From f7347eb7675479afbdd01f5c39e883151a360e96 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 01:15:13 +0000 Subject: [PATCH] fix(plugin-security): make the RLS emptied-membership deny guard polarity-aware An emptied pre-resolved membership set under a supported `not in` (`$not` wrapping `$in: []`) inverted to a constant-TRUE clause and compiled to allow-all on the read scope instead of the deny sentinel. The guard now fires on odd-polarity emptied memberships anywhere in the compiled filter tree (direct `$not`, `$not` arms inside `$or`/`$and`, `$not` over composites, multi-level `$not`, multi-key implicit AND) and keeps the legacy positive single-policy case, generalised through double negation. Empty `$nin` (intrinsically constant TRUE) is recognised defensively. Non-empty `not in` and inert positive composites are unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs --- .../rls-empty-membership-polarity-guard.md | 7 + .../plugin-security/src/rls-compiler.ts | 120 +++++++++++-- .../src/rls-empty-membership-polarity.test.ts | 168 ++++++++++++++++++ 3 files changed, 281 insertions(+), 14 deletions(-) create mode 100644 .changeset/rls-empty-membership-polarity-guard.md create mode 100644 packages/plugins/plugin-security/src/rls-empty-membership-polarity.test.ts diff --git a/.changeset/rls-empty-membership-polarity-guard.md b/.changeset/rls-empty-membership-polarity-guard.md new file mode 100644 index 0000000000..45d28251ef --- /dev/null +++ b/.changeset/rls-empty-membership-polarity-guard.md @@ -0,0 +1,7 @@ +--- +'@objectstack/plugin-security': patch +--- + +Security fix (fail-closed tightening, #13552): the RLS emptied-membership deny guard is now polarity-aware. A policy whose pre-resolved membership set resolves EMPTY under a negated membership test (`not in` — e.g. `using: '!(owner in current_user.org_user_ids)'`) now compiles to the deny sentinel (zero rows) instead of flowing through. Before this fix `$in: []` under `$not` inverted to a constant-TRUE clause (`NOT (1 = 0)` on the SQL read-scope lowering), so the policy the guard exists to turn into a DENY compiled to ALLOW-ALL on reads. The guard now fires at any composition depth: `$not` wrapping the membership directly, `$not` arms nested inside `$or`/`$and`, `$not` over a composite containing the membership, and multi-level `$not` (odd polarity anywhere; the bare positive case is unchanged). + +Blast radius, in plain terms: callers that were relying on that allow-all stop seeing rows. If a negated-membership policy was the only applicable policy and its membership set resolves empty (no active organization; an empty team/territory/blocked set), reads that previously returned EVERY row now return ZERO rows. The prior behaviour was a defect — an over-permissive read on a row-level-security scope — not a contract. If own-rows access must survive an emptied membership set, author it as a separate OR'd policy (e.g. `owner == current_user.id`): each policy's grant is compiled independently, and a sibling policy dropping does not take it down. A deliberate allow-all remains authorable as a literal `true` predicate. Unchanged: a NON-empty membership set under `not in` compiles and enforces exactly as before, and an emptied POSITIVE membership nested in `$or` (e.g. `owner in current_user.team_ids || owner == current_user.id`) still preserves the other arm's grant. diff --git a/packages/plugins/plugin-security/src/rls-compiler.ts b/packages/plugins/plugin-security/src/rls-compiler.ts index be82465a8c..9360869c63 100644 --- a/packages/plugins/plugin-security/src/rls-compiler.ts +++ b/packages/plugins/plugin-security/src/rls-compiler.ts @@ -66,19 +66,105 @@ export const RLS_DENY_FILTER: Record = Object.freeze({ }); /** - * Does this filter consist solely of an empty membership (`{ field: { $in: [] } }`)? - * Used to preserve the legacy "empty pre-resolved set drops the policy" semantics - * so the single-policy path fails closed via the deny sentinel rather than an - * always-false `$in: []`. + * Is this field constraint an emptied membership, and what CONSTANT does it + * evaluate to? `{ $in: [] }` is constant FALSE on every backend ("IN () + * matches nothing"); `{ $nin: [] }` is constant TRUE ("NOT IN () excludes + * nothing"). Returns that constant, or `null` when the spec is not an emptied + * membership. The CEL pushdown compiler only ever emits `$in` (negation wraps + * in `$not` — cel-to-filter.ts), but this guard's contract is over the + * FilterCondition shape, so the intrinsically-negated `$nin` spelling is + * recognised too rather than left to fail open should a future lowering emit it. */ -function isEmptyMembershipFilter(filter: Record): boolean { - const keys = Object.keys(filter); - if (keys.length !== 1) return false; - const inner = filter[keys[0]]; - if (!inner || typeof inner !== 'object') return false; - const innerKeys = Object.keys(inner as Record); - return innerKeys.length === 1 && Array.isArray((inner as Record).$in) - && ((inner as Record).$in as unknown[]).length === 0; +function emptyMembershipConstantTruth(spec: unknown): boolean | null { + if (!spec || typeof spec !== 'object' || Array.isArray(spec)) return null; + const rec = spec as Record; + if (Object.keys(rec).length !== 1) return null; + if (Array.isArray(rec.$in) && (rec.$in as unknown[]).length === 0) return false; + if (Array.isArray(rec.$nin) && (rec.$nin as unknown[]).length === 0) return true; + return null; +} + +/** + * Does this compiled filter LEAN ON an emptied membership in a way that must + * drop the policy (→ the deny sentinel upstream)? Preserves the legacy "empty + * pre-resolved set drops the policy" semantics so the single-policy path fails + * closed via the deny sentinel rather than an always-false `$in: []`. + * + * [#13552] POLARITY-AWARE. "An emptied membership is safe because `$in: []` + * matches nothing" is a polarity-DEPENDENT claim, and the pushdown subset + * contains negation (`not in` is first-class: `!(x in y)` → `$not` wrapping + * `$in`). The pre-#13552 guard shape-matched the bare positive form only, so + * `{ $not: { f: { $in: [] } } }` — constant TRUE for every row — flowed + * through and compiled to ALLOW-ALL on the read scope. Two rules now hold: + * + * 1. An emptied membership whose EFFECTIVE polarity is inverted (odd number of + * enclosing `$not`s for `$in: []`; zero/even for `$nin: []`) is a + * constant-TRUE clause. Anywhere in the tree — wrapping directly, as an arm + * of `$or`/`$and` (nested to any depth), inside a multi-key implicit AND, + * or under multi-level `$not` — it means the membership restriction the + * author wrote has evaporated: as an `$or` arm the whole filter is + * allow-all; as an `$and` arm the restriction silently vanishes. Either way + * the policy is degenerate → drop it (fail closed), exactly as the emptied + * POSITIVE single-policy case already does. + * 2. The legacy positive case, generalised through double negation: a filter + * that consists solely of an emptied `$in` membership under an even + * (incl. zero) number of `$not` wrappers is constant FALSE as a whole — + * prefer the deny sentinel over an always-false filter (same zero rows, + * one recognisable shape). + * + * What deliberately does NOT fire, in both cases matching pre-#13552 + * behaviour: a NON-empty membership under `$not` (the working `not in` + * feature), and an emptied POSITIVE membership nested in a composite — as an + * `$or` arm it is inert (`owner in || owner == me` must keep granting + * own rows), as an `$and` arm the filter is already constant FALSE (denies by + * itself). A deliberate allow-all stays authorable as literal `true` (compiles + * to `{}`), which never involves a membership set. + */ +export function isEmptyMembershipFilter(filter: Record): boolean { + // (Exported for direct shape tests — rls-empty-membership-polarity.test.ts; + // not part of the package surface: index.ts deliberately does not re-export it.) + return containsTautologicalEmptyMembership(filter, false) || isSolelyEmptyMembership(filter); +} + +/** Rule 1 above: an emptied membership that is constant TRUE in effective polarity. */ +function containsTautologicalEmptyMembership(node: unknown, negated: boolean): boolean { + if (!node || typeof node !== 'object' || Array.isArray(node)) return false; + const rec = node as Record; + // A BARE operator object (an emptied membership with no field key) is not a + // shape the CEL lowering emits, but the evaluator answers it fail-closed + // (constant FALSE — unknown top-level operator), which a wrapping `$not` + // inverts to constant TRUE. The pre-#13552 guard already fired on + // `{ $not: { $in: [] } }`; never be weaker than the predecessor on any shape. + if (emptyMembershipConstantTruth(rec) !== null) return negated; + for (const [key, value] of Object.entries(rec)) { + if (key === '$not') { + if (containsTautologicalEmptyMembership(value, !negated)) return true; + } else if (key === '$and' || key === '$or') { + if (Array.isArray(value) && value.some((arm) => containsTautologicalEmptyMembership(arm, negated))) return true; + } else if (!key.startsWith('$')) { + const truth = emptyMembershipConstantTruth(value); + if (truth !== null && (negated ? !truth : truth)) return true; + } + } + return false; +} + +/** Rule 2 above: solely an emptied `$in` membership under even (incl. zero) `$not`s. */ +function isSolelyEmptyMembership(filter: Record): boolean { + let node: Record = filter; + let negations = 0; + for (;;) { + const keys = Object.keys(node); + if (keys.length !== 1 || keys[0] !== '$not') break; + const inner = node.$not; + if (!inner || typeof inner !== 'object' || Array.isArray(inner)) return false; + node = inner as Record; + negations++; + } + if (negations % 2 !== 0) return false; // odd polarity → rule 1's walk owns it + const keys = Object.keys(node); + if (keys.length !== 1 || keys[0].startsWith('$')) return false; + return emptyMembershipConstantTruth(node[keys[0]]) === false; } /** @@ -228,7 +314,10 @@ export class RLSCompiler { * - an unresolved/absent `current_user.*` variable → `null` → fail closed * (the "no active organization" path); * - an empty pre-resolved membership set → `null` so the single-policy case - * yields the deny sentinel upstream rather than a permissive `$in: []`. + * yields the deny sentinel upstream rather than a permissive `$in: []` — + * in EITHER polarity ([#13552]): under a supported `not in` the emptied + * set would otherwise compile to a constant-TRUE `$not`/`$in: []` clause, + * i.e. allow-all, the exact fail-open this guard exists to prevent. */ compileExpression( expression: string, @@ -255,7 +344,10 @@ export class RLSCompiler { // Parity: an empty pre-resolved membership (`field in current_user.`) // compiles to `{ field: { $in: [] } }`. The legacy compiler dropped the // policy in this case; preserve that so the deny sentinel (not a literal - // empty-IN) is what the single-policy path returns. + // empty-IN) is what the single-policy path returns. [#13552] The guard is + // polarity-aware: the same emptied set under a supported `not in` + // (`$not` wrapping, at any composition depth) is dropped too — otherwise + // it inverts to a constant-TRUE clause and the policy compiles ALLOW-ALL. if (isEmptyMembershipFilter(result.filter as Record)) return null; return result.filter as Record; } diff --git a/packages/plugins/plugin-security/src/rls-empty-membership-polarity.test.ts b/packages/plugins/plugin-security/src/rls-empty-membership-polarity.test.ts new file mode 100644 index 0000000000..f0195bfc25 --- /dev/null +++ b/packages/plugins/plugin-security/src/rls-empty-membership-polarity.test.ts @@ -0,0 +1,168 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13552] The RLS emptied-membership deny guard must be POLARITY-AWARE. + * + * `isEmptyMembershipFilter` exists so a pre-resolved membership set that + * RESOLVES EMPTY drops the policy and the single-policy path fails closed via + * `RLS_DENY_FILTER`. Before #13552 it shape-matched the bare positive form + * (`{ f: { $in: [] } }`) only — but `not in` is a first-class pushdown shape + * (`!(x in y)` → `$not` wrapping `$in`, cel-to-filter.ts), and under `$not` an + * empty `$in: []` INVERTS from constant FALSE to constant TRUE: the policy the + * guard exists to turn into a DENY compiled to ALLOW-ALL on the read scope. + * + * This suite is the triage-mandated enumeration (issue #13552, grading + * comment): every negation/composition shape the guard must fire under, the + * shapes it must NOT fire under (the working `not in` feature; the legitimate + * positive-composite cases), and row-level evidence via the formula evaluator + * that the pre-fix filter really admitted everything while the deny sentinel + * admits nothing. + */ + +import { describe, it, expect } from 'vitest'; +import { isPushdownableCel, matchesFilterCondition } from '@objectstack/formula'; +import { RLSCompiler, RLS_DENY_FILTER, isEmptyMembershipFilter } from './rls-compiler.js'; + +/** Five-row fixture: distinct owners, one null — mirrors the issue's measurement. */ +const ROWS: Record[] = [ + { id: 'r1', owner: 'u_me', status: 'open' }, + { id: 'r2', owner: 'u_other', status: 'open' }, + { id: 'r3', owner: 'u_third', status: 'closed' }, + { id: 'r4', owner: null, status: 'open' }, + { id: 'r5', owner: 'u_fourth', status: 'closed' }, +]; + +const admitted = (filter: Record): number => + ROWS.filter((row) => matchesFilterCondition(row, filter as any)).length; + +const policy = (using: string): any => ({ object: 'task', operation: 'select', using }); + +/** Context whose membership sets all RESOLVE EMPTY (the degenerate context). */ +const EMPTY_CTX: any = { + userId: 'u_me', + tenantId: 'org-1', + positions: [], + org_user_ids: [], + rlsMembership: { team_ids: [], blocked_ids: [] }, +}; + +describe('[#13552] emptied membership under negation — the guard must fire (deny sentinel)', () => { + const compiler = new RLSCompiler(); + + // ── The decisive control first: the DANGER is real at the evaluator ────── + it('evaluator control: `$not` over an empty `$in` is constant TRUE — 5 of 5 rows', () => { + // Independent of the guard: this pins WHY the guard must fire. The same + // inversion holds at the analytics lowering (`read-scope-sql.ts`: + // `$in: []` → `1 = 0`, and `NOT (1 = 0)` is TRUE for every row). + expect(admitted({ $not: { owner: { $in: [] } } })).toBe(5); + // …and the deny sentinel admits nothing. + expect(admitted(RLS_DENY_FILTER as Record)).toBe(0); + }); + + // ── Enumeration: shapes the guard fires under, driven through authored CEL ── + const MUST_DENY: Array<[label: string, cel: string]> = [ + ['direct `$not` wrap — `not in` on an emptied set', + '!(owner in current_user.org_user_ids)'], + ['`$not` nested inside `$or`', + '!(owner in current_user.team_ids) || owner == current_user.id'], + ['`$not` nested inside `$and`', + '!(owner in current_user.blocked_ids) && status == "open"'], + ['`$not` over a composite containing the emptied membership ($and)', + '!(owner in current_user.team_ids && status == "open")'], + ['`$not` over a composite containing the emptied membership ($or)', + '!(owner in current_user.team_ids || status == "archived")'], + ['multi-level `$not`, odd (triple)', + '!(!(!(owner in current_user.org_user_ids)))'], + ['multi-level `$not`, even (double) — constant FALSE, sentinel preferred', + '!(!(owner in current_user.org_user_ids))'], + ['bare positive (the pre-#13552 behaviour, preserved)', + 'owner in current_user.org_user_ids'], + ]; + + it('every enumerated shape is an AUTHORABLE pushdown shape (isPushdownableCel ok)', () => { + for (const [label, cel] of MUST_DENY) { + expect(isPushdownableCel(cel), `${label}: ${cel}`).toEqual({ ok: true }); + } + }); + + for (const [label, cel] of MUST_DENY) { + it(`${label} → RLS_DENY_FILTER (zero rows)`, () => { + const filter = compiler.compileFilter([policy(cel)], EMPTY_CTX); + expect(filter, `policy: ${cel}`).toEqual(RLS_DENY_FILTER); + // Row-level: the compiled scope admits NOTHING. Before the #13552 fix + // the negated shapes compiled to a constant-TRUE filter admitting 5/5. + expect(admitted(filter as Record), `policy: ${cel}`).toBe(0); + }); + } + + // ── Shapes the guard must NOT fire under ───────────────────────────────── + it('NON-empty membership under `$not` keeps working — the `not in` feature', () => { + const ctx: any = { userId: 'u_me', tenantId: 'org-1', positions: [], org_user_ids: ['u_other', 'u_third'] }; + const filter = compiler.compileFilter([policy('!(owner in current_user.org_user_ids)')], ctx); + expect(filter).toEqual({ $not: { owner: { $in: ['u_other', 'u_third'] } } }); + // r1 (u_me), r4 (null owner — $in over null is false, $not inverts), r5 (u_fourth). + expect(admitted(filter as Record)).toBe(3); + }); + + it('emptied POSITIVE membership as an `$or` arm stays inert — own rows keep flowing', () => { + const filter = compiler.compileFilter( + [policy('owner in current_user.team_ids || owner == current_user.id')], + EMPTY_CTX, + ); + expect(filter).toEqual({ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] }); + expect(admitted(filter as Record)).toBe(1); // r1 only + }); + + it('deliberate allow-all stays authorable as literal `true`', () => { + const filter = compiler.compileFilter([policy('true')], EMPTY_CTX); + expect(filter).toEqual({}); + expect(admitted(filter as Record)).toBe(5); + }); + + it('multi-policy: a dropped negated-empty policy removes only its grant — the sibling still grants', () => { + const filter = compiler.compileFilter( + [policy('!(owner in current_user.org_user_ids)'), policy('owner == current_user.id')], + EMPTY_CTX, + ); + // The degenerate policy contributes nothing; the sibling's grant survives. + expect(filter).toEqual({ owner: 'u_me' }); + expect(admitted(filter as Record)).toBe(1); + }); +}); + +describe('[#13552] guard shape tests — FilterCondition forms CEL cannot author', () => { + // The guard's contract is over the compiled FilterCondition, which is wider + // than what cel-to-filter emits today. Direct shape pins so the defensive + // arms are not phantom checks. + const fires = (f: Record) => isEmptyMembershipFilter(f); + + it('multi-key implicit AND under `$not` (constant TRUE by De Morgan) fires', () => { + expect(fires({ $not: { owner: { $in: [] }, status: 'open' } })).toBe(true); + // Evaluator agreement: NOT(FALSE AND …) admits everything. + expect(admitted({ $not: { owner: { $in: [] }, status: 'open' } })).toBe(5); + }); + + it('bare `{ $not: { $in: [] } }` still fires — pre-#13552 guard parity', () => { + expect(fires({ $not: { $in: [] } })).toBe(true); + }); + + it('empty `$nin` (intrinsically constant TRUE) fires at positive polarity', () => { + // Not emitted by cel-to-filter today; recognised so a future lowering + // cannot fail open through the same blind spot ($nin: [] → `1 = 1` at the + // read-scope SQL lowering). + expect(fires({ owner: { $nin: [] } })).toBe(true); + expect(fires({ $or: [{ owner: { $nin: [] } }, { status: 'open' }] })).toBe(true); + }); + + it('non-membership shapes do not fire', () => { + expect(fires({ owner: 'u_me' })).toBe(false); + expect(fires({ $not: { owner: { $in: ['a'] } } })).toBe(false); + expect(fires({ $not: { owner: { $null: true } } })).toBe(false); + expect(fires({ $and: [{ owner: { $in: [] } }, { status: 'open' }] })).toBe(false); // constant FALSE — denies by itself + expect(fires({})).toBe(false); + }); + + it('even-`$not` emptied membership NESTED in a composite stays inert (constant FALSE arm)', () => { + expect(fires({ $or: [{ $not: { $not: { owner: { $in: [] } } } }, { owner: 'u_me' }] })).toBe(false); + }); +});