fix(plugin-approvals): the generic data door serves sys_approval_request rows by the approvals door's visibility (#22559)

[objectstack-fleet]

Fixes #22559
Clause-②: no

What this does

A read of sys_approval_request through the generic data door (find, findOne, count, aggregate on the engine, so GET /api/v1/data/sys_approval_request, the by-id route, POST .../query, the export route, the MCP data tool and every other engine read) now returns only the requests the approvals door (/api/v1/approvals/*) serves the same caller. Before this, once a deployment granted read on the object, that door served every request of the organization, including the payload snapshots of records the caller cannot open. The default (no grant, 403) is unchanged.

The rule is #8652's, applied as ruled: participants (submitter, current approver, past actor) and the override actor see a request; a reader of the record it is about sees it read-only, only on an object the deployment opted in (recordReaderVisibleObjects, default OFF). The declarative opt-in stays #22560's.

file	change
packages/plugins/plugin-approvals/src/request-read-gate.ts	new: the read middleware on sys_approval_request
packages/plugins/plugin-approvals/src/approval-service.ts	visibleRequestIdsFor(context, target), the approvals door's own visibleRequestIds made reachable; findRequestRow, the one by-id row read loadRequest and the gate share; the record-reader tier never anchors on sys_approval_request itself
packages/plugins/plugin-approvals/src/approvals-plugin.ts	binds the gate when the service runs; the option's doc names both doors
packages/plugins/plugin-approvals/src/payload-predicate-guard.ts	pinnedSubjectObject generalised to pinnedEquality(where, column), the one pin reader both seams use; behaviour of the guard unchanged
packages/plugins/plugin-approvals/src/sys-approval-request.object.ts	comment only
packages/plugins/plugin-approvals/src/request-read-gate.integration.test.ts	new: the pins
packages/plugins/plugin-approvals/src/payload-predicate-guard.test.ts	fixture re-judged (below)
.changeset/22559-approval-request-read-gate.md	patch

Which definition the gate consumes, and why

Hypothesis 1 held. The one definition is ApprovalService.visibleRequestIds: null for the override actor (a system context included), otherwise the participant set with the default-OFF record-reader tier (addRecordReaderVisibleIds) applied strictly on top. It was private, so the service gains one public method, visibleRequestIdsFor(context, target), which derives the organization the way listRequests / loadRequest do and calls it. The gate decides nothing itself: it reads which record the query names, asks the service, and ANDs an id constraint built from the ids the service returned. No second visibility rule exists.

Hypothesis 2 held; I used the approvals door's tier, not resolveReadableParentIds. That function lives in plugin-audit (comment-access-hooks.ts), is not exported from its index, and plugin-approvals does not depend on plugin-audit (the claim also excludes its source). The approvals door's tier already answers the same anchor the same way: one engine read of the record AS THE CALLER, so the record's own CRUD, sharing and RLS decide. Consuming it keeps one definition for both doors. Triage's "not the sys_activity shape" is pinned: switching the tier on for every object (ablation B) reddens exactly the tier-OFF reader pin.

Hypothesis 3: measured, and the target is read off the query. The data door's list arrives at the middleware with its filter already lowered into ast.where: implicit field filters (?object_name=…&record_id=…) and the filter-array spelling both arrive as conjunctive equalities, pinned in the tests through the real ObjectStackProtocolImplementation.findData. The gate maps the query to the approvals door's own read shapes:

• object_name and record_id both pinned (a record page's related list) is listRequests({ object, recordId });
• id pinned (GET /data/sys_approval_request/ID) is a request loaded by id, which carries its own anchor (loadRequest); the row is read through the same findRequestRow loadRequest uses;
• anything else is untargeted, the participant set alone, as for the inbox. A pin under $or / $not names nothing, so it reads untargeted: narrower, never wider.

Hypothesis 4 held. Every read the service makes of sys_approval_request runs as the system, and the shared definition answers a system context null before any read, so the gate adds nothing to them and never re-enters itself through them. One re-entry path did exist: the tier's anchor read runs as the caller, so with the opt-in naming sys_approval_request itself it would pass through the gate, which asks about the row that one names, unbounded on a cycle (measured: worker out of memory, see ablation C). The tier now never anchors on sys_approval_request; a request is no business record.

Hypothesis 5 held. The override actor gets null, so Setup → Approvals → Requests for an administrator is unnarrowed (pinned: list, count and by id).

How it composes with the attached-on-read viewer block (#22479)

viewer is attached by the approvals door only (attachViewers, on listRequests / getRequest). The gate decides which rows the data door serves; it never attaches viewer. So a row served on the data door carries no viewer, every decision action's visible predicate fails closed there, and a record reader the tier admits gets the row read-only on both doors: on the approvals door its viewer is { can_act: false, can_override: false, is_submitter: false }. Both halves are pinned. The other seams on this object are unchanged and compose: the snapshot query guard refuses before the gate runs, and the snapshot redaction narrows what a served row's payload_json holds.

Fail closed

• A failed visibility answer denies the read and logs one warning.
• A by-id read of a request the caller may not see answers exactly as a missing id does: 404 RECORD_NOT_FOUND (the door's real answer, measured through getData). No existence oracle.
• A list's total and a grouped count are narrowed like its rows.
• A context-less programmatic call on a bare kernel is not narrowed, as for the sibling read seams; every real transport carries a context.
• The gate binds whenever the service runs, not behind disableAutoHooks (that switch is documented as the record lock's). An engine with no middleware seam gets one loud warning at boot.

Tests and ablations

Rig: a real ObjectQL engine over better-sqlite3, the real ApprovalsServicePlugin.start() (so an unbound gate fails like an unwritten one), requests opened by the real openNodeRequest, and the real data-door normalizer. The one stand-in is record readability, one middleware on the business object. The subject resolves through src/: the test imports the plugin by relative path, so no dist/ sits on its resolution path and the ablations needed no rebuild.

Pins (14): a non-participant who cannot read the record sees no request in a list, a record's list, a count or a grouped count; by id, a hidden request answers as a missing one; a reader of the record sees nothing while the tier is off; a participant sees their own requests and no one else's; control: an administrator and the system are unchanged; for every caller, the data door's ids equal the approvals door's, tier off and on; with the tier on, a reader of the record sees its request on a read naming the record (both spellings), read-only, and nothing on an untargeted read or on a record it cannot read; a failed answer denies; a request about a request never re-enters the gate.

Each ablation was committed-first, landed through scripts/ablation-replace.mjs (anchor hit counted, blob changed), and restored to the HEAD blob with git diff HEAD empty:

ablation	mutation	result
A	gate unwired in approvals-plugin.ts	10 failed / 4 passed. Red: the five tier-off negatives and parity, the three tier-on negatives and parity, both fail-closed pins. Green, as they should be: the administrator and system controls, the tier-on reader positive, the filter-array positive. First failure: expected { ids: [ …(2) ], total: 2 } to deeply equal { ids: [], total: +0 }
B	record-reader tier on for every object (the activity-stream shape)	1 failed / 13 passed: a reader of the record who participates in nothing sees nothing while the tier is off, expected { …(2) } to deeply equal { ids: [], total: +0 }. Parity stays green, as it must: both doors loosen together
C	the self-anchor guard removed	with the fuse: 1 failed / 13 passed, expected 9 to be 1 (re-entry depth). Without the fuse, the first run of this pin held the lock 7m56s and ended in a worker out-of-memory crash, which is why the pin reads depth through a fuse
D	the gate's catch made to fail open	1 failed / 13 passed: a visibility answer that fails denies the read, and says so

Fixture re-judged. payload-predicate-guard.test.ts queried the generic door as three readers who participated in no request and asserted they were served rows "as before". After this change they are served nothing, so 7 of its pins went red for a reason outside what that file measures. Its readers are now the requests' current approvers (pending-approver index rows), so the rows they query are rows they may read and the file still measures the snapshot guard alone; its refusal pins were green throughout.

Verification (head 2a7dfed452, which merges origin/main d6c37919c7)

• pnpm turbo run build --filter='@objectstack/plugin-approvals...' --concurrency=1: 32 tasks, exit 0.
• pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2: 65 files, 930 passed, exit 0. The new file alone: 14 passed.
• pnpm --filter @objectstack/plugin-approvals typecheck: exit 0 (tsc build program, scripts program, and check:test-typecheck: the new test file adds no error; the ledger holds 8 files / 324 errors, unchanged).
• Lint, narrowed and proven: ESLint over the 7 changed .ts files, --no-inline-config --format json: 7 files, 0 errors, 0 warnings. Both new files are inside the linted population (isPathIgnored false, read from eslint.config.mjs), and the config enables no type-aware linting (no parserOptions.project / projectService), so this diff cannot move a verdict on an untouched file. The repo-wide pnpm lint is CI's.
• Declared to CI, not run here: the dogfood suites that boot the real stack with this plugin (approval-snapshot-masked-field, approval-snapshot-credential-field, activity-text-predicate, audit-log-admin-search). They resolve this package through dist/. Read through: their readers of sys_approval_request are its approvers (position holders), the platform administrator, or the system, so the gate admits each of them; the snapshot-guard refusals they pin happen before the gate runs.

Gates

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack at 2a7dfed452 derives 67 commands; all 67 ran and answered exit 0, and --ran reconciles: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN. Two answered 3 (PREREQUISITE NOT MET) in that run and were re-run once their prerequisite was built: pnpm check:i18n (its build closure, all cached) and pnpm check:dual-build-cjs-loads (a full turbo run build, 72 tasks). On an earlier head check-plugin-teardown-shape.mjs --self-test answered 3 because its pinned fixture commit was outside the shallow clone; after git fetch --unshallow origin main it answers 0. One went red on an earlier head and was fixed: pnpm check:query-options-erasure (the new test's engine options are now typed, not cast). The system-context census and the tenant-audit census are green with no regeneration: the gate reads no system flag of its own (the shared definition answers a system context), and the by-id row read moved into findRequestRow rather than being added.

Acceptance notes

• Same class, not fixed here (reported for filing): sys_approval_action and sys_approval_approver declare enable.apiMethods: ['get', 'list'] like sys_approval_request, and this package binds read middleware on sys_approval_request only. Under an app grant on either, the generic door is not narrowed to the approvals door's visibility (listActions gates through loadRequest). Not measured through HTTP here.
• disableAutoHooks: true also switches off the two snapshot seams on the generic door (the redaction and the predicate guard). This gate does not follow it. No production caller passes the option (git grep: tests only). Noted, not filed.
• disableService: true registers the five objects and binds no generic-door seam at all. Its one caller is packages/cli/scripts/check-app-nav-i18n.mjs, which reads navigation only. Noted, not filed.
• Cost: a non-administrator read of the object now pays the approvals door's participant probes (about four engine reads, plus the record read when the tier is on and a record is named), the same as one approvals-door list.
• The record-reader tier no longer anchors on sys_approval_request on the approvals door either. Only a deployment that names that object in the option sees a difference, and it is narrower.
• origin/main moved after the merge (25be87612d); not merged again, the PR's merge ref covers it.

Seat's append: rework round 1 (head a11d87ccaa)

Appended by domain:services seat 1 (session_013j5gkUCpqQiti4GgPqqmnt) at 2026-10-10T03:41Z, from the dev's round-1 report (6093383810 on #22559). Where the sentences above name visibleRequestIdsFor, or findRequestRow as a service member, this section supersedes them.

• The published surface is unchanged. The seat's round-0 review found that a new public ApprovalService.visibleRequestIdsFor widened the published class. It is gone.
  • The gate's visibility source is a module-scoped WeakMap that the ApprovalService constructor fills with an answer calling the class's own private visibleRequestIds.
  • The plugin reaches it through requestVisibilitySourceOf(service), which approval-service.ts exports and src/index.ts does not re-export (git diff origin/main -- packages/plugins/plugin-approvals/src/index.ts: 0 bytes).
  • findRequestRow is a module function, the one by-id row read loadRequest and the gate share.
  • ApprovalService's source member list equals origin/main's (106 members). The built dist/index.d.ts carries no visibleRequestIdsFor, requestVisibilitySourceOf or findRequestRow (positive control: listRequests( 1).
• Table row, approval-service.ts, superseded: the gate's visibility source, in-package only (above); findRequestRow, a module function; the record-reader tier never anchors on sys_approval_request itself.
• Merged with origin/main 99801d831f (merge 0522d7256e, clean). That brings feat(plugin-approvals): serve the approval request page through the manifest pages, and declare the thread reply as approval_comment (#22473) #22567 (the slotted sys_approval_request record page) and fix(plugin-approvals): a manager approver whose owner has no manager adds no slot, instead of manager:undefined #22583, in regions disjoint from this PR's.
  • The page's highlights and details read the request through the data door by id, and the gate serves that read exactly when getRequest does.
  • Two by-id parity pins hold this: 6 callers × 2 requests, with the tier off and on, and a refused request answering 404 RECORD_NOT_FOUND.
  • The page's Timeline tab reads sys_approval_action, which this PR does not gate. That is the sibling-tables card plugin-approvals: sys_approval_action (the decision log) and sys_approval_approver (the approver index) are served on the generic data door with no read narrowing, the sibling half of #22559 #22589.
• Round-1 verification:
  • The same 67 derived commands exit 0, and --ran reports 67 derived, 67 run, 0 NOT-MEASURED.
  • 66 files and 963 tests pass, and typecheck exits 0.
  • Ablation A (gate unwired) turns 12 of the suite's 16 pins red.
  • CI at a11d87ccaa: 31 success, 3 rostered skips.
• Clause-②: no stands. The surface is unchanged.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals, touching 29 documentable anchor(s).

44 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 99801d831faa07fa374cb21acc30a469fa6aa6e1.

⛔ 10 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see

• 4 name(s) were too generic to anchor anything (single lowercase words)
• the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
• a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 99801d831faa07fa374cb21acc30a469fa6aa6e1 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from df966dc37bf6030365cd91e2452b4533574a1579 — the merge of head a11d87ccaa9f3e738233185afdb168f99162308e into base 99801d831faa07fa374cb21acc30a469fa6aa6e1, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin df966dc37bf6030365cd91e2452b4533574a1579 && git checkout df966dc37bf6030365cd91e2452b4533574a1579
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 99801d831faa07fa374cb21acc30a469fa6aa6e1 a11d87ccaa9f3e738233185afdb168f99162308e && git checkout -B drift-repro 99801d831faa07fa374cb21acc30a469fa6aa6e1 && git merge --no-ff a11d87ccaa9f3e738233185afdb168f99162308e

node scripts/docs-audit/affected-docs.mjs --json 99801d831faa07fa374cb21acc30a469fa6aa6e1

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 99801d831faa07fa374cb21acc30a469fa6aa6e1 → pass the list as
args.docs, on the commit named under Which tree this was computed on.