Repository navigation
fix(plugin-security): explain answers a by-id write of a row the caller cannot read with the nonexistent id's whole answer, layers included - #22791
Conversation
…aller cannot read exactly as an id no row carries The read-absent question is now asked before the record story is written, and a row the caller cannot read is attributed through the same path as a nonexistent id: the decision's record verdict, every row-scoped layer's record and the tenant layer's verdict. Previously only the record verdict was rewritten after the fact. Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp Co-authored-by: Claude <noreply@anthropic.com>
…f a row the caller cannot read beside the door's 404 For update, delete and transfer on a controlled_by_parent detail: the explanation equals the nonexistent id's whole explanation, every status a layer names is the door's, and the readable control keeps the master check's 403 on both faces. Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp Co-authored-by: Claude <noreply@anthropic.com>
…ord story Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp Co-authored-by: Claude <noreply@anthropic.com>
…ite explanation at the engine level Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
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 e65f1405194296d20cf83f22fbfe9f07325f5b5e && git checkout e65f1405194296d20cf83f22fbfe9f07325f5b5e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin a2e94c2a053ab004b5cb9fea27824bb614e5759c 02f40a098c0feb6edf139b80fe69c523e0c37d97 && git checkout -B drift-repro a2e94c2a053ab004b5cb9fea27824bb614e5759c && git merge --no-ff 02f40a098c0feb6edf139b80fe69c523e0c37d97
node scripts/docs-audit/affected-docs.mjs --json a2e94c2a053ab004b5cb9fea27824bb614e5759c
|
Fixes #22761
Clause-②: no
It narrows what explain's layers say about a row the caller cannot read to what the door already answers. No accepted input changes and no public type moves.
What changed
packages/plugins/plugin-security/src/explain-engine.ts, the read-absent rewrite inexplainAccessonly. The read-absent question is now asked before the record story is written. A row the caller cannot read is then attributed through the same path as an id no row carries. The record verdict, every row-scoped layer'srecordblock and the tenant layer's verdict all come from that one path. Before, only the record verdict was replaced, after the layers had been written.capsDeny,crudAllowed) directly. It used to read the decider names the attribution had written. The cases it admits are the ones the comment always named: past the capability and object CRUD gates, for a principal with an identity, forrlsOpupdateordelete.masterWriteCheckRefusal, toapplyRecordAttributionor to any layer's own logic. The edit reaches theapplyRecordAttributioncall that opens the sameif (input.recordId)block, a few lines above the cited range, because the question now comes before that call.Measurement first
Measured at
origin/maine84aeb36cwith the realSecurityPluginover a realObjectQL, the SQL driver and the realSharingServicemiddleware. The cases wereupdate,deleteandtransferon acontrolled_by_parentdetail:Results:
404 RECORD_NOT_FOUND, and (c) answered403 PERMISSION_DENIED, for all three verbs.recordblocks differed:tenant_isolation,owd_baseline,depth,sharing,vama_bypassandrls.detail.owd_baselineandsharingalso differed inoutcomeandmatchesRecord, andowd_baselinein itsrules.tenant_isolation,sharingandrlsalso carried arowFilterkey that (b) does not.allowed,principal, each layer's ownverdict,detailandcontributors) were identical.sharingrecord named the master check's 403 beside the door's 404.Pins (
explain-read-absent-record-story.test.ts, 16 cases)RECORD_NOT_FOUNDwith status 404 (codeandstatusasserted), and the record verdict is still the missing-record shape.PERMISSION_DENIEDwith status 403, the record verdict is unchanged (visible: false,decidedBy: 'sharing'), thesharingrecord isexcludedand names the master check's leg, and the only status any layer names is the door's 403.Ablation (one-time proof, no permanent file)
The old behaviour was put back with
scripts/ablation-replace.mjs: the attribution read the stored row, and the record verdict was replaced afterwards.f0f52a3cto4ac19bb4.02f40a098: 9 failed, 7 passed of 16. The 9 are the negative pins: the equality pin, the status pin and the engine-level tenant pin, for each of the three verbs. The 7 are the armed case and the six controls.AssertionError: a status named in explain update's layers: expected '403 PERMISSION_DENIED' to be '404 RECORD_NOT_FOUND'AssertionError: expected { allowed: true, …(5) } to deeply equal { allowed: true, …(5) }. The diff starts at thetenant_isolationrecord.git checkout HEAD -- PATHon the absolute path. The blob equals the HEAD blobf0f52a3c, andgit diff HEADis empty.src/(relative imports), so nodist/leg applies.Verification
pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2at4da36f554: 199 files passed, 4030 tests passed, 45 skipped. The only commit since adds cases to the new test file. That file alone at02f40a098: 16 passed.pnpm --filter @objectstack/plugin-security typecheckat02f40a098: exit 0. The test layer is included (tsconfig.test.jsonlists the new file).node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackat02f40a098: 66 derived. 63 were run, all exit 0. 3 are NOT MEASURED:check:dual-build-cjs-loadsexited 3, PREREQUISITE NOT MET: it needs a whole-workspace build.check:i18nexited 3, PREREQUISITE NOT MET: it needs the CLI and its extract closure built.check:type-check-debt: its re-measure leg is a whole-workspace build plus every package's tsc. It was killed at the foreground cap at4da36f554and not re-run.--ranreconciles 66 of 66, 0 UNRUN. CI runs all three.02f40a098:check-changeset-fixed,check:error-code-casingandcheck:filter-alias-parity.--no-inline-config --format jsonover 2 files reports 0 errors and 0 warnings. The repository's eslint config enables no type-aware linting: the resolvedparserOptionsare onlyecmaVersionandsourceType, with noproject. So this diff cannot move any untouched file's verdict.Acceptance notes
object_crud. The attribution also writes that name on avisible: trueverdict that no other layer decided. The new guard keys on the gate inputs, so that path is covered too if it is reachable. Whether any configuration reaches it is NOT MEASURED. The one route tried, an update policy on the master, is refused by the master check first.Generated by Claude Code