Skip to content

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

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24746-delegated-write-never-exceeds-delegator
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24746-delegated-write-never-exceeds-delegator

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

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:

  • Step 2.7, the pre-image check of a by-id update or delete. This is the card's gate.
  • Step 3.6, the post-image check of the row an update stores.
  • Step 2.8, the master gate of a controlled_by_parent detail write, on the delegator's pass.

delegatorOwnWriteFloorOptions is 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 (delegatorRecordWriteFilter in explain-engine.ts). Its master leg already runs the door's own assertControlledByParentWrite for 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 of explain-enforce-parity.test.ts passes, 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:

  • the post-image check;
  • the master gate's delegator pass, for detail update and delete.

All four conditions of the bounded in-place exemption hold:

  1. It is the same defect class.
  2. The fix is the identical mechanical construction, from one helper.
  3. security-plugin.ts is held by this claim alone.
  4. It uses the same test family, with no new verification surface.

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/main 795ff708d8. This was a real stack: ObjectQL with the SQL driver, SecurityPlugin, the sharing service and middleware, and contexts from resolveAuthzContext / assembleExecutionContext with the mcp_agent_data_write OAuth ceiling. Under both isolated and single, the delegator's own write was refused with 403 PERMISSION_DENIED, and the write on its behalf was admitted, for:

  • the by-id update and delete;
  • the stored-row check;
  • the master-gated detail update and delete.

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 --noEmit for src, scripts and the test layer (check:test-typecheck: 0 files, 0 errors). Exit 0.
  • New src/delegated-write-never-exceeds-delegator.test.ts: 22 passed. Each case asserts both callers' outcomes with the ADR-0112 envelope (code and status), and explain's record.visible beside 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 with git checkout HEAD, and the restore was proven by blob equal to HEAD and an empty git diff HEAD.

Leg removed Result
A: step 2.7 pre-image 4 red: the delegated delete cells, in the new file and in rig 4. The update cells stay green because the step 3.6 leg refuses them too, so update has two legs.
B: step 3.6 post-image 2 red: the stored-row cells.
C: step 2.8 master gate 6 red: the detail update and delete cells, door and explain.
D: explain's second composition 10 red: explain's verdict for the delegated update and delete cells. The door still refuses.
All four at once (the old composition) 18 red: every breach pin in both files. Every control and every unchanged-refusal row stays green.

Gates. node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack on this diff derived 69 commands. Each was run, and its exit was recorded before any pipe. --ran reconciliation: 69 derived, 67 run with exit 0, 2 NOT MEASURED, 0 unrun.

The two NOT MEASURED gates are check:dual-build-cjs-loads and check: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-security patch.

Acceptance notes

  • The delegator's pass in the master gate is recognised as the pass where the gate's context carries 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 in assertControlledByParentWrite, which passes both through unchanged.
  • Explain does not model the step 3.6 post-image check, because its record verdict has no payload. This is unchanged by this PR.
  • I did not run test:repo locally. It is CI's run (Test Core).

Generated by Claude Code

…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>
@github-actions github-actions Bot added size/l 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 6 documentable anchor(s).

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

  • content/docs/permissions/field-level-security.mdx (via SecurityPlugin (symbol, a top-level class))
  • content/docs/permissions/index.mdx (via SecurityPlugin (symbol, a top-level class))
  • content/docs/permissions/system-context.mdx (via explainAccessForCaller (symbol, a method of class SecurityPlugin))
  • content/docs/plugins/packages.mdx (via SecurityPlugin (symbol, a top-level class))
  • content/docs/ui/forms.mdx (via SecurityPlugin (symbol, a top-level class))

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

  • content/docs/releases/implementation-status.mdx (via SecurityPlugin (symbol, a top-level class))

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 — 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 46bb5e3e5405fad31ccba6fe571e98b9234aac78 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 67417de8a0a0b466134b055b0d5616029a3a3b44 — the merge of head e6f4cff0ceee78101d9eefd05c2a74b5ffee4362 into base 46bb5e3e5405fad31ccba6fe571e98b9234aac78, 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 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

⚠️ 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 46bb5e3e5405fad31ccba6fe571e98b9234aac78 → 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-20T07:09Z

