diff --git a/.changeset/rls-using-tsdoc-grammar-rewrite.md b/.changeset/rls-using-tsdoc-grammar-rewrite.md new file mode 100644 index 0000000000..a588cbbfc0 --- /dev/null +++ b/.changeset/rls-using-tsdoc-grammar-rewrite.md @@ -0,0 +1,47 @@ +--- +"@objectstack/spec": patch +--- + +docs(spec): describe the RLS `using` grammar by what pushes down, not by a count (#6919) + +The TSDoc block above `RowLevelSecurityPolicySchema`'s `using` property still +opened with "The reference RLS compiler implements a deliberately **small, +fixed grammar** … **Exactly four forms compile**", then enumerated four SQL +spellings and declared "there is intentionally **no** support for `AND`/`OR`/ +`NOT`, comparison operators other than `=`". That contradicted the +`.describe()` on the *same property* — corrected in #6762 / PR #6918 — and it +contradicted the compiler. Measured against `isSupportedRlsExpression` +(`@objectstack/formula`, `src/rls-predicate.ts`): `!=` and the full ordering +comparisons, `in` over a `current_user.*` array **and** over an inline CEL list, +string `startsWith`/`endsWith`/`contains`, `&&`, `||`, parenthesised grouping +and a bare `true` all lower to a filter and genuinely enforce. + +PR #6918 could only park a `⚠️ STALE` marker on the block, because rewriting +~60 lines of grammar prose deserved its own review. This is that rewrite; the +marker is gone with it. + +The block is now written as the one question the compiler actually asks — +*does this predicate lower to an ObjectQL filter?* — with the forms that lower +listed as open categories rather than a numbered set, and the forms that fail +closed listed beside them. Replacing "four" with the current number would have +been the same defect, so no count appears. Canonical CEL leads; the SQL +spelling is presented as what it is, a deprecated transitional bridge +(`sqlPredicateToCel`, ADR-0058 D1) covering only `=` → `==` and `IN` → `in`. +The property's five `@example` strings, all SQL dialect, are now CEL. + +Two boundaries the old text got wrong in the *permissive* direction are stated +explicitly, because both are silent-fail-closed traps: SQL's parenthesised +value list does not survive the bridge (`status IN ('draft', 'pending')` fails +closed where `status in ['draft', 'pending']` lowers), and `!` negates a +parenthesised comparison but cannot negate a bare field. + +Also adds `rls-predicate-grammar-docs.pin.test.ts`, which holds the file's +three grammar faces — the published module docblock line, the property TSDoc, +and the property's `.describe()` — to one story: none may re-assert a +fixed-count or closed-set grammar, all must keep stating the fail-closed +contract, and the two operator-listing faces must name the same operators. +This grammar has now drifted twice in the same direction, and nothing compared +the faces to each other. + +No generated output changes: `gen:docs` never renders property-level TSDoc, so +`check:docs` reports all 231 files still in sync. diff --git a/packages/spec/src/security/rls-predicate-grammar-docs.pin.test.ts b/packages/spec/src/security/rls-predicate-grammar-docs.pin.test.ts new file mode 100644 index 0000000000..a6d6d968a7 --- /dev/null +++ b/packages/spec/src/security/rls-predicate-grammar-docs.pin.test.ts @@ -0,0 +1,197 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6919] The RLS predicate grammar is stated on THREE faces of + * `rls.zod.ts`, and they must not drift apart again. + * + * The three: + * + * 1. **module docblock** — the `ObjectStack RLS:` bullet list. `build-docs.ts` + * publishes the module block verbatim as the opening prose of + * `content/docs/references/security/rls.mdx`, so this face is READ BY USERS. + * 2. **property docblock** — the TSDoc block above `using`. No generator reads + * property-level TSDoc, so this face is read only by whoever opens the file + * (often an AI author, ADR-0033) — which is exactly why it rotted unnoticed. + * 3. **`.describe()` on `using`** — also published (it renders into the same + * page's property table). + * + * They have now drifted twice. Both times the same way: a face froze a + * *snapshot* of the compiler as a **closed enumeration with a count** + * ("Exactly four forms compile"; "equality, set-membership, always-true"), + * the compiler grew, and the prose stayed. #6762 / PR #6918 fixed faces 1 and + * 3; #6919 rewrote face 2 and deleted the interim `⚠️ STALE` marker PR #6918 + * had parked on it. Nothing compared the three, so face 2 contradicted face 3 + * — on the same property — across two majors. + * + * ⛔ Scope: **the claim shape, not the wording.** Rephrasing a sentence, + * reordering the bullets, or adding a newly-supported form is free. Reverting + * any face to a fixed-count / closed-set claim, dropping an operator that + * enforces, or dropping the fail-closed statement is not. + * + * Why a source-text pin is the right instrument here, given #6987's warning + * that source-scanning pins nail the wrong number when the pinned fact lives + * outside the source: the fact pinned here IS text — three prose faces of one + * file agreeing with each other. Reading the source is not a proxy for the + * fact, it is the fact. (The behavioural half — which predicates actually + * lower — is owned by `isSupportedRlsExpression`'s own tests in + * `@objectstack/formula`; this file must not restate it, and deliberately does + * not import a runtime package.) + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import url from 'node:url'; + +import { describe, it, expect } from 'vitest'; + +import { RowLevelSecurityPolicySchema } from './rls.zod'; + +const HERE = path.dirname(url.fileURLToPath(import.meta.url)); +const SOURCE = path.resolve(HERE, 'rls.zod.ts'); + +const source = fs.readFileSync(SOURCE, 'utf8'); + +/** + * Face 1 — the `ObjectStack RLS:` bullets of the module docblock. + * + * Read out of the raw source rather than out of the generated `.mdx`: the + * source is what a reader of the file sees AND what the generator copies, so + * one read covers both surfaces and it cannot go green because a regen was + * forgotten. + */ +function moduleFace(): string { + const blockEnd = source.indexOf('*/'); + const block = source.slice(0, blockEnd); + const anchor = block.indexOf('ObjectStack RLS:'); + expect(anchor, '`ObjectStack RLS:` heading not found in the module docblock').toBeGreaterThan(-1); + + // Collect the run of `* - ` bullets that follows, stopping when it ends. + const lines: string[] = []; + for (const line of block.slice(anchor).split('\n')) { + if (/^\s*\*\s*-\s+\S/.test(line)) { lines.push(line); continue; } + if (lines.length > 0) break; + } + return lines.join('\n'); +} + +/** Face 2 — the TSDoc block immediately above the `using` property. */ +function propertyFace(): string { + const decl = source.indexOf('\n using: z.string()'); + expect(decl, '`using: z.string()` declaration not found').toBeGreaterThan(-1); + const end = source.lastIndexOf('*/', decl); + const start = source.lastIndexOf('/**', end); + expect(start, 'no TSDoc block found above `using`').toBeGreaterThan(-1); + return source.slice(start, end + 2); +} + +/** Face 3 — the `.describe()` carried by the `using` property. */ +function describeFace(): string { + const shape = (RowLevelSecurityPolicySchema as unknown as { shape: Record }).shape; + return shape.using?.description ?? ''; +} + +/** + * Every operator the reference compiler lowers, spelled the way all three + * faces spell it (backticked, canonical CEL). Adding a row here when the + * compiler grows is the intended maintenance: it turns "the docs are stale" + * from something nobody notices into a red test. + */ +const ENFORCING_OPERATORS = ['`==`', '`!=`', '`<`', '`<=`', '`>`', '`>=`', '`in`', '`&&`', '`||`'] as const; + +/** A count attached to the accepted set — the exact defect that recurred. */ +const FIXED_COUNT_CLAIM = + /\b(?:exactly|precisely|only|just)\s+(?:\d+|one|two|three|four|five|six|seven|eight|nine|ten)\s+(?:forms?|shapes?|expressions?|predicates?)\b/i; + +/** The same defect spelled without the adverb ("four forms compile"). */ +const BARE_COUNT_CLAIM = + /\b(?:\d+|one|two|three|four|five|six|seven|eight|nine|ten)\s+(?:forms?|shapes?)\s+(?:compile|lower|are\s+supported)\b/i; + +const FACES: ReadonlyArray = [ + ['module docblock', moduleFace()], + ['property docblock', propertyFace()], + ['.describe()', describeFace()], +]; + +describe('[#6919] rls.zod.ts states one predicate grammar on all three faces', () => { + it('finds all three faces at all (anti-vacuity)', () => { + // Every assertion below is a search over a string. An empty haystack would + // make the negative ones pass forever the day someone moves a block. + for (const [name, text] of FACES) { + expect(text.length, `${name} face came back empty — the extractor no longer finds it`) + .toBeGreaterThan(200); + } + expect(propertyFace()).toContain('Supported expression grammar'); + }); + + it.each(FACES.map(([name, text]) => ({ name, text })))( + 'the $name face states no fixed count of accepted forms', + ({ name, text }) => { + // ⛔ #6919: replacing "four" with the current number is the SAME defect — + // the grammar is "whatever lowers to a filter", not a numbered list. + expect(FIXED_COUNT_CLAIM.test(text), `${name} re-introduced a counted accepted set`).toBe(false); + expect(BARE_COUNT_CLAIM.test(text), `${name} re-introduced a counted accepted set`).toBe(false); + }, + ); + + it.each(FACES.map(([name, text]) => ({ name, text })))( + 'the $name face does not re-assert a retracted under-statement', + ({ name, text }) => { + // The two literal sentences #6762 / #6918 / #6919 removed. + expect(text, `${name} re-asserts that only \`=\` compares`).not.toMatch(/comparison\s+operators?\s+other\s+than/i); + expect(text, `${name} re-asserts the closed three-item grammar`) + .not.toMatch(/equality,\s*set-membership,\s*always-true/i); + }, + ); + + it.each(FACES.map(([name, text]) => ({ name, text })))( + 'the $name face still says the compiler fails closed', + ({ name, text }) => { + // The one safety-relevant sentence. A face that drops it turns a + // "matches zero rows" contract into an unstated one. + expect(text, `${name} no longer states the fail-closed contract`).toMatch(/fails?\s+closed/i); + }, + ); + + it('the property docblock and `.describe()` name the same operator set', () => { + // The pair that literally contradicted each other on one property (#6919). + const property = propertyFace(); + const described = describeFace(); + for (const op of ENFORCING_OPERATORS) { + expect(property, `property docblock stopped naming ${op}`).toContain(op); + expect(described, `.describe() stopped naming ${op}`).toContain(op); + } + // The allow-all is a literal, not an operator, but it is the form most + // often dropped when someone "tidies" the list. + expect(property).toContain('`true`'); + expect(described).toContain('`true`'); + }); + + it('the module face describes an open grammar, not a closed list', () => { + // This face is ONE published line, so it cannot enumerate operators. What + // it must not do is name a finite set of categories again: it has to carry + // the composition operators, which are what a closed "equality / + // set-membership / always-true" list always omits. + const module = moduleFace(); + expect(module).toContain('`&&`'); + expect(module).toContain('`||`'); + expect(module).toMatch(/comparisons/i); + expect(module).toMatch(/set-membership/i); + }); + + it('carries no `STALE` marker on any face', () => { + // PR #6918 parked a `⚠️ STALE` marker on the property block as an interim + // measure and #6919 removed it with the rewrite. A marker coming back is a + // signal that the faces disagree again — which is what this pin is for. + for (const [name, text] of FACES) { + expect(text, `${name} carries a STALE marker again`).not.toMatch(/\bSTALE\b/); + } + }); + + it('presents CEL as the canonical spelling, SQL as the deprecated bridge', () => { + // ADR-0058 D1. `sqlPredicateToCel` is `@deprecated`; a face that leads with + // SQL sends an author to the dialect we are migrating off. + expect(propertyFace()).toMatch(/canonical CEL/); + expect(describeFace()).toMatch(/canonical CEL/); + expect(moduleFace()).toMatch(/CEL/); + }); +}); diff --git a/packages/spec/src/security/rls.zod.ts b/packages/spec/src/security/rls.zod.ts index 02ae77c0aa..1d9b54c0e4 100644 --- a/packages/spec/src/security/rls.zod.ts +++ b/packages/spec/src/security/rls.zod.ts @@ -294,44 +294,74 @@ export const RowLevelSecurityPolicySchema = lazySchema(() => strictObject( /** * USING clause - Filter condition for SELECT/UPDATE/DELETE. - * - * This is a constrained, SQL-like expression compiled into an ObjectQL + * + * A constrained CEL predicate (ADR-0058 D1) compiled into an ObjectQL * filter (see the supported grammar below). Only rows the compiled filter * matches are accessible. * * **Note**: For INSERT-only policies, USING is not required (only CHECK is needed). * For SELECT/UPDATE/DELETE operations, USING is required. * - * **Security Note**: the compiler maps each form to a structured filter and - * binds context values as parameters at the driver layer — context values - * are never string-concatenated into SQL. Policy `using` strings are - * authored by administrators, not end users. + * **Security Note**: the compiler lowers each predicate to a structured + * filter and binds context values as parameters at the driver layer — + * context values are never string-concatenated into SQL. Policy `using` + * strings are authored by administrators, not end users. * * **Supported expression grammar (reference compiler)** * - * ⚠️ **STALE — the enumeration below UNDER-states what compiles (#6919).** - * The `.describe()` on this property carries the current truth: `!=`, the - * ordering comparisons, `in` over an inline literal list, `&&`, `||` and a - * bare `true` all lower today. Rewriting this block is tracked in #6919; do - * not read the four-item list as the accepted set. + * There is no blessed list of forms to memorise here, and no count to + * quote: the grammar is defined by ONE question — *does the predicate lower + * to an ObjectQL filter?* `isSupportedRlsExpression` + * (`@objectstack/formula`, `src/rls-predicate.ts`) is that single decision + * procedure, and `@objectstack/lint` calls it at authoring time (ADR-0056 + * D4) so a predicate that would never enforce is rejected instead of + * silently dropped. Anything that does not lower **fails closed** — the + * policy matches zero rows, never more. * - * The reference RLS compiler implements a deliberately **small, fixed - * grammar** rather than a general SQL parser. Exactly four forms compile; - * anything else fails closed (the policy matches zero rows). Keep `using` - * to one of: + * What lowers, written in canonical CEL: * - * 1. `field = current_user.` — equality against a context value - * 2. `field = 'literal'` — equality against a single-quoted string literal - * 3. `field IN (current_user.)` — set membership against a - * pre-resolved id array (see "Dynamic membership" below) - * 4. `1 = 1` — always true / no restriction (privileged-position allow-all) + * - **Comparison** of a field against a literal or a `current_user.*` + * context value with `==`, `!=`, `<`, `<=`, `>` or `>=` — + * `owner_id == current_user.id`, `amount > 100`, `status != 'draft'`. + * Either operand may be the field; `current_user.id == owner_id` lowers + * to the same filter. + * - **Set membership** with `in`, against a pre-resolved `current_user.*` + * array (see "Dynamic membership" below) or an inline CEL list literal — + * `assigned_to_id in current_user.team_member_ids`, + * `status in ['draft', 'pending']`. + * - **String prefix / suffix / substring** tests — + * `name.startsWith('AC')`, `name.endsWith('_archived')`, + * `name.contains('demo')`. + * - **Composition** of the above with `&&`, `||` and parentheses — + * `organization_id == current_user.organization_id && status == 'published'`. + * `!` negates a *parenthesised comparison* (`!(status == 'draft')`); it + * cannot negate a bare field, because a bare field does not lower on its + * own. + * - **Allow-all**: the bare literal `true` (the privileged-position escape + * hatch). `1 == 1` lowers as an ordinary comparison and means the same. + * There is no bare-`false` deny-all — to make a policy inert, set + * `enabled: false`. * - * There is intentionally **no** support for `AND`/`OR`/`NOT`, comparison - * operators other than `=`, `IS NULL`/`IS NOT NULL`, `NOT IN`, `LIKE`/ - * `ILIKE`, regex (`~`/`!~`), `ANY`/`ALL`, subqueries, or `NOW()`/ - * `CURRENT_DATE`/`CURRENT_TIME`. Combine conditions by defining multiple - * policies (they OR-combine); express anything subquery-shaped as a - * pre-resolved `current_user.*` array instead. + * What does **not** lower, and therefore fails closed: SQL `AND` / `OR` / + * `NOT`, `NOT IN`, `IS NULL` / `IS NOT NULL`, `LIKE` / `ILIKE`, regex + * (`~` / `!~`), `ANY` / `ALL`, arithmetic (`amount + 1 > 2`), subqueries, + * `NOW()` / `CURRENT_DATE` / `CURRENT_TIME`, traversal across objects + * (`account.owner.id == current_user.id`), and a bare truthy field + * (`is_active`). Combine conditions with `&&` / `||`, or by defining + * multiple policies (they OR-combine); express anything subquery-shaped as + * a pre-resolved `current_user.*` array instead. + * + * **SQL spelling is a transitional bridge, not a second dialect.** Stored + * legacy predicates keep compiling because `sqlPredicateToCel` + * (`@deprecated` under ADR-0058 D1) rewrites `=` to `==` and `IN` to `in` + * before the one compiler sees them, so `owner_id = current_user.id`, + * `status = 'published'` and `assigned_to_id IN (current_user.team_member_ids)` + * still enforce. Only that subset is bridged. In particular SQL's + * parenthesised value list does **not** survive the bridge — + * `status IN ('draft', 'pending')` fails closed, where the CEL list + * `status in ['draft', 'pending']` lowers — and SQL keywords outside the + * subset (`AND`, `OR`, `NOT IN`, `IS NULL`, `LIKE`) are never rewritten. + * Author new policies in CEL. * * **Context values** — `current_user.*` resolves against the request's * execution context (camelCase fields map to snake_case placeholders): @@ -348,16 +378,16 @@ export const RowLevelSecurityPolicySchema = lazySchema(() => strictObject( * need a subquery ("tasks assigned to anyone I manage", "accounts in my * territories") is resolved by the runtime into * `ExecutionContext.rlsMembership` under a stable key, then referenced as - * `field IN (current_user.)`. This keeps the compiler subquery-free + * `field in current_user.`. This keeps the compiler subquery-free * while still supporting hierarchy- and sharing-based access. * * **Prohibited**: Dynamic SQL, DDL statements, DML statements (INSERT/UPDATE/DELETE) * - * @example "organization_id = current_user.organization_id" - * @example "owner_id = current_user.id" - * @example "status = 'published'" - * @example "assigned_to_id IN (current_user.team_member_ids)" // §7.3.1 pre-resolved - * @example "1 = 1" // privileged-position allow-all + * @example "organization_id == current_user.organization_id" + * @example "owner_id == current_user.id" + * @example "status == 'published'" + * @example "assigned_to_id in current_user.team_member_ids" // §7.3.1 pre-resolved + * @example "true" // privileged-position allow-all */ using: z.string() .optional()