Skip to content

fix(security): controlled_by_parent master-editability consults the same authored write widener the by-id path does (#8679) - #8758

Merged
qq9340100 merged 2 commits into
mainfrom
claude/issue-8679-controlled-by-parent-master-rls
Aug 14, 2026
Merged

fix(security): controlled_by_parent master-editability consults the same authored write widener the by-id path does (#8679)#8758
qq9340100 merged 2 commits into
mainfrom
claude/issue-8679-controlled-by-parent-master-rls

Conversation

@qq9340100

Copy link
Copy Markdown
Collaborator

Fixes#8679

The divergence

crm_campaign_member-shaped objects — ADR-0055 controlled_by_parent details — route every insert/update/delete through assertControlledByParentWrite, which asks whether the caller may EDIT the master. That gate's record-sharing leg hard-refused on canEdit === falsewithout ever asking whether an app-authored RLS update-widener admits the master row. The by-id write path has asked exactly that since #5493 (merged as PR #6909), where the deferral was installed on the sharing middleware's refusal branch — and deliberately only there, because folding it into canEdit would make the two authorities read each other in a circle.

So one principal, one master record and one operation got two different answers depending on who was asking: the master itself, allowed; a child of it, 403 ... (record sharing) — naming the very layer #6909 had already taught to defer.

The isolation, reproduced

Earned before implementing, on current main, driving the REAL SecurityPlugin middleware and the REAL sharing middleware over the REAL SharingService, wired through the same late binding the kernel uses. One variable: who created the master.

stepmaster by ADMINmaster by the caller
PATCH the master itself, by idallowedallowed
INSERT a childREFUSEDallowed
by-id UPDATE of a childREFUSEDallowed

Measured verdict on the unfixed tree, verbatim:

master marketing camp_admin -> {"ok":true,"message":"written"}
insert marketing camp_admin -> {"ok":false,"message":"[Security] Access denied: insert on
'crm_campaign_member' requires edit access to its master record
(master 'crm_campaign' not editable by this user (record sharing))"}

The first line is the witness that makes the second a divergence rather than a policy question: the platform already permits this principal to edit that exact master row.

The fix

The record-sharing leg now defers before it hard-refuses, consulting the SAME composition:

  • the verdict comes from checkAuthoredRowWrite — the method SharingService.probeAuthoredRowWrite passes straight through to — so the answer here is byte-for-byte the one a direct by-id write of that master would get. There is no second copy to drift, which matters because a duplicated permission composition is how these two paths diverged in the first place;
  • the operation asked is update, matching the two legs already above it. This gate's subject is edit access to the master, never the detail's own verb (deleting a child does not delete the master, and insert is not an AuthoredRowWriteOperation at all);
  • admit retracts only this leg's refusal. The object-level update grant and the master's own write-RLS leg run first and still refuse on their own terms — mirroring the by-id path, where an admit hands the row to the pre-image gate rather than authorizing anything;
  • every other outcome — abstain, no authored policy, a check-only policy, a principal-less or delegated context, a throwing probe — leaves the refusal byte-for-byte untouched. The method is fail-closed in the abstain direction and swallows its own throws, so no failure mode here can widen access.

This is not a relaxation, and the tests are built so it cannot become one. Ruling: reaching parity by widening what counts as master-editability beyond #6909's composition would be a permission-boundary expansion. Nothing here widens that definition — the same question is asked at a second call site.

Acceptance pins both directions

16 cases on one fixture. Every positive is mirrored by a negative on the same route with the same payload, differing only in the app-authored declaration:

  • the RLS-widened master permits the derived INSERT and the derived by-id UPDATE;
  • a principal with no widener and no share is still refused, same route, same shape;
  • a select-only authored policy admits no write, so the refusal stands (this separates "consults the authored WRITE widener" from "notices the caller authored something");
  • a principal-less context and a cross-tenant context are both abstain — Layer 0 still stands under the widest authored predicate;
  • the deferral is a REFUSAL-branch step: a satisfied sharing leg never consults it, pinned with a spy, which also pins that the probe is asked about the MASTER for update.

A transferred master (created by the caller, since handed to someone else) keeps the record-sharing leg itself pinned live: its write RLS admits via the platform ownership floor while record sharing refuses because the owner is another user, so that refusal can only come from the leg this PR edits. Deleting that leg outright would otherwise leave the whole suite green. An edit-level share admits the same row; a read-level share still refuses it.

Reverse verification

The expected direction was stated before running: reverting the fix should turn red exactly the cases that depend on the deferral, and leave every negative green. Observed exactly that — 4 red, 12 green, and precisely the 4 predicted:

x INSERT: a child under the RLS-widened master is permitted
x UPDATE: a by-id child update under the RLS-widened master is permitted
x the deferral asks about the MASTER, for `update`, whatever the detail's own verb is
x INVARIANT: direct master edit and derived child write agree, row by row
Tests 4 failed | 1152 passed (1156)

The negatives passing in BOTH states is the point: they pin the refusal, not the fix.

Scope

packages/plugins/plugin-security/src/** only, as dispatched. No file in packages/core, plugin-auth, plugin-sharing or packages/spec is touched — plugin-sharing was read to mirror the composition, never edited.

security/explain is deliberately unchanged: the card uses it as the platform's own contradicting verdict, so it is the witness, not the defect.

Verification

main moved twice during this card; it was merged and everything below re-measured at the merged head rather than reported green over a stale tree.

All of the following ran at 075477ee2, the head of this branch, after the final commit.

pnpm --filter @objectstack/plugin-security test 61 files, 1156 passed
pnpm --filter @objectstack/plugin-security typecheck tsc --noEmit, clean
pnpm --filter @objectstack/dogfood test 111 files, 779 passed, 3 skipped
pnpm --filter @objectstack/rest test 116 files, 1921 passed

The dogfood suite resolves the code under test from each package's built dist/, so the full workspace closure was rebuilt at the merged head before it ran — the run covers this change rather than a pre-merge artifact.

Gate union, all PASS at 075477ee2:

check:nul-bytes check:changeset-gate-self-tests
check:test-source-alias check:objectui-changeset
check:type-source-resolution check:query-options-erasure
check:authz-resolver check:i18n
check:cross-package-test-inputs check:type-check-coverage
check:engine-double-contract check:type-check-debt (--re-measure: 33 entries, none above ceiling)
check:error-code-casing check-adr-0087-registration / -changeset-no-major / -empty-changeset

The families were re-derived against this PR's actual changed paths with scripts/pm/dispatch-gates.mjs; check:engine-double-contract and check:error-code-casing are convention-triggered additions run beyond the dispatch list. The new test file declares a read-only engine double with no write verb, so it adds nothing to the double ledger.

Out of scope, filed separately

While isolating row 3 I measured a second, independent refusal on this route: the detail's OWN owner_only_writes ownership floor refuses a by-id update of a controlled_by_parent child created by another user, at the pre-image gate, before the master gate runs — and it is not droppable there, because checkEdit returns abstain for a controlled_by_parent object before it ever reaches the modifyAllRecords branch. A Modify-All-Data admin is refused too. That is a different gate and a different guard, so it is not touched here; it is filed rather than fixed. The acceptance cases above hold the detail's floor constant so this PR's verdicts are attributable to the master gate alone.

Filed as #8757 (unassigned, unlabeled for triage). #8757 is not addressed here.

Out of scope and untouched: #8688 remains open — it is this card's hard serial (same file, same guard, its own acceptance test), and nothing in this PR moves the missing-master envelope it is about.

Generated by Claude Code


Generated by Claude Code

…uthored write widener (#8679)
`assertControlledByParentWrite`'s record-sharing leg hard-refused on
`canEdit === false` without asking whether an app-authored RLS update-widener
admits the master row. The by-id write path has asked exactly that since #5493
(PR #6909), where the deferral was installed on the sharing middleware's
refusal branch — so one principal, one master record and one operation got two
different answers depending on whether the question was asked directly or on
behalf of a child.
The gate now consults the SAME composition: `checkAuthoredRowWrite`, the method
`SharingService.probeAuthoredRowWrite` passes straight through to, asked for
`update` to match the two legs already above it. `admit` retracts only this
leg's refusal; the object-level grant and the master's write-RLS leg still
refuse on their own terms, and every other verdict leaves the refusal byte-for-
byte untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn
@vercel

vercelBot commented Aug 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 14, 2026 6:56pm

Request Review

@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security.

12 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx(via @objectstack/plugin-security)
  • content/docs/kernel/runtime-services/sharing-service.mdx(via @objectstack/plugin-security)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/plugin-security)
  • content/docs/permissions/access-recipes.mdx(via packages/plugins/plugin-security)
  • content/docs/permissions/authorization.mdx(via @objectstack/plugin-security)
  • content/docs/permissions/explain.mdx(via @objectstack/plugin-security)
  • content/docs/permissions/permissions-matrix.mdx(via packages/plugins/plugin-security)
  • content/docs/permissions/sharing-rules.mdx(via @objectstack/plugin-security)
  • content/docs/plugins/index.mdx(via @objectstack/plugin-security)
  • content/docs/plugins/packages.mdx(via @objectstack/plugin-security)
  • content/docs/ui/audience-based-interfaces.mdx(via packages/plugins/plugin-security)
  • content/docs/ui/dashboards.mdx(via @objectstack/plugin-security)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/plugin-security)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@qq9340100
qq9340100 marked this pull request as ready for review August 14, 2026 19:17
@qq9340100
qq9340100 added this pull request to the merge queueAug 14, 2026
Merged via the queue into main with commit cf0d902Aug 14, 2026
27 checks passed
@qq9340100
qq9340100 deleted the claude/issue-8679-controlled-by-parent-master-rls branch August 14, 2026 19:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@qq9340100@claude