Repository navigation
fix(plugin-security)!: collectRLSPolicies applies an object's declared default row scope when no held set carries a row policy for it - #24451
Conversation
… default row scope on an empty collection Claude-Session: https://claude.ai/code/session_01Nk17Z26X5DB59iQztDBADH Co-authored-by: Claude <noreply@anthropic.com>
…tion point Claude-Session: https://claude.ai/code/session_01Nk17Z26X5DB59iQztDBADH Co-authored-by: Claude <noreply@anthropic.com>
…e reader does Claude-Session: https://claude.ai/code/session_01Nk17Z26X5DB59iQztDBADH Co-authored-by: Claude <noreply@anthropic.com>
…e pages and liveness counts Claude-Session: https://claude.ai/code/session_01Nk17Z26X5DB59iQztDBADH Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nk17Z26X5DB59iQztDBADH Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nk17Z26X5DB59iQztDBADH Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 146 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9bd3da7ba0ffa9b63420fd0e67f544980c18039c && git checkout 9bd3da7ba0ffa9b63420fd0e67f544980c18039c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 26501ae24feed649dade94c4a5cc2d6f4a7e9d82 7702f3ca2b0c5899763cca4b8fa96e84de0d1fbb && git checkout -B drift-repro 26501ae24feed649dade94c4a5cc2d6f4a7e9d82 && git merge --no-ff 7702f3ca2b0c5899763cca4b8fa96e84de0d1fbb
node scripts/docs-audit/affected-docs.mjs --json 26501ae24feed649dade94c4a5cc2d6f4a7e9d82
|
REWORK · patch round 1 ·
|
…ger's omit reason say the reader is in Claude-Session: https://claude.ai/code/session_01Nk17Z26X5DB59iQztDBADH Co-authored-by: Claude <noreply@anthropic.com>
…s-object-default-scope
Contract reviewServed-tier: Net diff judged: ① Derived judgments1. The collection point ( 2. The six call sites, each read in the cited code — RIGHT at every one.
3. Superuser bypass — RIGHT, with one boundary named. At 4. Fail-closed on a 5. Compiler mark and explain — RIGHT, and no public surface moves. 6. The registry read — RIGHT. 7. Spec texts ( 8. Liveness ledger — RIGHT. 9. 10. Pins against ruling item 6 — complete for this card. Five of the ruling's six pins are here (no-baseline The DELIBERATE CORRECTION —
|
席位核对ACCEPT · The contract review of record is PASS
Generated by Claude Code |
Fixes #24340
Clause-②: no (narrowing) — the narrowing is real only for a principal with no platform baseline whose permission sets carry no row policy for an object that declares a default (0 measured producers: no object in this tree declares
defaultRowScope, ruling E′ item 7).What changed
collectRLSPolicies(packages/plugins/plugin-security/src/security-plugin.ts), the one collection every read, write and explain door composes from, now applies an object's declaredObjectSchema.defaultRowScope(#24338) when the held sets' collection for that object and operation comes out empty. This is #24164 ruling E′ item 2, at the one collection point:objectimplied, built byobjectDefaultRowScopePolicy(new,object-default-row-scope.ts) and marked by identity.The shipped sets are not edited (
default-permission-sets.tsuntouched), so their behaviour and explain output do not move; the platform-objects declarations and the registry rule are #24341's.What "carries a row policy" means (measured, then pinned)
It is exactly what the collection already means by empty: the held sets' policies after the ADR-0105 D3 admission,
enabled, thepositionsdomain and'*'matching (RLSCompiler.getApplicablePolicies). So:'*'policy counts (the default is not applied);updatereads under its default);An object-level reading ("any held policy naming the object, any operation") was measured and rejected:
member_defaultships the wildcard ownership floor (owner_only_writes/owner_only_deletes, object'*'), so that reading would switch the default off for every baseline member on every object. The two readings agree on every declaration in sight: the identity defaults #24341 is to declare mirror the operations of the set literals they copy. The ADR-0095 D1 note words it as Layer 1 silence, which is the collection's emptiness.The default takes the path a held policy takes
From the admission on, the default is a held policy: it passes the same ADR-0105 D3 admission (
admitCollectedPolicy, extracted from the loop unchanged, so a default spelling a shipped tenant policy is stripped exactly as its set copy is, by the(object, name, using)provenance key), the samegetApplicablePoliciesfilter (its ownoperation,positions,enabled), and the same compiler (using,check).Call sites (every one reads the same collection)
resolvePreImageFloorDrop(:5326).some(isPlatformOwnershipFloorPolicy): is the platform floor in playcheckAuthoredRowWrite(:5448)using; empty answersabstainadmitfor a row it admits. Pinned.computeLayeredRlsFilterLayer 1 (:7222)getReadFilter/ analytics, explain; empty meanslayer1 = null, no filter:7369)update/deletewith no write-classusingre-collectsselectselectdefault scopes the derived write exactly as a held select policy does.computeWriteCheckFilter(:7657)writeCheckPolicies; empty means no checkcheck(or itsusing, by the schema's default) judges the post-image. Pinned: handing your own row away is refused.assertControlledByParentWritemaster leg (:8907).some(isPlatformOwnershipFloorPolicy)Which call sites read an empty collection as "no policy, no filter": the Layer 1 compile (
:7222, and:7369behind it) and the post-image check (:7657). Which already refuse on empty: none refuses;checkAuthoredRowWriteabstains (its fail-closed direction) and the two floor probes answerfalse. On a compile failure every door fails closed throughRLS_DENY_FILTER.Cost of the read
The declaration is read off
ql.getSchema(object), the ObjectQL registry's resolved object (the schema every find compiles against, and the sourcegetObjectSecurityMetaprefers): two in-memory map lookups behind the registry's merged-object cache, made only when the held collection is empty. No I/O and no parse on any path; a non-empty collection pays nothing.Explain names the object default
Explain already identifies policies inside a composed filter by identity (
compiledPolicyNameOf).RLSCompiler.compileFilternow also records when a compiled policy filter came from an object default (compiledObjectDefaultRowScopeOf), and explain reads it:rlslayer'sdetailgains one sentence naming the default byname, with itslabelanddescription, and the declaring object, saying it is not a permission set.contributorsstays[](the spec's contributor kinds arepermission_set/position/system; a structured object-level field would be a spec change, not taken).kind: 'rls_policy',nameset to the default's own name,via: 'object_default') instead ofbusiness_rls.A filter from held sets is explained byte-for-byte as before.
A
usingthat does not lower: why the runtime refusal suffices here (rider6255162631)The default reaches the same compiler an authored policy does, so a non-lowering
usingis dropped with the same "uncompilable predicate ... DROPPED" WARN and the clause fails closed toRLS_DENY_FILTER: zero rows, never a silent admission or a silent drop. Pinned on the same predicate in both forms (an object's default, and an authored set policy on a sibling object): both read zero rows, both WARN naming their policy, explain answersdeniesfor both.The authoring lint (
validate-rls-predicate-enforceability.ts) is not extended, for three reasons:RLS_PREDICATE_UNENFORCEABLE's explanation entry inpackages/lint/src/rule-explanations.tsto change: the entry's sharedRLS_DROPPED_USINGparagraph says the object "disappears for every holder of the permission set", which is wrong for a default. That file has another in-flight holder (lint: a locale pack'spicklists.<name>translations are never checked — an option key that is not in the list, or a list no package declares, passesos lint --strictand is dropped silently at runtime #23566), so per the claim it is not edited here; it is reported to the seat.extendcontributor that loosens a platform object's default (#24164 ruling E′) #24341) copy literals that already pass the lint where they live in the shipped sets.The key's describe now says plainly that authoring checks do not judge this predicate.
Spec-lane riders (rider
6256199121)packages/spec/liveness/object.json:defaultRowScopeand its eight members flip tolive, citingcollectRLSPolicies,objectDefaultRowScopePolicyand explain'sobjectDefaultRowScopeSourceas read points;authorWarn/authorHintdropped in the same edit.label/descriptionare live by the README's display-key rule: explain shows them to the administrator.state-counts/object.mdregenerated bygen:liveness-counts(live 53 to 61, planned 8 to 0).packages/spec/src/data/object.zod.ts: the key's TSDoc and describe say what the reader does, in place of "is to apply" and "no server reads this key".packages/spec/src/security/rls.zod.ts:DefaultRowScopeSchema's TSDoc likewise.packages/lint/src/validate-org-axis-red-lines.test.ts: the title and docblock namedefaultRowScopeas the one object-level policy; the assertion is unchanged.6259617731):.changeset/pre/24338-object-default-row-scope.md, the pending spec(data):ObjectSchemadeclares an object's default row scope — the existing RLS policy shape minusobject— applied when no held permission set carries a policy for it (#24164 ruling E′) #24338 release note, now says what the key does (see Acceptance notes), and the Studio form ledger's omit row forobject.defaultRowScopeinpackages/spec/src/system/metadata-form-zod-reconciliation.test.tsgives the reason that is true now.check:generated --fix, only the two it proved stale):content/docs/references/{data/object,api/metadata,system/migration}.mdx,packages/spec/liveness/state-counts/object.md.Pins (
packages/plugins/plugin-security/src/object-default-row-scope.test.ts)Real
SecurityPlugin+ObjectQLoverSqlDriver, the shipped sets, rows written past every scope. The identity objects are the platform's own declarations, each given in the fixture the default its sets copy today (sys_user's two literals as the one||policy the key allows, rider A);qa_noteis a test object.'*'read set: reads only its ownsys_user,sys_session,sys_scim_userrows plus its organisation's members onsys_user. CONTROL without the declarations: every row.'*'policy counts; a held policy for another operation, or outside the caller's positions, does not.RECORD_NOT_FOUND/ 404 (an unreadable target is absent), and handing your own row away fails the post-image check withPERMISSION_DENIED/ 403.checkAuthoredRowWriteanswersadmit/abstainfrom the default.via); CONTROL: an authored policy is not named as one.member_default,viewer_readonly,organization_adminand the platform admin, on the three identity objects, the served ids and theJSON.stringifyof explain (readandupdate) are identical with and without the declarations.usingdoes not lower is refused as an authored one is (zero rows, the WARN, explaindenies).Ablation (once, committed first, then reverted):
node scripts/ablation-replace.mjsreplacedif (held.length > 0) return held;withif (held.length >= 0) return held;(anchor 1 to 0, blob1e3792351de2toc54fe3ed549d), ran the file: 10 failed / 6 passed, the 10 being every default-dependent pin and the 6 exactly the controls (no-declaration control, authored-alone, held'*', platform admin, authored explain control, baseline byte-for-byte). Restored: blob1e3792351de2equalsHEAD,git diff HEADempty. Direction observed: turned red, as expected.Tests and gates
Final head
7702f3ca2: patch round 1 (ef9a7a01e, the two corrected texts) withorigin/main26501ae24merged in byscripts/pm/os-regen-merge.sh. The merge was a merge commit (no rebase, amend or force-push), and git carried the patch-round edit onto the moved.changeset/pre/24338-object-default-row-scope.md(blobd5154221dd). After a fullturbo run build:pnpm --filter @objectstack/spec test: 698 files, 21286 passed, 1 todo.typecheckexit 0.pnpm --filter @objectstack/spec check:generated: all 14 generated artifacts up to date, so no regeneration commit was owed after the merge.maintouched neither package's source, so their suites stand from the pre-merge head1081739c9(plugin-security 188 files, 3754 passed, 45 skipped; lint 145 files, 6864 passed; both typecheck exit 0).node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderives 117 commands for this diff (17 paths, +788 / -102 against merge base26501ae24). All 117 were run on7702f3ca2, each throughscripts/pm/os-verify-lock.shwith the command's own exit read from its VERDICT line, and--ranreconciles: 117 derived, 117 run, 0 NOT-MEASURED, 0 UNRUN. 115 exit 0.node scripts/check-empty-changeset.mjs --base origin/mainexits 1, by design: its foreign-changeset rule names.changeset/pre/24338-object-default-row-scope.mdas present on the merge base and changed here. That is the DELIBERATE CORRECTION class in Acceptance notes;Check Changesetis not a required context.pnpm check:platform-checklistexits 1 with one problem (ABSENT SYMBOLpackages/core/src/security/resolve-authz-context.ts#legacyGrantAnchorRetiredinareas/access-security.json). Control leg: the same command in a detached worktree at the merge base26501ae24exits 1 with the same single problem, and earlier ate22defb13likewise. Pre-existing, not this change's.check-adr-0087-registration --base origin/mainexit 0,check-changeset-no-major --base origin/mainexit 0.test:repo(Test Core), and every family CI runs beyond the derived list.Acceptance notes
.changeset/pre/24338-object-default-row-scope.mdis spec(data):ObjectSchemadeclares an object's default row scope — the existing RLS policy shape minusobject— applied when no held permission set carries a policy for it (#24164 ruling E′) #24338's changeset. The version release725a5b131moved it from.changeset/into.changeset/pre/, andrelease.ymlhas thepre exitGA version consume.changeset/pre/*.mdagain, so the GA changelog is built from it. Its bullet "What it does today: nothing ..." and its sentence naming theliveness-planned-propertywarning became false with this PR's reader. Patch round 1 rewrites that one bullet to what the key does now (collectRLSPoliciesapplies it when the caller's held sets carry no row policy for the object and the operation) and drops the warning sentence; level,Clause-②, heading and every other line are unchanged. The edit rides the moved path through the merge ofmain. The already-releasednextchangelog text stays as released.check-empty-changeset's foreign-changeset rule refuses this file by design (the DELIBERATE CORRECTION class: do not restore it; confirm it on the PR), soCheck Changeset, which is not a required context, stays red until the correction is confirmed.skip-changesetis not applied.object.defaultRowScope(packages/spec/src/system/metadata-form-zod-reconciliation.test.ts) now says the reader is in and a row policy is a composite form-face addition rather than a reconciliation. Prose only; the row's kind, path and the assertion are unchanged.validate-org-axis-red-lines.tsrule ①) walks permission-set RLS only, so adefaultRowScope.usingthat walks the org parent is not judged there. That is a lint not covering metadata the runtime honours, which is outside the filing classes.usingdoes not lower is explained as "composes to DENY ALL" without naming the default, exactly as an authored policy in the same state (the compiled identity is lost to the shared deny sentinel).check:platform-checklistexits 1 on this head and at the merge base alike (one ABSENT SYMBOL inareas/access-security.json,resolve-authz-context.ts#legacyGrantAnchorRetired); this diff touches neither file. The control leg is in the gates section above.Generated by Claude Code