Skip to content

Commit 486d526

Browse files
fix(spec): the RLS check clause's enumerated-values @example is CEL, and compiles (#6641) (#6729)
* fix(spec): the RLS check clause's enumerated-values @example is CEL, and compiles (#6641) `RowLevelSecurityPolicySchema.check` documented set membership as `status IN ('draft', 'pending')`, which does not compile. The deprecated SQL bridge (ADR-0058 D1) rewrites the word `IN` to `in` and never the parentheses, and CEL's list literal is bracketed, so the bridged `status in ('draft', 'pending')` is a parse error. `compileExpression` then returns null, `compileFilter` sees `filters.length === 0`, and a single-policy object falls to `RLS_DENY_FILTER` — an author copying the schema's own example gets a policy that denies every row, plus a lint error from `validateRlsPredicateEnforceability`. Route 1 (document side): the example now reads the canonical CEL `status in ['draft', 'pending']`. The deprecated bridge is deliberately NOT widened. The `using` examples and the neighbouring single-element `IN (current_user.<array>)` form were measured compilable and are unchanged. `@objectstack/formula`'s `rls-predicate.test.ts` gains the guard this defect class never had: every `@example` on `using` / `check` is read out of the spec source and pushed through the ADR-0056 D4 shape gate, plus a substance pin that the enumerated-values idiom compiles to a set membership and, under CHECK semantics, admits exactly the values it names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ffcE95NaMJcL9XJ9VDYgk * fix(formula): resolve this test file's directory once, keeping TEST_DEBT at 17 The new #6641 pin introduced a SECOND `import.meta.url` in `rls-predicate.test.ts`. This package's `tsconfig.json` excludes `*.test.ts`, so `pnpm typecheck` never reads the file — but `check-type-check-coverage --re-measure` reads it with the exclusion lifted and holds the count to a shrink-only TEST_DEBT ledger (#5278). Under the package's CommonJS-targeted config each `import.meta` costs two raw errors (TS1470 + TS2339), so the second occurrence took the entry from its recorded 17 to 19 and tripped the ratchet. Hoisted to one module-level `HERE` constant, read by both the existing import-graph assertion and the new `@example` pin. Measured with the gate's own method (formula's tsconfig, test exclusion lifted, its `spec` closure built): 19 before, 17 after — exactly the ledger's number, so no ledger edit is needed and no debt is being laundered. No pin was weakened: the `@example` extraction, the shape-gate round trip, the anti-vacuity guard and the post-image semantics all stand, and restoring the old example still turns both new tests red. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ffcE95NaMJcL9XJ9VDYgk --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c308064 commit 486d526

3 files changed

Lines changed: 154 additions & 5 deletions

File tree

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
fix(spec): the RLS `check` clause's enumerated-values `@example` is CEL, and compiles (#6641)
6+
7+
`RowLevelSecurityPolicySchema.check` in `security/rls.zod.ts` documented
8+
set membership as `status IN ('draft', 'pending')`. That predicate does not
9+
compile, and the failure is not cosmetic — an author who copies the schema's
10+
own example gets a policy that **denies every row**.
11+
12+
The runtime path is `RLSCompiler.compileExpression``sqlPredicateToCel`
13+
`compileCelToFilter`. The deprecated SQL bridge (ADR-0058 D1) rewrites the
14+
*word* `IN` to `in` and never the parentheses, and CEL's list literal is
15+
**bracketed**, so the bridged `status in ('draft', 'pending')` is a parse error
16+
(`Expected RPAREN, got COMMA`). `compileExpression` then returns `null`,
17+
`compileFilter` sees `filters.length === 0`, and a single-policy object falls to
18+
`RLS_DENY_FILTER`. `@objectstack/lint`'s `validateRlsPredicateEnforceability`
19+
rejects the same predicate at authoring time, so the symptom is "I followed the
20+
schema's example, and now lint errors and every query comes back empty".
21+
22+
The neighbouring `IN (current_user.team_member_ids)` examples were never broken
23+
and are unchanged: a single `(expr)` happens to be a legal CEL parenthesised
24+
group, so that spelling survives the bridge. It collapses only once the list
25+
holds a second element — which is why measuring one example never covered the
26+
other.
27+
28+
The example now reads `status in ['draft', 'pending']`, the canonical CEL
29+
spelling. Measured through the runtime's own path, it compiles to
30+
`{ status: { $in: ['draft', 'pending'] } }`, and under CHECK-clause semantics it
31+
accepts a post-image whose `status` is `draft` or `pending` while refusing
32+
`published`, `null`, and an absent field — which is what "Only allow certain
33+
statuses" has to mean.
34+
35+
Documentation only: no schema key, type, or accepted value changed, and the
36+
deprecated bridge was deliberately **not** widened to rewrite parenthesised SQL
37+
lists (that would add surface to a dialect being retired, and would change what
38+
compiles). `@objectstack/formula`'s `rls-predicate.test.ts` now pins every
39+
`@example` on `using` / `check` through the ADR-0056 D4 shape gate, so a
40+
documentation example that stops compiling fails a test instead of an author's
41+
first policy.

packages/formula/src/rls-predicate.test.ts

Lines changed: 112 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,23 @@ import { dirname, join } from 'node:path';
1818
import { fileURLToPath } from 'node:url';
1919

2020
import { isSupportedRlsExpression, sqlPredicateToCel } from './rls-predicate';
21-
import { isPushdownableCel } from './cel-to-filter';
21+
import { compileCelToFilter, isPushdownableCel } from './cel-to-filter';
22+
import { matchesFilterCondition } from './matches-filter';
23+
import type { FilterCondition } from '@objectstack/spec/data';
24+
25+
/**
26+
* This file's own directory, resolved ONCE.
27+
*
28+
* Deliberately a single module-level constant rather than a `const here = …` in
29+
* each test that needs it. This package's `tsconfig.json` excludes `*.test.ts`,
30+
* so `pnpm typecheck` never reads this file — but `check-type-check-coverage`
31+
* re-measures it with the exclusion lifted and holds the count to a shrink-only
32+
* TEST_DEBT ledger (#5278). `import.meta` costs two raw errors there (TS1470 +
33+
* TS2339) under the package's CommonJS-targeted config, so a SECOND occurrence
34+
* would raise the ledger by two for no behavioural reason. One occurrence, two
35+
* readers.
36+
*/
37+
const HERE = dirname(fileURLToPath(import.meta.url));
2238

2339
// ---------------------------------------------------------------------------
2440
// ADR-0056 D4 — RLS predicates that won't compile must not vanish in silence
@@ -130,14 +146,106 @@ describe('isSupportedRlsExpression — composition and dependency direction', ()
130146
* a dependency that is only wrong at build time produces no failing assertion.
131147
*/
132148
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');
149+
const source = readFileSync(join(HERE, 'rls-predicate.ts'), 'utf8');
135150
const specifiers = [...source.matchAll(/from\s+'([^']+)'/g)].map((m) => m[1]);
136151
expect(specifiers).toEqual(['./cel-to-filter']);
137152

138-
const pkg = JSON.parse(readFileSync(join(here, '..', 'package.json'), 'utf8')) as {
153+
const pkg = JSON.parse(readFileSync(join(HERE, '..', 'package.json'), 'utf8')) as {
139154
dependencies?: Record<string, string>;
140155
};
141156
expect(Object.keys(pkg.dependencies ?? {}).sort()).toEqual(['@marcbachmann/cel-js', '@objectstack/spec']);
142157
});
143158
});
159+
160+
// ---------------------------------------------------------------------------
161+
// #6641 — the schema's own `@example` predicates must COMPILE
162+
// ---------------------------------------------------------------------------
163+
//
164+
// `packages/spec/src/security/rls.zod.ts` documents `using` / `check` with
165+
// `@example` predicates, and an author copies them verbatim. An example that
166+
// does not compile is therefore not a typography defect: `compileExpression`
167+
// returns `null`, `compileFilter` sees `filters.length === 0` and returns
168+
// `RLS_DENY_FILTER`, so with a single policy the object denies EVERY row — and
169+
// `@objectstack/lint`'s `validateRlsPredicateEnforceability` rejects the same
170+
// predicate at authoring time, because it asks the very function above. The
171+
// symptom an author gets is "I followed the schema's example, and now lint
172+
// errors and every query is empty".
173+
//
174+
// #6641 was exactly that. `check`'s enumerated-values example read
175+
// `status IN ('draft', 'pending')`; `sqlPredicateToCel` rewrites the WORD `IN`
176+
// and never the parentheses, and CEL's list literal is BRACKETED, so the
177+
// bridged `status in ('draft', 'pending')` is a parse error. The neighbouring
178+
// `IN (current_user.<array>)` form survives only because a single `(expr)`
179+
// happens to be a legal CEL parenthesised group — it collapses the moment a
180+
// second element appears, which is why measuring one example never covered the
181+
// other.
182+
//
183+
// Asserted against the SOURCE TEXT on purpose: a documentation example is not
184+
// reachable from any import, so no ordinary unit test can ever go red on it.
185+
// This is the guard that whole defect class was missing.
186+
187+
/** `packages/spec/src/security/rls.zod.ts`, from this file's own location. */
188+
const RLS_ZOD_SOURCE = join(HERE, '..', '..', 'spec', 'src', 'security', 'rls.zod.ts');
189+
190+
/** The `@example "…"` predicates declared on ONE property's own TSDoc block. */
191+
function predicateExamples(property: 'using' | 'check'): string[] {
192+
const source = readFileSync(RLS_ZOD_SOURCE, 'utf8');
193+
const decl = source.indexOf(`\n ${property}: z.string()`);
194+
expect(decl, `\`${property}: z.string()\` not found in rls.zod.ts`).toBeGreaterThan(-1);
195+
const block = source.slice(source.lastIndexOf('/**', decl), decl);
196+
return [...block.matchAll(/@example\s+"([^"]+)"/g)].map((m) => m[1]!);
197+
}
198+
199+
describe('rls.zod.ts @example — the schema documents only predicates that compile (#6641)', () => {
200+
it.each(['using', 'check'] as const)('every `%s` @example passes the ADR-0056 D4 shape gate', (property) => {
201+
const examples = predicateExamples(property);
202+
// Anti-vacuity. A green loop over an empty list is this pin's own failure
203+
// mode: reshape the docblock, or move the examples, and a bare `for` would
204+
// keep reporting success while guarding nothing.
205+
expect(examples.length).toBeGreaterThanOrEqual(3);
206+
expect(examples.map((source) => ({ source, supported: isSupportedRlsExpression(source) })))
207+
.toEqual(examples.map((source) => ({ source, supported: true })));
208+
});
209+
210+
it('the `check` enumerated-values example is a CEL bracket list that means "one of these"', () => {
211+
// Substance, not wording: find the example by the idiom it demonstrates,
212+
// then read the expectations out of what it COMPILES to, so renaming the
213+
// field or the statuses keeps this green while breaking the idiom fails.
214+
// Case-INSENSITIVE on purpose: the SQL spelling `IN (…)` must be caught by
215+
// this test and fail on the bracket assertion below, not slip past the
216+
// filter and leave an empty list that a laxer pin would call green.
217+
const [enumerated, ...extra] = predicateExamples('check').filter((e) => /\bin\b/i.test(e));
218+
expect(extra).toEqual([]);
219+
expect(enumerated).toBeTypeOf('string');
220+
221+
// Bracketed, never parenthesised — `(a, b)` is not a CEL expression.
222+
expect(enumerated).toMatch(/\bin\s*\[/);
223+
// Already canonical CEL, so the deprecated SQL bridge is a no-op on it
224+
// (ADR-0058 D1): the example teaches the canonical dialect, not the bridge.
225+
expect(sqlPredicateToCel(enumerated!)).toBe(enumerated);
226+
227+
const compiled = compileCelToFilter(enumerated!, { variables: {} });
228+
expect(compiled.ok).toBe(true);
229+
const filter = (compiled as { ok: true; filter: unknown }).filter as Record<string, { $in?: unknown[] }>;
230+
const [field, ...moreFields] = Object.keys(filter);
231+
expect(moreFields).toEqual([]);
232+
const allowed = filter[field!]?.$in;
233+
// More than one member is the whole point — the one-element spelling was
234+
// never the broken case.
235+
expect(Array.isArray(allowed) && allowed.length > 1).toBe(true);
236+
237+
// CHECK-clause semantics: the write path validates the POST-IMAGE against
238+
// this filter (`plugin-security/security-plugin.ts` step 3.6, via
239+
// `matchesFilterCondition`). Only the enumerated values may be written;
240+
// anything else — including an absent or null field — is refused. That is
241+
// what "Only allow certain statuses" has to mean to be a true example.
242+
for (const value of allowed as unknown[]) {
243+
expect(matchesFilterCondition({ [field!]: value }, filter as FilterCondition)).toBe(true);
244+
}
245+
const notEnumerated = '__status_not_in_the_example__';
246+
expect(allowed).not.toContain(notEnumerated);
247+
expect(matchesFilterCondition({ [field!]: notEnumerated }, filter as FilterCondition)).toBe(false);
248+
expect(matchesFilterCondition({ [field!]: null }, filter as FilterCondition)).toBe(false);
249+
expect(matchesFilterCondition({}, filter as FilterCondition)).toBe(false);
250+
});
251+
});

packages/spec/src/security/rls.zod.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ export const RowLevelSecurityPolicySchema = lazySchema(() => strictObject(
374374
* - Restrict certain operations (e.g., only allow creating "draft" status)
375375
*
376376
* @example "organization_id = current_user.organization_id"
377-
* @example "status IN ('draft', 'pending')" - Only allow certain statuses
377+
* @example "status in ['draft', 'pending']" - Only allow certain statuses
378378
* @example "created_by = current_user.id" - Must be the creator
379379
*/
380380
check: z.string()

0 commit comments

Comments
 (0)