fix(plugin-security): compose controlled_by_parent across a chain (#11082)

[os-warren]

Fixes #11082

A controlled_by_parent detail whose master is itself controlled_by_parent was readable and writable org-wide. A two-level chain was enforced at level one and unenforced at level two, with metadata that reads as if it were narrowed — the dangerous direction: unenforced, and indistinguishable from enforced.

Draft, and it stays draft: Clause-② applies (this changes the runtime accept/reject of data operations on a declared security semantic). needs:contract-review remains on the card. This seat does not clear the label, mark ready, arm auto-merge, or merge.

Premise, re-derived

The card was measured on 17.1.0 and plugin-security was heavily edited afterwards by PR #11121. Every load-bearing fact was re-derived on the base commit 10485009a rather than taken from the card: the plugin-sharing opt-out at sharing-service.ts:101, computeControlledByParentFilter at security-plugin.ts:5428, assertControlledByParentWrite at :5545, the read call sites :2717 / :2730 / :3988 and the write call sites :2039 / :2049. The premise holds unchanged.

Two mechanisms, not one

Read. computeControlledByParentFilter composed exactly two halves of the master's read scope, and both answer "no restriction" for a derived master: the RLS half is null (a derived object authors no policy — declaring controlled_by_parent is its policy), and the sharing half is null too, because buildReadFilter opts out of every model that is not private and effectiveSharingModel maps controlled_by_parent to public. Composed null, the master query ran as system with an empty predicate and returned every master row.

Write. A separate mechanism with the same result. The master gate asks canEdit on the master row; checkEdit returns abstain for a public-mapped model, and abstain is not deny — so it answered true for every master row. A read-side fix does not touch this path, so it is pinned separately and ablated separately.

The fix

The read derivation AND-composes the master's own controlled_by_parent filter as a third layer, resolved through the very same method so the recursion cannot drift from the top-level answer. The write gate's three master-edit legs are extracted verbatim into assertMasterRowEditable and run on each hop until a master that governs its own rows is reached. The extraction is a re-indentation on purpose — the parameter is the CbpRelation itself, so the body still reads rel.master: a permission composition must not gain a second copy that can drift from the first.

Every refusal added on the write side is an authorization verdict and keeps #7474's envelope — 403 PERMISSION_DENIED, named for the caller's object and operation. The three non-verdict codes (422 metadata, 404 missing row, 422 null FK) stay scoped to the caller's own detail: telling a caller that "your master record does not exist" about a grandparent would be a false statement about their own write.

This is not a blanket refusal for chained declarations. That would deny the single-level case #5386 fixed and that the card's own measurement shows correct today. It is the load-bearing negative, so it is pinned in both directions in the same suite.

Reproduction table after the fix

Reproduced on the card's chain (crm_quote_line_item under crm_quote under crm_account), with the card's fixture: two accounts owned by another user, one of them shared to the rep through sys_record_share at access_level: edit, plus one the rep owns.

case	expected	measured
rep reads crm_quote (master crm_account)	still narrows	quote_us, quote_own
rep reads crm_quote_line_item	line_jp gone, reachable chain kept	line_us, line_own
rep writes line_jp (its quote unreachable)	DENIED	403 PERMISSION_DENIED
control: detail under a private master	unchanged	[] throughout

⚠️ One row of the dispatch's rendering of this table did not survive re-derivation, and the triage settles it. The dispatch wrote row 2 as "line_own only". That is the card's private-OWD row, not the controlled_by_parent row: line_us hangs off quote_us, which hangs off acct_us, which is shared to the rep at edit — its entire chain is reachable, so denying it is over-denial, which is exactly the failure mode the triage's own negative forbids. The triage comment states the intended answer directly: "line_JP must become unreadable and unwritable for the rep while quote_US-derived rows stay reachable". The implementation follows the triage. Both readings are pinned side by side in the suite (the walk STOPS at the first master that governs its own rows produces line_own alone under a private master), because reading one as the other is precisely how a fix ends up over-denying and still passing every leak test.

Cycle protection and the depth bound

Both fail closed — an empty master set on the read side, a denial on the write side — never to "no restriction", which is the failure being fixed here.

Cycle protection carries the set of objects already being resolved on the branch and refuses to re-enter one. Termination does not depend on the depth bound: the visited set grows strictly over a finite schema registry.

The depth bound is a cost ceiling, not a supported-chain-length statement, and no value for it is derivable from the tree — so the derivation is written down at the constant rather than implied. Measured in this repo, every authored controlled_by_parent object has a chain of exactly one hop (showcase_invoice_line, showcase_expense_line, crm_opportunity_line_item — in each case the master is public_read_write or private, never derived). The consumer that motivated this card needs two. The bound is 8: four times the deepest chain any consumer has asked for, so it cannot be reached by authoring that means anything, while capping the walk at 8 queries — the same order as the per-request cost ADR-0055 already books. At the bound the walk denies and logs the chain it refused; a bound that widened on overflow would simply reintroduce this defect at depth 9.

Both guards are pinned in both directions: a chain inside the bound must still resolve (otherwise the bound is the new blanket deny), and a chain over it must fail closed.

The delegator call site is covered — and pinned

The derivation has three read consumers: the CRUD middleware's caller leg, its ADR-0090 D10 delegator leg (security-plugin.ts:2730), and getReadFilter. The recursion lives inside the shared helper, so all three inherit it — but that is an argument, not evidence, so the delegator leg has its own case. Agent usr_rep reaches {quote_us, quote_own}; delegator usr_other reaches {quote_us, quote_jp}; the delegated read is the intersection, line_us alone. Before the fix the delegator's own derivation was org-wide, so the intersection collapsed to the agent's set and line_own came back too. The write gate's two call sites (:2039, :2049) are covered the same way, through one shared method.

Proof

Ablation signatures were predicted in writing before running, and the two legs are asymmetric — which is what shows the read and write defects are genuinely distinct mechanisms rather than one fix wearing two hats.

leg	predicted	measured
read fix removed (recursion + both guards)	8 red, all read-side	8 red, all read-side; every write case, both controls, level one and the inside-bound case stayed green
write walk collapsed to one hop	5 red, all write-side	5 red, all write-side; every read case, level-one writes and the private-master control stayed green

One prediction was wrong and is reported rather than smoothed over: a first, partial read ablation that removed only the composition and left the recursive call in place reddened 6, not 8. The two logging cases stayed green because they pin the cycle/depth guards, which that partial ablation left running — so those two cases belong to the guard, not to the composition. The full read ablation reddens all 8.

src vs dist: the ablated subject is imported by the suite as a relative specifier (./security-plugin.js), so vitest resolves it to the package's own source and no build sits between the mutation and the executed code. Falsifiable, and falsified in the right direction — both ablation legs changed the result. @objectstack/plugin-sharing is a genuine dist dependency here (this package's vitest config aliases only objectql and driver-sql), and it is unmodified and was rebuilt before every measurement. The subject was restored byte-identical after each leg, verified with git hash-object (ddd6915063775305b439bd9670f5480348a8ca1e), and re-run to a real green verdict.

