From e734c55378422692cbacc95ee45ecc82c0cdebbf Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 14:39:44 +0000 Subject: [PATCH 1/3] fix(formula): refuse comparands that are not one value at the CEL lowering and the write-check evaluator The CEL lowering refuses a list under every comparison (ordering operators included, and on the constant-fold branch), the variable root or an object under an ordering operator, and an `in` list whose member is itself a list. The write-check evaluator refuses an array under the ordering operators, an array member of `$in` / `$nin`, and a `{ $field }` comparison whose column holds a list or an object on the record being judged. Claude-Session: https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN Co-authored-by: Claude --- .../cel-to-filter-one-value-comparand.test.ts | 167 ++++++++++++++++++ packages/formula/src/cel-to-filter.ts | 120 +++++++++++-- .../matches-filter-array-comparand.test.ts | 82 ++++++++- packages/formula/src/matches-filter.ts | 113 ++++++++++-- 4 files changed, 445 insertions(+), 37 deletions(-) create mode 100644 packages/formula/src/cel-to-filter-one-value-comparand.test.ts diff --git a/packages/formula/src/cel-to-filter-one-value-comparand.test.ts b/packages/formula/src/cel-to-filter-one-value-comparand.test.ts new file mode 100644 index 00000000000..4c41bc35711 --- /dev/null +++ b/packages/formula/src/cel-to-filter-one-value-comparand.test.ts @@ -0,0 +1,167 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#19886 stage 2d] Every comparison the pushdown compiler lowers compares ONE + * value, and an `in` list holds one value per member. Stage 2c refused a list + * under `==` / `!=` opposite a field; the same fault stayed open one position + * over, each measured admitting and storing the writes a row-level `check` was + * written to refuse (real `SecurityPlugin` + ObjectQL, driver-sql and + * driver-memory): + * + * | predicate | lowered to (before) | write-check evaluator (before) | + * |---------------------------------------------|----------------------------------------------|--------------------------------| + * | `!(record.status in [['closed', 'archived']])` | `$not { status: { $in: [[…]] } }` | every write admitted | + * | `record.status > ['m']` | `{ status: { $gt: ['m'] } }` | compared as the string `'m'` | + * | `current_user.org_user_ids != 'x'` | `{}` — "no restriction" | every write admitted | + * | `current_user.org_user_ids > 'a'` | `{}` — folded on the coerced string | every write admitted | + * | `record.reviewer_id > current_user` | `{ reviewer_id: { $gt: } }` | compared as `[object Object]` | + * + * Each is now `unsupported`, so every consumer fails closed on the path it + * already has: the RLS compiler drops the policy (`RLS_DENY_FILTER` when + * nothing else applies), the sharing seeder skips the rule, and the authoring + * gate reports what the SOURCE shows (a list literal, the variable root). A + * variable's value exists per request, so a resolved list is refused at + * request time with the shape check still passing the source. + * + * NOT here, because the lowering cannot see it: a field compared with another + * field whose column holds a list (`record.status != record.tags`, `tags` a + * json or multiple field). The compiler knows the predicate's text, not the + * object's field types; that one is refused by the write-check evaluator + * (`matches-filter-array-comparand.test.ts`). + */ + +import { describe, expect, it } from 'vitest'; + +import { compileCelToFilter, isPushdownableCel } from './cel-to-filter'; +import { isSupportedRlsExpression } from './rls-predicate'; + +const VARS = { + current_user: { + id: 'u_me', + email: 'me@example.test', + org_user_ids: ['u_me', 'u_peer'], + // A membership set a host staged with a list member — the ExecutionContext + // contract declares string members, so only a host violating it supplies one. + nested_set: [['acc_secret_a', 'acc_secret_b']], + profile: { tier: 'gold' }, + }, +}; + +const refusedEverywhere = (source: string) => { + const compiled = compileCelToFilter(source, { variables: VARS }); + expect(compiled.ok).toBe(false); + expect(compiled.ok ? undefined : compiled.reason).toBe('unsupported'); + return compiled.ok ? '' : compiled.detail; +}; + +/** Refused per request AND by the authoring shape check: the source shows the fault. */ +const REFUSED_BY_SHAPE: Array<[string, string]> = [ + // (b) a nested list under `in` + ['(b) not-in with a nested list', "!(record.status in [['closed', 'archived']])"], + ['(b) in with a nested list', "record.status in [['closed', 'archived']]"], + ['(b) a nested list beside a scalar member', "record.status in ['open', ['closed']]"], + ['(b) an empty nested list', 'record.status in [[]]'], + ['(b) under && / ||', "record.owner_id == current_user.id || !(record.status in [['closed']])"], + // (c) an ordering operator against a list literal, either side, negated, constant + ['(c) > a list literal', "record.status > ['m']"], + ['(c) >= an empty list', 'record.amount >= []'], + ['(c) <= a list literal', 'record.amount <= [10, 20]'], + ['(c) negated <', "!(record.status < ['m'])"], + ['(c) the list on the left', "['m'] < record.status"], + ['(c) a constant ordering against a list literal', "['a'] > 'b'"], + // (e) a list literal on the constant branch + ['(e) a constant != against a list literal', "['a'] != 'x'"], + // (d) the variable root under an ordering operator + ['(d) > the variable root', 'record.reviewer_id > current_user'], + ['(d) the root on the left of <=', 'current_user <= record.reviewer_id'], + ['(d) a constant ordering of the root', "current_user > 'a'"], +]; + +describe('[#19886 stage 2d] a comparand that is not one value is refused — visible to the authoring gate', () => { + for (const [name, source] of REFUSED_BY_SHAPE) { + it(`${name}: ${source}`, () => { + refusedEverywhere(source); + expect(isPushdownableCel(source).ok).toBe(false); + expect(isSupportedRlsExpression(source)).toBe(false); + }); + } +}); + +/** Refused per request; the source passes the shape check because the value is per request. */ +const REFUSED_PER_REQUEST: Array<[string, string, RegExp]> = [ + ['(b) a membership set with a list member', '!(record.account in current_user.nested_set)', /current_user\.nested_set/], + ['(c) > a membership set', 'record.reviewer_id > current_user.org_user_ids', /current_user\.org_user_ids/], + ['(c) a membership set on the left of <=', 'current_user.org_user_ids <= record.reviewer_id', /current_user\.org_user_ids/], + ['(c) a constant ordering of a membership set', "current_user.org_user_ids > 'a'", /current_user\.org_user_ids/], + ['(e) a constant != of a membership set', "current_user.org_user_ids != 'x'", /current_user\.org_user_ids/], + ['(e) a constant == of a membership set', "current_user.org_user_ids == 'u_me'", /current_user\.org_user_ids/], + ['(d) > a key that resolves to an object', 'record.tier > current_user.profile', /current_user\.profile/], +]; + +describe('[#19886 stage 2d] a resolved comparand that is not one value is refused at request time', () => { + for (const [name, source, names] of REFUSED_PER_REQUEST) { + it(`${name}: ${source} — naming the variable, withholding its value`, () => { + const detail = refusedEverywhere(source); + expect(detail).toMatch(names); + for (const secret of ['u_me', 'u_peer', 'acc_secret_a', 'acc_secret_b', 'gold']) { + expect(detail).not.toContain(secret); + } + expect(isPushdownableCel(source).ok).toBe(true); + expect(isSupportedRlsExpression(source)).toBe(true); + }); + } +}); + +describe('[#19886 stage 2d] each refusal carries its own remedy', () => { + it('an ordering operator is told to take one bound, a range, or `in`', () => { + const detail = refusedEverywhere("record.status > ['m']"); + expect(detail).toContain('`>` orders against one value'); + expect(detail).toContain("record.f > 'm'"); + expect(detail).toContain('&&'); + }); + + it('a nested list is told to flatten, naming the member', () => { + const detail = refusedEverywhere("!(record.status in ['open', ['closed']])"); + expect(detail).toContain('member 1 of the list literal is itself a list'); + expect(detail).toContain("record.f in ['a', 'b']"); + }); + + it('`==` / `!=` keep the stage 2c remedy', () => { + const detail = refusedEverywhere("current_user.org_user_ids != 'x'"); + expect(detail).toContain('`!=` compares one value'); + expect(detail).toContain('!(record.f in current_user.org_user_ids)'); + }); +}); + +describe('[#19886 stage 2d] every neighbouring comparison lowers exactly as before', () => { + const ok = (source: string) => { + const r = compileCelToFilter(source, { variables: VARS }); + if (!r.ok) throw new Error(`expected "${source}" to lower, got ${r.reason}: ${r.detail}`); + return r.filter; + }; + + it('an ordering operator against one literal, one scalar key, and another field', () => { + expect(ok("record.status > 'm'")).toEqual({ status: { $gt: 'm' } }); + expect(ok('record.amount <= 10')).toEqual({ amount: { $lte: 10 } }); + expect(ok('record.owner_id >= current_user.id')).toEqual({ owner_id: { $gte: 'u_me' } }); + expect(ok('record.a > record.b')).toEqual({ a: { $gt: { $field: 'b' } } }); + }); + + it('`in` / `not in` against a flat list and a membership set', () => { + expect(ok("record.status in ['open', 'pending']")).toEqual({ status: { $in: ['open', 'pending'] } }); + expect(ok("!(record.status in ['closed'])")).toEqual({ $not: { status: { $in: ['closed'] } } }); + expect(ok('record.owner_id in current_user.org_user_ids')).toEqual({ owner_id: { $in: ['u_me', 'u_peer'] } }); + expect(ok('record.status in []')).toEqual({ status: { $in: [] } }); + }); + + it('constant comparisons over scalars still fold', () => { + expect(ok('1 == 1')).toEqual({}); + expect(ok("current_user.id != 'guest'")).toEqual({}); + expect(ok("current_user.id > 'a'")).toEqual({}); + }); + + it('field-to-field `==` / `!=` still lower — the column TYPE is not the lowering\'s to judge', () => { + expect(ok('record.status != record.tags')).toEqual({ status: { $ne: { $field: 'tags' } } }); + expect(ok('!(record.status == record.tags)')).toEqual({ $not: { status: { $eq: { $field: 'tags' } } } }); + }); +}); diff --git a/packages/formula/src/cel-to-filter.ts b/packages/formula/src/cel-to-filter.ts index 389f4f83178..4c6ef215947 100644 --- a/packages/formula/src/cel-to-filter.ts +++ b/packages/formula/src/cel-to-filter.ts @@ -43,8 +43,18 @@ * null/undefined MEMBER of a resolved membership array, which is the same * unresolved value one level in. See {@link lowerMembership} for why the member * is refused rather than dropped. The ROOT alone (`current_user`) is the whole - * context object, never a value: `==` / `!=` refuse it (see + * context object, never a value: every comparison refuses it (see * {@link variableObjectComparandRefusal}). + * + * ## One value per comparison (#19886) + * Each of the six comparisons (`==` `!=` `>` `>=` `<` `<=`) compares ONE value. + * A list operand — a list literal, or a variable that resolves to an array — + * is refused whichever side it is on, opposite a field or in a constant + * comparison ({@link arrayComparandRefusal}); so are the variable root and a + * variable resolving to an object ({@link variableObjectComparandRefusal}). + * An `in` list whose member is itself a list is refused too + * ({@link listMemberRefusal}). "One of these" is `in`, "none of these" is + * `!(… in …)`, and an ordering comparison takes one bound. */ import type { ASTNode } from '@marcbachmann/cel-js'; @@ -347,11 +357,11 @@ function lowerComparison(op: string, lNode: ASTNode, rNode: ASTNode, ctx: Ctx): // field-to-field comparison → `{ $field: otherPath }` reference. return emit((L as { path: string }).path, op, { $field: (R as { path: string }).path }, true); } - // [#19886] `==` / `!=` against a comparand that IS a list — a list literal, or - // a `current_user` variable that resolves to an array — is refused before it - // is emitted. See {@link arrayComparandRefusal}. [#19959] So is the variable - // root alone, and a variable that resolves to an object: see - // {@link variableObjectComparandRefusal}. + // [#19886] A comparison against a comparand that IS a list — a list literal, + // or a `current_user` variable that resolves to an array — is refused before + // it is emitted ([stage 2d] under the ordering operators too). See + // {@link arrayComparandRefusal}. [#19959] So is the variable root alone, and a + // variable that resolves to an object: see {@link variableObjectComparandRefusal}. if (lField) return emit((L as { path: string }).path, op, comparandOf(op, R, ctx), false); if (rField) return emit((R as { path: string }).path, FLIP[op] ?? op, comparandOf(op, L, ctx), false); @@ -361,8 +371,15 @@ function lowerComparison(op: string, lNode: ASTNode, rNode: ASTNode, ctx: Ctx): // variable root or object operand is refused first: `current_user != 'guest'` // folded to "no restriction" because an object is never strictly equal to a // literal, which is the same whole-object comparison one branch over. + // [#19886 stage 2d] …and so is a LIST operand, the same fault one step over: + // `current_user.org_user_ids != 'x'` folded to "no restriction" because an + // array is never strictly equal to a literal, and + // `current_user.org_user_ids > 'a'` folded on JavaScript's coercion of the + // array to a string. A list literal is refused in shape mode too. const lv = variableOperandOf(op, L, ctx); const rv = variableOperandOf(op, R, ctx); + if (Array.isArray(lv)) throw arrayComparandRefusal(op, L); + if (Array.isArray(rv)) throw arrayComparandRefusal(op, R); if (ctx.mode === 'shape') return {}; // shape check: structurally fine const truth = constFold(op, lv, rv); if (truth === true) return {}; @@ -408,6 +425,12 @@ function lowerMembership(elemNode: ASTNode, containerNode: ASTNode, ctx: Ctx): F ); } } + // [#19886 stage 2d] A member that is itself a LIST is refused, written or + // resolved: see {@link listMemberRefusal}. + if (Array.isArray(value)) { + const nested = value.findIndex((member) => Array.isArray(member)); + if (nested !== -1) throw listMemberRefusal(container, nested); + } return { [(elem as { path: string }).path]: { $in: value } } as FilterCondition; } @@ -477,11 +500,36 @@ function lowerStringMethod(args: [string, ASTNode, ASTNode[]], ctx: Ctx): Filter * "One of these values" is `in`; "none of these values" is `!(… in …)`. The * refusal message names the variable PATH the author wrote and never a value: * a resolved array is a membership set. + * + * ## The ordering operators, and the constant branch (stage 2d) + * + * The ruling's letter was `==` / `!=`, so the ordering operators kept lowering + * a list: `record.status > ['m']` became `{ status: { $gt: ['m'] } }`, which + * the write-check evaluator compared as the string `'m'` (JavaScript coerces + * the array), admitting whatever that string comparison admitted. An ordering + * takes one bound, so a list is refused under it too, with its own remedy (one + * bound, a range, or `in`). + * + * A comparison with no field at all folds to a constant, and the fold met the + * same list: `current_user.org_user_ids != 'x'` folded to "no restriction" + * (an array is never strictly equal to a literal), and + * `current_user.org_user_ids > 'a'` folded on the coerced string. A list + * operand is refused on that branch first, whichever side it is on. */ function arrayComparandRefusal(op: string, leaf: Leaf): CompileError { const list = leaf.kind === 'var' ? leaf.path.join('.') : '[...]'; const comparand = leaf.kind === 'var' ? `its comparand \`${list}\` resolves to a list` : 'its comparand is a list literal'; + if (op !== '==' && op !== '!=') { + // [#19886 stage 2d] An ORDERING operator: "one of these" is not what it + // meant, so the remedy is one bound (or a range, or membership). + return new CompileError( + 'unsupported', + `\`${op}\` orders against one value, but ${comparand} — compare against one bound ` + + `(\`record.f ${op} 'm'\`), write a range as two comparisons joined by \`&&\`, and test ` + + `membership with \`record.f in ${list}\``, + ); + } return new CompileError( 'unsupported', `\`${op}\` compares one value, but ${comparand} — spell "one of these" ` + @@ -490,6 +538,37 @@ function arrayComparandRefusal(op: string, leaf: Leaf): CompileError { ); } +/** + * [#19886 stage 2d] `in` compares the field with each MEMBER of its list, one + * value at a time, and a member that is itself a list is not one value. + * + * `!(record.status in [['closed', 'archived']])` used to lower to + * `{ $not: { status: { $in: [['closed', 'archived']] } } }`. The write-check + * evaluator compares strictly, so the nested member matched no record, `$in` + * was false for every post-image, and the negation admitted every write the + * policy was written to refuse — the `!=`-against-a-list bypass + * ({@link arrayComparandRefusal}) one level down. Refused here, it fails closed + * on every consumer's existing `unsupported` path, as that refusal does. + * + * Both kinds of list, as there: a LIST LITERAL is refused by the shape check + * too, so the authoring gate reports it; a `current_user` membership set that + * RESOLVES with a list member (the `ExecutionContext` contract declares its + * members as strings, so only a host violating it can supply one) is refused + * per request. The message names the member's index and the variable PATH, + * never a value. + */ +function listMemberRefusal(container: Leaf, index: number): CompileError { + const where = + container.kind === 'var' + ? `member ${index} of \`${container.path.join('.')}\` resolves to a list` + : `member ${index} of the list literal is itself a list`; + return new CompileError( + 'unsupported', + `\`in\` compares the field with each member of its list, one value at a time, but ${where} — ` + + `flatten it: \`record.f in ['a', 'b']\`, not \`record.f in [['a', 'b']]\``, + ); +} + /** * [#19959] `==` / `!=` compare ONE value, and a variable ROOT is not one value. * @@ -522,15 +601,23 @@ function arrayComparandRefusal(op: string, leaf: Leaf): CompileError { * of this published compiler. A `Date` is a literal comparand in the * `$eq` / `$ne` contract and passes. * - * Both guards apply wherever `==` / `!=` resolves an operand: opposite a field, + * Both guards apply wherever a comparison resolves an operand: opposite a field, * and on either side of a constant comparison, where `current_user != 'guest'` * folded to "no restriction" because an object is never strictly equal to a * literal. `{ $field }` references never reach here (the field-to-field branch * emits them with `isRef`), and a CEL map literal is refused by * {@link classify}, so a resolved variable is the only way an object ever became - * a comparand. Ordering operators keep their existing lowering: they are outside - * the `==` / `!=` letter this refusal carries. The message names the variable - * PATH the author wrote and never a value. + * a comparand. The message names the variable PATH the author wrote and never a + * value. + * + * [#19886 stage 2d] The ordering operators (`>` `>=` `<` `<=`) are judged too. + * They were left out when this refusal landed, because its letter was `==` / + * `!=`; left out, `record.reviewer_id > current_user` lowered to a `$gt` + * against the whole context object, which the write-check evaluator compared + * as the string `[object Object]` — measured admitting and storing the writes + * such a `check` was written to refuse. An ordering takes one bound, so the + * root and an object are refused under it for the same reason a list is + * ({@link arrayComparandRefusal}). */ function variableObjectComparandRefusal(op: string, path: string[]): CompileError { const written = path.join('.'); @@ -551,28 +638,27 @@ function isObjectComparand(value: unknown): boolean { } /** - * Resolve a comparison operand, refusing under `==` / `!=` a variable root + * Resolve a comparison operand, refusing under every comparison a variable root * before resolution and a variable resolving to an object after it * ({@link variableObjectComparandRefusal}). In shape mode a variable resolves to * the placeholder, so only the root is refused there. */ function variableOperandOf(op: string, leaf: Leaf, ctx: Ctx): unknown { - const equality = op === '==' || op === '!='; - if (equality && leaf.kind === 'var' && leaf.path.length === 1) throw variableObjectComparandRefusal(op, leaf.path); + if (leaf.kind === 'var' && leaf.path.length === 1) throw variableObjectComparandRefusal(op, leaf.path); const value = resolveValue(leaf, ctx); - if (equality && leaf.kind === 'var' && isObjectComparand(value)) throw variableObjectComparandRefusal(op, leaf.path); + if (leaf.kind === 'var' && isObjectComparand(value)) throw variableObjectComparandRefusal(op, leaf.path); return value; } /** - * Resolve the non-field side of a comparison, refusing under `==` / `!=` a list - * ({@link arrayComparandRefusal}) and a variable root or object + * Resolve the non-field side of a comparison, refusing under every comparison a + * list ({@link arrayComparandRefusal}) and a variable root or object * ({@link variableOperandOf}). In shape mode a variable resolves to the * placeholder, so only a literal list and a bare root are refused there. */ function comparandOf(op: string, leaf: Leaf, ctx: Ctx): unknown { const value = variableOperandOf(op, leaf, ctx); - if ((op === '==' || op === '!=') && Array.isArray(value)) throw arrayComparandRefusal(op, leaf); + if (Array.isArray(value)) throw arrayComparandRefusal(op, leaf); return value; } diff --git a/packages/formula/src/matches-filter-array-comparand.test.ts b/packages/formula/src/matches-filter-array-comparand.test.ts index ad47d5dc6bb..f094daba896 100644 --- a/packages/formula/src/matches-filter-array-comparand.test.ts +++ b/packages/formula/src/matches-filter-array-comparand.test.ts @@ -20,6 +20,20 @@ * * The first two rows are the bypass. The third failed closed only by accident * of polarity, which is why the equality position is refused with it. + * + * [#19886 stage 2d] The same fault one position over, each measured admitting + * and storing the writes a `check` was written to refuse: + * + * | shape | before | after | + * |---|---|---| + * | `{ s: { $gt: ['m'] } }` (and `$gte` / `$lt` / `$lte`) | the array compared as the string `'m'` | throws → 400 | + * | `{ $not: { s: { $in: [['a','b']] } } }`, `$nin` | `true` → every write **ALLOWED** | throws → 400 | + * | `{ s: { $ne: { $field: 'tags' } } }`, `tags` a list | `true` → every write **ALLOWED** | throws → 400 | + * + * The first two are authored shapes, refused before any record is judged like + * the rows above. The third is a `{ $field }` comparison whose column holds a + * list: the shape is legal, so it is judged on the record — refused when either + * compared column holds a list (or an object) on the record being judged. */ import { describe, it, expect } from 'vitest'; @@ -48,6 +62,13 @@ const SHAPES: Array<[string, (list: unknown[]) => Record]> = [ ['$ne with an array', (list) => ({ status: { $ne: list } })], ['a bare-array field spec (implicit equality)', (list) => ({ status: list })], ['$eq with an array', (list) => ({ status: { $eq: list } })], + // [#19886 stage 2d] the ordering operators, and a list member that is a list + ['$gt with an array', (list) => ({ status: { $gt: list } })], + ['$gte with an array', (list) => ({ status: { $gte: list } })], + ['$lt with an array', (list) => ({ status: { $lt: list } })], + ['$lte with an array', (list) => ({ status: { $lte: list } })], + ['$in with an array member', (list) => ({ status: { $in: ['open', list] } })], + ['$nin with an array member', (list) => ({ status: { $nin: [list] } })], ]; /** Every depth the refusal must reach — and the ones where the old answer was absorbed or inverted. */ @@ -147,15 +168,70 @@ describe('[#19886] every neighbouring shape answers exactly as before', () => { expect(m({ at: new Date(d.getTime()) }, { at: { $ne: d } })).toBe(false); }); - it('a { $field } reference is judged by what was AUTHORED, not by what it resolves to', () => { - // `tags` holds an array on this record; the reference itself is not one. - expect(m(RECORD, { tags: { $eq: { $field: 'tags' } } })).toBe(true); + it('a { $field } reference between two scalar columns compares them', () => { expect(m(RECORD, { other: { $ne: { $field: 'status' } } })).toBe(true); expect(m(RECORD, { status: { $ne: { $field: 'status' } } })).toBe(false); + expect(m(RECORD, { status: { $eq: { $field: 'status' } } })).toBe(true); + }); + + it('[#19886 stage 2d] a { $field } reference to a list-holding column is refused, even against itself', () => { + // Pinned `true` before stage 2d: a column holding a list compared with + // itself matched by reference identity. It is a list compared with a list, + // and it is refused like every other list in a one-value comparison. + const err = refusalOf({ tags: { $eq: { $field: 'tags' } } }); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); }); it('a scalar comparand against a STORED array is untouched — the refusal is about the comparand', () => { expect(m(RECORD, { tags: { $ne: 'a' } })).toBe(true); expect(m(RECORD, { tags: 'a' })).toBe(false); }); + + it('[#19886 stage 2d] ordering against one bound, and flat lists under $in / $nin', () => { + expect(m({ status: 'open' }, { status: { $gt: 'm' } })).toBe(true); + expect(m({ status: 'archived' }, { status: { $gt: 'm' } })).toBe(false); + expect(m({ n: 5 }, { n: { $lte: 5 } })).toBe(true); + expect(m({ status: 'open' }, { status: { $in: [] } })).toBe(false); + expect(m({ status: 'open' }, { status: { $nin: [] } })).toBe(true); + }); +}); + +describe('[#19886 stage 2d] a { $field } comparison whose column holds a list is refused on that record', () => { + const m = matchesFilterCondition; + const POST_IMAGE = { status: 'closed', reviewer: 'closed', tags: ['closed', 'archived'], meta: { k: 'v' } }; + + const REFUSED: Array<[string, Record]> = [ + ['$ne against a list-holding column (`record.status != record.tags`)', { status: { $ne: { $field: 'tags' } } }], + ['a negated $eq (`!(record.status == record.tags)`)', { $not: { status: { $eq: { $field: 'tags' } } } }], + ['the mirror — the list on the constrained side (`record.tags != record.status`)', { tags: { $ne: { $field: 'status' } } }], + ['an ordering operator against a list-holding column', { status: { $gt: { $field: 'tags' } } }], + ['a column holding an object', { status: { $ne: { $field: 'meta' } } }], + ['the offset form, judged on its base column', { $not: { status: { $eq: { $field: 'tags', addDays: 1 } } } }], + ['under $or beside an unsatisfied branch', { $or: [{ reviewer: 'nobody' }, { status: { $ne: { $field: 'tags' } } }] }], + ]; + + for (const [name, filter] of REFUSED) { + it(`${name}: INVALID_FILTER / 400`, () => { + const err = refusalOf(filter, POST_IMAGE); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + }); + } + + it('the same filter over a record whose columns hold one value each is compared, not refused', () => { + expect(m({ status: 'closed', tags: 'closed' }, { status: { $ne: { $field: 'tags' } } })).toBe(false); + expect(m({ status: 'open', tags: 'closed' }, { status: { $ne: { $field: 'tags' } } })).toBe(true); + expect(m({ status: 'open', tags: 'closed' }, { $not: { status: { $eq: { $field: 'tags' } } } })).toBe(true); + }); + + it('the message withholds the columns and their values', () => { + const err = refusalOf({ secret_scope_column: { $ne: { $field: 'secret_list' } } }, { + secret_scope_column: 'x', + secret_list: ['usr_member_one', 'usr_member_two'], + }); + for (const secret of ['secret_scope_column', 'secret_list', 'usr_member_one', 'usr_member_two']) { + expect(err.message).not.toContain(secret); + } + }); }); diff --git a/packages/formula/src/matches-filter.ts b/packages/formula/src/matches-filter.ts index c7416c8f9b6..1fb6f28c22e 100644 --- a/packages/formula/src/matches-filter.ts +++ b/packages/formula/src/matches-filter.ts @@ -173,16 +173,20 @@ function emptyFieldConstraintError(field: string, path: string): Error { function arrayComparandError(): Error { const err = new Error( 'A single-value comparison in this filter received an array as its comparand: an array ' + - 'under "$ne", or an array in the equality position ({ "field": [ ... ] } or "$eq"). A list ' + - 'is not one comparable value. For "one of these values" use "$in", and for "none of these ' + - 'values" use "$nin" — the list operators the filter protocol declares. It is refused before ' + - 'any record is judged rather than evaluated, because this evaluator compares strictly and ' + - 'no stored value ever equals an array: "$ne" matched EVERY record, and so did a negated ' + - 'equality, which on a row-level write check admitted every write the check was written to ' + - 'refuse. The field, the operator and the value are withheld from this message because the ' + - 'filter may be an access policy the caller did not write; in a row-level policy, look for ' + - 'a "!=" or "==" compared against a list literal or a current_user membership key, and ' + - 'rewrite it with "in" (for example "!(record.status in [\'closed\', \'archived\'])").', + 'under "$ne", or an array in the equality position ({ "field": [ ... ] } or "$eq"), or ' + + 'under an ordering operator ("$gt", "$gte", "$lt", "$lte"), or as a member of an "$in" / ' + + '"$nin" list, or a column on either side of a { "$field" } comparison that holds a list or ' + + 'an object. A list is not one comparable value. For "one of these values" use "$in", and ' + + 'for "none of these values" use "$nin" — the list operators the filter protocol declares; ' + + 'an ordering comparison takes one bound. It is refused rather than evaluated, because this ' + + 'evaluator compares strictly and no stored value ever equals an array: "$ne" matched EVERY ' + + 'record, and so did a negated equality or a negated "$in", which on a row-level write check ' + + 'admitted every write the check was written to refuse, and an ordering operator compared ' + + 'the array as a string. The field, the operator and the value are withheld from this ' + + 'message because the filter may be an access policy the caller did not write; in a ' + + 'row-level policy, look for a comparison against a list literal, a current_user membership ' + + 'key, or a json or multiple field, and rewrite it with "in" (for example ' + + '"!(record.status in [\'closed\', \'archived\'])").', ) as Error & { code?: string; status?: number }; err.code = StandardErrorCode.enum.INVALID_FILTER; err.status = 400; @@ -190,13 +194,32 @@ function arrayComparandError(): Error { } /** - * [#19886] The operators whose array comparand this face refuses: exactly the - * two the refusal was ruled for — the equality slot and its negation. The - * ordering operators (`$gt` / `$gte` / `$lt` / `$lte`) are deliberately NOT in - * this list; what they do with an array is a separate question this refusal - * does not answer. + * [#19886] The operators whose array comparand this face refuses: the six that + * compare ONE value. The equality slot and its negation were ruled first + * (stage 2a); [stage 2d] the four ordering operators joined them, because an + * ordering comparison against an array did not refuse or fail closed — it + * compared the array's JavaScript string form (`['m']` ordered as `'m'`), an + * answer no declared contract gives and no SQL backend shares. */ -const ARRAY_REFUSED_OPERATORS = ['$eq', '$ne'] as const; +const ARRAY_REFUSED_OPERATORS = ['$eq', '$ne', '$gt', '$gte', '$lt', '$lte'] as const; + +/** + * [#19886 stage 2d] The list operators, whose MEMBERS are each one comparable + * value. A member that is itself an array matched no record under `$in`, so + * `$nin` and a negated `$in` matched EVERY record — the `$ne` bypass one level + * down. + */ +const LIST_MEMBER_OPERATORS = ['$in', '$nin'] as const; + +/** One comparable value: not a list, and not a plain object (a `Date` is a value). */ +function isNonScalarValue(value: unknown): boolean { + return Array.isArray(value) || isOperatorMap(value); +} + +/** A `{ $field }` reference comparand — the same test {@link resolveValue} applies. */ +function isFieldReference(raw: unknown): raw is Record { + return raw !== null && typeof raw === 'object' && !Array.isArray(raw) && '$field' in (raw as Record); +} /** A plain object — an operator map rather than a comparand (`Date` is a comparand). */ function isOperatorMap(spec: unknown): spec is Record { @@ -243,11 +266,17 @@ function assertFilterShape(node: unknown, path: string): void { if (isEmptyFieldConstraint(val)) throw emptyFieldConstraintError(key, here); // [#19886] The equality position, spelled bare: `{ field: [...] }`. if (Array.isArray(val)) throw arrayComparandError(); - // [#19886] …and spelled with an operator: `$eq` / `$ne` carrying an array. + // [#19886] …and spelled with an operator: a one-value operator carrying an + // array ([stage 2d] the ordering operators included), or a list operator + // one of whose members is itself an array. if (isOperatorMap(val)) { for (const op of ARRAY_REFUSED_OPERATORS) { if (Array.isArray(val[op])) throw arrayComparandError(); } + for (const op of LIST_MEMBER_OPERATORS) { + const list = val[op]; + if (Array.isArray(list) && list.some((member) => Array.isArray(member))) throw arrayComparandError(); + } } } } @@ -315,6 +344,9 @@ function evalField(record: Record, field: string, spec: unknown } function evalOp(actual: unknown, op: string, raw: unknown, record: Record): boolean { + // [#19886 stage 2d] A `{ $field }` comparison whose column holds a list or an + // object ON THIS RECORD — either side. See {@link assertComparableReference}. + if (isFieldReference(raw)) assertComparableReference(actual, op, raw, record); const v = resolveValue(raw, record); // [#14104] An offset reference whose base is NULL is FALSE for every // operator — see {@link NO_OFFSET_BASE}. Before the switch, so `$ne`'s @@ -422,6 +454,53 @@ function evalOp(actual: unknown, op: string, raw: unknown, record: Record, + record: Record, +): void { + if (!(ARRAY_REFUSED_OPERATORS as readonly string[]).includes(op)) return; + const referent = getPath(record, String(raw.$field)); + if (isNonScalarValue(referent) || isNonScalarValue(actual)) throw arrayComparandError(); +} + /** * The inclusive-upper-bound comparison, with the calendar-day rule the rest of * the platform applies (ADR-0053 D-D, #3777): a bare `YYYY-MM-DD` bound means From bfd409eebb0e55895ad845ff88baf9580ba9cbd1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 14:42:49 +0000 Subject: [PATCH 2/3] test(plugin-security): pin one-value comparands failing closed on both clauses; register the ADR-0087 entry and changeset A table-driven pin through the real SecurityPlugin, ObjectQL, driver-sql and driver-sqlite-wasm: every leak row refuses both inserts and stores nothing, reads no rows or the driver's own refusal, and hands the analytics bridge the deny scope; the one-value spellings stay accepted and enforced. The semantic entry cel-predicate-one-value-comparand-refused is registered under protocol major 18 and the registry regenerated. Claude-Session: https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN Co-authored-by: Claude --- .../19886-one-value-comparand-refused.md | 38 +++ ...s-one-value-comparand-fails-closed.test.ts | 260 ++++++++++++++++++ ...l-predicate-one-value-comparand-refused.ts | 66 +++++ packages/spec/src/migrations/registry.ts | 62 +++++ 4 files changed, 426 insertions(+) create mode 100644 .changeset/19886-one-value-comparand-refused.md create mode 100644 packages/plugins/plugin-security/src/rls-one-value-comparand-fails-closed.test.ts create mode 100644 packages/spec/src/migrations/entries/semantic/18.cel-predicate-one-value-comparand-refused.ts diff --git a/.changeset/19886-one-value-comparand-refused.md b/.changeset/19886-one-value-comparand-refused.md new file mode 100644 index 00000000000..c4a7096a8f4 --- /dev/null +++ b/.changeset/19886-one-value-comparand-refused.md @@ -0,0 +1,38 @@ +--- +"@objectstack/formula": minor +"@objectstack/plugin-security": minor +"@objectstack/plugin-sharing": minor +"@objectstack/lint": minor +"@objectstack/objectql": minor +"@objectstack/spec": patch +--- + +A row-level or sharing-rule predicate whose comparison is handed something other than one value is refused at the CEL lowering or at the write-check evaluator, instead of admitting writes and reads it was written to refuse (#19886). + +**BREAKING** — an accept-set narrowing, shipped by `@objectstack/formula`, `@objectstack/plugin-security`, `@objectstack/plugin-sharing`, `@objectstack/lint` and `@objectstack/objectql` as `minor` under the repo's launch-window convention for accept-set narrowings. The hand-migration prescription is registered under protocol major 18 as `cel-predicate-one-value-comparand-refused`. + +Clause-②: no (narrowing) + +**Security fix for RLS write checks and reads.** Each shape below was measured through the real plugin-security on driver-sql and driver-memory: + +- `!(record.status in [['closed', 'archived']])` (a list nested in an `in` list) admitted and stored every write the `check` was written to refuse, and a `using` read returned every row on driver-memory. +- `current_user.org_user_ids != 'x'` and `current_user.org_user_ids > 'a'` (a membership set on a comparison with no field) folded to "no restriction": every write admitted, every row read, on every driver. +- `record.status > ['m']` compared the list as the string `'m'`, and `record.reviewer_id > current_user` compared the whole caller object as a string; the latter admitted and stored every write. +- `record.status != record.tags`, its negation `!(record.status == record.tags)`, and the mirror `record.tags != record.status`, with `tags` a `json` field or a `multiple` lookup, admitted and stored every write. + +What changes: + +- `@objectstack/formula`: `compileCelToFilter` refuses, with `unsupported`, a list comparand under every comparison (the ordering operators now included, and on the constant-fold branch, whichever side), the `current_user` root or a key resolving to an object under an ordering operator, and an `in` list whose member is itself a list. The authoring shape check (`isPushdownableCel`, `isSupportedRlsExpression`) reports each literal form; a resolved value is refused per request. `matchesFilterCondition` refuses, with `INVALID_FILTER` / 400, an array under `$gt` / `$gte` / `$lt` / `$lte`, an array member of `$in` / `$nin`, and a `{ $field }` comparison (`$eq`, `$ne` or an ordering operator) whose column holds a list or an object on the record being judged, on either side. The message withholds the field, the operator and the value. +- `@objectstack/plugin-security`: the RLS compiler drops a policy the compiler refuses and fails closed when no other policy applies (`RLS_DENY_FILTER`: reads return no rows, `check` writes are refused 403, and `getReadFilter` hands the analytics read scope the deny scope). A `check` comparing a field with a list-holding column is refused 400 and stores nothing. +- `@objectstack/plugin-sharing`: a declared sharing rule with such a `condition` is skipped at bootstrap and never seeded. +- `@objectstack/lint`: the literal forms are reported as `rls-predicate-unenforceable`, and an ordering comparison against a membership set through the reference pass. +- `@objectstack/objectql`: a `having` comparison against a `{ $field }` column whose aggregated row holds a list is refused 400 where the row carries the list itself (driver-memory); driver-sql rows carry the stored JSON text and compare as before. +- `@objectstack/spec`: the migration registry carries the entry. + +The stage 2a changeset's sentence that `{ $field }` references evaluate as before no longer holds for a column holding a list or an object: that comparison is now refused. + +**What to change.** "One of these values" is `record.status in ['open', 'pending']`, and "none of these values" is `!(record.status in ['closed', 'archived'])`, with the list flat. An ordering takes one bound (`record.status > 'm'`); a range is two comparisons joined by `&&`. Compare against one key of the caller (`record.reviewer_id > current_user.id`). A field compared with a `json` or `multiple` field has no pushdown form: compare with a single-valued column, or move the condition into a validation rule or hook. In a raw filter, use `$in` / `$nin` with flat lists and one bound per ordering operator. + +Not changed: a field compared with a `json` or `multiple` field still lowers and is not reported at authoring time, because the lowering sees the predicate's text and not the object's field types; driver-memory still answers a `{ $field }` comparison on a read without evaluating the reference. + + diff --git a/packages/plugins/plugin-security/src/rls-one-value-comparand-fails-closed.test.ts b/packages/plugins/plugin-security/src/rls-one-value-comparand-fails-closed.test.ts new file mode 100644 index 00000000000..aaf582dd534 --- /dev/null +++ b/packages/plugins/plugin-security/src/rls-one-value-comparand-fails-closed.test.ts @@ -0,0 +1,260 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#19886 stage 2d] A row-level policy whose comparison is handed something + * other than ONE value fails closed on both clauses, through the real plugin, + * the real engine and two SQL-family drivers. + * + * Each row below was measured before the fix through this same stack — on + * driver-sql and on driver-memory — admitting and STORING the insert its + * `check` was written to refuse, or folding to "no restriction" and reading + * every row: + * + * | leak | predicate | refused where | a `check` insert now | a `using` read now | + * |---|---|---|---|---| + * | a | `record.status != record.tags` (`tags` json), its negated `==`, the mirror | the write-check evaluator, on the record | `INVALID_FILTER` / 400 | driver-sql's own cross-field refusal (400) | + * | b | `!(record.status in [['closed', 'archived']])`, and a membership set with a list member | the CEL lowering | `PERMISSION_DENIED` / 403 | zero rows | + * | c | `record.status > ['m']`, `> current_user.org_user_ids`, `current_user.org_user_ids > 'a'` | the CEL lowering | 403 | zero rows | + * | d | `record.reviewer > current_user` | the CEL lowering | 403 | zero rows | + * | e | `current_user.org_user_ids != 'x'` | the CEL lowering | 403 | zero rows | + * + * A lowering refusal makes `RLSCompiler.compileFilter` drop the policy, so with + * nothing else applicable it answers `RLS_DENY_FILTER` — the filter the + * analytics read-scope bridge (`getReadFilter`) hands on too. The write-check + * refusal for (a) is judged on the post-image's VALUES, because the lowering + * sees the predicate's text and not the object's field types. + * + * The controls are the one-value spellings of each, accepted and enforced. + * Ground truth is read past every scope. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { SecurityPlugin } from './security-plugin.js'; +import { RLS_DENY_FILTER } from './rls-compiler.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; + +const OBJ = 'qa_ticket'; +const SYS_CTX = { isSystem: true, userId: 'usr_system' }; +const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; + +const DRIVERS: Array<[string, () => unknown]> = [ + ['driver-sql (better-sqlite3)', () => + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true })], + ['driver-sqlite-wasm', () => new SqliteWasmDriver({ filename: ':memory:' })], +]; + +const engines: ObjectQL[] = []; +afterEach(async () => { + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot(makeDriver: () => unknown, clause: 'using' | 'check', predicate: string) { + const engine = new ObjectQL(); + engine.registerDriver(makeDriver() as never, true); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.qa.rls-one-value-19886', + name: 'RLS one-value comparand', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + { + name: 'qa_person', + label: 'Person', + sharingModel: 'public_read_write', + fields: { id: { name: 'id', type: 'text', primaryKey: true }, name: { name: 'name', type: 'text' } }, + }, + { + name: OBJ, + label: 'Ticket', + sharingModel: 'public_read_write', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + status: { name: 'status', type: 'text' }, + reviewer: { name: 'reviewer', type: 'text' }, + tags: { name: 'tags', type: 'json' }, + watchers: { name: 'watchers', type: 'lookup', reference: 'qa_person', multiple: true }, + }, + }, + ], + } as never); + await engine.syncSchemas(); + engines.push(engine); + await engine.insert('qa_person', [{ id: 'p1', name: 'P1' }, { id: 'p2', name: 'P2' }], { context: SYS_CTX } as never); + + const set = PermissionSetSchema.parse({ + name: 'qa_ticket_guard', + objects: { [OBJ]: { allowRead: true, allowCreate: true, allowEdit: true }, qa_person: { allowRead: true } }, + rowLevelSecurity: [{ name: 'guard', object: OBJ, operation: 'all', [clause]: predicate }], + }); + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (_type: string, name: string) => engine.getSchema(name) ?? null, + list: async () => [MEMBER_DEFAULT, set], + }, + }; + const ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx as never); + await plugin.start(ctx as never); + vi.spyOn((engine as unknown as { logger: { warn: () => void } }).logger, 'warn').mockImplementation(() => undefined); + + const caller = { + userId: 'usr_member', + positions: ['qa_pos'], + permissions: [set.name], + posture: 'MEMBER', + org_user_ids: ['usr_member', 'usr_peer'], + // A membership set a host staged with a list member (the ExecutionContext + // contract declares string members; this is the host violating it). + rlsMembership: { nested_set: [['closed', 'archived']] }, + }; + const stored = async () => + ((await engine.find(OBJ, { context: SYS_CTX } as never)) as Array<{ id: string }>).map((r) => r.id).sort(); + const readFilter = () => + (plugin as unknown as { getReadFilter: (o: string, c: unknown) => Promise }).getReadFilter(OBJ, caller); + return { engine, caller, stored, readFilter }; +} + +type Envelope = { code: string; status: number }; +const DENIED: Envelope = { code: 'PERMISSION_DENIED', status: 403 }; +const INVALID: Envelope = { code: 'INVALID_FILTER', status: 400 }; + +interface Leak { + leak: string; + predicate: string; + /** A row the policy was written to refuse, and one it was written to admit. */ + forbidden: Record; + allowed: Record; + /** What BOTH inserts now get: the policy cannot be evaluated, so nothing is admitted. */ + write: Envelope; + /** The `using` read: zero rows, or the driver's own refusal. */ + read: 'no rows' | Envelope; + /** Whether the lowering drops the policy — then the analytics scope is the deny sentinel. */ + dropped: boolean; +} + +const LEAKS: Leak[] = [ + { leak: '(a) != a json column', predicate: 'record.status != record.tags', + forbidden: { status: 'closed', tags: ['closed', 'archived'] }, allowed: { status: 'open', tags: ['closed'] }, + write: INVALID, read: INVALID, dropped: false }, + { leak: '(a) a negated == against a json column', predicate: '!(record.status == record.tags)', + forbidden: { status: 'closed', tags: ['closed', 'archived'] }, allowed: { status: 'open', tags: ['closed'] }, + write: INVALID, read: INVALID, dropped: false }, + { leak: '(a) the mirror, the json column on the left', predicate: 'record.tags != record.status', + forbidden: { status: 'closed', tags: ['closed', 'archived'] }, allowed: { status: 'open', tags: ['closed'] }, + write: INVALID, read: INVALID, dropped: false }, + { leak: '(a) != a multiple lookup', predicate: 'record.reviewer != record.watchers', + forbidden: { reviewer: 'p1', watchers: ['p1'] }, allowed: { reviewer: 'p2', watchers: ['p1'] }, + write: INVALID, read: INVALID, dropped: false }, + { leak: '(b) not-in with a nested list', predicate: "!(record.status in [['closed', 'archived']])", + forbidden: { status: 'closed' }, allowed: { status: 'open' }, + write: DENIED, read: 'no rows', dropped: true }, + { leak: '(b) a membership set with a list member', predicate: '!(record.status in current_user.nested_set)', + forbidden: { status: 'closed' }, allowed: { status: 'open' }, + write: DENIED, read: 'no rows', dropped: true }, + { leak: '(c) > a list literal', predicate: "record.status > ['m']", + forbidden: { status: 'archived' }, allowed: { status: 'open' }, + write: DENIED, read: 'no rows', dropped: true }, + { leak: '(c) a negated <= a list literal', predicate: "!(record.status <= ['m'])", + forbidden: { status: 'archived' }, allowed: { status: 'open' }, + write: DENIED, read: 'no rows', dropped: true }, + { leak: '(c) > a membership set', predicate: 'record.status > current_user.org_user_ids', + forbidden: { status: 'archived' }, allowed: { status: 'zzz' }, + write: DENIED, read: 'no rows', dropped: true }, + { leak: '(c) a constant ordering of a membership set', predicate: "current_user.org_user_ids > 'a'", + forbidden: { status: 'closed' }, allowed: { status: 'open' }, + write: DENIED, read: 'no rows', dropped: true }, + { leak: '(d) > the variable root', predicate: 'record.reviewer > current_user', + forbidden: { reviewer: 'aaa' }, allowed: { reviewer: 'zzz' }, + write: DENIED, read: 'no rows', dropped: true }, + { leak: '(e) a constant != of a membership set', predicate: "current_user.org_user_ids != 'x'", + forbidden: { status: 'closed' }, allowed: { status: 'open' }, + write: DENIED, read: 'no rows', dropped: true }, +]; + +const ROWS = [ + { id: 'r_closed', status: 'closed', reviewer: 'closed', tags: ['closed', 'archived'], watchers: ['p1'] }, + { id: 'r_open', status: 'open', reviewer: 'x', tags: ['closed'], watchers: ['p2'] }, + { id: 'r_archived', status: 'archived', reviewer: 'y', tags: ['z'], watchers: ['p2'] }, +]; + +const envelopeOf = (e: unknown): Envelope => { + const x = e as { code?: string; status?: number; statusCode?: number }; + return { code: String(x?.code), status: Number(x?.statusCode ?? x?.status) }; +}; + +for (const [driverName, makeDriver] of DRIVERS) { + describe(`[#19886 stage 2d] ${driverName}: a comparand that is not one value fails closed`, () => { + for (const row of LEAKS) { + it(`${row.leak} — \`${row.predicate}\`: both inserts refused ${row.write.code} / ${row.write.status}, nothing stored`, async () => { + for (const rec of [row.forbidden, row.allowed]) { + const { engine, caller, stored } = await boot(makeDriver, 'check', row.predicate); + const err = await engine.insert(OBJ, { id: 'ins', ...rec }, { context: caller } as never) + .then(() => null, (e: unknown) => e); + expect(err, 'expected the insert to be refused, but it was admitted').not.toBeNull(); + expect(envelopeOf(err)).toEqual(row.write); + expect(await stored()).toEqual([]); + } + }); + + it(`${row.leak} — \`${row.predicate}\`: the using read ${row.read === 'no rows' ? 'returns no rows' : `is refused ${row.read.code} / ${row.read.status}`}`, async () => { + const { engine, caller, stored, readFilter } = await boot(makeDriver, 'using', row.predicate); + await engine.insert(OBJ, ROWS, { context: SYS_CTX } as never); + + const read = await engine.find(OBJ, { context: caller } as never).then( + (rows) => ({ rows: (rows as Array<{ id: string }>).map((r) => r.id) }), + (e: unknown) => ({ error: envelopeOf(e) }), + ); + if (row.read === 'no rows') expect(read).toEqual({ rows: [] }); + else expect(read).toEqual({ error: row.read }); + // The analytics read-scope bridge is handed the same compiled filter. + if (row.dropped) expect(await readFilter()).toEqual(RLS_DENY_FILTER); + expect(await stored()).toEqual(['r_archived', 'r_closed', 'r_open']); + }); + } + }); + + describe(`[#19886 stage 2d] ${driverName}: the one-value spellings are accepted and enforced`, () => { + const CONTROLS: Array<[string, Record, Record, string[]]> = [ + ['record.status != record.reviewer', { status: 'closed', reviewer: 'closed' }, { status: 'open', reviewer: 'closed' }, ['r_archived', 'r_open']], + ["!(record.status in ['closed', 'archived'])", { status: 'closed' }, { status: 'open' }, ['r_open']], + ["record.status > 'm'", { status: 'archived' }, { status: 'open' }, ['r_open']], + ["!(record.status in current_user.org_user_ids) && record.status > 'm'", { status: 'archived' }, { status: 'open' }, ['r_open']], + ]; + for (const [predicate, forbidden, allowed, visible] of CONTROLS) { + it(`\`${predicate}\`: refuses the forbidden insert (403), stores the allowed one, reads ${JSON.stringify(visible)}`, async () => { + const w1 = await boot(makeDriver, 'check', predicate); + const err = await w1.engine.insert(OBJ, { id: 'ins_bad', ...forbidden }, { context: w1.caller } as never) + .then(() => null, (e: unknown) => e); + expect(envelopeOf(err)).toEqual(DENIED); + expect(await w1.stored()).toEqual([]); + + const w2 = await boot(makeDriver, 'check', predicate); + await w2.engine.insert(OBJ, { id: 'ins_ok', ...allowed }, { context: w2.caller } as never); + expect(await w2.stored()).toEqual(['ins_ok']); + + const r = await boot(makeDriver, 'using', predicate); + await r.engine.insert(OBJ, ROWS, { context: SYS_CTX } as never); + const rows = (await r.engine.find(OBJ, { context: r.caller } as never)) as Array<{ id: string }>; + expect(rows.map((x) => x.id).sort()).toEqual(visible); + }); + } + }); +} diff --git a/packages/spec/src/migrations/entries/semantic/18.cel-predicate-one-value-comparand-refused.ts b/packages/spec/src/migrations/entries/semantic/18.cel-predicate-one-value-comparand-refused.ts new file mode 100644 index 00000000000..aacd7277f4a --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.cel-predicate-one-value-comparand-refused.ts @@ -0,0 +1,66 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// Stage 2d of #19886: the comparisons ruling A's letter (a list under != and in +// the equality slot) did not name, each measured admitting and storing writes a +// row-level check was written to refuse. Two faces refuse them: the CEL pushdown +// compiler every row-level policy and declared sharing rule compiles through, +// and the @objectstack/formula evaluator the write check runs. Recorded as its +// own entry because the surface an author rewrites is a CEL predicate string. +export const entry: SemanticMigration = { + id: 'cel-predicate-one-value-comparand-refused', + // No backticks in `surface` — build-upgrade-guide renders it inside a code + // span already, and a nested backtick would close it. + surface: + 'security.PermissionSet rowLevelSecurity[].using and .check, and sharingRules[].condition — a ' + + 'CEL predicate whose comparison is handed something other than one value: an ordering ' + + 'operator (>, >=, <, <=) against a list literal or a current_user membership set; an in ' + + 'list with a member that is itself a list; a comparison with no field at all against a ' + + 'membership set (current_user.org_user_ids != "x"); an ordering operator against the ' + + 'current_user root or a key that resolves to an object; and a field compared with another ' + + 'field (==, !=, or an ordering operator) where either column holds a list or an object on ' + + 'the record, as a json column or a multiple lookup does. In a filter passed to ' + + 'matchesFilterCondition, also $gt / $gte / $lt / $lte with an array, and $in / $nin with an ' + + 'array member', + replacement: + 'the comparison the predicate was standing in for. "One of these values" is in: ' + + 'record.status in ["open", "pending"], or record.reviewer_id in current_user.org_user_ids; ' + + '"none of these values" is !(record.status in ["closed", "archived"]), with the list flat. ' + + 'An ordering takes one bound: record.status > "m", and a range is two comparisons joined by ' + + '&&. A comparison against the caller names one key: record.reviewer_id > current_user.id. ' + + 'A field compared with a json or multiple field has no pushdown form: compare with a ' + + 'single-valued column, or move the condition into a validation rule or hook. One-value ' + + 'comparisons, flat in lists, and field-to-field comparisons between single-valued columns ' + + 'lower and evaluate exactly as before', + reason: + 'Ruling A on #19886 refused a list under != and in the equality slot; stage 2d closes the ' + + 'same fault one position over, measured through the real plugin-security on driver-sql and ' + + 'driver-memory. !(record.status in [["closed", "archived"]]) lowered to a negated $in whose ' + + 'only member was a list, which the strictly comparing write-check evaluator matched on no ' + + 'record, so the negation admitted and stored every write, and driver-memory returned every ' + + 'row on a read. record.status > ["m"] compared the list as the string "m". ' + + 'current_user.org_user_ids != "x" and current_user.org_user_ids > "a" folded to "no ' + + 'restriction": every write admitted and every row read. record.reviewer_id > current_user ' + + 'compared the whole caller object as a string. record.status != record.tags, with tags a ' + + 'json or multiple field, matched every post-image, so the check admitted and stored every ' + + 'write. The CEL compiler now refuses the first four with reason unsupported, so the RLS ' + + 'compiler drops the policy and fails closed when no other policy applies (reads return no ' + + 'rows, check writes are refused 403, the analytics read scope is the deny scope) and a ' + + 'declared sharing rule is skipped at bootstrap; the authoring lint reports what the source ' + + 'shows (a list literal, the current_user root). The compiler cannot see a column\'s type, so ' + + 'the last is refused by the write-check evaluator on the record whose compared column holds ' + + 'a list or an object: INVALID_FILTER / 400, nothing stored. Metadata AT REST is not ' + + 'rewritten and this entry adds no D2 conversion: the platform cannot tell which comparison a ' + + 'predicate was standing in for, and rewriting it on the author\'s behalf would change which ' + + 'writes and rows it admits, which is the policy author\'s decision. ADR-0058 D4 / ADR-0087 / ' + + 'ADR-0112.', + acceptanceCriteria: + 'Grep the rowLevelSecurity using and check predicates of your permission sets, and the ' + + 'condition of your sharing rules, for an ordering operator next to a list or to ' + + 'current_user alone, for an in list that nests a list, for a comparison with no record ' + + 'field against a current_user membership key, and for a field compared with a json or ' + + 'multiple field; rewrite each as the replacement says. Then re-check what each policy is ' + + 'supposed to admit rather than assuming what it admitted before was right: several of these ' + + 'admitted every write, and two folded to no restriction at all.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index b45e2f28368..ae179d6cb93 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -6388,6 +6388,68 @@ const step18: MigrationStep = { + 'or its negation. On driver-mongodb, ' + 'grep stored query filters for $ne with an array value and rewrite each with $nin.', }, + // Stage 2d of #19886: the comparisons ruling A's letter (a list under != and in + // the equality slot) did not name, each measured admitting and storing writes a + // row-level check was written to refuse. Two faces refuse them: the CEL pushdown + // compiler every row-level policy and declared sharing rule compiles through, + // and the @objectstack/formula evaluator the write check runs. Recorded as its + // own entry because the surface an author rewrites is a CEL predicate string. + { + id: 'cel-predicate-one-value-comparand-refused', + // No backticks in `surface` — build-upgrade-guide renders it inside a code + // span already, and a nested backtick would close it. + surface: + 'security.PermissionSet rowLevelSecurity[].using and .check, and sharingRules[].condition — a ' + + 'CEL predicate whose comparison is handed something other than one value: an ordering ' + + 'operator (>, >=, <, <=) against a list literal or a current_user membership set; an in ' + + 'list with a member that is itself a list; a comparison with no field at all against a ' + + 'membership set (current_user.org_user_ids != "x"); an ordering operator against the ' + + 'current_user root or a key that resolves to an object; and a field compared with another ' + + 'field (==, !=, or an ordering operator) where either column holds a list or an object on ' + + 'the record, as a json column or a multiple lookup does. In a filter passed to ' + + 'matchesFilterCondition, also $gt / $gte / $lt / $lte with an array, and $in / $nin with an ' + + 'array member', + replacement: + 'the comparison the predicate was standing in for. "One of these values" is in: ' + + 'record.status in ["open", "pending"], or record.reviewer_id in current_user.org_user_ids; ' + + '"none of these values" is !(record.status in ["closed", "archived"]), with the list flat. ' + + 'An ordering takes one bound: record.status > "m", and a range is two comparisons joined by ' + + '&&. A comparison against the caller names one key: record.reviewer_id > current_user.id. ' + + 'A field compared with a json or multiple field has no pushdown form: compare with a ' + + 'single-valued column, or move the condition into a validation rule or hook. One-value ' + + 'comparisons, flat in lists, and field-to-field comparisons between single-valued columns ' + + 'lower and evaluate exactly as before', + reason: + 'Ruling A on #19886 refused a list under != and in the equality slot; stage 2d closes the ' + + 'same fault one position over, measured through the real plugin-security on driver-sql and ' + + 'driver-memory. !(record.status in [["closed", "archived"]]) lowered to a negated $in whose ' + + 'only member was a list, which the strictly comparing write-check evaluator matched on no ' + + 'record, so the negation admitted and stored every write, and driver-memory returned every ' + + 'row on a read. record.status > ["m"] compared the list as the string "m". ' + + 'current_user.org_user_ids != "x" and current_user.org_user_ids > "a" folded to "no ' + + 'restriction": every write admitted and every row read. record.reviewer_id > current_user ' + + 'compared the whole caller object as a string. record.status != record.tags, with tags a ' + + 'json or multiple field, matched every post-image, so the check admitted and stored every ' + + 'write. The CEL compiler now refuses the first four with reason unsupported, so the RLS ' + + 'compiler drops the policy and fails closed when no other policy applies (reads return no ' + + 'rows, check writes are refused 403, the analytics read scope is the deny scope) and a ' + + 'declared sharing rule is skipped at bootstrap; the authoring lint reports what the source ' + + 'shows (a list literal, the current_user root). The compiler cannot see a column\'s type, so ' + + 'the last is refused by the write-check evaluator on the record whose compared column holds ' + + 'a list or an object: INVALID_FILTER / 400, nothing stored. Metadata AT REST is not ' + + 'rewritten and this entry adds no D2 conversion: the platform cannot tell which comparison a ' + + 'predicate was standing in for, and rewriting it on the author\'s behalf would change which ' + + 'writes and rows it admits, which is the policy author\'s decision. ADR-0058 D4 / ADR-0087 / ' + + 'ADR-0112.', + acceptanceCriteria: + 'Grep the rowLevelSecurity using and check predicates of your permission sets, and the ' + + 'condition of your sharing rules, for an ordering operator next to a list or to ' + + 'current_user alone, for an in list that nests a list, for a comparison with no record ' + + 'field against a current_user membership key, and for a field compared with a json or ' + + 'multiple field; rewrite each as the replacement says. Then re-check what each policy is ' + + 'supposed to admit rather than assuming what it admitted before was right: several of these ' + + 'admitted every write, and two folded to no restriction at all.', + }, // The variable-ROOT sibling of cel-predicate-list-comparand-refused, one // comparand kind over: the same pushdown compiler, the same consumers, the same // fail-closed path. Recorded as its own entry because the surface an author From 39379b9b98cecfcf76d28a291c4567184d9ec38f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 16:07:45 +0000 Subject: [PATCH 3/3] docs(changeset): state that #20212's comparand faces already drop the (d) policy in this release Claude-Session: https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN Co-authored-by: Claude --- .changeset/19886-one-value-comparand-refused.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/19886-one-value-comparand-refused.md b/.changeset/19886-one-value-comparand-refused.md index c4a7096a8f4..61350490338 100644 --- a/.changeset/19886-one-value-comparand-refused.md +++ b/.changeset/19886-one-value-comparand-refused.md @@ -17,7 +17,7 @@ Clause-②: no (narrowing) - `!(record.status in [['closed', 'archived']])` (a list nested in an `in` list) admitted and stored every write the `check` was written to refuse, and a `using` read returned every row on driver-memory. - `current_user.org_user_ids != 'x'` and `current_user.org_user_ids > 'a'` (a membership set on a comparison with no field) folded to "no restriction": every write admitted, every row read, on every driver. -- `record.status > ['m']` compared the list as the string `'m'`, and `record.reviewer_id > current_user` compared the whole caller object as a string; the latter admitted and stored every write. +- `record.status > ['m']` compared the list as the string `'m'` on the write check, while the analytics read scope bound the whole list as one SQL parameter. `record.reviewer_id > current_user` compared the whole caller object as a string and admitted and stored every write; in this release the RLS compiler's comparand faces (#20212) already drop that policy, and this change refuses it at the lowering for every caller of the compiler. - `record.status != record.tags`, its negation `!(record.status == record.tags)`, and the mirror `record.tags != record.status`, with `tags` a `json` field or a `multiple` lookup, admitted and stored every write. What changes: