Skip to content

Commit 6171331

Browse files
os-warrenclaude
andauthored
fix(plugin-security): compose controlled_by_parent across a chain (#11082) (#11183)
A `controlled_by_parent` detail whose master is ITSELF `controlled_by_parent` was readable and writable org-wide. #5386 made the derivation fold in the master's ownership and share grants, but it does not recurse, and both halves it composes answer "no restriction" for a derived master: the RLS half is null (a derived object authors no policy) and the sharing half is null too (`buildReadFilter` opts out of every non-`private` model, 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. The write half failed through a SEPARATE mechanism: the master gate asks `canEdit` on the master row, `checkEdit` returns `abstain` for a `public`-mapped model, and `abstain` is not `deny`. Read side: the master's own derivation is now AND-composed as a third layer. Write side: the three master-edit legs are extracted verbatim and run on each hop until a master that governs its own rows is reached; every added refusal keeps #7474's `403 PERMISSION_DENIED` envelope, named for the caller's own object and operation. Not a blanket deny: a detail whose whole chain is reachable stays reachable, and the single-level case is byte-for-byte unchanged. Cycle protection and a depth bound of 8 both fail CLOSED, never to "no restriction". Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1ddda1d commit 6171331

3 files changed

Lines changed: 928 additions & 6 deletions

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
"@objectstack/plugin-security": minor
3+
---
4+
5+
fix(plugin-security): `controlled_by_parent` composes across a chain — a child whose master is itself derived is no longer readable and writable org-wide (#11082)
6+
7+
**BREAKING** access tightening, shipped as `minor` under the repo's
8+
launch-window convention. It denies reads and writes that previously
9+
succeeded — which is the whole point: they were never authorized by any
10+
declaration, and the app author could not tell.
11+
12+
`controlled_by_parent` (ADR-0055) resolves a detail's access from its master.
13+
#5386 made that resolution fold in the master's ownership and its
14+
`sys_record_share` grants, not just the master's RLS policies. It did not
15+
recurse, and both halves it composes answer "no restriction" for a master that
16+
is **itself** `controlled_by_parent`:
17+
18+
- the RLS half is `null`, because a derived object authors no policy —
19+
declaring `controlled_by_parent` *is* its policy;
20+
- the sharing half is `null` too: `plugin-sharing`'s `buildReadFilter` opts out
21+
of every model that is not `private`, and `effectiveSharingModel` maps
22+
`controlled_by_parent` to `public`.
23+
24+
Composed: `null`. The derivation's master query then ran as **system** with an
25+
empty predicate and returned **every master row**, so a two-level chain was
26+
enforced at level one and org-wide at level two. The write half failed through
27+
a separate mechanism with the same result: the master gate asks `canEdit` on
28+
the master row, `checkEdit` returns `abstain` for a `public`-mapped model, and
29+
`abstain` is not `deny` — so it answered `true` for every master row.
30+
31+
Both halves now walk the chain. The read derivation composes the master's own
32+
`controlled_by_parent` filter as a third layer, and the write gate runs its
33+
three master-edit legs on each hop until it reaches a master that governs its
34+
own rows. The master set is therefore point-for-point equal to what a direct
35+
read of the master returns, at every level, which is the equality #5386
36+
established for one level.
37+
38+
This is **not** a blanket refusal for chained declarations: a detail whose
39+
whole chain is reachable stays readable and writable, and the single-level case
40+
is unchanged. Two guards bound the walk and both fail **closed**, never to "no
41+
restriction": a metadata cycle is refused, and so is a chain deeper than 8
42+
links (a cost ceiling, not a supported-length statement — termination is
43+
already guaranteed by the cycle guard).
44+
45+
What an app may observe: a detail under a `controlled_by_parent` master that
46+
was reachable before is now reachable only if the caller can reach the whole
47+
chain above it. Apps whose masters are `private`, `public_read` or
48+
`public_read_write` — every `controlled_by_parent` object authored in this
49+
repo — are unaffected.
50+
51+
<!-- adr-0087: not-required (no-migration-prescription) An access-derivation fix inside plugin-security. No spec surface is renamed, retired or re-shaped, no authorable metadata key changes meaning, and there is nothing for `objectstack migrate meta` to rewrite — a chained declaration that was silently unenforced is now enforced as it always read. -->

0 commit comments

Comments
 (0)