Rejection cases assert the code and statusCode of the envelope, never a bare "it threw": an unfixed path throwing some other error would otherwise read as a pass.

Adjacent files that encode the current semantics

Both were checked, and neither is falsified, so neither is edited (which also keeps spec-liveness-check.yml out of this PR's gate surface):

• packages/lint/src/validate-security-posture.ts:311 (re-derived) says computeControlledByParentFilter returns RLS_DENY_FILTER for a controlled_by_parent object with no relation to derive from. That branch is untouched and still does exactly that (security-plugin.ts:5506).
• packages/qa/dogfood/test/authz-conformance.matrix.ts:93, the controlled-by-parent conformance row, describes the Cross-cutting (verify/qa coverage): verify --rls cannot reach by-id-write, authz-conformance overclaims 'enforced', and the console permission-matrix editor is read-only by default #7685 single-level evidence. It neither claims nor denies chain composition; this change makes its enforced verdict more true, not false.

Verification

Gate union derived with node scripts/pm/dispatch-gates.mjs (no hand-supplied paths — the script reads the change set from the merge base itself) on the final commit 2d6c89b5d, and every result below is that commit's.

Path-derived, all EXIT=0: check:changeset-gate-self-tests, check:cross-package-test-inputs, check:objectui-changeset, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check-adr-0087-registration, check-changeset-no-major, check-ci-filter-parity, check-empty-changeset, check-plugin-teardown-shape, check-affected-docs.

Convention-triggered, all EXIT=0: check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:type-check-coverage, check:type-check-debt, check:i18n.

Run by hand on top of the derivation, all EXIT=0: check:route-envelope, check:dispatcher-error-vocabulary, check:ratchet-remedy-authority, check:nul-bytes, check:error-code-casing.

⚠️ check:i18n first answered PREREQUISITE NOT MET — the workspace CLI is not built with exit 1. That is not measured, never a pass; the workspace closure was built and it was re-run to a real verdict: check-i18n-bundles: OK (9 package(s) — all bundles in sync). check:type-check-debt has the same prerequisite and was run after the same build: check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured, 1896 raw tsc error(s) total, none above its recorded number. No ratchet baseline was written to, in either direction. plugin-auth's TEST_DEBT over-record was observed and left alone.

Exit codes were captured before any pipe throughout.

Tests: pnpm --filter @objectstack/plugin-security test — Test Files 73 passed (73), Tests 1397 passed (1397); plugin-sharing — 25 passed (25) / 624 passed (624). pnpm --filter @objectstack/plugin-security typecheck exit 0.

⚠️ packages/plugins/plugin-security/tsconfig.json excludes **/*.test.ts, so the package's own typecheck never reads a test file. An explicit test-inclusive program was run instead (a throwaway sibling config, not committed): it reports 11 pre-existing errors in other test files — the positive control proving it really includes tests — and zero in either file this PR touches.

Out of scope

Direction (3), the publish-time lint for a chained declaration, is deliberately not folded in, per triage. Filed as a finding, along with an ADR-0055 amendment: that ADR records "single-level only in v1" as an honest limit and lists transitive chains under Non-goals, which this change closes. docs/adr/ is a governed surface, so per AGENTS.md #13 the amendment is left for the maintainer rather than ridden in on this PR — a mixed diff is not a proportion question.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

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

4 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))
• content/docs/permissions/index.mdx (via SecurityPlugin (symbol))
• content/docs/plugins/packages.mdx (via SecurityPlugin (symbol))
• content/docs/ui/forms.mdx (via SecurityPlugin (symbol))

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

• content/docs/releases/implementation-status.mdx (via SecurityPlugin (symbol))

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

• the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 14 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 2f93d0f53a0ba50770819e068358cc0f25064df5 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 2f93d0f53a0ba50770819e068358cc0f25064df5

⚠️ 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 2f93d0f53a0ba50770819e068358cc0f25064df5 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[claude]

Contract review PASS at CONTRACT_REVIEW_TIER — verdict with the tier reading and basis on the card (#11082), one non-blocking rider there (ADR-0055 amendment must be a tracked card). needs:contract-review cleared.

---

Generated by Claude Code