fix(security)!: a disabled RLS policy no longer grants — security-subset liveness re-verification (#3896 follow-up) - #3980
Merged
Conversation
…up security-subset audit) RowLevelSecurityPolicySchema.enabled promises, verbatim: "Disabled policies are not evaluated." Nothing read it — not the collection site, not the projection round-trip, not the compiler. Because applicable policies OR-combine (any match allows access), a policy an admin switched off kept contributing its grant: disabling a too-permissive policy silently changed nothing. The #3896 shape one layer up — a documented security control whose real behaviour is wider than its contract. getApplicablePolicies now excludes enabled === false before any matching, at the single choke point both the find and analytics paths flow through — the same place, and the same ADR-0049 enforce-or-remove resolution, as the formerly-unenforced positions domain. Exact === false on purpose: the schema defaults enabled to true and projection rows may omit the key, so absent stays active. Four tests pin both directions. Access-narrowing only; nothing in-repo authors enabled:false. Found by re-verifying the liveness ledger's security subset: all 44 entries (permission 33, position 4, object sharing/access 7) call-graph-closed by hand and stamped verifiedAt — the subset's first-ever re-verification. Other corrections: - rowLevelSecurity.priority → dead + authorWarn: under OR-combination the promised "conflict resolution" semantics cannot exist. REMOVE candidate (#3715/#3950 precedent) while the v17 breaking window is open. - rowLevelSecurity.label/description/tags → dead (benign display, no consumer in either repo, not authorWarn'd). - tabPermissions was UNDERSTATED ("only 'hidden' is read" → the rank merge reads all four values; the me-apps dogfood test exercises it). Evidence upgraded; proof-binding candidate. - allowExport re-verified TRUE against the projection-only suspicion: the export route carries its own caller-level 403 gate, fail-closed. - allowTransfer/Restore/Purge notes re-confirmed (M2 still unshipped). - object.ownership evidence line had rotted — refreshed; 6 more object security entries re-cited and stamped. plugin-security: 32 files / 677 tests green. tsc clean, eslint clean, check:liveness green (48 dated entries, all paths resolve). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QuViRSR1j6GJjf9qGbnqFX
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
size/m
documentation
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 108 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
July 30, 2026 01:20
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…rity, bind tabPermissions + writeScope proofs (#3990) The three items #3980 flagged for the v17 window. 1. rls.priority removed — it promised "conflict resolution" that cannot exist: applicable policies OR-combine (most permissive wins, per the schema's own describe), so there is never a conflict to order and nothing ever read the key. Tombstoned via retiredKey (tsc never + parse-time prescription, both directions pinned), stripped mechanically by the permission-rls-priority-removed D2 conversion + D3 chain step, cleaned from factory helpers / the showcase example / the RLS docs, baselines updated. The docs table's enabled row now states the since-#3980-enforced contract. 2. permission.tabPermissions bound to the me-apps proof — after extending the proof to actually AUTHOR the property on the permission-set side (it had only mentioned tabPermissions in its header; binding then would have been the owner-anchor/allowTransfer anti-pattern). The new case pins hidden dropping the app from /me/apps, a more-visible grant winning it back (tabRank most-visible-wins), and unrelated principals untouched. 3. writeScope gets its runtime proof (showcase-scope-depth-write, 9/9, post-state assertions): 'unit' edits a BU co-member's row and does not descend; an ABSENT writeScope stays owner-only even under a 'unit' readScope (the axes gate independently); the hierarchy seam fails closed without the enterprise resolver. Bound as the scope-depth-write class. spec: 6879+ tests green incl. the chain-replay composability gate (union-merged over #3987's protocol-17 conversions); all gates green; dogfood fixture sweep corrected (the invoice-seed-isolation fixture no longer authors the retired key).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The security-subset re-verification of the liveness ledger, and the defect it found. Timed for the v17 window on purpose: if an audit like this turns up false compliance, the remedies (enforce / remove) are exactly the changes a major-release RC makes cheap.
The defect:
rowLevelSecurity.enabledwas a no-op, in the over-share directionRowLevelSecurityPolicySchema.enabledpromises, verbatim:Nothing read it. Not the collection site (
security-plugin.tspushes every policy, filtering only platform-tenant provenance), not the projection round-trip (verbatim JSON), not the compiler (getApplicablePoliciesmatched on object / positions / operation only).Direction matters: applicable policies OR-combine — "any match allows access" (
rls-compiler.ts's own doc). Each policy is a permissive grant widening row visibility. So a policy an admin switched off kept contributing its grant — disabling a too-permissive policy silently changed nothing. That is the #3896 shape one layer up: a documented security control whose real behaviour is wider than its written contract, discoverable only by reading the code.The ledger, meanwhile, claimed
livewith evidence pointing atrls-compiler.ts— a file that never mentioned the property. Exactly the "claim with a timestamp; code moves under it" failure the ledger README warns about, on the entry class where it costs most.The fix
getApplicablePoliciesnow excludesenabled === falsebefore any matching — the single choke point both the find path and the analytics path flow through, and the same spot where the formerly-unenforcedpositionsdomain got the identical ADR-0049 enforce-or-remove treatment. Exact=== falseon purpose: the schema defaultsenabledto true and rows round-tripped throughsys_permission_setmay omit the key, so absent stays active — no legacy row changes behaviour.Access-narrowing only: no policy grants more after this change. Nothing in-repo authors
enabled: false(checked examples/seeds), and the Studio permission form doesn't render a toggle for it, so no tenant flips it expecting the old (broken) behaviour. Four tests pin both directions, including "a disabled policy no longer contributes its OR-branch to the compiled filter".The audit around it
All 44 entries of the ledger's security subset —
permission(33),position(4),objectsharing/access (7) — call-graph-closed by hand per the README method (declaration → registration → accessor → non-test caller; grep only proves presence) and stampedverifiedAt: 2026-07-30. Repo-wide dated entries: 4 → 48. This subset had never been re-verified; the last sweep that leaned on preview-renderer citations went 10-for-13 wrong.Corrections beyond
enabled:rls.priorityrls.label/description/tagstabPermissionstabRankmerge reads all four visibility values across resolved sets. Theme-apps-and-everyone-baselinedogfood test exercises it; noted as a proof-binding candidate (the proof registry currently records it against the ungovernedapptype).object.ownershipregistry.ts:272registry.ts:292Suspicions the audit REFUTED (worth recording so nobody re-suspects them):
allowExportis not projection-only. The export route carries its own caller-level 403 gate (enforceExportPermission→canExport→checkObjectPermission('export')), fail-closed when the security service is present but cannot answer, separate from the object-level 405. The code comment even names the hazard: "without this the bit would only hide the client's Export button whilecurlstill drained the table."allowTransfer/allowRestore/allowPurgenotes are accurate: M2 operations still unshipped (no such ops inpackages/objectql), and the RBAC gates are pre-mapped with aDESTRUCTIVE_OPERATIONSfail-closed backstop, so they cannot ship ungated.position.json: all four entries verified, no corrections.object.json: seven security entries re-cited and stamped (sharingModel's fail-closed default — nosharingModel+ not system ⇒private— confirmed insharing-service.ts).Verification
plugin-security: 32 files / 677 tests green (4 new).tsc --noEmitclean, eslint clean on touched files.check:livenessgreen — 48 dated entries, every evidence path resolves, all bound proofs resolve.For the v17 triage (decisions this PR flags but does not take)
rls.priority? Semantically void; the breaking window is open now. Small schema-removal PR on request.permission.tabPermissionsto the me-apps proof once its authored surface is confirmed to be the permission-set side (ADR-0054 "bind only what the proof authors").writeScopeproof — sibling of proofedreadScope, stamped live with two confirmed readers but no dogfood proof of its own.Generated by Claude Code