Skip to content

fix(plugin-security): explain's record-grained write verdict composes the by-id write gate's authored-row-write deferral - #24713

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24647-explain-authored-row-write
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24647-explain-authored-row-write

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24647

Clause-②: no

What this changes

security/explain's record-grained write verdict now composes the by-id write gate's authored-row-write deferral, the composition the sharing middleware already runs. When record sharing's per-record update or delete gate refuses a row, the door asks whether an app-authored row-level policy admits that row for the gate's verb (ISecurityService.checkAuthoredRowWrite, reached through SharingService.probeAuthoredRowWrite), and on admit hands the row to the pre-image gate. Explain asked the gate alone, so it answered record.visible: false, decidedBy: 'sharing' for a row whose PATCH is admitted. The divergence was restrictive: explain refused what the door admits, and nothing leaked.

  • packages/plugins/plugin-security/src/explain-engine.ts: a new optional dependency, probeAuthoredRowWrite, asked only when the write gate answered false: for update and delete with the gate's verb, and for transfer as an update. Only the literal admit lifts the refusal. abstain, any other answer and a rejection keep it: the door's probe catches its own failure and keeps its refusal, so a failing probe is not reported as a failing request.
  • packages/plugins/plugin-security/src/security-plugin.ts: the explain binding wires it to the sharing service's own probe, when the service has one, with the context the gate is asked with (withWriteScope). A sharing service without the probe defers nothing, and explain reports the gate's refusal as before.
  • .changeset/24647-explain-authored-row-write.md: @objectstack/plugin-security patch.

What the record story reads for the new row

record: { recordId, visible: true, decidedBy: 'rls' }. rls is the layer that last admitted the row, which is the published meaning of decidedBy; no vocabulary value is added and packages/spec is untouched. The sharing layer's record keeps outcome: 'excluded' (record sharing itself still refuses), and its detail adds: "The by-id update does not stop at that refusal: an app-authored row-level update policy admits this record, so the write gate defers to row-level security (see the rls layer)." The rls layer's record reads admitted.

Why the composition sits in the engine (a deviation from the suggested route)

Composing at the binding alone (canEditRecord answering canEdit || admit) makes the row visible: true but leaves the story's sharing layer at admitted with "The sharing service grants write on this record (ownership or an edit/full share)" and decidedBy: 'sharing'. That is a wrong "why" naming a share that does not exist, which is the cost the card names. So the deferral is a dependency the engine composes beside the gate, the way it already composes the master-detail check. This touches explain-engine.ts, which the claim's file surface did not list. No other open PR touches it.

delete and transfer, included in place

  • The door's delete path defers too: buildSharingMiddleware probes with the refusing gate's verb for both update and delete. A transfer's door is the by-id PATCH of owner_id, which the update gate judges and the same deferral lifts, and explain's transfer verdict rides the update gate. Measured before this change (the ablated run below): all three answered visible: false, decidedBy: 'sharing' beside an admitted door.
  • The four in-place conditions hold: (1) the same defect class; (2) a mechanical fix, one composition with the verb its gate asked; (3) no other claim holds these files; (4) the same parity table, so no new validation surface. Each verb has its own parity row.
  • restore and purge are not deferred: they have no by-id door, and the object gate refuses both to every principal.

Security floor: no enforcement answer moves

  • The door (plugin-sharing) and the sys_attachment / sys_comment parent gates are not edited.
  • Both parent gates ask record sharing's checkEdit first and refuse on deny without asking explain. On the card's row the real SharingService.checkEdit answers deny under the gates' envelope, both with this change and with it ablated, so neither gate's answer can change. Each gate's own parity table already pins "sharing deny ⇒ refused, explain never called", with an explain double that would admit (comment-parent-update-parity.test.ts, attachment-access-hooks.test.ts).
  • No path was found where explain's verdict decides a write alone today. The other explain callers ask object-level create or read, and objectui reads the verdict for UI affordance only.

