From da11f13dc2e3a62a6b12af462528adf18ab8008f Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 21 Jun 2026 12:35:43 +0800 Subject: [PATCH 1/7] =?UTF-8?q?feat(formula):=20ADR-0058=20P1=20=E2=80=94?= =?UTF-8?q?=20canonical=20CEL=E2=86=92FilterCondition=20pushdown=20compile?= =?UTF-8?q?r?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Build the single, canonical lowering from the @marcbachmann/cel-js AST (the SAME AST the interpreter uses) to a Mongo-style FilterCondition — the one shape both backends already consume (ObjectQL engine `where` + analytics read-scope SQL). This replaces the THREE disconnected compile-to-filter front-ends ADR-0058 identified (plugin-security's 4-form regex, plugin-sharing's celToFilter, the ObjectUI array-AST path) and is the root fix for #1887. Supported subset (D2): == != > < >= <=, in (→$in), && || !, == null / != null (→$null), string methods startsWith/endsWith/contains. `not in` = !(x in y). Hard boundaries (ADR-0055 stands): single-column field paths only — a nested relation path (record.account.region), arithmetic, function calls, ternary, and any other non-pushdown shape is an authoring-time COMPILE ERROR, never silently dropped (failing closed is the security-correct outcome, ADR-0049/0056 D4). Value resolution: variableRoot leaves (current_user.*) resolve against a context to literals — current_user.org_user_ids → a pre-resolved membership array for $in (no subquery; the runtime pre-resolves the set per ADR-0055). An undefined/ null variable yields unresolved-variable (the "no active org" fail-closed path). Public API: compileCelToFilter (resolve), isPushdownableCel (shape-only gate), lowerCelAst (pre-parsed AST → one-AST-two-backends, D6). P1 = parity + full operator/variable test matrix (52 cases); NOT yet wired into the call sites (that is P2). Full formula suite green (181). Co-Authored-By: Claude Opus 4.8 --- packages/formula/src/cel-to-filter.test.ts | 218 +++++++++++ packages/formula/src/cel-to-filter.ts | 409 +++++++++++++++++++++ packages/formula/src/index.ts | 5 + 3 files changed, 632 insertions(+) create mode 100644 packages/formula/src/cel-to-filter.test.ts create mode 100644 packages/formula/src/cel-to-filter.ts diff --git a/packages/formula/src/cel-to-filter.test.ts b/packages/formula/src/cel-to-filter.test.ts new file mode 100644 index 0000000000..7b775604d1 --- /dev/null +++ b/packages/formula/src/cel-to-filter.test.ts @@ -0,0 +1,218 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, expect, it } from 'vitest'; + +import { compileCelToFilter, isPushdownableCel } from './cel-to-filter'; + +/** current_user context used across the value-resolution cases. */ +const VARS = { + current_user: { + id: 'u_me', + organization_id: 'org_1', + org_user_ids: ['u_me', 'u_peer'], + team_member_ids: ['u_me', 'u_report'], + department: 'sales', + }, +}; + +const ok = (src: string, vars = VARS) => { + const r = compileCelToFilter(src, { variables: vars }); + if (!r.ok) throw new Error(`expected ok for "${src}" but got ${r.reason}: ${r.detail}`); + return r.filter; +}; +const fail = (src: string, vars: Record = VARS) => + compileCelToFilter(src, { variables: vars }); + +describe('compileCelToFilter — equality & literals', () => { + it('field == variable → implicit equality with resolved value', () => { + expect(ok('owner_id == current_user.id')).toEqual({ owner_id: 'u_me' }); + }); + it('record.field == variable (strips record root)', () => { + expect(ok('record.organization_id == current_user.organization_id')).toEqual({ organization_id: 'org_1' }); + }); + it('field == string literal', () => { + expect(ok("record.region == 'EMEA'")).toEqual({ region: 'EMEA' }); + }); + it('field == number literal (bigint coerced to number)', () => { + expect(ok('record.amount == 1000')).toEqual({ amount: 1000 }); + }); + it('field == boolean literal', () => { + expect(ok('record.active == true')).toEqual({ active: true }); + }); + it('field != variable → $ne', () => { + expect(ok('record.owner_id != current_user.id')).toEqual({ owner_id: { $ne: 'u_me' } }); + }); +}); + +describe('compileCelToFilter — null & exists', () => { + it('field == null → $null:true', () => { + expect(ok('record.target_channels == null')).toEqual({ target_channels: { $null: true } }); + }); + it('field != null → $null:false', () => { + expect(ok('record.target_channels != null')).toEqual({ target_channels: { $null: false } }); + }); +}); + +describe('compileCelToFilter — comparisons (with right-side field flip)', () => { + it('>', () => expect(ok('record.amount > 1000')).toEqual({ amount: { $gt: 1000 } })); + it('>=', () => expect(ok('record.rating >= 4')).toEqual({ rating: { $gte: 4 } })); + it('<', () => expect(ok('record.amount < 500')).toEqual({ amount: { $lt: 500 } })); + it('<=', () => expect(ok('record.amount <= 500')).toEqual({ amount: { $lte: 500 } })); + it('flips when the field is on the right (100 > record.amount → amount < 100)', () => { + expect(ok('100 > record.amount')).toEqual({ amount: { $lt: 100 } }); + }); +}); + +describe('compileCelToFilter — membership (in → $in)', () => { + it('field in current_user. (the RLS membership IN-form)', () => { + expect(ok('id in current_user.org_user_ids')).toEqual({ id: { $in: ['u_me', 'u_peer'] } }); + }); + it('record.field in ', () => { + expect(ok("record.status in ['open','won']")).toEqual({ status: { $in: ['open', 'won'] } }); + }); + it('not in → !(x in y) → $not wrapping $in', () => { + expect(ok('!(record.status in [\'lost\'])')).toEqual({ $not: { status: { $in: ['lost'] } } }); + }); +}); + +describe('compileCelToFilter — string methods', () => { + it('startsWith → $startsWith', () => { + expect(ok("record.name.startsWith('Acme')")).toEqual({ name: { $startsWith: 'Acme' } }); + }); + it('endsWith → $endsWith', () => { + expect(ok("record.email.endsWith('@corp.com')")).toEqual({ email: { $endsWith: '@corp.com' } }); + }); + it('contains → $contains', () => { + expect(ok("record.name.contains('beta')")).toEqual({ name: { $contains: 'beta' } }); + }); +}); + +describe('compileCelToFilter — logical combinators (the #1887 compound-condition target)', () => { + it('&& → $and', () => { + expect(ok("record.stage == 'won' && record.amount >= 500")).toEqual({ + $and: [{ stage: 'won' }, { amount: { $gte: 500 } }], + }); + }); + it('|| → $or', () => { + expect(ok("record.tier == 'gold' || record.tier == 'platinum'")).toEqual({ + $or: [{ tier: 'gold' }, { tier: 'platinum' }], + }); + }); + it('! → $not', () => { + expect(ok('!(record.secret == true)')).toEqual({ $not: { secret: true } }); + }); + it('flattens nested same-operator (a && b && c → one $and)', () => { + expect(ok("record.a == 1 && record.b == 2 && record.c == 3")).toEqual({ + $and: [{ a: 1 }, { b: 2 }, { c: 3 }], + }); + }); + it('nested mixed precedence', () => { + expect(ok("record.region == 'EMEA' && (record.tier == 'gold' || record.amount > 10000)")).toEqual({ + $and: [{ region: 'EMEA' }, { $or: [{ tier: 'gold' }, { amount: { $gt: 10000 } }] }], + }); + }); +}); + +describe('compileCelToFilter — field-to-field ($field reference)', () => { + it('record.a == record.b → $eq $field', () => { + expect(ok('record.created_by == record.owner_id')).toEqual({ + created_by: { $eq: { $field: 'owner_id' } }, + }); + }); +}); + +describe('compileCelToFilter — allow-all constant fold', () => { + it('1 == 1 → {} (no restriction)', () => { + expect(ok('1 == 1')).toEqual({}); + }); + it('true → {}', () => { + expect(ok('true')).toEqual({}); + }); + it('1 == 2 → fails closed (not allow-all)', () => { + expect(fail('1 == 2').ok).toBe(false); + }); +}); + +describe('compileCelToFilter — fail-closed on non-pushdownable (ADR-0055 / D5)', () => { + const unsupported = [ + "record.name.matches('A.*')", // unsupported rcall method + 'record.amount + 1 > 2', // arithmetic + 'size(record.tags) > 0', // function call + 'record.account.region == \'X\'', // cross-object / nested relation traversal + 'account.region == \'X\'', // unknown-root relation traversal + "record.cond ? record.a : record.b == 1", // ternary + ]; + for (const src of unsupported) { + it(`refuses: ${src}`, () => { + const r = fail(src); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe('unsupported'); + }); + } + it('parse error is reported, not thrown', () => { + const r = fail('record.a == '); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe('parse-error'); + }); +}); + +describe('compileCelToFilter — unresolved variable fails closed', () => { + it('missing current_user.* → unresolved-variable', () => { + const r = compileCelToFilter('record.owner_id == current_user.id', { variables: { current_user: {} } }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe('unresolved-variable'); + }); + it('null variable (e.g. no active org) → unresolved-variable (fail closed)', () => { + const r = compileCelToFilter('record.organization_id == current_user.organization_id', { + variables: { current_user: { organization_id: null } }, + }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe('unresolved-variable'); + }); + it('empty membership array still compiles to $in:[] (caller decides)', () => { + expect(ok('id in current_user.org_user_ids', { current_user: { org_user_ids: [] } })).toEqual({ + id: { $in: [] }, + }); + }); +}); + +describe('isPushdownableCel — shape-only gate (no variables)', () => { + const supported = [ + 'owner_id == current_user.id', + "record.region == 'EMEA'", + 'record.amount > 1000', + 'id in current_user.org_user_ids', + "record.status in ['a','b']", + 'record.target_channels != null', + "record.a == 1 && record.b == 2", + "record.name.startsWith('A')", + '1 == 1', + ]; + for (const src of supported) { + it(`accepts: ${src}`, () => expect(isPushdownableCel(src).ok).toBe(true)); + } + const refused = [ + 'record.amount + 1 > 2', + 'size(record.tags) > 0', + "record.account.region == 'X'", + ]; + for (const src of refused) { + it(`rejects: ${src}`, () => expect(isPushdownableCel(src).ok).toBe(false)); + } +}); + +describe('compileCelToFilter — input shapes', () => { + it('accepts a { dialect, source } expression object', () => { + expect(ok({ dialect: 'cel', source: "record.region == 'EMEA'" } as unknown as string)).toBeUndefined; + const r = compileCelToFilter({ source: "record.region == 'EMEA'" }, { variables: VARS }); + expect(r.ok && r.filter).toEqual({ region: 'EMEA' }); + }); + it('custom variableRoots/fieldRoots', () => { + const r = compileCelToFilter('row.dept == ctx.department', { + fieldRoots: ['row'], + variableRoots: ['ctx'], + variables: { ctx: { department: 'sales' } }, + }); + expect(r.ok && r.filter).toEqual({ dept: 'sales' }); + }); +}); diff --git a/packages/formula/src/cel-to-filter.ts b/packages/formula/src/cel-to-filter.ts new file mode 100644 index 0000000000..29c261d307 --- /dev/null +++ b/packages/formula/src/cel-to-filter.ts @@ -0,0 +1,409 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Canonical CEL → FilterCondition pushdown compiler (ADR-0058 D1/D2/D6). + * + * ObjectStack has ONE authoring language (CEL) and ONE good interpreter + * (`cel-engine.ts`), but historically THREE disconnected "compile-to-filter" + * front-ends: `plugin-security/rls-compiler.ts`'s 4-form regex, `plugin-sharing`'s + * `celToFilter`, and the ObjectUI array-AST path. They diverged — which is the + * root of #1887 (a sharing `condition` that the interpreter understands but no + * compiler lowers, so it never enforces). + * + * This module is the single, canonical lowering. It takes the **same parsed + * `@marcbachmann/cel-js` AST the interpreter uses** (`env.parse(src).ast`) and + * lowers the pushdown-able subset to a Mongo-style {@link FilterCondition} — the + * one shape BOTH backends already consume: the ObjectQL engine `where` (AND-injected + * by plugin-security) and the analytics SQL backend + * (`service-analytics/read-scope-sql.ts`). One AST, two backends (D6). + * + * ## Supported subset (ADR-0058 D2) + * `==` `!=` `>` `<` `>=` `<=` · `in` (→ `$in`) · `&&` `||` `!` · + * `== null` / `!= null` (→ `$null`) · string methods `startsWith` / `endsWith` + * / `contains` (→ `$startsWith` / `$endsWith` / `$contains`). + * `not in` is `!(x in y)`. Negation wraps in `$not`. + * + * ## Hard boundaries (ADR-0055 stands) + * - **No subqueries, no cross-object traversal.** A field path is a SINGLE + * column (`record.region` → `region`, bare `owner` → `owner`). A multi-segment + * relation path (`record.account.region`) is an authoring-time compile error, + * not a silent join. + * - Arithmetic (`+ - * / %`), function calls (`size(...)`), ternary, maps, and + * any other non-pushdown shape are a compile error — NEVER silently dropped. + * A dropped predicate leaves an object unprotected; failing closed is the + * security-correct outcome (ADR-0049/0056 D4). + * + * ## Value resolution + * A leaf rooted at a `variableRoot` (default `current_user`) is resolved against + * `opts.variables` to a literal — `current_user.id` → the caller's id, + * `current_user.org_user_ids` → a pre-resolved membership array for `$in` + * (honours ADR-0055: the runtime pre-resolves the set; the compiler never emits + * a subquery). A variable that resolves to `undefined`/`null` yields + * `unresolved-variable` (the "no active org" fail-closed path). + */ + +import { Environment } from '@marcbachmann/cel-js'; +import type { ASTNode } from '@marcbachmann/cel-js'; +import type { FilterCondition } from '@objectstack/spec/data'; + +// --------------------------------------------------------------------------- +// Public contract +// --------------------------------------------------------------------------- + +export type CelFilterFailReason = + /** CEL did not parse (syntax error). */ + | 'parse-error' + /** Shape is not pushdown-able (arithmetic, function call, relation traversal, …). */ + | 'unsupported' + /** A required `variableRoot` reference was undefined/null in `variables`. */ + | 'unresolved-variable'; + +export type CelFilterCompileResult = + | { ok: true; filter: FilterCondition } + | { ok: false; reason: CelFilterFailReason; detail: string }; + +export interface CelFilterCompileOptions { + /** Member-access roots that denote a record FIELD path. Default `['record']`. */ + fieldRoots?: readonly string[]; + /** Roots resolved as VALUES against {@link variables}. Default `['current_user']`. */ + variableRoots?: readonly string[]; + /** + * Value-resolution context, keyed by variable root. e.g. + * `{ current_user: { id, organization_id, org_user_ids } }`. A `record.*` + * (field) reference is NEVER resolved here — only `variableRoot` leaves are. + */ + variables?: Record; +} + +/** Symbol returned for a variable leaf during a shape-only check (never executed). */ +const SHAPE_VALUE = Symbol('cel-filter-shape-placeholder'); + +class CompileError extends Error { + constructor(public reason: CelFilterFailReason, message: string) { + super(message); + this.name = 'CelFilterCompileError'; + } +} + +// A roots-permissive env: parsing is purely syntactic (we read `.ast`, never +// `.check()`/`.evaluate()`), so any identifier or method call parses. Built once. +let parseEnv: Environment | undefined; +function getParseEnv(): Environment { + if (!parseEnv) { + parseEnv = new Environment({ unlistedVariablesAreDyn: true, enableOptionalTypes: true }); + } + return parseEnv; +} + +/** Unwrap a CEL expression input — accepts a raw string or `{ source }`. */ +function toSource(input: string | { source?: string } | null | undefined): string | null { + if (typeof input === 'string') return input.trim() || null; + if (input && typeof input === 'object' && typeof input.source === 'string') { + return input.source.trim() || null; + } + return null; +} + +// --------------------------------------------------------------------------- +// Entry points +// --------------------------------------------------------------------------- + +/** + * Compile a CEL predicate into a {@link FilterCondition}, resolving `variableRoot` + * leaves against `opts.variables`. Returns a discriminated result — never throws + * for an authoring-level fault; a `false` result with a reason is the caller's + * cue to fail closed (deny) or surface a compile error. + */ +export function compileCelToFilter( + input: string | { source?: string }, + opts: CelFilterCompileOptions = {}, +): CelFilterCompileResult { + const source = toSource(input); + if (!source) return { ok: false, reason: 'parse-error', detail: 'empty expression' }; + let ast: ASTNode; + try { + ast = getParseEnv().parse(source).ast; + } catch (err) { + return { ok: false, reason: 'parse-error', detail: (err as Error).message?.split('\n')[0] ?? 'parse error' }; + } + return lowerCelAst(ast, opts, 'value'); +} + +/** + * Shape-only check: is this CEL predicate pushdown-able at all? Used by the + * authoring gate (ADR-0056 D4) to REJECT a predicate the runtime could only + * silently drop. Does not resolve `variables`. + */ +export function isPushdownableCel( + input: string | { source?: string }, + opts: Pick = {}, +): { ok: true } | { ok: false; reason: CelFilterFailReason; detail: string } { + const source = toSource(input); + if (!source) return { ok: false, reason: 'parse-error', detail: 'empty expression' }; + let ast: ASTNode; + try { + ast = getParseEnv().parse(source).ast; + } catch (err) { + return { ok: false, reason: 'parse-error', detail: (err as Error).message?.split('\n')[0] ?? 'parse error' }; + } + const res = lowerCelAst(ast, opts, 'shape'); + return res.ok ? { ok: true } : { ok: false, reason: res.reason, detail: res.detail }; +} + +/** + * Lower a pre-parsed cel-js AST node — the variant that lets the interpreter and + * the compiler share ONE parse (ADR-0058 D6, "one AST, two backends"). + */ +export function lowerCelAst( + ast: ASTNode, + opts: CelFilterCompileOptions = {}, + mode: 'value' | 'shape' = 'value', +): CelFilterCompileResult { + const ctx: Ctx = { + fieldRoots: new Set(opts.fieldRoots ?? ['record']), + variableRoots: new Set(opts.variableRoots ?? ['current_user']), + variables: opts.variables ?? {}, + mode, + }; + try { + return { ok: true, filter: lowerCondition(ast, ctx) }; + } catch (err) { + if (err instanceof CompileError) return { ok: false, reason: err.reason, detail: err.message }; + return { ok: false, reason: 'unsupported', detail: (err as Error).message ?? 'compile error' }; + } +} + +// --------------------------------------------------------------------------- +// Internals +// --------------------------------------------------------------------------- + +interface Ctx { + fieldRoots: Set; + variableRoots: Set; + variables: Record; + mode: 'value' | 'shape'; +} + +type Leaf = + | { kind: 'field'; path: string } + | { kind: 'literal'; value: unknown } + | { kind: 'var'; path: string[] }; + +const FLIP: Record = { '>': '<', '<': '>', '>=': '<=', '<=': '>=', '==': '==', '!=': '!=' }; +const CMP_OP: Record = { '>': '$gt', '>=': '$gte', '<': '$lt', '<=': '$lte' }; +const STRING_METHOD: Record = { startsWith: '$startsWith', endsWith: '$endsWith', contains: '$contains' }; + +/** Lower a boolean-valued node into a FilterCondition. Throws CompileError. */ +function lowerCondition(node: ASTNode, ctx: Ctx): FilterCondition { + const op = node.op; + const args = node.args as unknown; + switch (op) { + case '&&': + return combine('$and', node, ctx); + case '||': + return combine('$or', node, ctx); + case '!_': + return { $not: lowerCondition(args as ASTNode, ctx) }; + case '==': + case '!=': + case '>': + case '>=': + case '<': + case '<=': + return lowerComparison(op, (args as [ASTNode, ASTNode])[0], (args as [ASTNode, ASTNode])[1], ctx); + case 'in': + return lowerMembership((args as [ASTNode, ASTNode])[0], (args as [ASTNode, ASTNode])[1], ctx); + case 'rcall': + return lowerStringMethod(args as [string, ASTNode, ASTNode[]], ctx); + case 'value': { + // A bare boolean condition. `true` → no restriction; anything else fails + // closed (we never let a non-true constant become allow-all). + const v = coerceLiteral(args); + if (v === true) return {}; + throw new CompileError('unsupported', `constant non-true predicate (${String(v)})`); + } + default: + throw new CompileError('unsupported', `unsupported operator "${String(op)}"`); + } +} + +function combine(key: '$and' | '$or', node: ASTNode, ctx: Ctx): FilterCondition { + const [l, r] = node.args as [ASTNode, ASTNode]; + const parts: FilterCondition[] = []; + for (const child of [lowerCondition(l, ctx), lowerCondition(r, ctx)]) { + // Flatten same-key nesting so `a && b && c` is one `$and: [a,b,c]`. + const nested = (child as Record)[key]; + if (Array.isArray(nested) && Object.keys(child).length === 1) parts.push(...(nested as FilterCondition[])); + else parts.push(child); + } + return { [key]: parts } as FilterCondition; +} + +function lowerComparison(op: string, lNode: ASTNode, rNode: ASTNode, ctx: Ctx): FilterCondition { + const L = classify(lNode, ctx); + const R = classify(rNode, ctx); + const lField = L.kind === 'field'; + const rField = R.kind === 'field'; + + if (lField && rField) { + // field-to-field comparison → `{ $field: otherPath }` reference. + return emit((L as { path: string }).path, op, { $field: (R as { path: string }).path }, true); + } + if (lField) return emit((L as { path: string }).path, op, resolveValue(R, ctx), false); + if (rField) return emit((R as { path: string }).path, FLIP[op] ?? op, resolveValue(L, ctx), false); + + // Neither side is a field: a constant comparison. Fold the always-true case + // (`1 == 1`, the RLS allow-all) to "no restriction"; refuse the rest (a + // non-true constant must fail closed, never become allow-all). + const lv = resolveValue(L, ctx); + const rv = resolveValue(R, ctx); + if (ctx.mode === 'shape') return {}; // shape check: structurally fine + const truth = constFold(op, lv, rv); + if (truth === true) return {}; + throw new CompileError('unsupported', `constant ${op} predicate that is not always-true`); +} + +function lowerMembership(elemNode: ASTNode, containerNode: ASTNode, ctx: Ctx): FilterCondition { + const elem = classify(elemNode, ctx); + if (elem.kind !== 'field') { + throw new CompileError('unsupported', `\`in\` requires a field on the left (got ${elem.kind})`); + } + const container = classify(containerNode, ctx); + const value = resolveValue(container, ctx); + if (value !== SHAPE_VALUE && !Array.isArray(value)) { + throw new CompileError('unsupported', `\`in\` requires an array/list on the right`); + } + return { [(elem as { path: string }).path]: { $in: value } } as FilterCondition; +} + +function lowerStringMethod(args: [string, ASTNode, ASTNode[]], ctx: Ctx): FilterCondition { + const [method, receiver, callArgs] = args; + const mapped = STRING_METHOD[method]; + if (!mapped) throw new CompileError('unsupported', `unsupported method "${method}()"`); + const recv = classify(receiver, ctx); + if (recv.kind !== 'field') throw new CompileError('unsupported', `"${method}()" must be called on a field`); + if (!Array.isArray(callArgs) || callArgs.length !== 1) { + throw new CompileError('unsupported', `"${method}()" takes exactly one argument`); + } + const arg = resolveValue(classify(callArgs[0], ctx), ctx); + if (arg !== SHAPE_VALUE && typeof arg !== 'string') { + throw new CompileError('unsupported', `"${method}()" argument must be a string literal`); + } + return { [(recv as { path: string }).path]: { [mapped]: arg } } as FilterCondition; +} + +/** Build `{ field: value }`. `isRef` true → value is a `{ $field }` reference. */ +function emit(field: string, op: string, value: unknown, isRef: boolean): FilterCondition { + if (op === '==') { + if (!isRef && value === null) return { [field]: { $null: true } } as FilterCondition; + if (isRef) return { [field]: { $eq: value } } as FilterCondition; + return { [field]: value } as FilterCondition; // implicit equality + } + if (op === '!=') { + if (!isRef && value === null) return { [field]: { $null: false } } as FilterCondition; + return { [field]: { $ne: value } } as FilterCondition; + } + const cmp = CMP_OP[op]; + if (cmp) return { [field]: { [cmp]: value } } as FilterCondition; + throw new CompileError('unsupported', `unsupported comparison "${op}"`); +} + +/** Syntactically classify an operand node. Throws on a non-pushdown shape. */ +function classify(node: ASTNode, ctx: Ctx): Leaf { + switch (node.op) { + case 'value': + return { kind: 'literal', value: coerceLiteral(node.args) }; + case 'list': { + const items = (node.args as ASTNode[]).map((n) => { + const leaf = classify(n, ctx); + if (leaf.kind !== 'literal') { + throw new CompileError('unsupported', 'list elements must be literals'); + } + return leaf.value; + }); + return { kind: 'literal', value: items }; + } + case 'id': { + const name = node.args as string; + if (ctx.variableRoots.has(name)) return { kind: 'var', path: [name] }; + // Bare identifier = a single record field (RLS convention). + return { kind: 'field', path: name }; + } + case '.': + case '.?': { + const [recv, field] = node.args as [ASTNode, string]; + const chain = memberChain(recv, field); + if (!chain) throw new CompileError('unsupported', 'unsupported member-access expression'); + const [root, ...rest] = chain; + if (ctx.variableRoots.has(root)) return { kind: 'var', path: chain }; + if (ctx.fieldRoots.has(root)) { + if (rest.length !== 1) { + // `record.account.region` = cross-object traversal (ADR-0055): refuse. + throw new CompileError('unsupported', `cross-object/nested field path "${chain.join('.')}" is not pushdown-able`); + } + return { kind: 'field', path: rest[0] }; + } + // A `.`-chain rooted at an unknown identifier = relation traversal. + throw new CompileError('unsupported', `cross-object field path "${chain.join('.')}" is not pushdown-able`); + } + default: + throw new CompileError('unsupported', `unsupported operand "${String(node.op)}"`); + } +} + +/** Flatten a `.`-member chain into `[root, seg, seg, …]`, or null if not a pure path. */ +function memberChain(recv: ASTNode, field: string): string[] | null { + if (recv.op === 'id') return [recv.args as string, field]; + if (recv.op === '.' || recv.op === '.?') { + const [innerRecv, innerField] = recv.args as [ASTNode, string]; + const inner = memberChain(innerRecv, innerField); + return inner ? [...inner, field] : null; + } + return null; +} + +/** Resolve a leaf to its VALUE (literal directly; var via `variables`). */ +function resolveValue(leaf: Leaf, ctx: Ctx): unknown { + if (leaf.kind === 'literal') return leaf.value; + if (leaf.kind === 'field') { + throw new CompileError('unsupported', `expected a value but got field "${leaf.path}"`); + } + // var + if (ctx.mode === 'shape') return SHAPE_VALUE; + let cur: unknown = ctx.variables; + for (const seg of leaf.path) { + if (cur == null || typeof cur !== 'object') { + throw new CompileError('unresolved-variable', `variable "${leaf.path.join('.')}" is not resolvable`); + } + cur = (cur as Record)[seg]; + } + if (cur === undefined || cur === null) { + throw new CompileError('unresolved-variable', `variable "${leaf.path.join('.')}" is ${String(cur)}`); + } + return cur; +} + +/** Coerce a cel-js literal to a plain JS value (cel-js uses BigInt for ints). */ +function coerceLiteral(v: unknown): unknown { + if (typeof v === 'bigint') return Number(v); + if (v === null || typeof v === 'string' || typeof v === 'number' || typeof v === 'boolean') return v; + if (typeof v === 'object' && v !== null && typeof (v as { valueOf?: unknown }).valueOf === 'function') { + const prim = (v as { valueOf: () => unknown }).valueOf(); + if (typeof prim === 'bigint') return Number(prim); + if (typeof prim === 'number' || typeof prim === 'string' || typeof prim === 'boolean') return prim; + } + throw new CompileError('unsupported', `unsupported literal type "${typeof v}"`); +} + +/** Compile-time fold of a comparison between two concrete values. */ +function constFold(op: string, l: unknown, r: unknown): boolean | undefined { + switch (op) { + case '==': return l === r; + case '!=': return l !== r; + case '>': return (l as number) > (r as number); + case '>=': return (l as number) >= (r as number); + case '<': return (l as number) < (r as number); + case '<=': return (l as number) <= (r as number); + default: return undefined; + } +} diff --git a/packages/formula/src/index.ts b/packages/formula/src/index.ts index 219bb106c5..59134dbbef 100644 --- a/packages/formula/src/index.ts +++ b/packages/formula/src/index.ts @@ -16,6 +16,11 @@ export { templateEngine, TEMPLATE_FORMATTERS, formatValue } from './template-eng export { registerStdLib, buildScope } from './stdlib'; export { resolveSeed, resolveSeedRecord } from './seed-eval'; export { normalizeExpression, normalizeExpressionTree } from './normalize'; +// ADR-0058 — canonical CEL → FilterCondition pushdown compiler (one AST, +// two backends). Replaces the regex/celToFilter front-ends in plugin-security +// and plugin-sharing; honours ADR-0055 (no subquery / no cross-object traversal). +export { compileCelToFilter, isPushdownableCel, lowerCelAst } from './cel-to-filter'; +export type { CelFilterCompileResult, CelFilterCompileOptions, CelFilterFailReason } from './cel-to-filter'; // ADR-0032 — shared validator + introspection (one validator for build, // registration, and the agent-callable validate_expression tool). export { validateExpression, introspectScope, expectedDialect, CEL_STDLIB_FUNCTIONS } from './validate'; From 8a9e90ee697768cf6b65cb375368a950d332eef1 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 21 Jun 2026 12:46:06 +0800 Subject: [PATCH 2/7] =?UTF-8?q?refactor(security,sharing):=20ADR-0058=20P2?= =?UTF-8?q?=20=E2=80=94=20cut=20RLS=20+=20sharing=20over=20to=20the=20one?= =?UTF-8?q?=20compiler?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the two bespoke compile-to-filter front-ends with delegation to the canonical @objectstack/formula CEL→FilterCondition compiler (P1): - plugin-security/rls-compiler.ts: compileExpression + isSupportedRlsExpression now delegate to the compiler. A thin sqlPredicateToCel() bridges the legacy SQL-ish `using` subset (`=`→`==`, `IN`→`in`, quoted literals untouched) so authored policies keep working while the bespoke 4-form regex is retired. compileFilter's orchestration (userCtx build, §7.3.1 membership merge, null→ RLS_DENY aggregation) is unchanged; an isEmptyMembershipFilter() guard preserves the "empty pre-resolved set drops the policy" parity so the single- policy path still yields the deny sentinel, not a permissive `$in: []`. - plugin-sharing celToFilter: delegates to the compiler. A sharing condition is a pure record predicate, so compound criteria (AND/OR, comparisons, null, in) now lower to criteria_json instead of being skipped — the substance of #1887 (proven e2e in P3). Security contract unchanged: non-pushdownable shapes (arithmetic, functions, subqueries, cross-object, SQL AND) and unresolved current_user.* still fail closed. The shape gate (isSupportedRlsExpression) legitimately BROADENS — `==`, comparisons and CEL compound predicates now report supported because the runtime genuinely enforces them; the gate stays shape-only (variable exposure is a separate concern). Tests updated to the new, larger enforceable set with rationale. Parity verified green: plugin-security 112, plugin-sharing 63, service-analytics 137 (the RLS→SQL read-scope bridge). Co-Authored-By: Claude Opus 4.8 --- packages/plugins/plugin-security/package.json | 3 +- .../plugin-security/src/rls-compiler.ts | 135 +++++++++--------- .../src/security-plugin.test.ts | 27 +++- packages/plugins/plugin-sharing/package.json | 3 +- .../src/bootstrap-declared-sharing-rules.ts | 36 ++--- pnpm-lock.yaml | 6 + 6 files changed, 115 insertions(+), 95 deletions(-) diff --git a/packages/plugins/plugin-security/package.json b/packages/plugins/plugin-security/package.json index 2b9b3058f9..fa22142fdd 100644 --- a/packages/plugins/plugin-security/package.json +++ b/packages/plugins/plugin-security/package.json @@ -2,7 +2,7 @@ "name": "@objectstack/plugin-security", "version": "9.11.0", "license": "Apache-2.0", - "description": "Security Plugin for ObjectStack — RBAC, RLS, and Field-Level Security Runtime", + "description": "Security Plugin for ObjectStack \u2014 RBAC, RLS, and Field-Level Security Runtime", "main": "dist/index.js", "types": "dist/index.d.ts", "exports": { @@ -18,6 +18,7 @@ }, "dependencies": { "@objectstack/core": "workspace:*", + "@objectstack/formula": "workspace:*", "@objectstack/platform-objects": "workspace:*", "@objectstack/spec": "workspace:*" }, diff --git a/packages/plugins/plugin-security/src/rls-compiler.ts b/packages/plugins/plugin-security/src/rls-compiler.ts index a14faffa34..0b54806e73 100644 --- a/packages/plugins/plugin-security/src/rls-compiler.ts +++ b/packages/plugins/plugin-security/src/rls-compiler.ts @@ -2,6 +2,7 @@ import type { RowLevelSecurityPolicy } from '@objectstack/spec/security'; import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { compileCelToFilter, isPushdownableCel } from '@objectstack/formula'; /** * RLS User Context @@ -71,13 +72,48 @@ export const RLS_DENY_FILTER: Record = Object.freeze({ * object unprotected. A `false` here means "this predicate will never enforce". */ export function isSupportedRlsExpression(expression: string): boolean { - if (!expression) return false; - const e = expression.trim(); - if (/^\s*1\s*=\s*1\s*$/.test(e)) return true; - if (/^\s*\w+\s*=\s*current_user\.\w+\s*$/.test(e)) return true; - if (/^\s*\w+\s*=\s*'[^']*'\s*$/.test(e)) return true; - if (/^\s*\w+\s+IN\s+\(\s*current_user\.\w+\s*\)\s*$/i.test(e)) return true; - return false; + if (!expression || !expression.trim()) return false; + // ADR-0058 D1: a single canonical shape gate. We bridge the legacy SQL-ish + // subset (`=`, `IN`) to canonical CEL, then ask the ONE pushdown compiler + // whether the shape lowers to a FilterCondition at all. This is broader than + // the historical 4 forms — comparisons (`amount > 100`) and `==` now ENFORCE + // (the compiler lowers them), so the gate correctly reports them supported. + // It is SHAPE-only: whether a referenced `current_user.*` variable is exposed + // at runtime is a separate availability concern (an unexposed var fails closed + // at resolution — see compileExpression). + return isPushdownableCel(sqlPredicateToCel(expression)).ok; +} + +/** + * Bridge the legacy SQL-ish RLS `using` subset to canonical CEL so it flows + * through the ONE compiler: `=` → `==`, `IN` → `in`. Quoted string literals are + * left untouched. Only this historically-supported subset is bridged — compound + * predicates should be authored in canonical CEL (`&&` / `||`); anything outside + * the subset (subqueries, SQL `AND`/`OR`, `LIKE`) stays unparseable and so fails + * closed, exactly as before. + */ +export function sqlPredicateToCel(expression: string): string { + return expression.replace(/'[^']*'|\bIN\b|(?=!])=(?!=)/gi, (m) => { + if (m[0] === "'") return m; // quoted literal — never rewrite its contents + if (m === '=') return '=='; + return 'in'; // IN / in / In → CEL membership operator + }); +} + +/** + * 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: []`. + */ +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; } /** @@ -171,71 +207,40 @@ export class RLSCompiler { } /** - * Compile a single RLS expression into a query filter. + * Compile a single RLS predicate into a query filter (ADR-0058 D1/D2). * - * This reference compiler recognizes exactly four forms — anything else - * returns `null` and (via {@link compileFilter}) fails closed: - * - `field = current_user.property` → `{ field: }` - * - `field = 'literal_value'` → `{ field: 'literal_value' }` - * - `field IN (current_user.array)` → `{ field: { $in: [...] } }` - * (the array may be a §7.3.1 pre-resolved membership set) - * - `1 = 1` → `{}` (always-true / no restriction) - * - * There is intentionally no support for subqueries, `LIKE`/`ILIKE`, - * regex, `ANY`/`ALL`, `AND`/`OR`/`NOT`, or `NULL` checks — express those - * needs as a `current_user.*` property the runtime pre-resolves instead. + * Delegates to the ONE canonical CEL → FilterCondition pushdown compiler in + * `@objectstack/formula`, after bridging the legacy SQL-ish subset to CEL + * ({@link sqlPredicateToCel}). `current_user.*` references resolve against the + * pre-resolved {@link RLSUserContext} (incl. §7.3.1 membership sets). The + * supported subset is now broader than the historical four forms — `==`/`!=`, + * comparisons, `in`, `&&`/`||`/`!`, null checks and string ops all lower — but + * the security contract is unchanged: + * - a non-pushdownable shape (subquery, arithmetic, cross-object, SQL `AND`) + * → `null` → {@link compileFilter} fails closed; + * - 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: []`. */ compileExpression( expression: string, userCtx: RLSUserContext ): Record | null { if (!expression) return null; - - // Always-true literal: "1 = 1" → no restriction (match every row). - // Lets RLS.allowAllPolicy ('1 = 1' for privileged roles) grant access - // instead of silently failing closed. An empty filter AND's onto the - // caller's where clause as a no-op. - if (/^\s*1\s*=\s*1\s*$/.test(expression)) { - return {}; - } - - // Handle simple equality: "field = current_user.property" - const eqMatch = expression.match(/^\s*(\w+)\s*=\s*current_user\.(\w+)\s*$/); - if (eqMatch) { - const [, field, prop] = eqMatch; - const value = userCtx[prop]; - // Skip when the user-context value is missing (undefined or null). - // A `null` `organization_id` means "no active organization" — applying - // the filter as `organization_id IS NULL` would silently expose every - // un-tenanted row across tenants and break system tables that lack the - // column entirely. Treating null as "skip this policy" makes the - // tenant_isolation rule safely opt-out for users without an active org - // while still applying when one is set. - if (value === undefined || value === null) return null; - return { [field]: value }; - } - - // Handle literal equality: "field = 'value'" - const litMatch = expression.match(/^\s*(\w+)\s*=\s*'([^']*)'\s*$/); - if (litMatch) { - const [, field, value] = litMatch; - return { [field]: value }; - } - - // Handle IN: "field IN (current_user.array_property)" - const inMatch = expression.match(/^\s*(\w+)\s+IN\s+\(\s*current_user\.(\w+)\s*\)\s*$/i); - if (inMatch) { - const [, field, prop] = inMatch; - const value = userCtx[prop]; - if (!Array.isArray(value) || value.length === 0) return null; - return { [field]: { $in: value } }; - } - - // Unsupported expression: return null (no additional RLS filter applied). - // Note: callers should treat absence of RLS policies as "allow all" only when - // no policies are defined. If policies exist but cannot be compiled, the caller - // may want to deny access as a safety measure. - return null; + const result = compileCelToFilter(sqlPredicateToCel(expression), { + variables: { current_user: userCtx as Record }, + }); + // Any fault — unsupported shape, parse error, or an unresolved/null + // `current_user.*` variable — drops the policy. With a single applicable + // policy this surfaces as RLS_DENY_FILTER upstream (fail closed). + if (!result.ok) return null; + // 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. + if (isEmptyMembershipFilter(result.filter as Record)) return null; + return result.filter as Record; } /** diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index 0c01aae2b6..3e94b48c3c 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -1199,17 +1199,28 @@ describe('RLSCompiler', () => { // --------------------------------------------------------------------------- describe('RLSCompiler D4 — uncompilable predicates are surfaced', () => { it('isSupportedRlsExpression accepts the compilable shapes', () => { + // Legacy SQL-ish subset (bridged `=`/`IN`). expect(isSupportedRlsExpression('owner_id = current_user.id')).toBe(true); expect(isSupportedRlsExpression('owner = current_user.email')).toBe(true); expect(isSupportedRlsExpression("status = 'published'")).toBe(true); expect(isSupportedRlsExpression('id IN (current_user.org_user_ids)')).toBe(true); expect(isSupportedRlsExpression('1 = 1')).toBe(true); - }); - - it('isSupportedRlsExpression rejects shapes the runtime would drop', () => { - expect(isSupportedRlsExpression('owner == current_user.name')).toBe(false); // `==` - expect(isSupportedRlsExpression('a = current_user.id AND b = 1')).toBe(false); // AND - expect(isSupportedRlsExpression('amount > 100')).toBe(false); // range + // ADR-0058: the canonical compiler lowers a broader pushdown subset, so the + // shape gate now (correctly) reports these as enforceable — `==`/`!=`, + // comparisons, and CEL compound predicates all compile to a FilterCondition. + expect(isSupportedRlsExpression('owner == current_user.id')).toBe(true); // `==` + expect(isSupportedRlsExpression('amount > 100')).toBe(true); // comparison + expect(isSupportedRlsExpression('region != null')).toBe(true); // null check + expect(isSupportedRlsExpression('a == 1 && b == 2')).toBe(true); // CEL compound + }); + + it('isSupportedRlsExpression rejects genuinely non-pushdownable shapes', () => { + // These cannot lower to a FilterCondition for ANY input, so the gate must + // reject them (ADR-0055 / ADR-0056 D4) — they fail closed at runtime. + expect(isSupportedRlsExpression('a = current_user.id AND b = 1')).toBe(false); // SQL AND ≠ CEL && (unparseable) + expect(isSupportedRlsExpression('amount + 1 > 2')).toBe(false); // arithmetic + expect(isSupportedRlsExpression('id IN (SELECT id FROM users)')).toBe(false); // subquery + expect(isSupportedRlsExpression('record.a.b == 1')).toBe(false); // cross-object traversal expect(isSupportedRlsExpression('')).toBe(false); }); @@ -1217,7 +1228,9 @@ describe('RLSCompiler D4 — uncompilable predicates are surfaced', () => { const warned: string[] = []; const compiler = new RLSCompiler(); compiler.setLogger({ warn: (message: string) => warned.push(message) }); - const policy: any = { name: 'bad', object: 'thing', operation: 'select', using: 'owner == current_user.name' }; + // ADR-0058: a genuinely non-pushdownable shape (arithmetic) — no input can + // lower it, so it must WARN (vs a valid shape whose var is merely absent). + const policy: any = { name: 'bad', object: 'thing', operation: 'select', using: 'amount + 1 > 2' }; const filter = compiler.compileFilter([policy], { userId: 'u1' } as any); expect(filter).toEqual(RLS_DENY_FILTER); // only policy dropped → fail-closed expect(warned.length).toBe(1); diff --git a/packages/plugins/plugin-sharing/package.json b/packages/plugins/plugin-sharing/package.json index f1dfd18b46..c0bd9b6c1b 100644 --- a/packages/plugins/plugin-sharing/package.json +++ b/packages/plugins/plugin-sharing/package.json @@ -2,7 +2,7 @@ "name": "@objectstack/plugin-sharing", "version": "9.11.0", "license": "Apache-2.0", - "description": "Record-level sharing for ObjectStack — sys_record_share + middleware that enforces sharingModel + ISharingService.", + "description": "Record-level sharing for ObjectStack \u2014 sys_record_share + middleware that enforces sharingModel + ISharingService.", "main": "dist/index.js", "types": "dist/index.d.ts", "exports": { @@ -18,6 +18,7 @@ }, "dependencies": { "@objectstack/core": "workspace:*", + "@objectstack/formula": "workspace:*", "@objectstack/objectql": "workspace:*", "@objectstack/platform-objects": "workspace:*", "@objectstack/spec": "workspace:*" diff --git a/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts b/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts index b2f273cec7..fc9138e868 100644 --- a/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts +++ b/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts @@ -23,6 +23,7 @@ import type { SharingRuleService } from './sharing-rule-service.js'; import type { SharingRuleRecipientType, ShareAccessLevel } from '@objectstack/spec/contracts'; +import { compileCelToFilter } from '@objectstack/formula'; const SYSTEM_CTX = { isSystem: true, roles: [], permissions: [] } as const; @@ -42,29 +43,22 @@ function mapRecipientType(t: unknown): SharingRuleRecipientType | null { } /** - * Reduce a simple CEL predicate to a JSON FilterCondition. Handles the common - * `record.field == ` shape (string/number/bool). Anything else (AND/OR, - * comparisons, `current_user.*`, functions) returns null → caller skips. + * Compile a sharing-rule CEL `condition` into the runtime `criteria_json` + * FilterCondition (ADR-0058 D1, the substance of #1887). + * + * Delegates to the ONE canonical CEL → FilterCondition pushdown compiler in + * `@objectstack/formula`. A sharing condition is a pure record predicate — no + * `current_user.*` — so it resolves with the default `record` field root and an + * empty variable scope. This lowers the full pushdown subset (`==`/`!=`, + * comparisons, `in`, `&&`/`||`/`!`, `== null`, string ops), not just the former + * `record.field == ` shape, so compound criteria now SEED and ENFORCE + * instead of being skipped as experimental. Anything non-pushdownable (functions, + * cross-object traversal) still returns null → the caller skips it (logged), + * never seeding a permissive match-all (ADR-0049). */ export function celToFilter(cel: unknown): Record | null { - // The spec coerces `condition` to an ExpressionInput object - // ({ dialect: 'cel', source: '...' }); accept that or a raw string. - let src: string | null = null; - if (typeof cel === 'string') src = cel; - else if (cel && typeof cel === 'object' && typeof (cel as any).source === 'string') src = (cel as any).source; - if (!src) return null; - const m = src.trim().match(/^record\.([A-Za-z_][A-Za-z0-9_]*)\s*==\s*(.+)$/); - if (!m) return null; - const field = m[1]; - const raw = m[2].trim(); - let val: unknown; - if ((raw.startsWith("'") && raw.endsWith("'")) || (raw.startsWith('"') && raw.endsWith('"'))) { - val = raw.slice(1, -1); - } else if (raw === 'true') val = true; - else if (raw === 'false') val = false; - else if (/^-?\d+(\.\d+)?$/.test(raw)) val = Number(raw); - else return null; // references / non-literal → not statically translatable - return { [field]: val }; + const result = compileCelToFilter(cel as string | { source?: string }, { variables: {} }); + return result.ok ? (result.filter as Record) : null; } function readDeclared(engine: any, type: string): any[] { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 40a496ed9f..b52ce288bd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1531,6 +1531,9 @@ importers: '@objectstack/core': specifier: workspace:* version: link:../../core + '@objectstack/formula': + specifier: workspace:* + version: link:../../formula '@objectstack/platform-objects': specifier: workspace:* version: link:../../platform-objects @@ -1553,6 +1556,9 @@ importers: '@objectstack/core': specifier: workspace:* version: link:../../core + '@objectstack/formula': + specifier: workspace:* + version: link:../../formula '@objectstack/objectql': specifier: workspace:* version: link:../../objectql From 74801d7d2a1bb31e1db5cebe17e938dc04aec174 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 21 Jun 2026 12:56:06 +0800 Subject: [PATCH 3/7] =?UTF-8?q?feat(sharing):=20ADR-0058=20D3=20=E2=80=94?= =?UTF-8?q?=20close=20#1887,=20compound=20sharing=20conditions=20enforced?= =?UTF-8?q?=20e2e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sharing-rule CEL `condition` is now compiled to a compound `criteria_json` by the canonical compiler (P2) and enforced end-to-end: records matching the full predicate materialise sys_record_share grants. Drop the "⚠️ EXPERIMENTAL — NOT ENFORCED" block on SharingRuleSchema; only `owner`-type rules and `group`/`guest` recipients remain experimental (no static/runtime mapping), and a non-pushdownable condition is skipped + logged (never a permissive match-all). Proof (plugin-sharing): a compound `record.amount >= 100000 && record.name == "Big1"` lowers to `{$and:[…]}`, persists as criteria_json, and shares ONLY opp1 (the full-AND match) — not opp2 (right amount, wrong name). Also proves `||` + `== null` lower and that a non-pushdownable condition returns null. Co-Authored-By: Claude Opus 4.8 --- .../src/bootstrap-declared-sharing-rules.ts | 4 +- .../plugin-sharing/src/sharing-rule.test.ts | 59 +++++++++++++++++++ packages/spec/src/security/sharing.zod.ts | 19 +++--- 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts b/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts index fc9138e868..457091c90b 100644 --- a/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts +++ b/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts @@ -13,7 +13,9 @@ * over-sharing would be worse than not enforcing (ADR-0049): * - `owner`-type rules (`ownedBy`): role membership is dynamic, no static * `criteria_json` equivalent. - * - CEL `condition` the mini-translator can't reduce to a field equality. + * - a CEL `condition` the canonical compiler cannot lower (functions, + * cross-object traversal) — ADR-0058 D2. Compound predicates (AND/OR, + * comparisons, null, in) DO lower now and are enforced (ADR-0058 D3, #1887). * - `sharedWith.type` of `group`/`guest`: no runtime recipient mapping. * * Seeding upserts via `SharingRuleService.defineRule` (idempotent by name) and diff --git a/packages/plugins/plugin-sharing/src/sharing-rule.test.ts b/packages/plugins/plugin-sharing/src/sharing-rule.test.ts index af0b7d8aff..9bd65ba1f9 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule.test.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule.test.ts @@ -5,6 +5,7 @@ import { SharingService } from './sharing-service.js'; import { SharingRuleService } from './sharing-rule-service.js'; import { TeamGraphService, expandPrincipal } from './team-graph.js'; import { BusinessUnitGraphService } from './business-unit-graph.js'; +import { celToFilter } from './bootstrap-declared-sharing-rules.js'; interface Row { [k: string]: any } @@ -346,3 +347,61 @@ describe('SharingRuleService', () => { expect(new Set(engine._tables.sys_record_share.map(s => s.recipient_id))).toEqual(new Set(['alice', 'bob'])); }); }); + +// --------------------------------------------------------------------------- +// #1887 — sharing CEL `condition` compiled & ENFORCED end-to-end (ADR-0058 D3) +// +// Before P2, a compound CEL condition returned null from celToFilter and the +// rule was SKIPPED (decorative metadata, #1887). Now the canonical compiler +// lowers it to a compound `criteria_json` that the runtime matching honours, so +// exactly the records satisfying the full predicate materialise grants. +// --------------------------------------------------------------------------- +describe('#1887 — compound sharing condition compiled + enforced (ADR-0058 D3)', () => { + let engine: ReturnType; + let sharing: SharingService; + let rules: SharingRuleService; + const SYS = { isSystem: true, organizationId: 'org1' } as any; + + beforeEach(() => { + engine = makeEngine(); + engine._tables.opportunity = [ + { id: 'opp1', name: 'Big1', amount: 200000 }, // matches amount>=100k AND name=Big1 + { id: 'opp2', name: 'Big2', amount: 150000 }, // amount ok but wrong name + { id: 'opp3', name: 'Small', amount: 5000 }, // neither + ]; + sharing = new SharingService({ engine: engine as any }); + rules = new SharingRuleService({ engine: engine as any, sharing }); + }); + + it('celToFilter lowers a COMPOUND CEL condition to a compound FilterCondition', () => { + expect(celToFilter('record.amount >= 100000 && record.name == "Big1"')) + .toEqual({ $and: [{ amount: { $gte: 100000 } }, { name: 'Big1' }] }); + // accepts the { dialect, source } authoring shape + expect(celToFilter({ dialect: 'cel', source: 'record.amount >= 100000' })) + .toEqual({ amount: { $gte: 100000 } }); + // null-check & disjunction lower too (no longer field-equality-only) + expect(celToFilter('record.region == null || record.tier == "gold"')).toEqual({ + $or: [{ region: { $null: true } }, { tier: 'gold' }], + }); + // non-pushdownable → null → caller skips (never a permissive match-all) + expect(celToFilter('size(record.tags) > 0')).toBeNull(); + }); + + it('shares ONLY the records satisfying the full AND (not just one conjunct)', async () => { + const r = await rules.defineRule({ + name: 'big1_hv', label: 'Big1 high-value', object: 'opportunity', + criteria: celToFilter('record.amount >= 100000 && record.name == "Big1"'), + recipientType: 'user', recipientId: 'alice', accessLevel: 'read', + }, SYS); + // The CEL condition persisted as a COMPOUND criteria_json (not skipped). + expect(JSON.parse(engine._tables.sys_sharing_rule[0].criteria_json)) + .toEqual({ $and: [{ amount: { $gte: 100000 } }, { name: 'Big1' }] }); + + const res = await rules.evaluateRule(r.id, SYS); + expect(res.matchedRecords).toBe(1); // only opp1 — opp2 fails the name conjunct + const shared = (engine._tables.sys_record_share ?? []) + .filter((sh: any) => sh.recipient_id === 'alice') + .map((sh: any) => sh.record_id); + expect(shared).toEqual(['opp1']); + }); +}); diff --git a/packages/spec/src/security/sharing.zod.ts b/packages/spec/src/security/sharing.zod.ts index 291158f026..b155c940c1 100644 --- a/packages/spec/src/security/sharing.zod.ts +++ b/packages/spec/src/security/sharing.zod.ts @@ -94,13 +94,18 @@ export const OwnerSharingRuleSchema = lazySchema(() => BaseSharingRuleSchema.ext /** * Master Sharing Rule Schema * - * ⚠️ EXPERIMENTAL — NOT ENFORCED as authored (ADR-0049, #1887). - * The live engine enforces a divergent runtime model (`sys_sharing_rule`: JSON - * `criteria_json`, recipient enum `user`/`team`/`department`/`role`/`queue`). - * This spec's CEL `condition` is never compiled (unparsable CEL degrades to - * "match nothing"), and recipients such as `role_and_subordinates`/`guest` have - * no runtime mapping. Authoring a rule via this schema does NOT grant access — - * use `sys_sharing_rule` directly until reconciliation. Tracked by #1887 (M2). + * ADR-0058 D3 — closes #1887. The CEL `condition` of a criteria-based rule is + * COMPILED to the runtime `criteria_json` FilterCondition by the canonical + * `@objectstack/formula` compiler at seed / `defineRule` time, and ENFORCED: + * records matching the criteria materialise `sys_record_share` grants for the + * resolved recipients. Supported recipients: `user` / `team` / `business_unit` / + * `role` / `role_and_subordinates` (→ `unit_and_subordinates`, ADR-0057 D5). + * + * Still `[experimental — not enforced]` (ADR-0049): `owner`-type rules + * (`ownedBy` — depends on live role membership, with no static `criteria_json` + * equivalent) and `group` / `guest` recipients (no runtime recipient mapping). + * A `condition` the compiler cannot lower (functions, cross-object traversal) is + * skipped and logged — never seeded as a permissive match-all. */ export const SharingRuleSchema = lazySchema(() => z.discriminatedUnion('type', [ CriteriaSharingRuleSchema, From c7fa21541b5fdf7016e9c55be3a5e5e19dbfaf44 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 21 Jun 2026 13:06:02 +0800 Subject: [PATCH 4/7] =?UTF-8?q?feat(security):=20ADR-0058=20D4=20=E2=80=94?= =?UTF-8?q?=20enforce=20the=20RLS=20`check`=20clause=20on=20writes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The RLS `check` clause (the PostgreSQL WITH CHECK analog) was declared in the schema but never read — a declared-but-unenforced gap (ADR-0049). Close it: - @objectstack/formula gains matchesFilterCondition(record, filter) — the single-record backend for the canonical FilterCondition shape (ADR-0058 D6), completing the round-trip: compile CEL → filter, run as `where`, lower to SQL, and now match one in-memory record for write validation. Fail-closed on any unknown operator / malformed node (20-case test). - RLSCompiler.compileFilter gains a `clause` selector; the write pass sources the predicate from `check ?? using`. Also fixes a latent bug surfaced by check-only policies: when no applicable policy carries a predicate for the pass, return null (no filter) instead of the deny sentinel — a check-only policy no longer spuriously denies the `using` read/pre-image path. - security-plugin step 3.6: on insert/update, compile the declared `check` clauses and match the POST-IMAGE (insert → new row; by-id update → pre-image ∪ change set) against them; a violating write is denied (fail closed, D5). Scoped to policies that explicitly declare `check`, so objects governed only by `using` are unaffected (zero blast radius — no seeded policy declares check). Bulk updates are governed by the using-scoped where and logged as not post-image-validated. Proof: 7 cases — compliant/violating/missing-field insert, post-image satisfy/violate/unchanged update, delete-unaffected. Suites green: security 119, formula 201, analytics 137, dogfood 21. Co-Authored-By: Claude Opus 4.8 --- packages/formula/src/index.ts | 1 + packages/formula/src/matches-filter.test.ts | 110 +++++++++++++++++ packages/formula/src/matches-filter.ts | 115 ++++++++++++++++++ .../plugin-security/src/rls-compiler.ts | 35 ++++-- .../src/security-plugin.test.ts | 79 ++++++++++++ .../plugin-security/src/security-plugin.ts | 83 +++++++++++++ 6 files changed, 413 insertions(+), 10 deletions(-) create mode 100644 packages/formula/src/matches-filter.test.ts create mode 100644 packages/formula/src/matches-filter.ts diff --git a/packages/formula/src/index.ts b/packages/formula/src/index.ts index 59134dbbef..5395009283 100644 --- a/packages/formula/src/index.ts +++ b/packages/formula/src/index.ts @@ -21,6 +21,7 @@ export { normalizeExpression, normalizeExpressionTree } from './normalize'; // and plugin-sharing; honours ADR-0055 (no subquery / no cross-object traversal). export { compileCelToFilter, isPushdownableCel, lowerCelAst } from './cel-to-filter'; export type { CelFilterCompileResult, CelFilterCompileOptions, CelFilterFailReason } from './cel-to-filter'; +export { matchesFilterCondition } from './matches-filter'; // ADR-0032 — shared validator + introspection (one validator for build, // registration, and the agent-callable validate_expression tool). export { validateExpression, introspectScope, expectedDialect, CEL_STDLIB_FUNCTIONS } from './validate'; diff --git a/packages/formula/src/matches-filter.test.ts b/packages/formula/src/matches-filter.test.ts new file mode 100644 index 0000000000..a9853abc90 --- /dev/null +++ b/packages/formula/src/matches-filter.test.ts @@ -0,0 +1,110 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, expect, it } from 'vitest'; + +import { matchesFilterCondition as m } from './matches-filter'; + +const rec = { + id: 'r1', owner_id: 'u1', org: 'org1', amount: 1000, stage: 'won', + region: null as string | null, name: 'Acme Beta', created_by: 'u1', +}; + +describe('matchesFilterCondition — basics', () => { + it('null/empty filter matches everything', () => { + expect(m(rec, null)).toBe(true); + expect(m(rec, {})).toBe(true); + }); + it('implicit equality', () => { + expect(m(rec, { owner_id: 'u1' })).toBe(true); + expect(m(rec, { owner_id: 'u2' })).toBe(false); + }); + it('{ field: null } → IS NULL', () => { + expect(m(rec, { region: null })).toBe(true); + expect(m(rec, { stage: null })).toBe(false); + }); + it('multiple keys are AND-ed', () => { + expect(m(rec, { owner_id: 'u1', stage: 'won' })).toBe(true); + expect(m(rec, { owner_id: 'u1', stage: 'lost' })).toBe(false); + }); +}); + +describe('matchesFilterCondition — operators', () => { + it('$eq / $ne', () => { + expect(m(rec, { stage: { $eq: 'won' } })).toBe(true); + expect(m(rec, { stage: { $ne: 'lost' } })).toBe(true); + expect(m(rec, { stage: { $ne: 'won' } })).toBe(false); + }); + it('$gt/$gte/$lt/$lte', () => { + expect(m(rec, { amount: { $gte: 1000 } })).toBe(true); + expect(m(rec, { amount: { $gt: 1000 } })).toBe(false); + expect(m(rec, { amount: { $lt: 2000 } })).toBe(true); + expect(m(rec, { amount: { $lte: 999 } })).toBe(false); + }); + it('$in / $nin', () => { + expect(m(rec, { stage: { $in: ['won', 'open'] } })).toBe(true); + expect(m(rec, { stage: { $in: ['lost'] } })).toBe(false); + expect(m(rec, { stage: { $nin: ['lost'] } })).toBe(true); + expect(m(rec, { stage: { $nin: ['won'] } })).toBe(false); + }); + it('$between', () => { + expect(m(rec, { amount: { $between: [500, 1500] } })).toBe(true); + expect(m(rec, { amount: { $between: [1100, 1500] } })).toBe(false); + }); + it('string ops', () => { + expect(m(rec, { name: { $contains: 'Beta' } })).toBe(true); + expect(m(rec, { name: { $startsWith: 'Acme' } })).toBe(true); + expect(m(rec, { name: { $endsWith: 'Beta' } })).toBe(true); + expect(m(rec, { name: { $notContains: 'Zeta' } })).toBe(true); + expect(m(rec, { name: { $startsWith: 'Zzz' } })).toBe(false); + }); + it('$null / $exists', () => { + expect(m(rec, { region: { $null: true } })).toBe(true); + expect(m(rec, { stage: { $null: false } })).toBe(true); + expect(m(rec, { stage: { $exists: true } })).toBe(true); + expect(m(rec, { missing: { $exists: false } })).toBe(true); + }); + it('$field reference (field-to-field)', () => { + expect(m(rec, { created_by: { $eq: { $field: 'owner_id' } } })).toBe(true); + expect(m(rec, { created_by: { $eq: { $field: 'org' } } })).toBe(false); + }); +}); + +describe('matchesFilterCondition — combinators', () => { + it('$and', () => { + expect(m(rec, { $and: [{ stage: 'won' }, { amount: { $gte: 500 } }] })).toBe(true); + expect(m(rec, { $and: [{ stage: 'won' }, { amount: { $gte: 5000 } }] })).toBe(false); + }); + it('$or', () => { + expect(m(rec, { $or: [{ stage: 'lost' }, { amount: { $gte: 500 } }] })).toBe(true); + expect(m(rec, { $or: [{ stage: 'lost' }, { amount: { $gte: 5000 } }] })).toBe(false); + expect(m(rec, { $or: [] })).toBe(false); // empty OR matches nothing + }); + it('$not', () => { + expect(m(rec, { $not: { stage: 'lost' } })).toBe(true); + expect(m(rec, { $not: { stage: 'won' } })).toBe(false); + }); + it('nested compound (the compiled compound-condition shape)', () => { + const f = { $and: [{ org: 'org1' }, { $or: [{ stage: 'won' }, { amount: { $gt: 9999 } }] }] }; + expect(m(rec, f)).toBe(true); + expect(m({ ...rec, org: 'org2' }, f)).toBe(false); + }); +}); + +describe('matchesFilterCondition — FAIL CLOSED', () => { + it('unknown operator → false', () => { + expect(m(rec, { amount: { $regex: '.*' } as never })).toBe(false); + }); + it('unknown top-level operator → false', () => { + expect(m(rec, { $weird: [] } as never)).toBe(false); + }); + it('nested relation object (non-$ key) → false', () => { + expect(m(rec, { account: { region: 'EMEA' } } as never)).toBe(false); + }); + it('bare array value → false', () => { + expect(m(rec, { stage: ['won'] } as never)).toBe(false); + }); + it('malformed (array/scalar) filter → false', () => { + expect(m(rec, [] as never)).toBe(false); + expect(m(rec, 'nope' as never)).toBe(false); + }); +}); diff --git a/packages/formula/src/matches-filter.ts b/packages/formula/src/matches-filter.ts new file mode 100644 index 0000000000..b51760f9b4 --- /dev/null +++ b/packages/formula/src/matches-filter.ts @@ -0,0 +1,115 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * matchesFilterCondition — evaluate a Mongo-style {@link FilterCondition} against + * ONE in-memory record (ADR-0058 D4/D6). + * + * This is the third backend for the canonical filter shape, completing the + * round-trip: `compileCelToFilter` lowers CEL → FilterCondition; the engine runs + * it as a `where`; `read-scope-sql` lowers it to SQL; and THIS evaluates it + * against a single record for write-side validation — the RLS `check` clause + * (post-image of an insert/update), where there is no query to push down to. + * + * Security posture: **fail closed.** Anything it cannot evaluate — a malformed + * node, an unknown operator, a nested relation object a flat record can't + * satisfy — returns `false` (the write is denied), never `true`. The operator + * vocabulary mirrors `read-scope-sql.ts` so the in-memory and SQL backends agree. + */ + +import type { FilterCondition } from '@objectstack/spec/data'; + +/** True iff `record` satisfies `filter`. A null/empty filter matches everything. */ +export function matchesFilterCondition(record: Record, filter: FilterCondition | null | undefined): boolean { + if (filter == null) return true; + if (typeof filter !== 'object' || Array.isArray(filter)) return false; + return evalNode(record, filter as Record); +} + +function evalNode(record: Record, node: Record): boolean { + // A node is the AND of all its entries. + for (const [key, val] of Object.entries(node)) { + if (key === '$and') { + if (!Array.isArray(val) || !val.every((c) => evalNode(record, c as Record))) return false; + } else if (key === '$or') { + if (!Array.isArray(val) || val.length === 0 || !val.some((c) => evalNode(record, c as Record))) return false; + } else if (key === '$not') { + if (val == null || typeof val !== 'object') return false; + if (evalNode(record, val as Record)) return false; + } else if (key.startsWith('$')) { + return false; // unknown top-level operator → fail closed + } else { + if (!evalField(record, key, val)) return false; + } + } + return true; +} + +function evalField(record: Record, field: string, spec: unknown): boolean { + const actual = getPath(record, field); + // `{ field: null }` → IS NULL. + if (spec === null) return actual == null; + // Scalar / Date → implicit equality. + if (typeof spec !== 'object' || spec instanceof Date) return looseEq(actual, spec); + // A bare array value is not a valid field spec (must be `{ $in: [...] }`). + if (Array.isArray(spec)) return false; + + const ops = spec as Record; + const keys = Object.keys(ops); + // Must be all-operators; a non-`$` key means a nested relation a flat record + // cannot satisfy → fail closed. + if (keys.length === 0 || keys.some((k) => !k.startsWith('$'))) return false; + for (const op of keys) { + if (!evalOp(actual, op, ops[op], record)) return false; + } + return true; +} + +function evalOp(actual: unknown, op: string, raw: unknown, record: Record): boolean { + const v = resolveValue(raw, record); + switch (op) { + case '$eq': return v === null ? actual == null : looseEq(actual, v); + case '$ne': return v === null ? actual != null : !looseEq(actual, v); + case '$gt': return actual != null && v != null && (actual as never) > (v as never); + case '$gte': return actual != null && v != null && (actual as never) >= (v as never); + case '$lt': return actual != null && v != null && (actual as never) < (v as never); + case '$lte': return actual != null && v != null && (actual as never) <= (v as never); + case '$in': return Array.isArray(v) && v.some((x) => looseEq(actual, x)); + case '$nin': return Array.isArray(v) && !v.some((x) => looseEq(actual, x)); + case '$between': + return Array.isArray(v) && v.length === 2 && actual != null + && (actual as never) >= (v[0] as never) && (actual as never) <= (v[1] as never); + case '$contains': return typeof actual === 'string' && typeof v === 'string' && actual.includes(v); + case '$notContains': return !(typeof actual === 'string' && typeof v === 'string' && actual.includes(v)); + case '$startsWith': return typeof actual === 'string' && typeof v === 'string' && actual.startsWith(v); + case '$endsWith': return typeof actual === 'string' && typeof v === 'string' && actual.endsWith(v); + case '$null': return v === true ? actual == null : actual != null; + case '$exists': return v === true ? actual !== undefined : actual === undefined; + default: return false; // unknown operator → fail closed + } +} + +/** Resolve a `{ $field: 'path' }` reference against the record; else passthrough. */ +function resolveValue(raw: unknown, record: Record): unknown { + if (raw && typeof raw === 'object' && !Array.isArray(raw) && '$field' in (raw as Record)) { + return getPath(record, String((raw as Record).$field)); + } + return raw; +} + +function getPath(record: Record, path: string): unknown { + if (!path.includes('.')) return record[path]; + let cur: unknown = record; + for (const seg of path.split('.')) { + if (cur == null || typeof cur !== 'object') return undefined; + cur = (cur as Record)[seg]; + } + return cur; +} + +/** Equality that treats Dates by time-value; otherwise strict. */ +function looseEq(a: unknown, b: unknown): boolean { + if (a instanceof Date && b instanceof Date) return a.getTime() === b.getTime(); + if (a instanceof Date && (typeof b === 'string' || typeof b === 'number')) return a.getTime() === new Date(b).getTime(); + if (b instanceof Date && (typeof a === 'string' || typeof a === 'number')) return new Date(a).getTime() === b.getTime(); + return a === b; +} diff --git a/packages/plugins/plugin-security/src/rls-compiler.ts b/packages/plugins/plugin-security/src/rls-compiler.ts index 0b54806e73..f7ed792463 100644 --- a/packages/plugins/plugin-security/src/rls-compiler.ts +++ b/packages/plugins/plugin-security/src/rls-compiler.ts @@ -144,7 +144,8 @@ export class RLSCompiler { */ compileFilter( policies: RowLevelSecurityPolicy[], - executionContext?: ExecutionContext + executionContext?: ExecutionContext, + clause: 'using' | 'check' = 'using', ): Record | null { if (policies.length === 0) return null; @@ -173,25 +174,39 @@ export class RLSCompiler { } const filters: Record[] = []; + let applicable = 0; for (const policy of policies) { - if (!policy.using) continue; - const filter = this.compileExpression(policy.using, userCtx); + // [ADR-0058 D4] On a WRITE (check) pass, the post-image is validated against + // the `check` clause, defaulting to `using` when omitted. Reads use `using`. + const predicate = clause === 'check' + ? ((policy as { check?: string }).check ?? policy.using) + : policy.using; + // A policy that carries no predicate for THIS clause (e.g. a check-only + // policy on the `using` read pass) is not applicable here — skip it + // WITHOUT counting it toward the fail-closed deny below. + if (!predicate) continue; + applicable++; + const filter = this.compileExpression(predicate, userCtx); if (filter) { filters.push(filter); - } else if (!isSupportedRlsExpression(policy.using)) { - // ADR-0056 D4: an UNSUPPORTED-SHAPE predicate (e.g. `==`, AND/OR, ranges) - // compiles to nothing and would silently vanish, leaving the object - // unprotected. Surface it instead of dropping in silence. (A SUPPORTED - // shape that returned null is the intentional "context var absent" path — - // it fails closed downstream and is not warned here.) + } else if (!isSupportedRlsExpression(predicate)) { + // ADR-0056 D4: an UNSUPPORTED-SHAPE predicate (e.g. arithmetic, functions, + // subqueries) compiles to nothing and would silently vanish, leaving the + // object unprotected. Surface it instead of dropping in silence. (A + // SUPPORTED shape that returned null is the intentional "context var + // absent" path — it fails closed downstream and is not warned here.) this.logger?.warn?.( `[RLS] policy '${(policy as { name?: string }).name ?? '(unnamed)'}' on '${(policy as { object?: string }).object ?? '?'}' ` + - `has an uncompilable predicate and was DROPPED (no enforcement): ${policy.using}`, + `has an uncompilable predicate (${clause} clause) and was DROPPED (no enforcement): ${predicate}`, ); } } + // No policy carried a predicate for this clause → nothing to apply (NOT a + // deny). e.g. a check-only policy seen on the `using` read pass. + if (applicable === 0) return null; + if (filters.length === 0) { // Policies *were* applicable but every one of them depended on a // `current_user.*` variable that wasn't populated (or used an diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index 3e94b48c3c..8e146f57f6 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -689,6 +689,85 @@ describe('SecurityPlugin', () => { await orig.call(harness, opCtx); // No throw — find is not a write operation. }); + + + // --------------------------------------------------------------------------- + // ADR-0058 D4 — RLS `check` clause (write post-image validation) + // + // `using` gates which EXISTING rows a write may target (pre-image, step 2.7); + // `check` validates the NEW / CHANGED row (post-image) on insert/update — the + // PostgreSQL WITH CHECK analog — compiled by the canonical compiler and matched + // in-memory. Fail closed (D5). Scoped to policies that explicitly declare check. + // --------------------------------------------------------------------------- + describe('RLS check enforcement (ADR-0058 D4)', () => { + const checkPolicySet: PermissionSet = { + name: 'member_default', + label: 'Member', + isProfile: true, + objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }, + rowLevelSecurity: [ + { name: 'emea_insert_check', object: '*', operation: 'insert', check: "region == 'EMEA'" }, + { name: 'emea_update_check', object: '*', operation: 'update', check: "region == 'EMEA'" }, + ], + } as any; + + const started = async (findOneImpl?: (q: any) => any) => { + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + const harness = makeMiddlewareCtx({ + permissionSets: [checkPolicySet], + objectFields: ['id', 'region', 'owner_id', 'name'], + findOneImpl, + }); + await plugin.init(harness.ctx); + await plugin.start(harness.ctx); + return harness; + }; + const ctx = () => ({ userId: 'u1', tenantId: 'org-1', roles: [], permissions: ['member_default'] }); + + it('INSERT whose post-image satisfies the check succeeds', async () => { + const h = await started(); + const opCtx: any = { object: 'task', operation: 'insert', data: { name: 'A', region: 'EMEA' }, context: ctx() }; + await expect(h.run(opCtx)).resolves.toBeDefined(); + }); + + it('INSERT whose post-image VIOLATES the check is denied (fail closed)', async () => { + const h = await started(); + const opCtx: any = { object: 'task', operation: 'insert', data: { name: 'A', region: 'APAC' }, context: ctx() }; + await expect(h.run(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('INSERT missing the checked field is denied (fail closed)', async () => { + const h = await started(); + const opCtx: any = { object: 'task', operation: 'insert', data: { name: 'A' }, context: ctx() }; + await expect(h.run(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('UPDATE whose post-image (pre-image ∪ change) satisfies the check succeeds', async () => { + // Pre-image is APAC; the update moves it to EMEA → post-image is valid. + const h = await started(() => ({ id: 'r1', region: 'APAC', name: 'X' })); + const opCtx: any = { object: 'task', operation: 'update', data: { id: 'r1', region: 'EMEA' }, context: ctx() }; + await expect(h.run(opCtx)).resolves.toBeDefined(); + }); + + it('UPDATE that changes a valid row to violate the check is denied', async () => { + // Pre-image is EMEA (valid); the update moves it to APAC → post-image invalid. + const h = await started(() => ({ id: 'r1', region: 'EMEA', name: 'X' })); + const opCtx: any = { object: 'task', operation: 'update', data: { id: 'r1', region: 'APAC' }, context: ctx() }; + await expect(h.run(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('UPDATE leaving the checked field unchanged uses the pre-image value (valid stays valid)', async () => { + const h = await started(() => ({ id: 'r1', region: 'EMEA', name: 'X' })); + const opCtx: any = { object: 'task', operation: 'update', data: { id: 'r1', name: 'renamed' }, context: ctx() }; + await expect(h.run(opCtx)).resolves.toBeDefined(); + }); + + it('DELETE is unaffected by check (no new row to validate)', async () => { + const h = await started(() => ({ id: 'r1', region: 'APAC' })); + const opCtx: any = { object: 'task', operation: 'delete', data: { id: 'r1' }, context: ctx() }; + await expect(h.run(opCtx)).resolves.toBeDefined(); + }); + }); }); // --------------------------------------------------------------------------- describe('PermissionEvaluator', () => { diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 49fcc088db..2476e96074 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -5,6 +5,7 @@ import type { PermissionSet, RowLevelSecurityPolicy } from '@objectstack/spec/se import { PermissionEvaluator } from './permission-evaluator.js'; import { bootstrapDeclaredRoles } from './bootstrap-declared-roles.js'; import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js'; +import { matchesFilterCondition } from '@objectstack/formula'; import { FieldMasker } from './field-masker.js'; import { PermissionDeniedError } from './errors.js'; import { bootstrapPlatformAdmin } from './bootstrap-platform-admin.js'; @@ -535,6 +536,68 @@ export class SecurityPlugin implements Plugin { } } + // 3.6. [ADR-0058 D4] RLS WRITE `check` — post-image validation. + // + // `using` gates which EXISTING rows a write may target (the #1994 + // pre-image, step 2.7). `check` validates the NEW / CHANGED row + // (post-image) on insert/update — the PostgreSQL WITH CHECK analog. We + // compile the declared `check` clauses with the canonical compiler and + // match the resolved FilterCondition against the post-image in-memory + // (the single-record backend for the same filter shape, ADR-0058 D6). A + // row that fails the check is DENIED (fail closed, D5) — never silently + // written. Scoped to policies that EXPLICITLY declare `check`, so an + // object governed only by `using` is unaffected. + if ( + (opCtx.operation === 'insert' || opCtx.operation === 'update') && + opCtx.data && + typeof opCtx.data === 'object' && + !Array.isArray(opCtx.data) && + permissionSets.length > 0 && + !!opCtx.context?.userId + ) { + const checkFilter = await this.computeWriteCheckFilter( + permissionSets, + opCtx.object, + opCtx.operation, + opCtx.context, + ); + if (checkFilter) { + // Build the post-image. Insert → the new row. Update by-id → the + // pre-image merged with the change set (so a check on an unchanged + // field still sees its value). A bulk update (no single id) cannot + // form a post-image here — it is governed by the using-based AST + // scoping (step 3); we log and skip rather than guess. + let postImage: Record | null = { ...(opCtx.data as Record) }; + if (opCtx.operation === 'update') { + const targetId = this.extractSingleId(opCtx); + if (targetId == null) { + this.logger.warn?.( + `[Security] RLS check on bulk update '${opCtx.object}' is not post-image validated ` + + `(governed by the using-scoped where); single-id writes are checked.`, + ); + postImage = null; + } else if (this.ql) { + let pre: any = null; + try { + pre = await this.ql.findOne(opCtx.object, { where: { id: targetId }, context: opCtx.context }); + } catch { + pre = null; + } + if (pre && typeof pre === 'object') postImage = { ...(pre as Record), ...(opCtx.data as Record) }; + } + } + if (postImage && !matchesFilterCondition(postImage, checkFilter as any)) { + this.logger.warn?.( + `[Security] RLS check FAILED on ${opCtx.operation} '${opCtx.object}' — write denied (fail-closed)`, + ); + throw new PermissionDeniedError( + `[Security] Access denied: the ${opCtx.operation} would violate a row-level CHECK on '${opCtx.object}'`, + { operation: opCtx.operation, object: opCtx.object, roles, permissionSets: explicitPermissionSets }, + ); + } + } + } + // 3. RLS filter injection. The policy collection + field-existence // safety + compile (incl. the fail-closed deny sentinel) is shared with // the public getReadFilter service via computeRlsFilter, so the engine @@ -865,6 +928,26 @@ export class SecurityPlugin implements Plugin { return rlsFilter; } + /** + * [ADR-0058 D4] Compile the WRITE `check` predicate for a post-image + * validation. Scoped to applicable policies that EXPLICITLY declare a `check` + * clause — an object governed only by `using` (the pre-image path) yields no + * check filter and is unaffected. The compiled FilterCondition is matched + * against the post-image record by the caller (fail closed). + */ + private async computeWriteCheckFilter( + permissionSets: PermissionSet[], + object: string, + operation: string, + context: any, + ): Promise | null> { + const withCheck = this.collectRLSPolicies(permissionSets, object, operation).filter( + (p) => typeof (p as { check?: string }).check === 'string' && (p as { check?: string }).check!.trim() !== '', + ); + if (withCheck.length === 0) return null; + return this.rlsCompiler.compileFilter(withCheck, context, 'check'); + } + /** * Resolve a controlled_by_parent object's master-detail relation (the FK field * key + the master object name), or null. Prefers a required `master_detail` From 901b90366706511c6eef7de329704936a7666040 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 21 Jun 2026 13:10:46 +0800 Subject: [PATCH 5/7] =?UTF-8?q?test(dogfood):=20ADR-0058=20D7=20=E2=80=94?= =?UTF-8?q?=20Expression=20Surface=20Conformance=20ledger=20+=20CI=20ratch?= =?UTF-8?q?et?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One classification per expression-holding declaration across the spec (27 surfaces): dialect, mode (compile|interpret), state, fail-policy (D5), evaluator/ compiler site, and — for the security-critical COMPILE surfaces (RLS using/check, sharing condition) — a proof. The companion test makes it a CHECKED artifact: - every COMPILE row must be security fail-closed, name the canonical compiler (compileCelToFilter / celToFilter / matchesFilterCondition), and carry a proof file that exists on disk; - every surface is covered by exactly one row (no double classification); - THE RATCHET: the test re-discovers every `ExpressionInputSchema` field in packages/spec/src (plus the RLS using/check string predicates) and fails if any is unclassified or any `covers` entry is stale. A new declared-but-unwired predicate — the #1887 class — now breaks the build until it is classified. Verified the ratchet has teeth (dropping a covers entry fails CI with an actionable message). 7 assertions green. Co-Authored-By: Claude Opus 4.8 --- .../dogfood/test/authz-conformance.matrix.ts | 3 + .../test/expression-conformance.ledger.ts | 148 ++++++++++++++++++ .../test/expression-conformance.test.ts | 117 ++++++++++++++ 3 files changed, 268 insertions(+) create mode 100644 packages/dogfood/test/expression-conformance.ledger.ts create mode 100644 packages/dogfood/test/expression-conformance.test.ts diff --git a/packages/dogfood/test/authz-conformance.matrix.ts b/packages/dogfood/test/authz-conformance.matrix.ts index 3cf25e68f5..4739de7379 100644 --- a/packages/dogfood/test/authz-conformance.matrix.ts +++ b/packages/dogfood/test/authz-conformance.matrix.ts @@ -31,6 +31,9 @@ export const AUTHZ_CONFORMANCE: AuthzPrimitive[] = [ enforcement: 'plugin-security/security-plugin.ts computeRlsFilter (AND-injected)', proof: 'rls-fixture.dogfood.test.ts' }, { id: 'rls-by-id-write', summary: 'by-id write enforcement (#1994)', state: 'enforced', enforcement: 'plugin-security/security-plugin.ts pre-image re-read', proof: 'rls-fixture.dogfood.test.ts' }, + { id: 'rls-write-check', summary: 'RLS `check` write post-image validation (ADR-0058 D4)', state: 'enforced', + enforcement: 'plugin-security/security-plugin.ts step 3.6 — compileCelToFilter + matchesFilterCondition against the post-image (fail-closed)', + note: 'Unit-proven in plugin-security/security-plugin.test.ts (RLS check enforcement); see ADR-0058 D7 ledger.' }, { id: 'owd-private', summary: 'OWD private (owner-only)', state: 'enforced', enforcement: 'plugin-sharing/sharing-service.ts effectiveSharingModel=private', proof: 'showcase-private-owd.dogfood.test.ts' }, { id: 'owd-public-read', summary: 'OWD public_read (everyone reads, owner writes)', state: 'enforced', diff --git a/packages/dogfood/test/expression-conformance.ledger.ts b/packages/dogfood/test/expression-conformance.ledger.ts new file mode 100644 index 0000000000..a940af153f --- /dev/null +++ b/packages/dogfood/test/expression-conformance.ledger.ts @@ -0,0 +1,148 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// ADR-0058 D7 — Expression Surface Conformance ledger. +// +// The durable encoding of the ADR-0058 audit: ONE classification per expression- +// holding declaration across the spec. Every surface is in exactly one honest +// state, names its evaluator/compiler site, declares its fail-policy (ADR-0058 +// D5), and — for the security-critical COMPILE surfaces — references a proof. +// +// `mode` is a property of the surface (ADR-0058 D6): RLS `using`/`check` and the +// sharing `condition` are COMPILE (pushdown via the canonical +// @objectstack/formula compiler); everything else is INTERPRET (per-record, via +// the celEngine). There is NO silent fallback from compile to interpret. +// +// The companion test (`expression-conformance.test.ts`) RE-DISCOVERS every +// `ExpressionInputSchema` field declaration in `packages/spec/src` (plus the RLS +// `using`/`check` string predicates) and asserts each is `covers`-ed by exactly +// one row. A NEW expression surface that nobody classified — the #1887 class of +// "declared-but-unwired predicate" — breaks the build. + +export type ExprMode = 'compile' | 'interpret'; +export type ExprDialect = 'cel' | 'cron' | 'template' | 'js'; +export type ExprState = 'enforced' | 'experimental' | 'removed'; +/** ADR-0058 D5 fail-policy tiers. */ +export type FailPolicy = 'compile-error' | 'fail-closed' | 'fail-soft-log' | 'throw'; + +export interface ExprSurface { + id: string; + summary: string; + dialect: ExprDialect; + mode: ExprMode; + state: ExprState; + failPolicy: FailPolicy; + /** Runtime evaluator / compiler site. */ + site: string; + /** `file:field` surfaces (relative to packages/spec/src) this row classifies — the ratchet keys. */ + covers: string[]; + /** Proof path (repo-root-relative). Required for ENFORCED COMPILE (security) rows. */ + proof?: string; + /** Rationale for experimental/removed, or a roadmap pointer. */ + note?: string; +} + +export const EXPRESSION_SURFACE: ExprSurface[] = [ + // ── COMPILE (pushdown) — security-critical; canonical-compiler-reachable + proven ── + { + id: 'rls-using', + summary: 'RLS `using` read / pre-image predicate', + dialect: 'cel', mode: 'compile', state: 'enforced', failPolicy: 'fail-closed', + site: 'plugin-security/rls-compiler.ts → @objectstack/formula compileCelToFilter (legacy SQL bridged); AND-injected by security-plugin computeRlsFilter + service-analytics read-scope-sql', + covers: ['security/rls.zod.ts:using'], + proof: 'packages/dogfood/test/rls-fixture.dogfood.test.ts', + }, + { + id: 'rls-check', + summary: 'RLS `check` write post-image validation (ADR-0058 D4)', + dialect: 'cel', mode: 'compile', state: 'enforced', failPolicy: 'fail-closed', + site: 'plugin-security/security-plugin.ts step 3.6 → compileCelToFilter + @objectstack/formula matchesFilterCondition', + covers: ['security/rls.zod.ts:check'], + proof: 'packages/plugins/plugin-security/src/security-plugin.test.ts', + }, + { + id: 'sharing-condition', + summary: 'sharing-rule `condition` → criteria_json (ADR-0058 D3, closes #1887)', + dialect: 'cel', mode: 'compile', state: 'enforced', failPolicy: 'fail-closed', + site: 'plugin-sharing/bootstrap-declared-sharing-rules.ts celToFilter → compileCelToFilter; matched by sharing-rule-service findMatchingRecords', + covers: ['security/sharing.zod.ts:condition'], + proof: 'packages/plugins/plugin-sharing/src/sharing-rule.test.ts', + }, + + // ── INTERPRET (per-record) — classified, fail-soft per ADR-0058 D5 ── + { + id: 'cel-validation', + summary: 'object validation predicate (condition / when)', + dialect: 'cel', mode: 'interpret', state: 'enforced', failPolicy: 'fail-soft-log', + site: '@objectstack/formula celEngine (interpret) via the validation runner', + covers: ['data/validation.zod.ts:condition', 'data/validation.zod.ts:when'], + }, + { + id: 'cel-hook', + summary: 'hook gate condition', + dialect: 'cel', mode: 'interpret', state: 'enforced', failPolicy: 'fail-soft-log', + site: '@objectstack/formula celEngine (interpret) via the hook runner', + covers: ['data/hook.zod.ts:condition'], + }, + { + id: 'cel-formula', + summary: 'computed / formula field + mapping / graphql / feature expressions', + dialect: 'cel', mode: 'interpret', state: 'enforced', failPolicy: 'fail-soft-log', + site: '@objectstack/formula celEngine (interpret)', + covers: [ + 'data/field.zod.ts:expression', + 'shared/mapping.zod.ts:expression', + 'api/graphql.zod.ts:expression', + 'kernel/feature.zod.ts:expression', + ], + }, + { + id: 'cel-field-rule', + summary: 'field UI rules (requiredWhen / readonlyWhen / visibleWhen / conditionalRequired)', + dialect: 'cel', mode: 'interpret', state: 'enforced', failPolicy: 'fail-soft-log', + site: '@objectstack/formula celEngine (interpret) — console (objectui) + server', + covers: [ + 'data/field.zod.ts:requiredWhen', + 'data/field.zod.ts:readonlyWhen', + 'data/field.zod.ts:visibleWhen', + 'data/field.zod.ts:conditionalRequired', + ], + }, + { + id: 'cel-ui', + summary: 'UI visibility / routing / submit predicates', + dialect: 'cel', mode: 'interpret', state: 'enforced', failPolicy: 'fail-soft-log', + site: 'console (objectui) SchemaRenderer + server celEngine (interpret)', + covers: [ + 'data/object.zod.ts:visibleOn', + 'ui/action.zod.ts:visible', + 'ui/app.zod.ts:visible', + 'ui/page.zod.ts:visibility', + 'ui/view.zod.ts:condition', + 'ui/view.zod.ts:visibleOn', + 'ui/component.zod.ts:onSubmit', + 'system/settings-manifest.zod.ts:visible', + ], + }, + { + id: 'cel-flow', + summary: 'flow / sync / loader branching + filter predicates', + dialect: 'cel', mode: 'interpret', state: 'enforced', failPolicy: 'throw', + site: '@objectstack/formula celEngine (interpret) via the automation runtime', + covers: [ + 'automation/flow.zod.ts:condition', + 'automation/sync.zod.ts:condition', + 'kernel/metadata-loader.zod.ts:filter', + ], + }, + { + id: 'cel-advanced-policy', + summary: 'advanced security / versioning policy conditions', + dialect: 'cel', mode: 'interpret', state: 'experimental', failPolicy: 'fail-closed', + site: '(no runtime consumer yet)', + covers: [ + 'kernel/plugin-security-advanced.zod.ts:condition', + 'kernel/plugin-versioning.zod.ts:condition', + ], + note: 'EXPERIMENTAL — declared policy conditions with no runtime evaluator yet (ADR-0056 D8 / ADR-0049 tracking).', + }, +]; diff --git a/packages/dogfood/test/expression-conformance.test.ts b/packages/dogfood/test/expression-conformance.test.ts new file mode 100644 index 0000000000..3c68d9c710 --- /dev/null +++ b/packages/dogfood/test/expression-conformance.test.ts @@ -0,0 +1,117 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// ADR-0058 D7 — the Expression Surface Conformance ledger is a CHECKED artifact. +// These assertions make "every expression-holding declaration is classified in +// exactly one honest state, every COMPILE security surface is reachable by the +// canonical compiler and proven, and no new surface slips in unclassified" a +// green CI gate. A new ExpressionInputSchema field with no ledger row — the +// #1887 class of declared-but-unwired predicate — breaks the build. + +import { describe, it, expect } from 'vitest'; +import { existsSync, readFileSync, readdirSync, statSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { dirname, join, relative } from 'node:path'; +import { EXPRESSION_SURFACE, type ExprSurface } from './expression-conformance.ledger.js'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = join(HERE, '../../..'); +const SPEC_SRC = join(REPO_ROOT, 'packages/spec/src'); + +const MODES = new Set(['compile', 'interpret']); +const STATES = new Set(['enforced', 'experimental', 'removed']); +const FAIL_POLICIES = new Set(['compile-error', 'fail-closed', 'fail-soft-log', 'throw']); +const DIALECTS = new Set(['cel', 'cron', 'template', 'js']); + +/** Re-discover every expression surface in the spec — the SAME scan the ledger encodes. */ +function discoverSurfaces(): Set { + const found = new Set(); + const walk = (dir: string) => { + for (const e of readdirSync(dir)) { + const p = join(dir, e); + if (statSync(p).isDirectory()) walk(p); + else if (e.endsWith('.zod.ts')) { + const rel = relative(SPEC_SRC, p); + for (const line of readFileSync(p, 'utf8').split('\n')) { + const m = line.match(/^\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*:\s*ExpressionInputSchema\b/); + if (m) found.add(`${rel}:${m[1]}`); + } + } + } + }; + walk(SPEC_SRC); + // RLS using/check are expression predicates too (legacy z.string() fields, not + // ExpressionInputSchema) — classify them explicitly so they cannot drift. + found.add('security/rls.zod.ts:using'); + found.add('security/rls.zod.ts:check'); + return found; +} + +describe('ADR-0058 D7 — expression surface conformance ledger', () => { + it('has no duplicate ids', () => { + const ids = EXPRESSION_SURFACE.map((s) => s.id); + expect(new Set(ids).size).toBe(ids.length); + }); + + it('every row has a valid mode / state / dialect / fail-policy', () => { + for (const s of EXPRESSION_SURFACE) { + expect(MODES.has(s.mode), `${s.id}: mode '${s.mode}'`).toBe(true); + expect(STATES.has(s.state), `${s.id}: state '${s.state}'`).toBe(true); + expect(DIALECTS.has(s.dialect), `${s.id}: dialect '${s.dialect}'`).toBe(true); + expect(FAIL_POLICIES.has(s.failPolicy), `${s.id}: failPolicy '${s.failPolicy}'`).toBe(true); + expect(s.site && s.site.length > 0, `${s.id}: missing site`).toBe(true); + expect(Array.isArray(s.covers) && s.covers.length > 0, `${s.id}: empty covers`).toBe(true); + } + }); + + it('every COMPILE row is security fail-closed, names the canonical compiler, and is proven', () => { + for (const s of EXPRESSION_SURFACE.filter((x) => x.mode === 'compile')) { + expect(s.failPolicy, `${s.id}: a compile/security surface must fail closed`).toBe('fail-closed'); + // Compiler-reachable: the site must reference the canonical compiler entry. + expect(/compileCelToFilter|celToFilter|matchesFilterCondition/.test(s.site), `${s.id}: site does not name the canonical compiler`).toBe(true); + expect(s.proof, `${s.id}: an enforced compile surface must carry a proof`).toBeTruthy(); + } + }); + + it('every referenced proof FILE EXISTS (the proof ratchet)', () => { + const broken: string[] = []; + for (const s of EXPRESSION_SURFACE as ExprSurface[]) { + if (s.proof && !existsSync(join(REPO_ROOT, s.proof))) broken.push(`${s.id} → ${s.proof}`); + } + expect(broken, `ledger proofs missing on disk: ${broken.join(', ')}`).toEqual([]); + }); + + it('every experimental/removed row carries a note (honest rationale)', () => { + const missing = EXPRESSION_SURFACE.filter((s) => s.state !== 'enforced' && !s.note).map((s) => s.id); + expect(missing, `non-enforced rows missing a note: ${missing.join(', ')}`).toEqual([]); + }); + + // ── THE RATCHET ────────────────────────────────────────────────────────── + it('classifies EVERY expression surface in the spec — no unclassified declaration', () => { + const discovered = discoverSurfaces(); + const covered = new Set(EXPRESSION_SURFACE.flatMap((s) => s.covers)); + + // (a) every discovered surface is classified by some row + const unclassified = [...discovered].filter((s) => !covered.has(s)).sort(); + expect( + unclassified, + `NEW unclassified expression surface(s) — add a row to expression-conformance.ledger.ts ` + + `(ADR-0058 D7): ${unclassified.join(', ')}`, + ).toEqual([]); + + // (b) no stale `covers` entry that no longer exists in the spec + const stale = [...covered].filter((s) => !discovered.has(s)).sort(); + expect(stale, `STALE ledger covers (surface removed from spec): ${stale.join(', ')}`).toEqual([]); + }); + + it('each surface is covered by EXACTLY ONE row (no double classification)', () => { + const seen = new Map(); + const dup: string[] = []; + for (const s of EXPRESSION_SURFACE) { + for (const c of s.covers) { + if (seen.has(c)) dup.push(`${c} (in ${seen.get(c)} and ${s.id})`); + else seen.set(c, s.id); + } + } + expect(dup, `surfaces classified by more than one row: ${dup.join(', ')}`).toEqual([]); + }); +}); From 626c44584adb06211b628ca951e5cae2a641cd66 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 21 Jun 2026 13:15:30 +0800 Subject: [PATCH 6/7] chore(changeset): ADR-0058 expression compiler (closes #1887) Co-Authored-By: Claude Opus 4.8 --- .changeset/adr-0058-expression-compiler.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .changeset/adr-0058-expression-compiler.md diff --git a/.changeset/adr-0058-expression-compiler.md b/.changeset/adr-0058-expression-compiler.md new file mode 100644 index 0000000000..ec7aaff812 --- /dev/null +++ b/.changeset/adr-0058-expression-compiler.md @@ -0,0 +1,16 @@ +--- +"@objectstack/formula": minor +"@objectstack/plugin-security": minor +"@objectstack/plugin-sharing": minor +--- + +ADR-0058 — expression & predicate surface unification. Adds the canonical +CEL→FilterCondition pushdown compiler in `@objectstack/formula` +(`compileCelToFilter`, `isPushdownableCel`, `lowerCelAst`) plus an in-memory +`matchesFilterCondition` backend (one AST, three backends). `plugin-security` +(RLS `using`, via a SQL bridge) and `plugin-sharing` (`celToFilter`) cut over to +it, retiring the bespoke regex/field-equality front-ends. Compound sharing +conditions now compile and enforce end-to-end (closes #1887). The RLS `check` +clause is now enforced on the write post-image (insert/by-id update), fail-closed. +Non-pushdownable predicates (arithmetic, functions, subqueries, cross-object) are +an authoring compile error, never silently dropped (ADR-0049/0055). From 70de896a146126793bd2fabc6ab51f5a483a13fa Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 21 Jun 2026 14:20:29 +0800 Subject: [PATCH 7/7] fix(expr): resolve 2 CodeQL alerts in the ADR-0058 changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - expression-conformance.test.ts (HIGH js/file-system-race): the ratchet's spec walk did statSync(p) then readFileSync(p) — a check-then-use TOCTOU window. Read the entry type from the single readdir syscall via withFileTypes instead. - cel-to-filter.ts coerceLiteral (warning, comparison between inconvertible types): `v !== null` is dead after the preceding line returns for null — drop it; the early return already guarantees non-null. No behavior change; cel-to-filter 52 + expression-conformance 7 still green. Co-Authored-By: Claude Opus 4.8 --- packages/dogfood/test/expression-conformance.test.ts | 12 +++++++----- packages/formula/src/cel-to-filter.ts | 4 +++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/dogfood/test/expression-conformance.test.ts b/packages/dogfood/test/expression-conformance.test.ts index 3c68d9c710..ffec9ba6db 100644 --- a/packages/dogfood/test/expression-conformance.test.ts +++ b/packages/dogfood/test/expression-conformance.test.ts @@ -8,7 +8,7 @@ // #1887 class of declared-but-unwired predicate — breaks the build. import { describe, it, expect } from 'vitest'; -import { existsSync, readFileSync, readdirSync, statSync } from 'node:fs'; +import { existsSync, readFileSync, readdirSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { dirname, join, relative } from 'node:path'; import { EXPRESSION_SURFACE, type ExprSurface } from './expression-conformance.ledger.js'; @@ -26,10 +26,12 @@ const DIALECTS = new Set(['cel', 'cron', 'template', 'js']); function discoverSurfaces(): Set { const found = new Set(); const walk = (dir: string) => { - for (const e of readdirSync(dir)) { - const p = join(dir, e); - if (statSync(p).isDirectory()) walk(p); - else if (e.endsWith('.zod.ts')) { + // `withFileTypes` reads the entry type from the single readdir syscall — no + // stat-then-read window (avoids a file-system TOCTOU race; CodeQL). + for (const ent of readdirSync(dir, { withFileTypes: true })) { + const p = join(dir, ent.name); + if (ent.isDirectory()) walk(p); + else if (ent.isFile() && ent.name.endsWith('.zod.ts')) { const rel = relative(SPEC_SRC, p); for (const line of readFileSync(p, 'utf8').split('\n')) { const m = line.match(/^\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*:\s*ExpressionInputSchema\b/); diff --git a/packages/formula/src/cel-to-filter.ts b/packages/formula/src/cel-to-filter.ts index 29c261d307..c255e96deb 100644 --- a/packages/formula/src/cel-to-filter.ts +++ b/packages/formula/src/cel-to-filter.ts @@ -387,7 +387,9 @@ function resolveValue(leaf: Leaf, ctx: Ctx): unknown { function coerceLiteral(v: unknown): unknown { if (typeof v === 'bigint') return Number(v); if (v === null || typeof v === 'string' || typeof v === 'number' || typeof v === 'boolean') return v; - if (typeof v === 'object' && v !== null && typeof (v as { valueOf?: unknown }).valueOf === 'function') { + // `v` is already non-null here (the line above returns for null), so a + // further `v !== null` would be a dead comparison; rely on the early return. + if (typeof v === 'object' && typeof (v as { valueOf?: unknown }).valueOf === 'function') { const prim = (v as { valueOf: () => unknown }).valueOf(); if (typeof prim === 'bigint') return Number(prim); if (typeof prim === 'number' || typeof prim === 'string' || typeof prim === 'boolean') return prim;