Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-security): a public_read_write OWD opens row-level writes, not just the creator's (#8023) - #8072
Conversation
… not just the creator's (#8023) The platform's wildcard `owner_only_writes` floor (object '*', operation update, `created_by == current_user.id`, positions ['org_member']) stayed composed into Layer 1 on objects declaring `sharingModel: 'public_read_write'`, because the by-id write pre-image gate only lets ISharingService REPLACE the floor on a positive `allow` and a public object makes the service abstain. Net effect: the OWD declared 'everyone reads and writes' and the runtime enforced 'only the creator writes' — a declared-but-unenforced security property on a published surface. The floor is now conditioned on the object's DECLARED OWD at collection time, before #7665's derive-from-select branch, so the write class falls through to the caller's select narrowing and by-id write visibility (#7792) is preserved. Only `update` is opened (owner_only_deletes survives), only the canonical `public_read_write` spelling qualifies (controlled_by_parent and an unset model on a system object do not), and only the PLATFORM's floor is dropped — an app-authored policy with the same predicate still reaches the compiler. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 12 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#8023
An object declaring
sharingModel: 'public_read_write'promised "everyone can see and edit" and delivered "everyone can see, only the creator can edit".Premise: reproduced, with one correction to the card
The card's mechanism is confirmed, and so is the PM's correction of its file attribution (the shipped seed is
objects/default-permission-sets.ts:415;platform-ownership-policies.tsonly documents the floor). Reproduced end-to-end against the real showcase app before any edit:POST /api/v1/security/explainreproduced the card's other measurement exactly — therlslayer reportingnarrowsforoperation: 'update'on an object with zero authored RLS, whilereadreportednot_applicable.One precondition the card does not state, and it matters for anyone re-running this. The floor is positions-gated to
org_member, which a principal holds only through asys_memberrow. In an org-less harness a fresh sign-up getspositions: ['everyone'], the floor never applies, and the PATCH answers 200 — the defect is invisible. My first fixture was org-less and passed on the broken build; that is recorded in the test file's header, because a fixture that cannot fail is worse than no fixture. The reproduction above stands up the default organization the membership reconciler binds new users to (ADR-0093 D1), which is what a real deployment boots into.The cause
member_defaultshipsowner_only_writes(object'*', operationupdate,created_by == current_user.id, positions['org_member']). The by-id write pre-image gate letsISharingService's tri-state verdict replace that floor, but only on a positiveallow— and on a public object the service abstains, correctly, because record sharing does not enforce there. An abstain keeps the floor, so the floor became the object's only row-level write gate and silently overrode its OWD.The fix, and why it is where it is
An object whose author declared
public_read_writeno longer inherits the wildcardupdatefloor at collection time, insidecomputeLayeredRlsFilter.The placement is the fix, not a style choice. Removing the floor there leaves the update class empty, so #7665's derive-from-select branch then supplies the write scope from the caller's SELECT narrowing — which is exactly what preserves #7792's by-id write visibility. Removing it at the existing
dropPlatformOwnershipFloorfilter further down would instead compile Layer 1 tonulland hand back an ungated by-id write: the hole #7665 closed, reopened by a fix for a different bug.Three boundaries are deliberate, each with a control case in the test:
deleteis unchanged.public_read_writeis "see and edit"; the legacyfullalias that also covered transfer/delete was refused a mechanical conversion for being wider than it (ADR-0090 D4,spec/conversions/registry.ts).owner_only_deletesstill refuses a non-creator delete.plugin-sharing'seffectiveSharingModel, which foldscontrolled_by_parentand an unset model on a system object into the same'public'bucket. A detail derives access from its master; an unset model on asys_*table is a legacy default, not a statement. Opening either would have handed members cross-creator writes on the platform's identity tables. An unresolvable schema fails closed.I did not take the composition-semantics route (reinterpreting what an abstain means):
abstaincovers three distinct facts — public OWD, no owner field, bypass-listed internal — and only the first says anything about writes being open. Widening the abstain would re-open #5492's E2 measurement on the other two.plugin-sharingis untouched.Verification
Acceptance evidence is at the layer the harm is at — HTTP
PATCHthrough the real REST stack — in a new dogfood fixture whose three objects are byte-identical apart from their OWD.Predict-then-mutate ablation. The predicted red/green split was written down before running, then the decision-site hunk alone was removed (helper and plumbing left in place). Measured result matched the prediction exactly: 2 failed / 7 passed, failing set
{[A public_read_write], [D #7792]}, with[D]failing on its first assertion (the in-scope PATCH — the narrowed persona is a non-creator too), precisely as predicted.[C public_read],[C private],[B object gate]and[E delete floor]stayed green on both sides; that invariance is the security claim rather than a coverage gap.Acceptance criteria, each with a positive pin (rejection cases assert
codeand status, never a bare throw):edit:truepersona PATCHes a row it did not create[A]— 403 -> 2xx, value persisted[B]— 403PERMISSION_DENIED, asserts the object-level sentence is present and the record-level one is notprivate/public_read[C]x2 — 403PERMISSION_DENIED, record-level sentence, nothing persisted[D]— out-of-select-scope row unreadable and unwritable; in-scope row writable[E]— non-creator DELETE still refused, row survives[F]—explainupdate flipsnarrows->not_applicable, whiledeleteon the same object still reportsnarrowsGates re-derived for the final file surface;
check:authz-resolverfrom the dispatch no longer matches becauseobjects/default-permission-sets.tsis not edited.Not settled here
The regression window
92f26f75 -> b602d536is not repeated as fact — no build ablation at the parent commit was run, so the attribution remains the card's mechanism evidence rather than a differential build.The card's cross-object confirmation (a
sys_user_positionrow readable but not PATCHable by a non-creator) is not fixed:sys_*objects reach the same abstain through the unset-model path, which this change deliberately leaves closed. Whether platform system tables should open cross-creator writes is a separate contract question and would be a widening, not a restoration.Generated by Claude Code