Skip to content

Commit bf1edef

Browse files
feat(formula,lint): wire ADR-0056 D4's RLS authoring gate, from the runtime's own predicate (#4983) (#5008)
`isSupportedRlsExpression` was written so an authoring command could REJECT a predicate the runtime silently drops, and no authoring command ever called it — a declared-but-never-read helper whose whole purpose was fixing declared-but-never-read. Two steps, in this order. 1. Hoist `sqlPredicateToCel` + `isSupportedRlsExpression` FROM plugin-security/src/rls-compiler.ts TO formula/src/rls-predicate.ts. Executable code unchanged — an address change. plugin-security consumes them from @objectstack/formula and keeps no copy; neither symbol was ever exported from plugin-security's entry point, so no import path outside the two packages moves. @objectstack/lint may depend on spec and never on a runtime, so the alternative was forking the SQL->CEL bridge, whose boundary conditions (quoted literals never rewritten; canonical CEL idempotent) ARE the gate's red/green line. ADR-0058 D1: a single canonical shape gate. 2. New lint rule `validateRlsPredicateEnforceability`, error, on all three authoring commands, over permissions[].rowLevelSecurity[].using/.check: - rls-predicate-unenforceable: parses as CEL, outside the pushdown subset. - rls-predicate-unparseable: does not parse even after the legacy SQL bridge. The verdict is `isSupportedRlsExpression` itself — the same function RLSCompiler.compileFilter consults to decide whether a dropped policy earns its WARN — so lint and runtime are one boolean by construction, pinned in both directions over a shared corpus. Runtime consequence, read from plugin-security rather than inferred: the policy is DROPPED with one request-time WARN; on the read path, when it is the only applicable policy, compileFilter returns RLS_DENY_FILTER, so every select/update/delete matches zero rows; on the ADR-0058 D4 write path the post-image check becomes that sentinel and every insert/update raises PermissionDeniedError. Fail-closed, hence survivable — a policy that reads as an authorization and behaves as a blanket refusal, with nothing at authoring time naming the line. Measured: every RLS predicate declared anywhere in this repo (platform seeds, examples, dogfood fixtures, the authoring skill) is supported, so the gate turns nothing red that works today. Claude-Session: https://claude.ai/code/session_018iARDqtrhQgz6fVHDeDkbQ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 55dbbba commit bf1edef

12 files changed

Lines changed: 962 additions & 80 deletions
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
---
2+
"@objectstack/formula": minor
3+
"@objectstack/lint": minor
4+
"@objectstack/plugin-security": patch
5+
---
6+
7+
feat(formula,lint): wire ADR-0056 D4's RLS authoring gate, from the runtime's own predicate (#4983)
8+
9+
`isSupportedRlsExpression` has carried the same docblock since ADR-0056 D4:
10+
"exposed so an authoring-time gate (`objectstack compile`) can REJECT a
11+
predicate the runtime would silently drop … A `false` here means 'this
12+
predicate will never enforce'." It had **no non-test consumer anywhere** — the
13+
function written to fix declared-but-never-read was itself declared and never
14+
read. This lands the consumer, in two steps that had to happen in this order.
15+
16+
**1. `sqlPredicateToCel` and `isSupportedRlsExpression` move FROM
17+
`@objectstack/plugin-security` (`src/rls-compiler.ts`) TO `@objectstack/formula`
18+
(`src/rls-predicate.ts`), and are exported from its root.** Executable code
19+
unchanged — a change of address, not of behaviour; `plugin-security` now imports
20+
them from `@objectstack/formula` and keeps no copy, so there is still exactly
21+
one definition. No import path outside the two packages changes: neither symbol
22+
was ever exported from `@objectstack/plugin-security`'s entry point. The move is
23+
what makes step 2 possible at all — `@objectstack/lint` may depend on
24+
`@objectstack/spec` and never on a runtime, so with the predicate living in a
25+
runtime the gate's only other door was copying the SQL→CEL bridge, whose
26+
boundary conditions (quoted literals are never rewritten; canonical CEL passes
27+
through unchanged) *are* the gate's red/green line. A fork drifting by one
28+
character rejects policies the runtime executes correctly — the false-positive
29+
direction, which is worse than the gap. ADR-0058 D1 asks for a single canonical
30+
shape gate; the bridge is part of that gate.
31+
32+
**2. New `@objectstack/lint` rule `validateRlsPredicateEnforceability`,
33+
`error`, on all three authoring commands**, over
34+
`permissions[].rowLevelSecurity[].using` and `.check`:
35+
36+
- **`rls-predicate-unenforceable`** — parses as CEL, outside the pushdown
37+
subset: a function call (`size(...)`, `has(...)`), arithmetic, a ternary, a
38+
cross-object path (`record.account.region`).
39+
- **`rls-predicate-unparseable`** — does not parse as CEL even after the legacy
40+
SQL bridge (`=``==`, `IN``in`): SQL `AND` / `OR` / `LIKE`, a subquery.
41+
Its own id because the fix is different — write CEL (`&&`, `||`), not a
42+
different shape.
43+
44+
What the gate prevents, measured through `plugin-security` rather than inferred:
45+
`RLSCompiler` drops the policy and logs one request-time WARN. On the read path,
46+
when it is the only applicable policy, `compileFilter` returns the
47+
`RLS_DENY_FILTER` sentinel instead, which is AND-ed onto the where clause — so
48+
every select / update / delete on the object matches **zero rows**. On the
49+
ADR-0058 D4 write path the post-image `check` becomes that same sentinel, which
50+
no record satisfies, so every insert / update fails with `PermissionDeniedError`.
51+
The runtime fails closed, which is why this was survivable: the result is not a
52+
hole but a policy that reads as an authorization and behaves as a blanket
53+
refusal, with nothing at authoring time pointing at the line that caused it.
54+
55+
Fix a flagged predicate by rewriting it inside the lowerable subset — `==` `!=`
56+
`>` `<` `>=` `<=`, `in`, `&&` `||` `!`, `== null` / `!= null`, and
57+
`startsWith` / `endsWith` / `contains` over single-column field paths (ADR-0058
58+
D2), against a literal or a `current_user.*` value. Two specific migrations:
59+
`has(x)` / `size(x) > 0``x != null` (a function call is correct in an object
60+
*validation* rule, which is interpreted, and wrong here, where the predicate is
61+
compiled to a filter); and a related record's field → denormalise it onto this
62+
object (formula/rollup) and test that column, since RLS cannot join (ADR-0055).
63+
64+
Same construction as the sharing-rule gate (#4698): the rule does not model the
65+
consumer or grep for it — it calls `isSupportedRlsExpression`, the exact
66+
function `RLSCompiler.compileFilter` consults to decide whether a dropped policy
67+
earns its warning, so the two verdicts are one boolean by construction, pinned
68+
in both directions over a shared corpus. Measured before shipping: every RLS
69+
predicate declared anywhere in this repo — the `plugin-security` platform seeds,
70+
the examples, the dogfood fixtures, the authoring skill — is supported, so the
71+
gate turns nothing red that works today. Unlike the sharing-rule gate, CEL
72+
*syntax* is reported here rather than deferred to `expression-invalid`:
73+
`validateStackExpressions` does not walk `rowLevelSecurity` at all, and could not
74+
judge this field correctly if it did, because `owner_id = current_user.id` is a
75+
CEL syntax error and a working RLS predicate at the same time.

packages/formula/src/index.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ export { normalizeExpression, normalizeExpressionTree } from './normalize';
2525
// and plugin-sharing; honours ADR-0055 (no subquery / no cross-object traversal).
2626
export { compileCelToFilter, isPushdownableCel, lowerCelAst } from './cel-to-filter';
2727
export type { CelFilterCompileResult, CelFilterCompileOptions, CelFilterFailReason } from './cel-to-filter';
28+
// ADR-0056 D4 / ADR-0058 D1 — the RLS predicate shape gate and its legacy
29+
// SQL→CEL bridge. Hoisted out of plugin-security in #4983 so the runtime that
30+
// enforces the predicate and the authoring gate that rejects it share ONE
31+
// definition: `@objectstack/lint` may depend on this package and never on a
32+
// runtime, so the alternative was forking the bridge, whose `=`/`IN` boundary
33+
// conditions ARE the red/green line.
34+
export { isSupportedRlsExpression, sqlPredicateToCel } from './rls-predicate';
2835
export { matchesFilterCondition } from './matches-filter';
2936
// ADR-0032 — shared validator + introspection (one validator for build,
3037
// registration, and the agent-callable validate_expression tool).
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The unit tests that travelled with `isSupportedRlsExpression` /
5+
* `sqlPredicateToCel` when #4983 hoisted them out of
6+
* `@objectstack/plugin-security` (`security-plugin.test.ts`, describe block
7+
* "RLSCompiler D4 — uncompilable predicates are surfaced"). The two shape cases
8+
* are reproduced VERBATIM below: the hoist is a change of address, so a moved
9+
* test that also changes its assertions would hide the one thing the move has
10+
* to prove. The consumer-side half — that `RLSCompiler` still warns, still
11+
* fails closed, and still agrees with this predicate — stayed in
12+
* plugin-security, where the consumer is.
13+
*/
14+
15+
import { describe, it, expect } from 'vitest';
16+
import { readFileSync } from 'node:fs';
17+
import { dirname, join } from 'node:path';
18+
import { fileURLToPath } from 'node:url';
19+
20+
import { isSupportedRlsExpression, sqlPredicateToCel } from './rls-predicate';
21+
import { isPushdownableCel } from './cel-to-filter';
22+
23+
// ---------------------------------------------------------------------------
24+
// ADR-0056 D4 — RLS predicates that won't compile must not vanish in silence
25+
// (moved verbatim from plugin-security/src/security-plugin.test.ts, #4983)
26+
// ---------------------------------------------------------------------------
27+
describe('isSupportedRlsExpression — the ADR-0056 D4 shape gate', () => {
28+
it('isSupportedRlsExpression accepts the compilable shapes', () => {
29+
// Legacy SQL-ish subset (bridged `=`/`IN`).
30+
expect(isSupportedRlsExpression('owner_id = current_user.id')).toBe(true);
31+
expect(isSupportedRlsExpression('owner = current_user.email')).toBe(true);
32+
expect(isSupportedRlsExpression("status = 'published'")).toBe(true);
33+
expect(isSupportedRlsExpression('id IN (current_user.org_user_ids)')).toBe(true);
34+
expect(isSupportedRlsExpression('1 = 1')).toBe(true);
35+
// ADR-0058: the canonical compiler lowers a broader pushdown subset, so the
36+
// shape gate now (correctly) reports these as enforceable — `==`/`!=`,
37+
// comparisons, and CEL compound predicates all compile to a FilterCondition.
38+
expect(isSupportedRlsExpression('owner == current_user.id')).toBe(true); // `==`
39+
expect(isSupportedRlsExpression('amount > 100')).toBe(true); // comparison
40+
expect(isSupportedRlsExpression('region != null')).toBe(true); // null check
41+
expect(isSupportedRlsExpression('a == 1 && b == 2')).toBe(true); // CEL compound
42+
});
43+
44+
it('isSupportedRlsExpression rejects genuinely non-pushdownable shapes', () => {
45+
// These cannot lower to a FilterCondition for ANY input, so the gate must
46+
// reject them (ADR-0055 / ADR-0056 D4) — they fail closed at runtime.
47+
expect(isSupportedRlsExpression('a = current_user.id AND b = 1')).toBe(false); // SQL AND ≠ CEL && (unparseable)
48+
expect(isSupportedRlsExpression('amount + 1 > 2')).toBe(false); // arithmetic
49+
expect(isSupportedRlsExpression('id IN (SELECT id FROM users)')).toBe(false); // subquery
50+
expect(isSupportedRlsExpression('record.a.b == 1')).toBe(false); // cross-object traversal
51+
expect(isSupportedRlsExpression('')).toBe(false);
52+
});
53+
});
54+
55+
// ---------------------------------------------------------------------------
56+
// The bridge's boundary conditions — the reason a COPY of it was unacceptable
57+
// ---------------------------------------------------------------------------
58+
//
59+
// `sqlPredicateToCel` is a regex rewrite, and its edge cases are precisely the
60+
// red/green line of the authoring gate built on it (#4983). A second
61+
// implementation drifting by one character would make `os validate` reject
62+
// policies the runtime executes correctly — the false-positive direction, which
63+
// is worse than the gap. Pinning them here is what makes ONE definition worth
64+
// insisting on.
65+
66+
describe('sqlPredicateToCel — the legacy bridge, pinned at its boundaries', () => {
67+
it('rewrites the historically-supported SQL subset', () => {
68+
expect(sqlPredicateToCel('owner_id = current_user.id')).toBe('owner_id == current_user.id');
69+
expect(sqlPredicateToCel('id IN (current_user.org_user_ids)')).toBe('id in (current_user.org_user_ids)');
70+
expect(sqlPredicateToCel('1 = 1')).toBe('1 == 1');
71+
});
72+
73+
it('never rewrites inside a quoted string literal', () => {
74+
expect(sqlPredicateToCel("status = 'a = b'")).toBe("status == 'a = b'");
75+
expect(sqlPredicateToCel("note = 'IN transit'")).toBe("note == 'IN transit'");
76+
});
77+
78+
it('is IDEMPOTENT on canonical CEL — an authored predicate passes through unchanged', () => {
79+
for (const cel of [
80+
'owner_id == current_user.id',
81+
'id in current_user.org_user_ids',
82+
'amount >= 100',
83+
'amount <= 100',
84+
'region != null',
85+
"a == 1 && b == 'x'",
86+
]) {
87+
expect(sqlPredicateToCel(cel)).toBe(cel);
88+
expect(sqlPredicateToCel(sqlPredicateToCel(cel))).toBe(cel);
89+
}
90+
});
91+
92+
it('leaves comparison operators containing `=` alone', () => {
93+
// The lookbehind/lookahead exist for these: `>=`, `<=`, `!=`, `==`.
94+
expect(sqlPredicateToCel('a >= 1')).toBe('a >= 1');
95+
expect(sqlPredicateToCel('a <= 1')).toBe('a <= 1');
96+
expect(sqlPredicateToCel('a != 1')).toBe('a != 1');
97+
});
98+
});
99+
100+
// ---------------------------------------------------------------------------
101+
// The composition the gate depends on
102+
// ---------------------------------------------------------------------------
103+
104+
describe('isSupportedRlsExpression — composition and dependency direction', () => {
105+
it('is exactly `isPushdownableCel(sqlPredicateToCel(x)).ok` for a non-blank predicate', () => {
106+
const corpus = [
107+
'owner_id = current_user.id',
108+
"status = 'published'",
109+
'id IN (current_user.org_user_ids)',
110+
'amount > 100',
111+
'a == 1 && b == 2',
112+
'amount + 1 > 2',
113+
'size(record.tags) > 0',
114+
"record.account.region == 'EU'",
115+
'a = current_user.id AND b = 1',
116+
];
117+
for (const source of corpus) {
118+
expect({ source, ok: isSupportedRlsExpression(source) })
119+
.toEqual({ source, ok: isPushdownableCel(sqlPredicateToCel(source)).ok });
120+
}
121+
});
122+
123+
/**
124+
* #4983's hard constraint: the direction is `plugin-security` → `formula` and
125+
* `lint` → `formula`, NEVER the reverse. `@objectstack/formula` depends on
126+
* `@objectstack/spec` alone (see its package.json), and this module may not
127+
* quietly acquire a runtime import — that would put the hoisted predicate back
128+
* out of `@objectstack/lint`'s reach ("Depends on @objectstack/spec; never on
129+
* a runtime") and undo the whole move. Asserted against the source, because
130+
* a dependency that is only wrong at build time produces no failing assertion.
131+
*/
132+
it('never imports a runtime — the hoist direction is pinned, not just intended', () => {
133+
const here = dirname(fileURLToPath(import.meta.url));
134+
const source = readFileSync(join(here, 'rls-predicate.ts'), 'utf8');
135+
const specifiers = [...source.matchAll(/from\s+'([^']+)'/g)].map((m) => m[1]);
136+
expect(specifiers).toEqual(['./cel-to-filter']);
137+
138+
const pkg = JSON.parse(readFileSync(join(here, '..', 'package.json'), 'utf8')) as {
139+
dependencies?: Record<string, string>;
140+
};
141+
expect(Object.keys(pkg.dependencies ?? {}).sort()).toEqual(['@marcbachmann/cel-js', '@objectstack/spec']);
142+
});
143+
});
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The RLS predicate shape gate, and the legacy SQL→CEL bridge it stands on.
5+
*
6+
* **Hoisted here from `@objectstack/plugin-security` (`src/rls-compiler.ts`) in
7+
* #4983 — executable code unchanged, address changed.** Both functions were
8+
* already pure `(string) => …` over `isPushdownableCel`; neither ever read a
9+
* runtime service, an `ExecutionContext` or a policy record, so nothing about
10+
* them needed a runtime to live in. What their old address DID do was put the
11+
* one decision procedure for "will this RLS predicate ever enforce?" behind a
12+
* package `@objectstack/lint` is forbidden to import ("Depends on
13+
* @objectstack/spec; never on a runtime"), which left the ADR-0056 D4
14+
* authoring gate two impossible doors: import a runtime, or fork the bridge.
15+
* A forked bridge is the worse one — `sqlPredicateToCel`'s `=` / `IN` boundary
16+
* conditions (quoted literals are never rewritten; CEL input passes through
17+
* unchanged) ARE the red/green line, so one drifting character makes the linter
18+
* reject policies the runtime executes correctly. Hoisting keeps ONE definition
19+
* and lets the gate call the consumer's own verdict (ADR-0058 D1: a single
20+
* canonical shape gate).
21+
*
22+
* `plugin-security` imports both from here; `@objectstack/lint` imports
23+
* {@link isSupportedRlsExpression} for the authoring gate. The dependency
24+
* direction is security → formula and lint → formula, never the reverse —
25+
* pinned by `rls-predicate.test.ts`'s import-graph assertion.
26+
*/
27+
28+
import { isPushdownableCel } from './cel-to-filter';
29+
30+
/**
31+
* Recognize whether an RLS `using` / `check` expression matches one of the SHAPES
32+
* the compiler can compile (equality against a `current_user.*` var, equality
33+
* against a string literal, set-membership against a `current_user.*` array, or
34+
* the `1 = 1` allow-all). This is SHAPE-only — it does not check whether the
35+
* referenced context variable is populated at runtime.
36+
*
37+
* ADR-0056 D4: exposed so an authoring-time gate (`objectstack compile`) can REJECT
38+
* a predicate the runtime would silently drop — the class of bug where
39+
* `owner == current_user.name` (`==`, unsupported) compiled to nothing and left an
40+
* object unprotected. A `false` here means "this predicate will never enforce".
41+
*
42+
* That gate exists as of #4983: `validateRlsPredicateEnforceability` in
43+
* `@objectstack/lint` calls THIS function on every
44+
* `permissions[].rowLevelSecurity[].using` / `.check`, so the sentence above is
45+
* no longer aspirational. Until then the function had no non-test consumer
46+
* anywhere — a declared-but-never-read helper written to fix
47+
* declared-but-never-read.
48+
*/
49+
export function isSupportedRlsExpression(expression: string): boolean {
50+
if (!expression || !expression.trim()) return false;
51+
// ADR-0058 D1: a single canonical shape gate. We bridge the legacy SQL-ish
52+
// subset (`=`, `IN`) to canonical CEL, then ask the ONE pushdown compiler
53+
// whether the shape lowers to a FilterCondition at all. This is broader than
54+
// the historical 4 forms — comparisons (`amount > 100`) and `==` now ENFORCE
55+
// (the compiler lowers them), so the gate correctly reports them supported.
56+
// It is SHAPE-only: whether a referenced `current_user.*` variable is exposed
57+
// at runtime is a separate availability concern (an unexposed var fails closed
58+
// at resolution — see RLSCompiler.compileExpression).
59+
return isPushdownableCel(sqlPredicateToCel(expression)).ok;
60+
}
61+
62+
/**
63+
* @deprecated Transitional bridge (ADR-0058 D1). Canonical RLS predicates are
64+
* CEL; this exists ONLY so stored/legacy SQL-ish `using`/`check` keeps compiling
65+
* until it is migrated. Bridge the legacy SQL subset to canonical CEL so it flows
66+
* through the ONE compiler: `=` → `==`, `IN` → `in`. Quoted string literals are
67+
* left untouched. It is IDEMPOTENT on CEL input (a `==`/`in` predicate is
68+
* unchanged), so authored-CEL seeds pass through as no-ops (no deprecation warn). Only this historically-supported subset is bridged — compound
69+
* predicates should be authored in canonical CEL (`&&` / `||`); anything outside
70+
* the subset (subqueries, SQL `AND`/`OR`, `LIKE`) stays unparseable and so fails
71+
* closed, exactly as before.
72+
*/
73+
export function sqlPredicateToCel(expression: string): string {
74+
return expression.replace(/'[^']*'|\bIN\b|(?<![<>=!])=(?!=)/gi, (m) => {
75+
if (m[0] === "'") return m; // quoted literal — never rewrite its contents
76+
if (m === '=') return '==';
77+
return 'in'; // IN / in / In → CEL membership operator
78+
});
79+
}

0 commit comments

Comments
 (0)