Repository navigation
fix(plugin-security): an on-behalf-of by-id write never admits a row its delegator's own by-id write refuses (ADR-0090 D10) - #24792
Conversation
…elegator's own write ADR-0090 D10: a delegated principal never exceeds its delegator. Each delegated row gate of a by-id write (the step 2.7 pre-image, the step 3.6 post-image check and the master gate's delegator pass) now ANDs the delegator's own direct composition of that write beside the composition the delegated path already used. The AND only narrows. Explain's record verdict composes the same two filters for the delegator. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
…eeds the delegator's own write Real-stack pins under `isolated` and `single`: the delegated by-id update, delete, stored row and master-gated detail write are refused wherever the delegator's own write is, with the ADR-0112 envelope and the delegator's own request as the control; rows the delegator's own write admits and the delegated write has always refused stay refused. Explain's record verdict is asserted beside the door, and the parity table gains the matching rig 4 rows. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
…legated-write-never-exceeds-delegator
📓 Docs Drift CheckThis PR changes 1 package(s): 5 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 67417de8a0a0b466134b055b0d5616029a3a3b44 && git checkout 67417de8a0a0b466134b055b0d5616029a3a3b44
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 46bb5e3e5405fad31ccba6fe571e98b9234aac78 e6f4cff0ceee78101d9eefd05c2a74b5ffee4362 && git checkout -B drift-repro 46bb5e3e5405fad31ccba6fe571e98b9234aac78 && git merge --no-ff e6f4cff0ceee78101d9eefd05c2a74b5ffee4362
node scripts/docs-audit/affected-docs.mjs --json 46bb5e3e5405fad31ccba6fe571e98b9234aac78
|
席位核对 · ACCEPT ·
|
Fixes #24746
Clause-②: no
What was wrong
ADR-0090 D10 says an on-behalf-of (delegated) principal never exceeds its delegator. The by-id write's pre-image check (
security-plugin.ts, step 2.7) already composes the delegator's own write filter beside the principal's. But it composed that filter differently from the filter the delegator's own direct write of the same row gets. Where an app-authored row-level write policy bounds the delegator, an OAuth agent acting for that delegator could therefore be admitted for a row that the delegator's own by-id write refuses. Since #24745 (the #24646 card), explain's on-behalf-of write verdict composes the same delegated path, so it gave the same admit.The fix, in rule terms
Every row gate that judges a delegated by-id write now also composes the delegator's filter as the delegator's own direct write of that row gets it, and requires both compositions to pass:
updateordelete. This is the card's gate.updatestores.controlled_by_parentdetail write, on the delegator's pass.delegatorOwnWriteFloorOptionsis the one helper. It returns the composition knobs the delegator's own direct by-id write of the row would get, asked as the delegator and not as the delegated context. The three gates each AND that composition beside the one they already used.The change only narrows. A write is admitted only where today's composition admits it AND the delegator's own composition admits it. No row refused before is admitted. Writes that are not delegated, bulk writes, and every other gate are unchanged.
Explain.
security.explain's on-behalf-of record verdict composes the same two filters (delegatorRecordWriteFilterinexplain-engine.ts). Its master leg already runs the door's ownassertControlledByParentWritefor both principals, so the step 2.8 change reaches explain without a second edit. Explain's record verdict still matches the door: every rig 4 row ofexplain-enforce-parity.test.tspasses, and so do 14 new rig 4 rows for this card.Bounded in-place fix: steps 3.6 and 2.8
The claim's file surface names step 2.7. I measured on
origin/main(795ff708d8), in both postures: the same rule broke at two more gates of the same delegated write, for the same reason, in the same file:updateanddelete.All four conditions of the bounded in-place exemption hold:
security-plugin.tsis held by this claim alone.The evidence is the pins below and ablations B and C. The claim's declared file surface needs the same amendment. That is the seat's write, and it is named in the report.
Not changed, deliberately
There are shapes where the delegator's own direct write admits a row that the delegated write has always refused. Those rows stay refused. Two such rows are pinned as unchanged in the new file, and rig 4 already pins two more. Making the delegated write match the delegator's own composition exactly there would WIDEN a security boundary. That is the maintainer's call, so it is not done here. It goes to the seat as an open question.
This PR does not touch
plugin-sharing,rls-compiler.ts,packages/spec/**or the shipped permission sets.Evidence
Measured first, on
origin/main795ff708d8. This was a real stack: ObjectQL with the SQL driver,SecurityPlugin, the sharing service and middleware, and contexts fromresolveAuthzContext/assembleExecutionContextwith themcp_agent_data_writeOAuth ceiling. Under bothisolatedandsingle, the delegator's own write was refused with 403PERMISSION_DENIED, and the write on its behalf was admitted, for:updateanddelete;updateanddelete.Explain admitted those writes as well. After the fix, every one of them is refused for both callers. Explain matches the door on each, and the controls are unchanged.
Tests, at head
e6f4cff0ce:pnpm --filter @objectstack/plugin-security test: 191 files, 3972 passed, 45 skipped. Exit 0.pnpm --filter @objectstack/plugin-security typecheck:tsc --noEmitforsrc, scripts and the test layer (check:test-typecheck: 0 files, 0 errors). Exit 0.src/delegated-write-never-exceeds-delegator.test.ts: 22 passed. Each case asserts both callers' outcomes with the ADR-0112 envelope (codeandstatus), and explain'srecord.visiblebeside the door. It covers both postures, and every refusal has the delegator's own request as its control.src/explain-enforce-parity.test.ts: 164 passed. The 14 new rig 4 rows are labelled#24746.Ablations. Each removes one leg's new composition through
scripts/ablation-replace.mjs. The anchor must hit once, and the blob must change on disk. Each run was restored withgit checkout HEAD, and the restore was proven by blob equal to HEAD and an emptygit diff HEAD.deletecells, in the new file and in rig 4. Theupdatecells stay green because the step 3.6 leg refuses them too, soupdatehas two legs.updateanddeletecells, door and explain.updateanddeletecells. The door still refuses.Gates.
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackon this diff derived 69 commands. Each was run, and its exit was recorded before any pipe.--ranreconciliation: 69 derived, 67 run with exit 0, 2 NOT MEASURED, 0 unrun.The two NOT MEASURED gates are
check:dual-build-cjs-loadsandcheck:i18n. Both exited 3, PREREQUISITE NOT MET, because they read a whole-repo build that this container does not have. CI runs them.The changeset is
.changeset/24746-delegated-write-never-exceeds-delegator.md, a@objectstack/plugin-securitypatch.Acceptance notes
contextcarries no delegation link while the operation context does. This is the same reading the existing floor exclusion uses there, which reads the link off the operation context. The method's only caller is the chain walk inassertControlledByParentWrite, which passes both through unchanged.test:repolocally. It is CI's run (Test Core).Generated by Claude Code