Skip to content

Commit 09b0d7b

Browse files
os-steveclaude
andauthored
fix(plugin-security): make the RLS emptied-membership deny guard polarity-aware (#13570)
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. Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs Co-authored-by: Claude <noreply@anthropic.com>
1 parent d475838 commit 09b0d7b

3 files changed

Lines changed: 281 additions & 14 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
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).
6+
7+
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.

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

Lines changed: 106 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -66,19 +66,105 @@ export const RLS_DENY_FILTER: Record<string, unknown> = Object.freeze({
6666
});
6767

6868
/**
69-
* Does this filter consist solely of an empty membership (`{ field: { $in: [] } }`)?
70-
* Used to preserve the legacy "empty pre-resolved set drops the policy" semantics
71-
* so the single-policy path fails closed via the deny sentinel rather than an
72-
* always-false `$in: []`.
69+
* Is this field constraint an emptied membership, and what CONSTANT does it
70+
* evaluate to? `{ $in: [] }` is constant FALSE on every backend ("IN ()
71+
* matches nothing"); `{ $nin: [] }` is constant TRUE ("NOT IN () excludes
72+
* nothing"). Returns that constant, or `null` when the spec is not an emptied
73+
* membership. The CEL pushdown compiler only ever emits `$in` (negation wraps
74+
* in `$not` — cel-to-filter.ts), but this guard's contract is over the
75+
* FilterCondition shape, so the intrinsically-negated `$nin` spelling is
76+
* recognised too rather than left to fail open should a future lowering emit it.
7377
*/
74-
function isEmptyMembershipFilter(filter: Record<string, unknown>): boolean {
75-
const keys = Object.keys(filter);
76-
if (keys.length !== 1) return false;
77-
const inner = filter[keys[0]];
78-
if (!inner || typeof inner !== 'object') return false;
79-
const innerKeys = Object.keys(inner as Record<string, unknown>);
80-
return innerKeys.length === 1 && Array.isArray((inner as Record<string, unknown>).$in)
81-
&& ((inner as Record<string, unknown>).$in as unknown[]).length === 0;
78+
function emptyMembershipConstantTruth(spec: unknown): boolean | null {
79+
if (!spec || typeof spec !== 'object' || Array.isArray(spec)) return null;
80+
const rec = spec as Record<string, unknown>;
81+
if (Object.keys(rec).length !== 1) return null;
82+
if (Array.isArray(rec.$in) && (rec.$in as unknown[]).length === 0) return false;
83+
if (Array.isArray(rec.$nin) && (rec.$nin as unknown[]).length === 0) return true;
84+
return null;
85+
}
86+
87+
/**
88+
* Does this compiled filter LEAN ON an emptied membership in a way that must
89+
* drop the policy (→ the deny sentinel upstream)? Preserves the legacy "empty
90+
* pre-resolved set drops the policy" semantics so the single-policy path fails
91+
* closed via the deny sentinel rather than an always-false `$in: []`.
92+
*
93+
* [#13552] POLARITY-AWARE. "An emptied membership is safe because `$in: []`
94+
* matches nothing" is a polarity-DEPENDENT claim, and the pushdown subset
95+
* contains negation (`not in` is first-class: `!(x in y)` → `$not` wrapping
96+
* `$in`). The pre-#13552 guard shape-matched the bare positive form only, so
97+
* `{ $not: { f: { $in: [] } } }` — constant TRUE for every row — flowed
98+
* through and compiled to ALLOW-ALL on the read scope. Two rules now hold:
99+
*
100+
* 1. An emptied membership whose EFFECTIVE polarity is inverted (odd number of
101+
* enclosing `$not`s for `$in: []`; zero/even for `$nin: []`) is a
102+
* constant-TRUE clause. Anywhere in the tree — wrapping directly, as an arm
103+
* of `$or`/`$and` (nested to any depth), inside a multi-key implicit AND,
104+
* or under multi-level `$not` — it means the membership restriction the
105+
* author wrote has evaporated: as an `$or` arm the whole filter is
106+
* allow-all; as an `$and` arm the restriction silently vanishes. Either way
107+
* the policy is degenerate → drop it (fail closed), exactly as the emptied
108+
* POSITIVE single-policy case already does.
109+
* 2. The legacy positive case, generalised through double negation: a filter
110+
* that consists solely of an emptied `$in` membership under an even
111+
* (incl. zero) number of `$not` wrappers is constant FALSE as a whole —
112+
* prefer the deny sentinel over an always-false filter (same zero rows,
113+
* one recognisable shape).
114+
*
115+
* What deliberately does NOT fire, in both cases matching pre-#13552
116+
* behaviour: a NON-empty membership under `$not` (the working `not in`
117+
* feature), and an emptied POSITIVE membership nested in a composite — as an
118+
* `$or` arm it is inert (`owner in <empty> || owner == me` must keep granting
119+
* own rows), as an `$and` arm the filter is already constant FALSE (denies by
120+
* itself). A deliberate allow-all stays authorable as literal `true` (compiles
121+
* to `{}`), which never involves a membership set.
122+
*/
123+
export function isEmptyMembershipFilter(filter: Record<string, unknown>): boolean {
124+
// (Exported for direct shape tests — rls-empty-membership-polarity.test.ts;
125+
// not part of the package surface: index.ts deliberately does not re-export it.)
126+
return containsTautologicalEmptyMembership(filter, false) || isSolelyEmptyMembership(filter);
127+
}
128+
129+
/** Rule 1 above: an emptied membership that is constant TRUE in effective polarity. */
130+
function containsTautologicalEmptyMembership(node: unknown, negated: boolean): boolean {
131+
if (!node || typeof node !== 'object' || Array.isArray(node)) return false;
132+
const rec = node as Record<string, unknown>;
133+
// A BARE operator object (an emptied membership with no field key) is not a
134+
// shape the CEL lowering emits, but the evaluator answers it fail-closed
135+
// (constant FALSE — unknown top-level operator), which a wrapping `$not`
136+
// inverts to constant TRUE. The pre-#13552 guard already fired on
137+
// `{ $not: { $in: [] } }`; never be weaker than the predecessor on any shape.
138+
if (emptyMembershipConstantTruth(rec) !== null) return negated;
139+
for (const [key, value] of Object.entries(rec)) {
140+
if (key === '$not') {
141+
if (containsTautologicalEmptyMembership(value, !negated)) return true;
142+
} else if (key === '$and' || key === '$or') {
143+
if (Array.isArray(value) && value.some((arm) => containsTautologicalEmptyMembership(arm, negated))) return true;
144+
} else if (!key.startsWith('$')) {
145+
const truth = emptyMembershipConstantTruth(value);
146+
if (truth !== null && (negated ? !truth : truth)) return true;
147+
}
148+
}
149+
return false;
150+
}
151+
152+
/** Rule 2 above: solely an emptied `$in` membership under even (incl. zero) `$not`s. */
153+
function isSolelyEmptyMembership(filter: Record<string, unknown>): boolean {
154+
let node: Record<string, unknown> = filter;
155+
let negations = 0;
156+
for (;;) {
157+
const keys = Object.keys(node);
158+
if (keys.length !== 1 || keys[0] !== '$not') break;
159+
const inner = node.$not;
160+
if (!inner || typeof inner !== 'object' || Array.isArray(inner)) return false;
161+
node = inner as Record<string, unknown>;
162+
negations++;
163+
}
164+
if (negations % 2 !== 0) return false; // odd polarity → rule 1's walk owns it
165+
const keys = Object.keys(node);
166+
if (keys.length !== 1 || keys[0].startsWith('$')) return false;
167+
return emptyMembershipConstantTruth(node[keys[0]]) === false;
82168
}
83169

84170
/**
@@ -228,7 +314,10 @@ export class RLSCompiler {
228314
* - an unresolved/absent `current_user.*` variable → `null` → fail closed
229315
* (the "no active organization" path);
230316
* - an empty pre-resolved membership set → `null` so the single-policy case
231-
* yields the deny sentinel upstream rather than a permissive `$in: []`.
317+
* yields the deny sentinel upstream rather than a permissive `$in: []` —
318+
* in EITHER polarity ([#13552]): under a supported `not in` the emptied
319+
* set would otherwise compile to a constant-TRUE `$not`/`$in: []` clause,
320+
* i.e. allow-all, the exact fail-open this guard exists to prevent.
232321
*/
233322
compileExpression(
234323
expression: string,
@@ -255,7 +344,10 @@ export class RLSCompiler {
255344
// Parity: an empty pre-resolved membership (`field in current_user.<empty>`)
256345
// compiles to `{ field: { $in: [] } }`. The legacy compiler dropped the
257346
// policy in this case; preserve that so the deny sentinel (not a literal
258-
// empty-IN) is what the single-policy path returns.
347+
// empty-IN) is what the single-policy path returns. [#13552] The guard is
348+
// polarity-aware: the same emptied set under a supported `not in`
349+
// (`$not` wrapping, at any composition depth) is dropped too — otherwise
350+
// it inverts to a constant-TRUE clause and the policy compiles ALLOW-ALL.
259351
if (isEmptyMembershipFilter(result.filter as Record<string, unknown>)) return null;
260352
return result.filter as Record<string, unknown>;
261353
}
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#13552] The RLS emptied-membership deny guard must be POLARITY-AWARE.
5+
*
6+
* `isEmptyMembershipFilter` exists so a pre-resolved membership set that
7+
* RESOLVES EMPTY drops the policy and the single-policy path fails closed via
8+
* `RLS_DENY_FILTER`. Before #13552 it shape-matched the bare positive form
9+
* (`{ f: { $in: [] } }`) only — but `not in` is a first-class pushdown shape
10+
* (`!(x in y)` → `$not` wrapping `$in`, cel-to-filter.ts), and under `$not` an
11+
* empty `$in: []` INVERTS from constant FALSE to constant TRUE: the policy the
12+
* guard exists to turn into a DENY compiled to ALLOW-ALL on the read scope.
13+
*
14+
* This suite is the triage-mandated enumeration (issue #13552, grading
15+
* comment): every negation/composition shape the guard must fire under, the
16+
* shapes it must NOT fire under (the working `not in` feature; the legitimate
17+
* positive-composite cases), and row-level evidence via the formula evaluator
18+
* that the pre-fix filter really admitted everything while the deny sentinel
19+
* admits nothing.
20+
*/
21+
22+
import { describe, it, expect } from 'vitest';
23+
import { isPushdownableCel, matchesFilterCondition } from '@objectstack/formula';
24+
import { RLSCompiler, RLS_DENY_FILTER, isEmptyMembershipFilter } from './rls-compiler.js';
25+
26+
/** Five-row fixture: distinct owners, one null — mirrors the issue's measurement. */
27+
const ROWS: Record<string, unknown>[] = [
28+
{ id: 'r1', owner: 'u_me', status: 'open' },
29+
{ id: 'r2', owner: 'u_other', status: 'open' },
30+
{ id: 'r3', owner: 'u_third', status: 'closed' },
31+
{ id: 'r4', owner: null, status: 'open' },
32+
{ id: 'r5', owner: 'u_fourth', status: 'closed' },
33+
];
34+
35+
const admitted = (filter: Record<string, unknown>): number =>
36+
ROWS.filter((row) => matchesFilterCondition(row, filter as any)).length;
37+
38+
const policy = (using: string): any => ({ object: 'task', operation: 'select', using });
39+
40+
/** Context whose membership sets all RESOLVE EMPTY (the degenerate context). */
41+
const EMPTY_CTX: any = {
42+
userId: 'u_me',
43+
tenantId: 'org-1',
44+
positions: [],
45+
org_user_ids: [],
46+
rlsMembership: { team_ids: [], blocked_ids: [] },
47+
};
48+
49+
describe('[#13552] emptied membership under negation — the guard must fire (deny sentinel)', () => {
50+
const compiler = new RLSCompiler();
51+
52+
// ── The decisive control first: the DANGER is real at the evaluator ──────
53+
it('evaluator control: `$not` over an empty `$in` is constant TRUE — 5 of 5 rows', () => {
54+
// Independent of the guard: this pins WHY the guard must fire. The same
55+
// inversion holds at the analytics lowering (`read-scope-sql.ts`:
56+
// `$in: []` → `1 = 0`, and `NOT (1 = 0)` is TRUE for every row).
57+
expect(admitted({ $not: { owner: { $in: [] } } })).toBe(5);
58+
// …and the deny sentinel admits nothing.
59+
expect(admitted(RLS_DENY_FILTER as Record<string, unknown>)).toBe(0);
60+
});
61+
62+
// ── Enumeration: shapes the guard fires under, driven through authored CEL ──
63+
const MUST_DENY: Array<[label: string, cel: string]> = [
64+
['direct `$not` wrap — `not in` on an emptied set',
65+
'!(owner in current_user.org_user_ids)'],
66+
['`$not` nested inside `$or`',
67+
'!(owner in current_user.team_ids) || owner == current_user.id'],
68+
['`$not` nested inside `$and`',
69+
'!(owner in current_user.blocked_ids) && status == "open"'],
70+
['`$not` over a composite containing the emptied membership ($and)',
71+
'!(owner in current_user.team_ids && status == "open")'],
72+
['`$not` over a composite containing the emptied membership ($or)',
73+
'!(owner in current_user.team_ids || status == "archived")'],
74+
['multi-level `$not`, odd (triple)',
75+
'!(!(!(owner in current_user.org_user_ids)))'],
76+
['multi-level `$not`, even (double) — constant FALSE, sentinel preferred',
77+
'!(!(owner in current_user.org_user_ids))'],
78+
['bare positive (the pre-#13552 behaviour, preserved)',
79+
'owner in current_user.org_user_ids'],
80+
];
81+
82+
it('every enumerated shape is an AUTHORABLE pushdown shape (isPushdownableCel ok)', () => {
83+
for (const [label, cel] of MUST_DENY) {
84+
expect(isPushdownableCel(cel), `${label}: ${cel}`).toEqual({ ok: true });
85+
}
86+
});
87+
88+
for (const [label, cel] of MUST_DENY) {
89+
it(`${label} → RLS_DENY_FILTER (zero rows)`, () => {
90+
const filter = compiler.compileFilter([policy(cel)], EMPTY_CTX);
91+
expect(filter, `policy: ${cel}`).toEqual(RLS_DENY_FILTER);
92+
// Row-level: the compiled scope admits NOTHING. Before the #13552 fix
93+
// the negated shapes compiled to a constant-TRUE filter admitting 5/5.
94+
expect(admitted(filter as Record<string, unknown>), `policy: ${cel}`).toBe(0);
95+
});
96+
}
97+
98+
// ── Shapes the guard must NOT fire under ─────────────────────────────────
99+
it('NON-empty membership under `$not` keeps working — the `not in` feature', () => {
100+
const ctx: any = { userId: 'u_me', tenantId: 'org-1', positions: [], org_user_ids: ['u_other', 'u_third'] };
101+
const filter = compiler.compileFilter([policy('!(owner in current_user.org_user_ids)')], ctx);
102+
expect(filter).toEqual({ $not: { owner: { $in: ['u_other', 'u_third'] } } });
103+
// r1 (u_me), r4 (null owner — $in over null is false, $not inverts), r5 (u_fourth).
104+
expect(admitted(filter as Record<string, unknown>)).toBe(3);
105+
});
106+
107+
it('emptied POSITIVE membership as an `$or` arm stays inert — own rows keep flowing', () => {
108+
const filter = compiler.compileFilter(
109+
[policy('owner in current_user.team_ids || owner == current_user.id')],
110+
EMPTY_CTX,
111+
);
112+
expect(filter).toEqual({ $or: [{ owner: { $in: [] } }, { owner: 'u_me' }] });
113+
expect(admitted(filter as Record<string, unknown>)).toBe(1); // r1 only
114+
});
115+
116+
it('deliberate allow-all stays authorable as literal `true`', () => {
117+
const filter = compiler.compileFilter([policy('true')], EMPTY_CTX);
118+
expect(filter).toEqual({});
119+
expect(admitted(filter as Record<string, unknown>)).toBe(5);
120+
});
121+
122+
it('multi-policy: a dropped negated-empty policy removes only its grant — the sibling still grants', () => {
123+
const filter = compiler.compileFilter(
124+
[policy('!(owner in current_user.org_user_ids)'), policy('owner == current_user.id')],
125+
EMPTY_CTX,
126+
);
127+
// The degenerate policy contributes nothing; the sibling's grant survives.
128+
expect(filter).toEqual({ owner: 'u_me' });
129+
expect(admitted(filter as Record<string, unknown>)).toBe(1);
130+
});
131+
});
132+
133+
describe('[#13552] guard shape tests — FilterCondition forms CEL cannot author', () => {
134+
// The guard's contract is over the compiled FilterCondition, which is wider
135+
// than what cel-to-filter emits today. Direct shape pins so the defensive
136+
// arms are not phantom checks.
137+
const fires = (f: Record<string, unknown>) => isEmptyMembershipFilter(f);
138+
139+
it('multi-key implicit AND under `$not` (constant TRUE by De Morgan) fires', () => {
140+
expect(fires({ $not: { owner: { $in: [] }, status: 'open' } })).toBe(true);
141+
// Evaluator agreement: NOT(FALSE AND …) admits everything.
142+
expect(admitted({ $not: { owner: { $in: [] }, status: 'open' } })).toBe(5);
143+
});
144+
145+
it('bare `{ $not: { $in: [] } }` still fires — pre-#13552 guard parity', () => {
146+
expect(fires({ $not: { $in: [] } })).toBe(true);
147+
});
148+
149+
it('empty `$nin` (intrinsically constant TRUE) fires at positive polarity', () => {
150+
// Not emitted by cel-to-filter today; recognised so a future lowering
151+
// cannot fail open through the same blind spot ($nin: [] → `1 = 1` at the
152+
// read-scope SQL lowering).
153+
expect(fires({ owner: { $nin: [] } })).toBe(true);
154+
expect(fires({ $or: [{ owner: { $nin: [] } }, { status: 'open' }] })).toBe(true);
155+
});
156+
157+
it('non-membership shapes do not fire', () => {
158+
expect(fires({ owner: 'u_me' })).toBe(false);
159+
expect(fires({ $not: { owner: { $in: ['a'] } } })).toBe(false);
160+
expect(fires({ $not: { owner: { $null: true } } })).toBe(false);
161+
expect(fires({ $and: [{ owner: { $in: [] } }, { status: 'open' }] })).toBe(false); // constant FALSE — denies by itself
162+
expect(fires({})).toBe(false);
163+
});
164+
165+
it('even-`$not` emptied membership NESTED in a composite stays inert (constant FALSE arm)', () => {
166+
expect(fires({ $or: [{ $not: { $not: { owner: { $in: [] } } } }, { owner: 'u_me' }] })).toBe(false);
167+
});
168+
});

0 commit comments

Comments
 (0)