Skip to content

fix(plugin-approvals): a payload_display lookup target is titled only for a target record the reader may read - #23672

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-23637-lookup-target-row-access
Oct 15, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-23637-lookup-target-row-access

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #23637
Clause-②: no

What this changes

plugin-approvals' enrichRows titles each lookup in a request's payload snapshot (payload_display). The title was read under the system context and narrowed only by the target's declared API exposure (#22738) and, since PR #23629, by the reader's field-level security on the target object. Nothing asked whether the reader may read the target RECORD.

Now, after the exposure gate:

  1. Object-level admission. The security service's canReadObject is asked once per target object, with the reader's envelope minus the operation-private keys. A refused object (or a throw, or any answer but true) is not read as the reader at all, so the security middleware has no denial to log for it. No security service, or one without the optional member, leaves every target to step 2.
  2. Row access. resolveReadableParentIds (@objectstack/core, the one caller-scoped "which of these records may this caller read" answer the comment, activity, attachment and mail gates ask) is asked once per admitted target object per read, with the reader's context. Sharing and row-level rules decide, as they do for the data door's $expand.
  3. Title. Only the readable ids are titled, by the existing read and FLS pick (titlesAsServed, unchanged). An unreadable target gets no display value and the snapshot's stored id stands, the answer a deleted target already gets.

The SUBJECT is not asked: its record_title, label and FLS-redacted snapshot are served exactly as before, whatever the reader's access to the subject record (#7497 ruling C, #10749).

Files:

  • packages/plugins/plugin-approvals/src/approval-service.ts: the narrowing in enrichRows, the new private admittedTargetObjects, the attachObjectReadAdmission attach point and its two exported types (ObjectReadAdmission, ObjectReadAdmissionSource; not re-exported from index.ts), docblocks.
  • packages/plugins/plugin-approvals/src/approvals-plugin.ts: one wiring call handing the service the registered security service, resolved lazily on every read like the field-visibility source beside it. Without it the admission would be dead in the composed runtime (route-and-surface rule 2).
  • packages/plugins/plugin-approvals/src/lookup-target-title-row-access.integration.test.ts: new pins.
  • packages/plugins/plugin-approvals/src/implicit-reference-target-lookup.test.ts: one existing pin counted exactly one sys_user read for an implicit-target user lookup. It now asserts the two reads that happen: the reader's row-access ask, then the system title read.
  • .changeset/23637-lookup-target-title-row-access.md: @objectstack/plugin-approvals patch.

The canReadObject pre-filter is used at the coordinator's offer: the same shape as PR #23663 (not landed) uses in plugin-audit's gate. It is CALLED, through the security service plugin-approvals already resolves. Nothing in plugin-security, plugin-audit or packages/core changes, and no log is muted, caught or lowered: a reader's own read of an object they may not read still logs its denial.

Measurement (function level)

Booted on main at 286eb9ef32 with the real SecurityPlugin composed in front of a real ObjectQL engine over SQLite, with a routed approver who may read the target object's display field (FLS answers the full field set):

  • Before: getRequest and listRequests served the target's title in payload_display for two denial classes. (1) The target row was hidden from that reader by a row-level rule on an object they may read. (2) The reader had no object-level read on the target object, so the data door refused it. In both classes the data door did not show the record. Served.
  • After, without the admission wired: both titles were withheld, and a readable row of the same object kept its title. The caller-scoped read of the object with no object-level read added 1 WARN denial line per inbox read (getRequest: 1, listRequests: 1, one per refused target object per read).
  • After, with the admission wired (the production wiring): the served answer was the same and 0 WARN lines were added.

The measurement rig was a throwaway test file in this package's tree. It imported plugin-security's source by relative path read-only, ran three times, and was deleted. It was never committed.

Pins (lookup-target-title-row-access.integration.test.ts)

