Uh oh!
There was an error while loading. Please reload this page.
feat(expr): ADR-0058 P1–P4 — canonical CEL→FilterCondition compiler, RLS/sharing cutover, check enforcement, conformance ledger - #2090
Merged
Conversation
…ompiler 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 <noreply@anthropic.com>
…he one compiler 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 <noreply@anthropic.com>
… enforced e2e 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…CI ratchet 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 <noreply@anthropic.com>
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📓 Docs Drift CheckThis PR changes 5 package(s): 89 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- 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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements ADR-0058 (expression & predicate surface) P1–P4. The ADR was merged in #2087; this is the implementation.
The problem (ADR-0058 audit)
ObjectStack had ONE authoring language (CEL) + ONE good interpreter, but a fragmented compile-to-filter path: three disconnected front-ends (plugin-security's 4-form regex, plugin-sharing's
celToFilter, the ObjectUI array-AST path). That divergence is the root of #1887 — a sharingconditionthe interpreter understands but no compiler lowers, so it never enforces.What landed
P1 — canonical compiler (
@objectstack/formula)compileCelToFilterlowers the same@marcbachmann/cel-jsAST the interpreter uses to a Mongo-styleFilterCondition— the one shape both backends already consume (ObjectQLwhere+ analyticsread-scope-sql). Supported subset (D2):== != > < >= <=,in,&& || !,== null, string methods. Hard boundaries (ADR-0055): single-column field paths only; arithmetic / functions / cross-object / subqueries are an authoring compile error, never silently dropped.isPushdownableCel(shape gate) +lowerCelAst(one-AST-two-backends, D6). 52-case matrix.P2 — cut over the two front-ends
rls-compiler.ts(compileExpression+isSupportedRlsExpression) and plugin-sharing'scelToFilternow delegate to the canonical compiler. A thinsqlPredicateToCelbridges the legacy SQL-ish RLSusingsubset (=→==,IN→in).compileFilterorchestration (membership merge, null→deny aggregation) and the empty-membership parity are preserved. Output parity verified: plugin-security 119, plugin-sharing 65, service-analytics 137.P3/D3 — close#1887
Sharing
conditionnow compiles to a compoundcriteria_jsonand enforces e2e: arecord.amount >= 100000 && record.name == "Big1"shares ONLY the full-AND match. The⚠️ EXPERIMENTAL — NOT ENFORCEDblock onSharingRuleSchemais removed; onlyowner-type +group/guestremain experimental.P3/D4 — enforce RLS
checkNew
matchesFilterCondition(formula) — the single-record backend for the canonical filter shape (D6). security-plugin step 3.6 validates the post-image (insert → new row; by-id update → pre-image ∪ change) against the compiledcheck; violating writes fail closed (D5). Scoped to policies that explicitly declarecheck→ zero blast radius (no seeded policy declares one). 7 proof cases.P4/D7 — conformance ledger + CI ratchet
expression-conformance.ledger.tsclassifies all 27 expression surfaces (dialect / mode / state / fail-policy / site / proof). The companion test re-discovers everyExpressionInputSchemafield inpackages/spec/src(+ RLS using/check) and fails the build if any surface is unclassified — the #1887 class can't recur silently. Compile-mode rows must be compiler-reachable + carry an existing proof.Verification (local, all green)
Full workspace build 75/75 · formula 201 · plugin-security 119 · plugin-sharing 65 · service-analytics 137 · spec 6599 · dogfood 130.
Closes#1887.
🤖 Generated with Claude Code