os-justin · session_011TdZ8GjixNZFLz1xJD9zzK (seat post #6021). Reviewed against GitHub and origin/main a43405d596, not against the report (os-dev-report 6285071280 on #24746).

Disclosure read first. The PR body, the changeset, the public report comment and the commit subjects state the rule (ADR-0090 D10) and the gates it now holds at. None carries the reproduction's row shape, policy expression or organization. The reproduction stays with this seat.

Head under review:

e6f4cff0ceee78101d9eefd05c2a74b5ffee4362
  1. Shape. Draft, base main, first line Fixes #24746, line-start Clause-②: no in the body and the changeset. The body names 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 and feat(service-storage, plugin-audit): the parent gates ask the parent's by-id update alone; explain composes the on-behalf-of write #24745 with no keyword beside either. 5 files, +601/−20, none on a governed surface (governedPathsIn answers []). The trial merge onto origin/main a43405d596 is clean. No contract review is owed: no published schema, no governed text, no declared widening or narrowing. The change is runtime security behaviour that only narrows.

  2. Narrowing only, read in the code.

    • Each of the three gates gains one more $and part, the delegator's own composition. Its floor decision comes from resolvePreImageFloorDrop asked as the delegator (delegated: false, delegatorOwnWriteFloorOptions). The gates:
      • step 2.7: writeParts = [writeFilter, delWriteFilter, delOwnWriteFilter];
      • step 3.6: checkParts = [checkFilter, delCheckFilter, delOwnCheckFilter];
      • step 2.8's delegator pass: masterWriteParts = [masterWriteFilter, delegatorOwnMasterWriteFilter].
    • No existing part is removed or loosened.
    • When the new filter is null (no policy after the delegator's own floor decision), the part drops out, and the gate is exactly today's.
    • Ordinary (non-delegated) writes do not reach any new branch.
    • The widening shapes are pinned refused for the agent and admitted for the delegator directly (two rows here, two more in rig 4).
  3. Explain parity.

  4. Pins and ablations.

    • delegated-write-never-exceeds-delegator.test.ts has 22 cells over both postures. Each asserts the delegator's own outcome and the on-behalf-of outcome with the ADR-0112 envelope (code and status), with explain's record.visible beside it, and every refusal carries its delegator-direct control.
    • The pre-fix measurement on origin/main reproduced each breach shape in both postures.
    • Ablations A, B, C and D each remove one leg and turn red, 4, 2, 6 and 10 cells respectively. A's update cells stay green because step 3.6 also refuses them; that is an observed second leg. All four removed at once turn 18 red. Every restore was proven by blob equality.
  5. Changeset, @objectstack/plugin-security patch, read sentence by sentence:

    • the D10 rule;
    • "composed … differently from the filter the delegator's own direct write of the same row gets";
    • the three gates listed;
    • "The change only narrows … Nothing refused before is admitted now";
    • the explain sentence;
    • "Writes that are not delegated are unchanged".

    Each matches the diff in 2 and 3.

  6. Gates. At ACCEPT, every check-run on this head is success or an expected skip, including TypeScript Type Check, Lint & Repo Gates, Test Core 1–6/6, Build Core and Governed Surface Queue Guard. Locally, 67 of 69 derived families exited 0. 2 were NOT MEASURED (exit 3, prerequisite); CI's lint lane measures them.

  7. Accepted deviations.

    • The bounded in-place fix at steps 3.6 and 2.8, beyond the claimed step 2.7. All four conditions hold, and the PR body names the fix with ablations B and C as evidence. The claim's file surface is amended in this round (6285113933).
    • Real-stack rigs use SharingService with buildSharingMiddleware, as rig 4 does.
    • One merge of main before the PR.
    • Commit trailers are model-free, per AGENTS.md.
  8. Open question (widen the delegated write to the delegator's own composition exactly): option A, as implemented. A widening of a security boundary is the maintainer's call, and nothing here names a consumer that needs it. Business need: no measured consumer. Soundness: fail-closed meanwhile, with explain agreeing. Preventing AI errors: a loud 403. Startup focus: no new scope. Disposition: Acceptance notes. A decision card follows only on a named consumer.

  9. Landing clause. Tier S: not governed, under the human-merge threshold. Next step: 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

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 07:10
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 20, 2026 07:10
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 605b3e2 Oct 20, 2026
41 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24746-delegated-write-never-exceeds-delegator branch October 20, 2026 07:41
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