A real ApprovalService on a real ObjectQL over SqlDriver (better-sqlite3, in-memory). Record readability is the one stand-in for plugin-security: one middleware per business object refuses a caller with no object read and scopes one with read to its rows. Pins:

  • (a) A reader denied the target row gets no title for it, on getRequest and listRequests, and the stored id stands. The three classes are a row rule on a readable object, no object read, and a self-lookup to a hidden row. The row shape equals that of a deleted target.
  • (a-row) Per record, not per object: in one list, the readable row of an object keeps its title while a hidden row of the same object does not.
  • (b) Control: a reader who may read every row gets every title.
  • (c) One row-access ask per target object per read, naming every id of that object the read titles, on the single read and on a multi-row list.
  • (d) The subject is untouched: a reader who may read no subject row gets the subject's title, label and snapshot exactly as the control does.
  • (e) With an admission attached:
    • an object it refuses is not read as the reader, and the answer is unchanged;
    • an admitted object is still narrowed by row;
    • the admission is asked once per target object per read, without the operation-private keys.
  • (e-closed) A throwing admission, or one that answers anything but true, refuses that object; the throw is said once at warn. A service without the member reads as before.
  • (f) Composed: the real ApprovalsServicePlugin.start() hands its service the registered security service's admission.

Red before. The first five pins were run on the base's source (286eb9ef32) before the fix was written: 3 failed / 2 passed. The failures were (a), (a-row) and (c); (b) and (d) passed, as predicted. Pins (e), (e-closed) and (f) were written later; ablation A below restores the old behaviour and reddens all three.

Ablations. Each was run through scripts/ablation-replace.mjs in wrap mode under an EXIT/INT/TERM restore trap, over the pin file's 8 tests, with the red set predicted before each run:

  • A, the old behaviour (every exposed target titled, no ask): 6 failed / 2 passed, = (a), (a-row), (c), (e), (e-closed), (f). As predicted.
  • B, admission ignored: 3 failed / 5 passed, = (e), (e-closed), (f). As predicted.
  • C, one row-access ask per id instead of per object: 2 failed / 6 passed, = (c), (e). As predicted.
  • D, the approvals-plugin.ts wiring call deleted: 1 failed / 7 passed, = (f). As predicted.

Every restore was proven by blob equal to HEAD (0e12be2ed246 for A to C, ffdaebd5269e for D) and an empty git diff HEAD. There was no dist step: both subjects are imported from src by relative path.

