Repository navigation
fix(plugin-security): explain's record-grained write verdict composes the by-id write gate's authored-row-write deferral - #24713
Conversation
…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>
…plain-authored-row-write
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # 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
|
席位核对 · ACCEPT ·
|
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 throughSharingService.probeAuthoredRowWrite), and onadmithands the row to the pre-image gate. Explain asked the gate alone, so it answeredrecord.visible: false,decidedBy: 'sharing'for a row whosePATCHis 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 answeredfalse: forupdateanddeletewith the gate's verb, and fortransferas anupdate. Only the literaladmitlifts 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-securitypatch.What the record story reads for the new row
record: { recordId, visible: true, decidedBy: 'rls' }.rlsis the layer that last admitted the row, which is the published meaning ofdecidedBy; no vocabulary value is added andpackages/specis untouched. Thesharinglayer's record keepsoutcome: '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)." Therlslayer's record readsadmitted.Why the composition sits in the engine (a deviation from the suggested route)
Composing at the binding alone (
canEditRecordansweringcanEdit || admit) makes the rowvisible: truebut leaves the story'ssharinglayer atadmittedwith "The sharing service grants write on this record (ownership or an edit/full share)" anddecidedBy: '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 touchesexplain-engine.ts, which the claim's file surface did not list. No other open PR touches it.deleteandtransfer, included in placebuildSharingMiddlewareprobes with the refusing gate's verb for bothupdateanddelete. A transfer's door is the by-idPATCHofowner_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 answeredvisible: false,decidedBy: 'sharing'beside an admitted door.restoreandpurgeare not deferred: they have no by-id door, and the object gate refuses both to every principal.Security floor: no enforcement answer moves
plugin-sharing) and thesys_attachment/sys_commentparent gates are not edited.checkEditfirst and refuse ondenywithout asking explain. On the card's row the realSharingService.checkEditanswersdenyunder 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).explaincallers ask object-levelcreateorread, 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 throughscripts/pm/os-verify-lock.sh, verdict linecommand-exit 0:pnpm --filter '@objectstack/plugin-security^...' build(dependency closure), then the./packages/**build after themainmerge.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, andcheck:test-typecheck: OK(the test layer compiles undertsconfig.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, explainvisible: true), the two controls (a row no authored policy admits: door 403PERMISSION_DENIED, explainvisible: false), and three cases: the story (decidedBy: 'rls',sharinglayerexcluded,rlslayeradmitted), the parent gates' first question (checkEditisdenyon the row explain now admits) and its positive control (the writer's own row:checkEditallow, explain and the door admit).Ablation (a one-time proof; no permanent test file)
node scripts/ablation-replace.mjsonsecurity-plugin.ts, removing the binding's wiring of the probe (anchor count 1 to 0), on head2fdf7b1987:Tests 5 failed | 101 passed (106), failing in the expected direction (red). The update, delete and transfer rows show explain atrecord: {"recordId":"l_theirs","visible":false,"decidedBy":"sharing"}besideenforcement {"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'scheckEditassertion (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.git checkout HEAD -- ABS_PATH, blob equal to HEAD (56d547d5ed1f),git diff HEADempty. The same ablation on the pre-merge headd2e245471gave the same 5 failures.Gates
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, run on head2fdf7b1987(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 answeredPREREQUISITE NOT MET(exit 3):check-plugin-teardown-shape --self-test(its pinned fixture commit was not reachable yet),check:i18n(CLI not built) andcheck: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
mainwas merged once before this PR opened (merge commit2fdf7b1987): the claim's serial constraint said whichever of this card and feat(platform-objects)!: the identity objects declare their default row scope, and the registry refuses an extend that sets one #24623 lands second mergesmain, and feat(platform-objects)!: the identity objects declare their default row scope, and the registry refuses an extend that sets one #24623 landed (d11fa8cc19) with a change tosecurity-plugin.tsat a disjoint region. The merge was clean. The build, the suite, the typecheck and the gate union were re-run on the merge head.[#5493 / ADR-0105 D3], following this file's existing tag forcheckAuthoredRowWrite. ADR-0105 D3's own text is about stripping only the platform's wildcard tenant policies; the deferral's provenance is the On objects that carry sharing rules, sharing middleware answers FORBIDDEN before RLS update-wideners are consulted — the identical widener works on rule-less objects #5493 ruling. A citation observation, not filed.purgerecord verdict asks the update gate (engineOp === 'delete' ? canDeleteRecord : canEditRecord), though a purge destroys like a delete. The object gate refusespurgeto every principal, so the record verdict never reaches that gate. Dormant, not filed.checkAuthoredRowWrite's docblock says a private-OWD cross-owner by-id write is still refused end to end. That holds for a caller whose read scope hides the row (the dogfood pin's principal). Withorgread depth the write is admitted, which the new parity row measures. Not filed.Generated by Claude Code