Pins a sibling card flips: packages/plugins/plugin-security/src/explain-enforce-parity.test.ts, test "an app-authored row-level write policy the by-id write gate defers to (ADR-0105 D3) › the parent gates' first question, record sharing, still refuses the row explain now admits". #24646 remains open. Its ruling (option B, comment 6279880675) has both gates ask only explain's record-grained update verdict, and that flips this pin: the gate then admits exactly what the parent's by-id update admits, which for this scenario, with this deferral, is admitted. The flip is pre-registered in a comment beside the assertion.

Tests

All on head 2fdf7b1987, each through scripts/pm/os-verify-lock.sh, verdict line command-exit 0:

  • pnpm --filter '@objectstack/plugin-security^...' build (dependency closure), then the ./packages/** build after the main merge.
  • pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2: Test Files 189 passed (189), Tests 3794 passed | 45 skipped (3839).
  • pnpm --filter @objectstack/plugin-security typecheck: exit 0, and check:test-typecheck: OK (the test layer compiles under tsconfig.test.json, so the new rows are type-checked).
  • vitest run --reporter=verbose src/explain-enforce-parity.test.ts: Tests 106 passed (106). New cases: the table rows for update, delete and transfer of a row an authored policy admits (door admitted, explain visible: true), the two controls (a row no authored policy admits: door 403 PERMISSION_DENIED, explain visible: false), and three cases: the story (decidedBy: 'rls', sharing layer excluded, rls layer admitted), the parent gates' first question (checkEdit is deny on the row explain now admits) and its positive control (the writer's own row: checkEdit allow, explain and the door admit).

Ablation (a one-time proof; no permanent test file)

node scripts/ablation-replace.mjs on security-plugin.ts, removing the binding's wiring of the probe (anchor count 1 to 0), on head 2fdf7b1987:

  • Mutated: Tests 5 failed | 101 passed (106), failing in the expected direction (red). The update, delete and transfer rows show explain at record: {"recordId":"l_theirs","visible":false,"decidedBy":"sharing"} beside enforcement {"kind":"admitted"}, which is the card's divergence reproduced. The story case and the parent-gate case fail at the explain assertion. The parent-gate case's checkEdit assertion (deny) passes before that point, so the gates' first answer is the same with and without the change. The positive control and both refusal controls stay green.
  • Restored: git checkout HEAD -- ABS_PATH, blob equal to HEAD (56d547d5ed1f), git diff HEAD empty. The same ablation on the pre-merge head d2e245471 gave the same 5 failures.

Gates

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, run on head 2fdf7b1987 (the merge head) against the real diff: 69 commands, each run with its exit code captured before any pipe. All 69 exit 0. dispatch-gates --ran: "69 derived famil(ies) accounted for — 69 run, 0 NOT-MEASURED". The derived list is identical to the one derived before the merge. Before the merge, three families first answered PREREQUISITE NOT MET (exit 3): check-plugin-teardown-shape --self-test (its pinned fixture commit was not reachable yet), check:i18n (CLI not built) and check:dual-build-cjs-loads (not every package built). All three were re-run once their prerequisite was met, and all exit 0 in the final union. CI's own check-runs on this PR are the authority for every family CI runs.

Acceptance notes


Generated by Claude Code

…id write gate's authored-row-write deferral

Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK
Co-authored-by: Claude <noreply@anthropic.com>
…oes, as an update; changeset

Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK
Co-authored-by: Claude <noreply@anthropic.com>
…te pin; changeset names transfer

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/permissions/system-context.mdx (via explainAccessForCaller (symbol, a method of class SecurityPlugin))
What this run could not see
  • 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 — 15 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 73d4cc7391bb5c2dc5d40e23af8cd505721242af → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 76d6a4ceb098716e855ca6ea6360fb47d0a933d5 — the merge of head 2fdf7b1987056b36cfa3152caef1746048116141 into base 73d4cc7391bb5c2dc5d40e23af8cd505721242af, 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 76d6a4ceb098716e855ca6ea6360fb47d0a933d5 && git checkout 76d6a4ceb098716e855ca6ea6360fb47d0a933d5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 73d4cc7391bb5c2dc5d40e23af8cd505721242af 2fdf7b1987056b36cfa3152caef1746048116141 && git checkout -B drift-repro 73d4cc7391bb5c2dc5d40e23af8cd505721242af && git merge --no-ff 2fdf7b1987056b36cfa3152caef1746048116141

node scripts/docs-audit/affected-docs.mjs --json 73d4cc7391bb5c2dc5d40e23af8cd505721242af

⚠️ 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 73d4cc7391bb5c2dc5d40e23af8cd505721242af → 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

席位核对 · ACCEPT · domain:services seat 1 · 2026-10-20T01:25Z

os-justin · session_011TdZ8GjixNZFLz1xJD9zzK (seat post #6021). Reviewed against GitHub and origin/main 73d4cc7391, not against the report. Disclosure is read first: the PR body and the report state only the restrictive divergence the card already states publicly, and give no recipe against any door. Head under review:

2fdf7b1987056b36cfa3152caef1746048116141
  1. Shape. Draft, base main, first line Fixes #24647, line-start Clause-②: no in the body and the changeset. Decision needed: should the sys_attachment / sys_comment parent gates drop their own-depth sharing step now that they ask the parent's by-id update judgement (#7144 revisited)? #24646 appears only as "remains open", with no keyword beside it. The changeset is @objectstack/plugin-security patch, and each sentence was read against the diff: the deferral covers update, delete and transfer, is asked only on the gate's refusal, and only the literal admit lifts it. The story reads decidedBy: 'rls', which is already a value of the published decidedBy enum (packages/spec/src/security/explain.zod.ts:336–347), so no spec change. Not governed, 234 changed lines.
  2. Security floor, measured by the seat, not taken from the report.
    • Every non-test caller of explain on origin/main was enumerated. Two ask a record-grained update: the comment gate (comment-access-hooks.ts:737) and the attachment gate (attachment-access-hooks.ts:398). The rest ask object-level create or read (rest-server.ts:2775, file-attachment-service.ts:237, sharing-service.ts:1193, runtime/src/domains/automation.ts:336), or are the explain endpoint itself (security-routes.ts).
    • Both gates ask checkEdit first and ask explain only after it admits. checkEdit abstains only where record sharing does not enforce at all: no schema, a public model, or no owner field (sharing-service.ts, checkEdit at :853). canEdit folds that abstention into true.
    • The new deferral fires only when explain's canEdit, asked at the caller's real write depth (never narrower than the gates' own), answers false. Wherever it fires, the gates' checkEdit has already answered deny. No gate answer can move, structurally, before Decision needed: should the sys_attachment / sys_comment parent gates drop their own-depth sharing step now that they ask the parent's by-id update judgement (#7144 revisited)? #24646.
    • Verdict order on the head: rlsExcluded and the master leg decide before the new branch, so a row-level refusal still wins.
  3. Gates. At ACCEPT, these check-runs on this head are success: Lint · repo & tooling gates, Lint · ESLint, Lint · PM dispatch-gates, Lint · scoped · query-options erasure / slot lookup & corpus / tooling self-tests, Type Check · source gates, Type Check · debt ledger, Dogfood Regression Gate, filter, "No other open PR may claim the same single-writer path", "Flag docs affected by code changes" and Auto Label. Test Core 1–6/6, Build Core, Type Check · workspace / consumer gates and Lint · CI, release & engine gates / docs & code guards are in_progress. Expected skips are on the roster. Locally, 69 of 69 derived families exited 0 on the merge head.
  4. Accepted deviations.
  5. Out-of-scope dispositions. Each goes to Acceptance notes; none is filed.
  6. Landing clause. No contract review is owed: it declares no widening and no narrowing, touches no published schema and no governed text, and no enforcement answer moves (2). Ready + auto-merge through the relay once every check on the current head is green or an expected skip. A push resets this reading.

Generated by Claude Code

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/m tests tooling

Projects

None yet

2 participants