Tests and gates

  • pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2: 78 files / 1080 tests passed at 9e88df4551. Since then: a changeset text edit, and two merges of origin/main (1ced0f82bd and 881eeff315, both .claude/** and scripts/pm/** only).
  • pnpm --filter @objectstack/plugin-approvals typecheck: exit 0. check:test-typecheck held at 8 files / 324 errors / 27 signatures. tsc --listFiles on tsconfig.test.json compiles both edited test files, with 0 errors in them.
  • eslint --no-inline-config over the 4 changed TS files: exit 0, no findings.
  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 67 commands. All 67 were run at 9aed095a6e, each exit captured before any pipe:
    • 65 answered 0 on the first run.
    • check:dual-build-cjs-loads and check:i18n first answered 3, PREREQUISITE NOT MET (no workspace dist). After a workspace build (75 tasks, exit 0) both answered 0.
    • Re-derived at 37573fbd67: the same 67. --ran: 67 derived, 67 run, 0 NOT-MEASURED (a derived zero), 0 UNRUN.
    • The ratchet and census families (12) were re-run on 37573fbd67: all exit 0.
  • Not run locally, CI's: the full farm, pnpm lint.

Acceptance notes

  • Log interplay with Every console page load by a non-admin logs ~16 "[Security] Access denied" WARN lines: the header activity feed's GET /api/v1/data/sys_activity fans out to objects the caller cannot read and logs each refusal #23279 (measured). Without the admission, one inbox read added one [Security] Access denied WARN per target object the reader may not read at all. With the admission wired, it added zero. No log is muted, caught or lowered here.
  • Degrade in the console (read from source, not browser-measured). objectui's inbox summary card (ApprovalsInboxPage.tsx payloadSummary, at objectui 28fb2a0) gives a reference the server could not resolve no row. So an unreadable target renders as a deleted target already does.
  • Fewer field-visibility asks, same answers. The target object's FLS answer is now asked only for an object with at least one readable id.
  • Observation, not filed: participant names. submitter_name and pending_approver_names are still read under the system context. They are the approval's own participants, part of the inbox entry ruling C gives an approver, and not referenced records.
  • Observation, not filed: a self-lookup to the subject record itself. A lookup that points at the request's own subject record now follows the reader's row access to that record, while record_title is served regardless. This is not a disclosure: it is the same title.
  • Ruling C. This narrowing reads no new meaning into Design: does approver routing imply record read visibility? (#7345 model half) #7497 ruling C. What the ruling gives an approver (the inbox entry, the FLS-redacted snapshot holding the target's id, the actions) is unchanged. Record access to the referenced record stays with object permissions, sharing and row-level rules, which is what is asked here. Nothing is granted.

Deviations

  • File surface beyond the claim's approval-service.ts and its tests: approvals-plugin.ts, one wiring call for the coordinator-offered admission (see above).
  • Commit trailers carry the repository's model-free pair, and this body ends with the repository's session-URL footer (AGENTS.md).

Generated by Claude Code

… for a target record the reader may read

enrichRows asks the platform's caller-scoped row-access answer
(resolveReadableParentIds, @objectstack/core) for the lookup targets of a
read, once per target object, and titles only the readable ones. An
unreadable target gets no display value and its stored id stands, as a
deleted one does. The subject's title and snapshot are unchanged.

Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ
Co-authored-by: Claude <noreply@anthropic.com>
… is skipped by the object-level read admission before any caller-scoped read

enrichRows asks the security service's canReadObject for each exposed
target object, with the reader's envelope minus the operation-private
keys, before the caller-scoped row-access read, so an object the reader
may not read at all is not read as the reader and the security
middleware logs no denial for it. Fails closed on a throw or a non-true
answer; with no service, or one without the member, every target is left
to the caller-scoped read. Wired by ApprovalsServicePlugin through the
registered security service, resolved on every read.

Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx (via ApprovalService (symbol, a top-level class))
  • content/docs/permissions/system-context.mdx (via canReadObject (literal, a string literal in ObjectReadAdmission))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-5.mdx (via canReadObject (literal, a string literal in ObjectReadAdmission))
  • content/docs/releases/v17/17-6.mdx (via ApprovalService (symbol, a top-level class), canReadObject (literal, a string literal in ObjectReadAdmission))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 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 57994c311f4acc576e1a1eb9ad4162d191d15905 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 8a31df4b49df12fda0e391f574d82a1adbd0e49e — the merge of head 37573fbd67a8ab04a0373248da587cb63895a17b into base 57994c311f4acc576e1a1eb9ad4162d191d15905, 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 8a31df4b49df12fda0e391f574d82a1adbd0e49e && git checkout 8a31df4b49df12fda0e391f574d82a1adbd0e49e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 57994c311f4acc576e1a1eb9ad4162d191d15905 37573fbd67a8ab04a0373248da587cb63895a17b && git checkout -B drift-repro 57994c311f4acc576e1a1eb9ad4162d191d15905 && git merge --no-ff 37573fbd67a8ab04a0373248da587cb63895a17b

node scripts/docs-audit/affected-docs.mjs --json 57994c311f4acc576e1a1eb9ad4162d191d15905

⚠️ 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 57994c311f4acc576e1a1eb9ad4162d191d15905 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 37573fbd67a8ab04a0373248da587cb63895a17b
Local-runs: none

Read: card #23637 (body and all four comments: triage 6188373141, claim 6190546282, os-dev-report 6191744169, regrade 6191803622); PR #23672 body, its 5-file list and the net diff against main at the head (merge-base 881eeff315, +592/-11); the 39 check-runs on the head, latest per name, read twice without waiting; #7497 ruling C (6051659422); payload-redaction.ts's header (#10749) on the head; PR #23629's contract record (6187371379); PR #23663 (#23279) as landed on main at 94924e01c1 (parent-record-read-gate.ts, audit-plugin.ts); resolveReadableParentIds and withoutOperationPrivateKeys in packages/core; the canReadObject contract (packages/spec/src/contracts/security-service.ts) and plugin-security's implementation; the REST approvals routes on the head. Function level only, per the card's security gate.

① Derived judgments

Accept-set and public-surface changes the diff implies, each judged.

  1. (1) Every payload_display lookup-target title is now served only for a target RECORD the reader may read, on both inbox doors. On the head, payload_display is set in exactly one place (enrichRows, the one assignment at its tail), and enrichRows is reached from exactly two call sites: listRequests and loadRequest, the latter being getRequest's body. Both pass the context the request's own visibility was judged by. In enrichRows the order is now: the exposure gate (servesPayloadDisplayTarget, security(approvals, audit): a lookup target's title is resolved into the approvals inbox payload and the activity summary without asking the target's exposure (census rows 5-6 of #22661) #22738, unchanged, first), then admittedTargetObjects (object-level, the reader's envelope), then resolveReadableParentIds (@objectstack/core, one caller-scoped read per admitted object, the reader's envelope), then targets is the admitted objects with at least one readable id, then servedFields (FLS, security(approvals): two more approval-path titles are read under the system context without the reader's field-level security (lookup-target payload_display titles; the RECORD_LOCKED refusal) — #23594's family, measure first #23618, unchanged) and titlesAsServed with readableIds.get(object) in place of refIds.get(object). So the system-context title read now takes only the ids the reader's own read returned; a refused object, a hidden row and a deleted record all come back absent and the stored id stands. Right: this is the card's point 2 as filed and as triage pre-ruled it (narrow through the platform's existing row-access door, degrade as a deleted target does), and resolveReadableParentIds is that door (its header: the one answer the comment, activity, ledger, attachment and mail gates ask; do not hand-copy). The code-line diff of the service is only the two imports, the two types, the field, the attach point, admittedTargetObjects, the four lines in enrichRows and the one argument change; nothing else moves.
  2. (1) No other producer of payload_display is left out. A grep of packages, services and apps at the head finds the key produced nowhere but enrichRows and declared in packages/spec's ApprovalRequestRow (optional). The two REST routes (GET .../approvals/requests and .../requests/:id) call the two doors with the resolveExecCtx context after enforceAuth. The other getRequest callers are listActions and the decision-attachment visibility check, both with the caller's context, and resolveActionToken under SYSTEM_CTX. That system read yields whole titles (plugin-security's isSystem bypass in canReadObject, and an unscoped read), but its only consumers are peekActionToken and redeemActionToken, served by the plugin's self-hosted mount through action-link-pages.ts, which reads submitter_name, submitter_id, step_label, record_id, process_name, process_label, object_name, object_label and current_step and never payload or payload_display (the word appears once, in a comment). Unchanged by this diff, and the same finding as PR fix(plugin-approvals): lookup-target titles and the record-lock refusal title are served only from fields the caller may read #23629's record item 4. Right.
  3. (2) Nothing is granted, and the SUBJECT is untouched. The only code lines changed narrow the set of target ids that are titled. resolveSubjectDisplays, servedSubjectFields, redactPayloads, record_title, object_label and the FLS-redacted snapshot have no code-line change; the new reads are reads (fields: ['id'], bounded by the id count), the exposure gate still runs first, and the target read itself (titlesAsServed, SYSTEM_CTX) is unchanged in body. A routed approver with no read on the subject record is served the subject exactly as before (pin (d) asserts record_title, object_label and payload equal to the control's, on both doors). What ruling C gives an approver (the inbox entry, the FLS-redacted snapshot holding the target's id, the actions) is unchanged; what the ruling keeps with object permissions, sharing and row-level rules (record access) is what is now asked of the referenced record. Right, and the fork did not open: the diff reads no new meaning into ruling C. The reviewer agrees with the dev and triage here: PR fix(plugin-approvals): lookup-target titles and the record-lock refusal title are served only from fields the caller may read #23629's record called this a design decision beside ruling C and left it to a card of its own; security(approvals): a lookup target's title in the approvals inbox (payload_display) is read under the system context without asking whether the reader may read that target record (row-level, sharing), unlike $expand and dataset labels; measure first #23637 is that card, and triage pre-ruled its fix on measurement. Nothing is widened, which is the only act the card reserves for the maintainer.
  4. (3) Fail-closed posture, and the same posture PR fix(plugin-audit): the parent-record read gates skip an object the caller cannot read without reading it, so no denial is logged for the skip #23663 landed. admittedTargetObjects resolves the source inside a try/catch (a throwing resolver is read as absent); a service without the optional member returns every object to step 2; an answer that is not === true refuses the object; a throw refuses the object and is said once at warn as a fault. admittedParentObjects in parent-record-read-gate.ts at 94924e01c1 is the same shape line for line (try/catch around the resolver, typeof canReadObject !== 'function' as the absent state, === true, warn on throw, exclude otherwise), and approvals-plugin.ts's resolver is the same as audit-plugin.ts's getReadAdmission (ctx.getService('security') in a try/catch returning undefined). Can the absent state widen what is served? No: with no admission, every exposed object goes to resolveReadableParentIds as the reader, which is the reader-scoped read alone; the admission only removes objects from that set, and by the contract (canReadObject is the middleware's own object-level arms, computed from the same resolution) an object it refuses is one the reader-scoped read would have refused. So the served set under absence equals the reader-scoped read's, and under presence is a subset of it. A reader whose envelope carries no principal is refused by both (ADR-0096 D5 in canReadObject; the middleware's principal-less refusal on the read), so an undefined context closes too; the REST doors never reach here unauthenticated. The two reads a refused object would otherwise log (Every console page load by a non-admin logs ~16 "[Security] Access denied" WARN lines: the header activity feed's GET /api/v1/data/sys_activity fans out to objects the caller cannot read and logs each refusal #23279's interplay) are avoided without muting, catching or lowering any log: the dev measured 1 WARN per refused object per read without the admission and 0 with it, and nothing in plugin-security changes. Right.
  5. (4) The reader's envelope is the one the request was judged by; no second derivation. listRequests and getRequest take the resolveAuthzContext envelope (the Re-annotate plugin-approvals + plugin-reports onto the full ExecutionContext and delete the now-redundant casts (services half of #7070) #7135 full-envelope note at the file's head), judge the request's visibility by it (actingCaller, visibleRequestIds) and hand the same context object to enrichRows. admittedTargetObjects asks canReadObject with withoutOperationPrivateKeys(context) copied, and resolveReadableParentIds sends the same strip of the same context; neither rebuilds a session or re-derives permissions, and the strip drops only the __ operation-private widening keys, in the safe direction operation-private-keys.ts documents. Pin (e) asserts the admission is asked with permissions, positions and userId and without a __readScope the caller carried. Right. The approvals-plugin.ts wiring line is within the claim's purpose: route-and-surface rule 2 (a capability the host should get is mounted explicitly); without it the admission is dead in the composed runtime (ablation D, pin (f)); it is one lazy resolver beside the field-visibility one, the same shape PR fix(plugin-approvals): lookup-target titles and the record-lock refusal title are served only from fields the caller may read #23629's record accepted at its judgment 8 and the shape audit-plugin.ts landed at 94924e01c1; the dev declared it in the PR body and the report. Right.
  6. (4) The two new exported types are not a public-surface change. ObjectReadAdmission (a Pick of ISecurityService's canReadObject) and ObjectReadAdmissionSource are exported from the module approval-service.ts only. index.ts re-exports from that module by a named list that does not include them (verified on the head), and package.json's exports map is . only, served from dist/index.*, so they are package-private, as ParentObjectReadAdmission is in plugin-audit. attachObjectReadAdmission is a new public method on the exported ApprovalService class, additive and optional (an unattached service reads as before); a new optional attach point on a service class is within patch for this package, as attachFieldVisibility was. Right; semver unaffected.
  7. Test accept-set changes. implicit-reference-target-lookup.test.ts's one pin now asserts the two sys_user reads that happen for an implicit user lookup: the reader's row-access ask (not system) and then the system title read, in that order. Right: it states the new accept set for the implicit-target class (a user lookup follows the reader's row access to sys_user, as $expand on it would). The new pin file drives the real service and the real ApprovalsServicePlugin.start() on a real engine over sqlite with one middleware per business object standing in for the security middleware's two answers (a refusal, a narrowed result): (a) three denial classes on both doors, equal to a deleted target's shape, the stored id standing, the titles absent from the serialised row; (a-row) per record, not per object; (b) control; (c) one row-access ask per target object per read naming every id; (d) the subject untouched; (e) admission attached: a refused object not read as the reader, an admitted one still narrowed by row, one ask per object without the __ keys; (e-closed) throw and non-true refuse, a service without the member reads as before; (f) composed wiring. Red-before (3 failed / 2 passed, the predicted set) and four restore-proven ablations with predicted sets equal to observed are reported; not re-run here. The real-SecurityPlugin measurement (before: served on both doors for a row-rule class and a no-object-read class; after: withheld, a readable row of the same object keeps its title) is the card's point 1 and is reported; its finding agrees with the source read here.

Check-runs on the head, as read (latest per name, 39 runs, read twice, not waited for): 29 completed success, among them Check Changeset, Governed Surface Queue Guard, Build Core, every Lint family, every Type Check family, Dogfood Regression Gate, Test Core (5/6) and (6/6), both claim guards, the single-writer guard and the Part-of guard; 6 skipped by design (Build Docs, Console Pin Gate, Dogfood matrix shard, Dogfood Verify CLI, Packed-tarball smoke, Temporal Conformance). In progress at both reads: Test Core (1/6), (2/6), (3/6), (4/6). None failed. The queue's every-check-green condition is the seat's to read at landing time; this verdict is on the contract.

② Semver level

Changeset .changeset/23637-lookup-target-title-row-access.md: @objectstack/plugin-approvals patch. Matches the diff. The package name matches package.json. Nothing published moves: no packages/spec file is touched; ApprovalRequestRow.payload_display is already optional on the contract, so an absent key is within the declared shape; enrichRows, admittedTargetObjects, titlesAsServed and servedFields are private; the two new types are package-private (judgment 6); the new attach point is additive and optional; no route, query parameter, error code, translation key, notification field or export changes; nothing in packages/core, plugin-security or plugin-audit changes. A narrowing of served values is patch. skip-changeset does not apply and was not used. Check Changeset is green.

Clause-②: no is on the PR body, the changeset and the claim, and all three agree with the diff: no published schema or public surface is declared or changed. Right.

Governed surfaces: none in the 5-file list (no docs/adr/**, .claude/**, skills/**, .github/workflows/**, AGENTS.md, CLAUDE.md, docs/NORTH-STAR.md). The Governed Surface Queue Guard is green. The branch's two origin/main merges before opening touched .claude/** and scripts/pm/** on main's side only; the net diff against the merge-base 881eeff315 is the 5 files.

③ Boundary flags

open_questions: empty. Nothing to answer.

Deviation 1 (file surface beyond the claim's approval-service.ts and its tests: the one wiring call in approvals-plugin.ts). Answered: within purpose (judgment 5); declared in the PR body and the report; the claim's reserved paths (packages/core, plugin-audit, plugin-security) are untouched, as the file list shows. Within surface.

Deviation 2 (a throwaway measurement file in this package's tree importing plugin-security's source by relative path, run three times and deleted, never committed). Answered: the net diff carries no such file and no relative cross-package import, so nothing of it ships; the measurement is what the card's point 1 asked for first, and its finding agrees with the source read here (judgment 7). No contract effect.

Deviation 3 (push order: the pins and the fix went up together after the empty-branch probe). Answered: the card's security rule that nothing red lands on the remote before the fix governs; no contract effect.

Deviation 4 (gates run under os-verify-lock as dispatched; one batch-wrapper timeout on check:type-check-debt, re-run to completion at exit 0). Answered: a process note; the check-runs on the head are the gate verdicts of record (Type Check · debt ledger is green). No contract effect.

Deviation 5 (two origin/main merges before opening; the package suite not re-run after them). Answered: the merge-base is 881eeff315 and the net diff is the 5 files (section ②); the head's Build Core, every Type Check family and Test Core (5/6) and (6/6) are green, the remaining Test Core shards in progress at read time. No contract effect.

Deviation 6 (model-free commit trailers and the repository's session footer). Answered: a process note with no contract effect; the repository's instructions govern the seat.

out_of_scope_findings entry 1: submitter_name and pending_approver_names are read under the system context (resolveUserNames, sys_user by id and by email, fields id, name, email). Judged: not a defect this card names, and not a disclosure of the class this card names; the PR is complete on the card's terms. The card names a lookup target in the payload snapshot, a referenced business record whose title the snapshot never stored. The participant names belong to the inbox entry itself, which ruling C gives the approver (who submitted, who is still pending), and the entry is already tenant-scoped by buildRequestWhere and findRequestRow. Whether a directory name should be served to a reader who holds no read on sys_user at all is the same shape of question as this card's, for a different object and a different class of consumer; if the seat wants it settled it is a card of its own, filed with a measurement first, not an incompleteness of this PR. The seat decides any filing.

out_of_scope_findings entry 2: a lookup pointing at the request's own subject record now follows the reader's row access to that record, while record_title is served regardless. Judged: not a defect, and not a disclosure. Both values are the same record's title, picked by the same titlesAsServed and the same served-fields answer (a target that is also the subject object reuses the subject's answer); withholding the copy in payload_display discloses nothing the row does not already carry in record_title, and serving record_title is ruling C's text. A self-lookup to a DIFFERENT record of the subject object (pin (a)'s third class) is correctly withheld. A parity oddity in the row shape, not a contract question; nothing to file.

Implemented-by: claude/issue-23637-lookup-target-row-access
Reviewed-by: session_017WLmd5in9RXu7AjyHR8fKQ

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

domain:services seat 1 (#6021) · os-bill · session_017WLmd5in9RXu7AjyHR8fKQ · 2026-10-15T12:25Z · reviewer of record for #23637 (claim 6190546282; dev report 6191744169; regrade 6191803622). Read on GitHub and on origin/main, ⛔ not from the report. Function level only (a security card).

Verdict: ACCEPT at head 37573fbd67a8ab04a0373248da587cb63895a17b. The contract-tier record 6191970854 (Served-tier: CONTRACT_REVIEW_TIER, VERDICT: PASS) is on this head. Landing waits on every check green.

  1. Shape.

    • Draft, base main, first line Fixes #23637, and no other card number beside a closing keyword.
    • Assignee os-bill = the card's.
    • 5 files, +592/−11, all in plugin-approvals plus one changeset.
    • No reproduction recipe in the body or the public report (scanned before anything else).
  2. Premise, measured: served. The measurement is on the card, and triage's pre-ruled p1 regrade was applied at 6191803622. Seat spot reads at the head, in enrichRows:

    • the exposure gate, unchanged;
    • then canReadObject per target object, with the reader's envelope minus its operation-private keys;
    • then resolveReadableParentIds per admitted object, as the reader;
    • then the unchanged system-context title read, over the readable ids only.

    An unreadable target gets no display value and the stored id stands. The subject's title, label and snapshot have no code-line change.

    Tests. Red before: 3 of 5 pins on the base source. Four ablations, each matching its predicted red set, restores proven.

  3. Gates.

    • Locally: 67 derived, 67 run, --ran reconciled with 0 NOT-MEASURED.
    • On this head at this read: 30 success, 6 skipped (this lane's expected-skip roster), and Test Core 2/6–4/6 in progress. None failed.
    • ⛔ No landing act until every check is green.
  4. Accepted deviations.

    • The approvals-plugin.ts wiring line. It hands the service the registered security service's admission, resolved lazily. The contract record judges it within purpose: without it, the admission is dead in the composed runtime.
    • The canReadObject pre-filter, at this seat's mid-task relay. It is the shape PR fix(plugin-audit): the parent-record read gates skip an object the caller cannot read without reading it, so no denial is logged for the skip #23663 landed as 94924e01c1. With it wired, the reader-scoped read adds 0 denial lines, against 1 per refused object per read without it. No log is muted.
    • The throwaway measurement rig (deleted, never committed), the push order, and the two origin/main merges that touched .claude/** and scripts/pm/** only: as declared.
  5. Out of scope, judged by the contract record and not filed.

    • Participant names (submitter_name, pending_approver_names, read under the system context). They are the inbox entry's own content, which ruling C gives an approver, and they are tenant-scoped. This seat does not file it: showing an approver who submitted the request and who else is pending is what the inbox entry is for. ⛔ No measured pull for narrowing it.
    • The self-lookup to the subject record: the same title is already on the row as record_title, so nothing is disclosed.
  6. Landing clause. No governed path, and the contract-tier PASS is on this head. When every check on the then-current head is green, this seat flips the PR ready and arms auto-merge through the relay. It then confirms MERGED from the queue membership and origin/main, and clears pm:dispatched and the assignee on security(approvals): a lookup target's title in the approvals inbox (payload_display) is read under the system context without asking whether the reader may read that target record (row-level, sharing), unlike $expand and dataset labels; measure first #23637.


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 15, 2026 12:31
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 15, 2026 12:31
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit 7e6e4e1 Oct 15, 2026
41 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-23637-lookup-target-row-access branch October 15, 2026 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants