Skip to content

Commit 94a0bbc

Browse files
authored
fix(security)!: a disabled RLS policy no longer grants — security-subset liveness re-verification (#3980)
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. getApplicablePolicies now excludes enabled === false before any matching, at the single choke point both the find and analytics paths flow through — the same ADR-0049 enforce-or-remove resolution as the formerly-unenforced positions domain. Exact === false: the schema defaults enabled to true and projection rows may omit the key, so absent stays active. Access-narrowing only; four tests pin both directions. 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 (4 → 48 dated entries repo-wide). Other corrections: rls.priority → dead+authorWarn (semantically void under OR-combination — remove candidate); rls.label/description/tags → dead (benign); tabPermissions evidence upgraded (was understated — the rank merge reads all four values); object.ownership evidence line rot refreshed. Suspicions refuted and recorded: allowExport is enforced server-side (caller-level 403 gate, fail-closed), and the allowTransfer/Restore/Purge notes are accurate (M2 unshipped, gates pre-mapped fail-closed). plugin-security: 32 files / 677 tests green. check:liveness green — every evidence path resolves.
1 parent a47ac06 commit 94a0bbc

7 files changed

Lines changed: 206 additions & 47 deletions

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
"@objectstack/plugin-security": patch
3+
"@objectstack/spec": patch
4+
---
5+
6+
fix(security)!: a disabled RLS policy no longer grants — found by re-verifying the ledger's security subset (#3896 follow-up)
7+
8+
**The fix.** `RowLevelSecurityPolicySchema.enabled` promises, verbatim: *"Disabled
9+
policies are not evaluated."* Nothing read it — not the collection site, not the
10+
projection round-trip, not the compiler. Because applicable policies OR-combine
11+
(any match allows access), a policy an admin switched off **kept contributing its
12+
grant**: disabling a too-permissive policy silently changed nothing. That is the
13+
#3896 shape — a documented security control whose real behaviour is wider than
14+
its contract — one layer up, on RLS instead of sharing rules.
15+
16+
`getApplicablePolicies` now excludes `enabled === false` before any matching, at
17+
the single choke point both the find path and the analytics path flow through —
18+
the same place, and the same ADR-0049 enforce-or-remove resolution, as the
19+
formerly-unenforced `positions` domain. Exact `=== false` on purpose: the schema
20+
defaults `enabled` to true and projection rows may omit the key, so absent stays
21+
active. Four tests pin both directions. Access-narrowing only: no policy grants
22+
MORE after this change, and nothing in-repo authors `enabled: false`.
23+
24+
**The audit that found it.** All 44 entries of the liveness ledger's security
25+
subset (`permission` 33, `position` 4, `object` sharing/access 7) were
26+
call-graph-closed by hand and stamped `verifiedAt: 2026-07-30` — the subset's
27+
first-ever re-verification (previously 4 dated entries repo-wide, and the last
28+
sweep that cited preview renderers went 10-for-13 wrong). Beyond `enabled`:
29+
30+
- `rowLevelSecurity.priority`**dead + authorWarn**. Not merely unimplemented:
31+
policies OR-combine (the schema's own describe says most-permissive-wins), so
32+
the promised "conflict resolution" semantics cannot exist. A REMOVE candidate
33+
per the #3715/#3950 precedent while the v17 breaking window is open.
34+
- `rowLevelSecurity.label` / `description` / `tags` → dead (benign display —
35+
no consumer in either repo; deliberately not authorWarn'd).
36+
- `tabPermissions` was UNDERSTATED: the note said only `'hidden'` is read, but
37+
hono's rank merge reads all four visibility values across resolved sets, and
38+
the `me-apps-and-everyone-baseline` dogfood test exercises it. Evidence
39+
upgraded; noted as a proof-binding candidate.
40+
- `allowExport` re-verified TRUE against the suspicion that it was
41+
projection-only: the export route carries its own caller-level 403 gate
42+
(`enforceExportPermission`), fail-closed when the security service cannot
43+
answer, separate from the object-level 405.
44+
- `allowTransfer/Restore/Purge` notes re-confirmed accurate (M2 operations still
45+
unshipped; the RBAC gates are pre-mapped fail-closed).
46+
- `object.ownership` evidence had rotted (line drift) — refreshed; six other
47+
object-level security entries re-cited and stamped.
48+
49+
No other runtime behaviour changes.

packages/plugins/plugin-security/src/rls-compiler.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,17 @@ export class RLSCompiler {
285285
const rlsOp = this.mapOperationToRLS(operation);
286286

287287
return allPolicies.filter(policy => {
288+
// A policy switched off is not evaluated — the schema's exact promise
289+
// ("Disabled policies are not evaluated", RowLevelSecurityPolicySchema).
290+
// Formerly unread ANYWHERE: because applicable policies OR-combine (any
291+
// match allows access), a disabled policy kept contributing its grant —
292+
// an admin who switched off a too-permissive policy kept sharing the
293+
// rows. Same enforce-or-remove shape as `positions` below (ADR-0049).
294+
// Exact `=== false` on purpose: the schema defaults `enabled` to true,
295+
// and rows round-tripped through sys_permission_set may omit the key —
296+
// absent means active, only an explicit false disables.
297+
if ((policy as { enabled?: boolean }).enabled === false) return false;
298+
288299
// Check object match
289300
if (policy.object !== objectName && policy.object !== '*') return false;
290301

packages/plugins/plugin-security/src/security-plugin.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2760,6 +2760,55 @@ describe('RLSCompiler', () => {
27602760
expect(contactFind).toHaveLength(2); // contact all + * all
27612761
});
27622762

2763+
// `enabled` — "Disabled policies are not evaluated" (the schema's promise).
2764+
// Found unread by the 2026-07 security-subset liveness re-verification:
2765+
// applicable policies OR-combine (any match ALLOWS access), so a disabled
2766+
// policy kept granting the rows an admin believed they had switched off.
2767+
// These pin the gate in both directions.
2768+
it('excludes a policy explicitly disabled with enabled:false', () => {
2769+
const compiler = new RLSCompiler();
2770+
const policies: any[] = [
2771+
{ object: 'task', operation: 'select', using: 'owner_id = current_user.id' },
2772+
{ object: 'task', operation: 'select', using: "visibility = 'public'", enabled: false },
2773+
];
2774+
const applicable = compiler.getApplicablePolicies('task', 'find', policies);
2775+
expect(applicable).toHaveLength(1);
2776+
expect(applicable[0]!.using).toBe('owner_id = current_user.id');
2777+
});
2778+
2779+
it('treats enabled:true and an ABSENT enabled key as active (schema default)', () => {
2780+
// Rows round-tripped through sys_permission_set may omit the key entirely —
2781+
// absent must mean active, or every legacy row would silently stop enforcing.
2782+
const compiler = new RLSCompiler();
2783+
const policies: any[] = [
2784+
{ object: 'task', operation: 'select', using: 'owner_id = current_user.id', enabled: true },
2785+
{ object: 'task', operation: 'select', using: "status = 'open'" },
2786+
];
2787+
expect(compiler.getApplicablePolicies('task', 'find', policies)).toHaveLength(2);
2788+
});
2789+
2790+
it('a disabled policy no longer contributes its OR-branch to the compiled filter', () => {
2791+
// The over-share this closes: owner-only policy + a disabled public-read
2792+
// policy must compile to owner-only, not owner-OR-public.
2793+
const compiler = new RLSCompiler();
2794+
const policies: any[] = [
2795+
{ object: 'task', operation: 'select', using: 'owner_id = current_user.id' },
2796+
{ object: 'task', operation: 'select', using: "visibility = 'public'", enabled: false },
2797+
];
2798+
const ctx: any = { userId: 'user-42', positions: [] };
2799+
const applicable = compiler.getApplicablePolicies('task', 'find', policies);
2800+
const filter = compiler.compileFilter(applicable, ctx);
2801+
expect(filter).toEqual({ owner_id: 'user-42' });
2802+
});
2803+
2804+
it('disabling the ONLY policy leaves no applicable policies (caller decides posture, not a silent grant)', () => {
2805+
const compiler = new RLSCompiler();
2806+
const policies: any[] = [
2807+
{ object: 'task', operation: 'select', using: "visibility = 'public'", enabled: false },
2808+
];
2809+
expect(compiler.getApplicablePolicies('task', 'find', policies)).toHaveLength(0);
2810+
});
2811+
27632812
// §7.3.1 dynamic membership — arbitrary pre-resolved sets in rlsMembership
27642813
it('should resolve IN against a §7.3.1 pre-resolved rlsMembership set', () => {
27652814
// Manager hierarchy: the runtime resolved the manager's reports into

packages/spec/liveness/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,8 +467,8 @@ EOF
467467
| flow | 26 || 5 || dead = description/template/nodes.outputSchema/errorHandling.fallbackNodeId (engine uses fault edges) + `active` CORRECTED to dead 2026-07 (deprecated no-op — `status` is what gates binding/execution since 497bda853; the file `_note` claiming otherwise is fixed) |
468468
| action | 34 | 0 | 2 || `type:'form'` CORRECTED to live (objectui ActionRunner.executeForm, #2377); dead `timeout` REMOVED (#2377); `disabled` live for real since objectui#2863 (six surfaces); `shortcut` + `bulkEnabled` CORRECTED to dead 2026-07 — registered into ActionEngine but their accessors have no non-test caller (#3686 sweep); `undoable` CORRECTED to live 2026-07 — understated, two objectui readers gate the toast's Undo and the record restore (#3714) |
469469
| hook | 11 || 2 || model-healthy; only label/description dead (benign) |
470-
| permission | 32 || 0 || CRUD/FLS/RLS live; dead `contextVariables` REMOVED (ADR-0105 D11 — RLS resolves only the `current_user.*` built-ins plus runtime-staged `rlsMembership` sets) |
471-
| position | 4 |||| (role's ADR-0090 successor) fully live |
470+
| permission | 29 || 4 || CRUD/FLS/RLS live; dead `contextVariables` REMOVED (ADR-0105 D11 — RLS resolves only the `current_user.*` built-ins plus runtime-staged `rlsMembership` sets). 2026-07-30 security-subset re-verification (all 33 entries `verifiedAt`-stamped): `rowLevelSecurity.enabled` was live-with-wrong-evidence and UNREAD — a disabled policy kept contributing its OR-branch grant; ENFORCED same day in rls-compiler (`getApplicablePolicies`), the `positions` ADR-0049 resolution repeated. `rowLevelSecurity.priority` CORRECTED to dead+authorWarn — semantically void under OR-combination (no conflict exists to order), a REMOVE candidate. `rls.label`/`description`/`tags` CORRECTED to dead (benign display, no consumer in either repo). `tabPermissions` was UNDERSTATED ("only hidden read" → the rank merge reads all four values; me-apps dogfood test exercises it). `allowExport` re-verified TRUE end-to-end (server-side 403 gate, not just the /me projection) |
471+
| position | 4 |||| (role's ADR-0090 successor) fully live; all 4 `verifiedAt`-stamped 2026-07-30 |
472472
| agent | 13 | 5 | 1 || dead `tenantId` + `planning.strategy`/`allowReplan` REMOVED (#2377) — only `planning.maxIterations` live; autonomy tier experimental; `knowledge` CORRECTED to dead 2026-07 — `search_knowledge` takes `sourceIds` from the LLM's tool-call args, never from the agent record (#1878 §3 recheck) |
473473
| tool | 5 | 1 | 4 || the whole authoring surface is inert: `permissions` (not permission-gated), plus `category`/`active`/`builtIn` CORRECTED to dead 2026-07 (#3686 sweep). `requiresConfirmation` REMOVED (#3715, ADR-0033 §2) — SAFETY-shaped and unenforced on every path, so it was false compliance, not merely dead; ToolSchema is now `.strict()` so the retired key REJECTS with the FROM → TO prescription instead of being silently stripped |
474474
| skill | 8 || 1 || `permissions` REMOVED 2026-07 (never gated anything — owner call was prune, #3704); `triggerPhrases` CORRECTED to dead — phrases are never matched against user messages; activation is `triggerConditions` + the agent's `skills[]` + explicit /skill-name pinning (#3686 sweep) |

packages/spec/liveness/object.json

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,9 @@
102102
},
103103
"ownership": {
104104
"status": "live",
105-
"evidence": "packages/objectql/src/registry.ts:272",
106-
"note": "#3175 record-ownership model. applySystemFields injects the reassignable owner_id lookup by default (ownership:'user'); 'org'|'none' opt out (Dataverse-style catalog/junction tables). Proven in objectql/src/registry.test.ts."
105+
"verifiedAt": "2026-07-30",
106+
"evidence": "packages/objectql/src/registry.ts:292 (applySystemFields reads schema.ownership)",
107+
"note": "#3175 record-ownership model. applySystemFields injects the reassignable owner_id lookup by default (ownership:'user'); 'org'|'none' opt out (Dataverse-style catalog/junction tables). Proven in objectql/src/registry.test.ts. Evidence line refreshed 2026-07-30 (was :272, drifted)."
107108
},
108109
"access": {
109110
"status": "live",
@@ -112,7 +113,8 @@
112113
},
113114
"requiredPermissions": {
114115
"status": "live",
115-
"evidence": "packages/plugins/plugin-security/src/security-plugin.ts",
116+
"verifiedAt": "2026-07-30",
117+
"evidence": "packages/plugins/plugin-security/src/security-plugin.ts:132 (NormalizedRequiredPermissions — per-CRUD buckets) + packages/plugins/plugin-security/src/permission-evaluator.ts (crudBucketForOperation keeps the buckets in lockstep with OPERATION_TO_PERMISSION)",
116118
"note": "ADR-0066 D3 object capability contract — the security middleware denies unless the caller's systemPermissions union covers it (AND-gate, before the CRUD grant). Mirrors App.requiredPermissions. Unit + full-middleware proven in plugin-security/security-plugin.test.ts."
117119
},
118120
"userActions": {
@@ -121,18 +123,21 @@
121123
},
122124
"systemFields": {
123125
"status": "live",
124-
"evidence": "packages/objectql/src/registry.ts",
126+
"verifiedAt": "2026-07-30",
127+
"evidence": "packages/objectql/src/registry.ts (column injection) + packages/plugins/plugin-security/src/security-plugin.ts:3281 (systemFields.tenant === false read as the tenancy opt-out)",
125128
"note": "organization_id auto-inject gate."
126129
},
127130
"sharingModel": {
128131
"status": "live",
129-
"evidence": "packages/plugins/plugin-sharing/src/sharing-service.ts:54",
132+
"verifiedAt": "2026-07-30",
130133
"proof": "packages/qa/dogfood/test/controlled-by-parent.dogfood.test.ts#cbp-controlled-by-parent",
134+
"evidence": "packages/plugins/plugin-sharing/src/sharing-service.ts:54",
131135
"note": "ADR-0055 high-risk class (sharing): the `controlled_by_parent` value derives a detail object's access from its master — the security layer injects `masterFK IN (accessible master ids)` on reads and requires master edit-access on by-id writes. The proof asserts a member who cannot read the master can neither read nor by-id-write the detail, and is not over-blocked on a master they own."
132136
},
133137
"publicSharing": {
134138
"status": "live",
135-
"evidence": "packages/plugins/plugin-sharing/src/share-link-service.ts:56"
139+
"verifiedAt": "2026-07-30",
140+
"evidence": "packages/plugins/plugin-sharing/src/share-link-service.ts:48 (policy extraction) — share-link creation on an object without publicSharing.enabled=true is rejected 422 (share-link-service.ts:170)"
136141
},
137142
"tenancy": {
138143
"children": {
@@ -191,7 +196,8 @@
191196
},
192197
"isSystem": {
193198
"status": "live",
194-
"evidence": "packages/plugins/plugin-sharing/src/sharing-service.ts:74",
199+
"verifiedAt": "2026-07-30",
200+
"evidence": "packages/plugins/plugin-sharing/src/sharing-service.ts:75",
195201
"note": "effectiveSharingModel: an object with no sharingModel and isSystem===true defaults to 'public' (else 'private') — fail-closed org-wide-default posture; ORed with the sys_ name-prefix fallback. Also read by lint/src/validate-security-posture.ts:98 (isSystemObject exempts system objects from the master-detail CRUD-grant lint) and mirrored in metadata-protocol/src/protocol.ts:106. The 2026-06 audit mis-classified as dead (sharing/lint readers not re-verified)."
196202
},
197203
"searchableFields": {
@@ -200,8 +206,9 @@
200206
},
201207
"externalSharingModel": {
202208
"status": "planned",
209+
"verifiedAt": "2026-07-30",
203210
"authorWarn": true,
204-
"note": "[ADR-0090 D11] P1 lands the SPEC SHAPE only (validated external<=internal at authoring; Studio surfaces the Ext badge). Runtime consumption (audience-aware evaluator branch substituting the external dial for external principals) is scheduled with the principal-taxonomy semantics phase; tracked on #2696. `planned` + authorWarn per enforce-or-mark: authors get told the dial does not evaluate yet. Was a bespoke 'authorable' status outside the documented vocabulary."
211+
"note": "[ADR-0090 D11] P1 lands the SPEC SHAPE only (validated external<=internal at authoring; Studio surfaces the Ext badge). Runtime consumption (audience-aware evaluator branch substituting the external dial for external principals) is scheduled with the principal-taxonomy semantics phase; tracked on #2696. `planned` + authorWarn per enforce-or-mark: authors get told the dial does not evaluate yet. Was a bespoke 'authorable' status outside the documented vocabulary. Re-verified 2026-07-30: still no runtime consumer (authoring-time external<=internal validation in lint/src/validate-security-posture.ts:175 only, plus the ADR-0090 D4 retired-alias value check); nothing external-principal-shaped evaluates it, matching planned."
205212
},
206213
"fileAccessDelegate": {
207214
"status": "live",

0 commit comments

Comments
 (0)