diff --git a/.changeset/19886-formula-array-comparand-refused.md b/.changeset/19886-formula-array-comparand-refused.md new file mode 100644 index 00000000000..c784dc472a8 --- /dev/null +++ b/.changeset/19886-formula-array-comparand-refused.md @@ -0,0 +1,24 @@ +--- +"@objectstack/formula": minor +"@objectstack/plugin-security": minor +"@objectstack/spec": patch +--- + +`matchesFilterCondition` refuses an array comparand under `$ne` and in the equality position (`{ field: [...] }`, `$eq: [...]`) with `INVALID_FILTER` / 400, before any record is judged (#19886). + +**BREAKING** — an accept-set narrowing, shipped by `@objectstack/formula` and `@objectstack/plugin-security` as `minor` under the repo's launch-window convention for accept-set narrowings. The hand-migration prescription is registered under protocol major 18 as `rls-predicate-array-comparand-refused`. + +Clause-②: no (narrowing) + +**Security fix for row-level write checks.** This evaluator is what `@objectstack/plugin-security` runs against the post-image of an insert or update to enforce a row-level `check`. It compared strictly, and no stored value ever equals an array, so: + +- a `check` written `record.status != ['closed', 'archived']`, or `!=` against a `current_user` membership array, lowered to `{ status: { $ne: [...] } }` and matched **every** post-image; +- a `check` written `!(record.status == ['closed', 'archived'])` lowered to `{ $not: { status: [...] } }` and did the same. + +Every write such a policy was written to refuse was admitted and stored. Both shapes now fail the write with `INVALID_FILTER` / 400, the envelope driver-sql and driver-memory already give the same shape on the read side. The explain engine's record attribution reads the same evaluator, so it refuses too, instead of reporting a row as admitted that the enforcing read refuses. The positive `record.status == ['open', 'pending']` already refused every write (403); it now refuses with the 400 instead. + +The message withholds the field, the operator and the value, because the filter is usually an access policy the caller did not write, and the comparand may be a resolved membership set. + +**What to change.** A `check` or `using` predicate that means "one of these values" or "none of these values" is spelled with `in`: `record.status in ['open', 'pending']`, or `!(record.status in ['closed', 'archived'])`. Those, scalar `!=` / `==`, `null`, `Date` comparands and `{ $field }` references evaluate exactly as before. + + diff --git a/packages/formula/src/matches-filter-array-comparand.test.ts b/packages/formula/src/matches-filter-array-comparand.test.ts new file mode 100644 index 00000000000..ad47d5dc6bb --- /dev/null +++ b/packages/formula/src/matches-filter-array-comparand.test.ts @@ -0,0 +1,161 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#19886] An ARRAY where one comparable value is expected — under `$ne`, or in + * the equality position (a bare-array field spec, or `$eq`) — is REFUSED by + * `matchesFilterCondition` with the `INVALID_FILTER` / 400 envelope driver-sql + * and driver-memory already raise for the same shape. + * + * # Why this face's old answer was a write-gate bypass + * + * This evaluator IS the enforcement of a row-level `check` (plugin-security + * matches it against the post-image; there is no query to push down to). It + * compares strictly, and no stored scalar is `===` an array: + * + * | `check` policy (the CEL it lowers from) | before | after | + * |---|---|---| + * | `{ s: { $ne: ['a','b'] } }` (`record.s != ['a','b']`) | `true` → every write **ALLOWED** | throws → 400 | + * | `{ $not: { s: ['a','b'] } }` (`!(record.s == ['a','b'])`) | `true` → every write **ALLOWED** | throws → 400 | + * | `{ s: ['a','b'] }` (`record.s == ['a','b']`) | `false` → every write DENIED | throws → 400 | + * + * 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. + */ + +import { describe, it, expect } from 'vitest'; +import { matchesFilterCondition } from './matches-filter'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +const RECORD = { id: '1', status: 'closed', owner: 'u1', tags: ['a', 'b'], other: 'x' }; + +const refusalOf = (filter: unknown, record: Record = RECORD): WireBearingError => { + try { + matchesFilterCondition(record, filter as never); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the evaluator to refuse this filter, but it answered'); +}; + +const LIST = ['closed', 'archived']; + +/** Each refused shape, spelled once per operator position. */ +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 } })], +]; + +/** Every depth the refusal must reach — and the ones where the old answer was absorbed or inverted. */ +const POSITIONS: Array<[string, (leaf: Record) => Record]> = [ + ['top level', (leaf) => leaf], + ['inside $and', (leaf) => ({ $and: [{ owner: 'u1' }, leaf] })], + ['inside $or, beside a satisfied branch', (leaf) => ({ $or: [{ owner: 'u1' }, leaf] })], + ['inside $not', (leaf) => ({ $not: leaf })], + ['nested two combinators deep', (leaf) => ({ $and: [{ $or: [{ $not: leaf }] }] })], + ['after a sibling that already fails for this record', (leaf) => ({ owner: 'nobody', ...leaf })], +]; + +describe('[#19886] matchesFilterCondition refuses an array comparand in a single-value position', () => { + for (const [shapeName, shape] of SHAPES) { + for (const [positionName, position] of POSITIONS) { + it(`${shapeName} — ${positionName}: INVALID_FILTER / 400`, () => { + const err = refusalOf(position(shape(LIST))); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + }); + } + + it(`${shapeName} — an EMPTY array is refused too`, () => { + const err = refusalOf(shape([])); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + }); + + it(`${shapeName} — refused for EVERY record, before any row is judged`, () => { + // An empty record, a record whose value is a member of the list, a record + // whose value is the very same array reference, and a record that would + // have failed an earlier sibling: the verdict may not depend on the row. + const records: Array> = [ + {}, + { status: 'closed' }, + { status: LIST }, + { status: 'open', owner: 'nobody' }, + ]; + for (const record of records) { + const err = refusalOf(shape(LIST), record); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + } + }); + } + + it('the remedy names the list operators by their FieldOperatorsSchema spelling', () => { + const err = refusalOf({ status: { $ne: LIST } }); + expect(err.message).toContain('"$in"'); + expect(err.message).toContain('"$nin"'); + }); + + it('the message withholds the field and the comparand — the filter may be a policy the caller did not write', () => { + const secretField = 'secret_scope_column'; + const secretIds = ['usr_member_one', 'usr_member_two']; + for (const filter of [ + { [secretField]: { $ne: secretIds } }, + { [secretField]: secretIds }, + { [secretField]: { $eq: secretIds } }, + ]) { + const err = refusalOf(filter, { [secretField]: 'x' }); + expect(err.message).not.toContain(secretField); + for (const id of secretIds) expect(err.message).not.toContain(id); + } + }); +}); + +describe('[#19886] every neighbouring shape answers exactly as before', () => { + const m = matchesFilterCondition; + + it('$in / $nin — the list operators — still evaluate their array', () => { + expect(m({ status: 'closed' }, { status: { $in: LIST } })).toBe(true); + expect(m({ status: 'open' }, { status: { $in: LIST } })).toBe(false); + expect(m({ status: 'closed' }, { status: { $nin: LIST } })).toBe(false); + expect(m({ status: 'open' }, { status: { $nin: LIST } })).toBe(true); + expect(m({ status: 'open' }, { $not: { status: { $in: LIST } } })).toBe(true); + expect(m({ status: 'closed' }, { $not: { status: { $in: LIST } } })).toBe(false); + }); + + it('scalar $ne / $eq / implicit equality are untouched', () => { + expect(m({ status: 'closed' }, { status: { $ne: 'closed' } })).toBe(false); + expect(m({ status: 'open' }, { status: { $ne: 'closed' } })).toBe(true); + expect(m({ status: 'closed' }, { status: { $eq: 'closed' } })).toBe(true); + expect(m({ status: 'closed' }, { status: 'closed' })).toBe(true); + expect(m({ status: 'open' }, { $not: { status: 'closed' } })).toBe(true); + }); + + it('null comparands are untouched', () => { + expect(m({ status: null }, { status: null })).toBe(true); + expect(m({ status: 'x' }, { status: { $ne: null } })).toBe(true); + expect(m({ status: null }, { status: { $eq: null } })).toBe(true); + }); + + it('a Date comparand is still a comparand, not an operator map', () => { + const d = new Date('2026-01-01T00:00:00.000Z'); + expect(m({ at: new Date(d.getTime()) }, { at: d })).toBe(true); + 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); + expect(m(RECORD, { other: { $ne: { $field: 'status' } } })).toBe(true); + expect(m(RECORD, { status: { $ne: { $field: 'status' } } })).toBe(false); + }); + + 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); + }); +}); diff --git a/packages/formula/src/matches-filter-empty-field-constraint.test.ts b/packages/formula/src/matches-filter-empty-field-constraint.test.ts index 77c01fbe8f8..9793274767a 100644 --- a/packages/formula/src/matches-filter-empty-field-constraint.test.ts +++ b/packages/formula/src/matches-filter-empty-field-constraint.test.ts @@ -109,9 +109,9 @@ describe('[#5240] matchesFilterCondition refuses a zero-operator field constrain expect(matchesFilterCondition(RECORD, { stage: { nested: 'won' } } as never)).toBe(false); }); - it('a bare array field spec', () => { - expect(matchesFilterCondition(RECORD, { stage: ['won'] } as never)).toBe(false); - }); + // [#19886] A bare array field spec is no longer in this group: it is + // REFUSED (INVALID_FILTER / 400), with `$eq` / `$ne` carrying an array — + // pinned in `matches-filter-array-comparand.test.ts`. it('an unknown top-level operator', () => { expect(matchesFilterCondition(RECORD, { $nope: 1 } as never)).toBe(false); diff --git a/packages/formula/src/matches-filter.test.ts b/packages/formula/src/matches-filter.test.ts index 65bc586fa74..3afbf26ffe5 100644 --- a/packages/formula/src/matches-filter.test.ts +++ b/packages/formula/src/matches-filter.test.ts @@ -125,8 +125,11 @@ describe('matchesFilterCondition — FAIL CLOSED', () => { 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('bare array value → REFUSED, not answered (#19886; the full pins are matches-filter-array-comparand.test.ts)', () => { + let err: { code?: string; status?: number } | undefined; + try { m(rec, { stage: ['won'] } as never); } catch (e) { err = e as { code?: string; status?: number }; } + expect(err?.code).toBe('INVALID_FILTER'); + expect(err?.status).toBe(400); }); it('malformed (array/scalar) filter → false', () => { expect(m(rec, [] as never)).toBe(false); diff --git a/packages/formula/src/matches-filter.ts b/packages/formula/src/matches-filter.ts index 087ba9fc88f..d6efb91bac0 100644 --- a/packages/formula/src/matches-filter.ts +++ b/packages/formula/src/matches-filter.ts @@ -63,6 +63,19 @@ * where such a constraint sat under an `$or` beside a satisfied branch, or under * a `$not`, the old `false` was ABSORBED and the write was allowed. Those writes * now fail. See {@link emptyFieldConstraintError}. + * + * [#19886] A second shape is refused the same way: an ARRAY where a single + * comparable value is expected — under `$ne`, or in the equality position + * (`{ field: [...] }`, `{ field: { $eq: [...] } }`). It is the one position + * where this face's answer was not merely silent but the WRONG way round for a + * write gate: a strict comparison never equals an array, so `$ne` matched EVERY + * record, and a negated equality (`$not` around `{ field: [...] }`, which is + * what `!(record.f == [...])` lowers to) did too. On the ADR-0058 D4 `check` + * that admitted every write the policy was written to refuse, measured through + * the real `plugin-security` on three drivers. The query faces the read side + * runs on — driver-sql's unbindable-comparand refusal and driver-memory's + * array-comparand refusal — already refuse the shape with `INVALID_FILTER` / + * 400; this face now gives the same envelope. See {@link arrayComparandError}. */ import type { FilterCondition } from '@objectstack/spec/data'; @@ -118,6 +131,83 @@ function emptyFieldConstraintError(field: string, path: string): Error { return err; } +/** + * [#19886] An ARRAY where one comparable value is expected — under `$ne`, or in + * the equality position (a bare-array field spec, or `$eq`) — is REFUSED, not + * evaluated, with the envelope the other faces already give the same shape + * (`INVALID_FILTER` / 400). + * + * # Why refused rather than answered + * + * The answer this face used to give was unsafe on the surface it exists for. + * `looseEq` is a strict comparison, and no stored scalar is ever `===` an array, + * so: + * + * | shape | old answer, every record | + * |:----------------------------------------|:-------------------------| + * | `{ f: { $ne: [...] } }` | `true` | + * | `{ f: [...] }` / `{ f: { $eq: [...] } }`| `false` | + * | `{ $not: { f: [...] } }` | `true` | + * + * An RLS `check` is authored as CEL, and `record.f != ['a', 'b']` (or `!=` + * against a `current_user` membership array) lowers to the first row, so every + * write the policy was written to refuse was admitted and stored. The positive + * equality row only failed closed by accident, and inverted the moment it was + * negated. There is no answer here that is right in every polarity — which is + * the #5240 argument for refusing rather than choosing. + * + * # What stays exactly as it was + * + * `$in` / `$nin` (the list operators — this is what they are FOR), scalars, + * `null`, `Date`, and `{ $field }` references. The refusal reads the AUTHORED + * comparand, never a resolved one: a `{ $field }` reference whose column + * happens to hold an array is untouched. + * + * # Why the message names nothing from the filter + * + * This face's callers evaluate access policies — the write gate's `check`, and + * the explain engine's record attribution — and the caller who receives the + * 400 is usually not the author of the predicate. The comparand may be a + * resolved membership set (other users' ids), which must not be echoed to them. + * So the field, the operator and the value are withheld, the posture + * driver-sql's withheld-diagnostic ruling took for the same reason, and the + * message carries the refusal's identity and the remedy only. + */ +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\'])").', + ) as Error & { code?: string; status?: number }; + err.code = StandardErrorCode.enum.INVALID_FILTER; + err.status = 400; + return err; +} + +/** + * [#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. + */ +const ARRAY_REFUSED_OPERATORS = ['$eq', '$ne'] as const; + +/** A plain object — an operator map rather than a comparand (`Date` is a comparand). */ +function isOperatorMap(spec: unknown): spec is Record { + if (spec === null || typeof spec !== 'object' || Array.isArray(spec) || spec instanceof Date) return false; + const proto = Object.getPrototypeOf(spec); + return proto === Object.prototype || proto === null; +} + /** 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; @@ -134,9 +224,11 @@ export function matchesFilterCondition(record: Record, filter: /** * [#5240] Walk the whole condition tree and refuse any zero-operator field - * constraint. Shapes this evaluator already answers fail-closed (a non-node - * `$and` element, an unknown `$`-operator, a bare array field spec) are left to - * it — this walk adds exactly one refusal and changes nothing else. + * constraint. [#19886] The same walk refuses an array comparand under `$ne` + * or in the equality position ({@link arrayComparandError}), at any depth under + * `$and` / `$or` / `$not`. Shapes this evaluator already answers fail-closed in + * every polarity (a non-node `$and` element, an unknown `$`-operator) are left + * to it — this walk adds those two refusals and changes nothing else. */ function assertFilterShape(node: unknown, path: string): void { if (node == null || typeof node !== 'object' || Array.isArray(node)) return; @@ -152,6 +244,14 @@ function assertFilterShape(node: unknown, path: string): void { } if (key.startsWith('$')) continue; 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. + if (isOperatorMap(val)) { + for (const op of ARRAY_REFUSED_OPERATORS) { + if (Array.isArray(val[op])) throw arrayComparandError(); + } + } } } @@ -195,6 +295,9 @@ function evalField(record: Record, field: string, spec: unknown // 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: [...] }`). + // [#19886] Refused up front by `assertFilterShape` on the public entry point, + // so this arm is a floor for a recursive call on a subtree, not this face's + // answer to the shape — the same standing as the `keys.length === 0` arm below. if (Array.isArray(spec)) return false; const ops = spec as Record; diff --git a/packages/plugins/plugin-security/src/rls-check-array-comparand-refusal.test.ts b/packages/plugins/plugin-security/src/rls-check-array-comparand-refusal.test.ts new file mode 100644 index 00000000000..be8b5685754 --- /dev/null +++ b/packages/plugins/plugin-security/src/rls-check-array-comparand-refusal.test.ts @@ -0,0 +1,128 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#19886] A row-level `check` that compares a field against a LIST with `!=` + * (or negates `==` against one) no longer admits the write it was written to + * refuse. + * + * Both CEL spellings lower to a shape the write gate's evaluator + * (`matchesFilterCondition`, `@objectstack/formula`) could not answer safely: + * + * `record.status != ['closed', 'archived']` → `{ status: { $ne: [...] } }` + * `!(record.status == ['closed', 'archived'])` → `{ $not: { status: [...] } }` + * + * It compared strictly, so both matched EVERY post-image and the forbidden + * insert was admitted and stored — measured through this plugin on driver-sql, + * driver-sqlite-wasm and driver-memory. The evaluator now refuses both shapes + * before any row is judged, with `INVALID_FILTER` / 400, and the refusal + * propagates out of the engine's post-hook check seam: the insert fails and + * nothing lands. The correct spelling (`!(record.status in [...])`) is what the + * refusal's remedy points at. + * + * One pin per spelling, through the real plugin and the real engine, asserting + * the envelope (never a bare `toThrow()`) and the ground truth off the table. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { SecurityPlugin } from './security-plugin.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 engines: ObjectQL[] = []; +afterEach(async () => { + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function bootWithCheck(check: string) { + const engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) as never, + true, + ); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.qa.rls-check-array-comparand-19886', + name: 'RLS check array comparand', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [{ + name: OBJ, + label: 'Ticket', + sharingModel: 'public_read_write', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + status: { name: 'status', type: 'text' }, + }, + }], + } as never); + await engine.syncSchemas(); + engines.push(engine); + + // The package-metadata door: the published schema admits the predicate. + const set = PermissionSetSchema.parse({ + name: 'qa_ticket_guard', + objects: { [OBJ]: { allowRead: true, allowCreate: true, allowEdit: true } }, + rowLevelSecurity: [{ name: 'no_closed_tickets', object: OBJ, operation: 'all', check }], + }); + 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' }; + const stored = async () => + ((await engine.find(OBJ, { context: SYS_CTX } as never)) as Array>).map((r) => r.id); + return { engine, caller, stored }; +} + +async function refusalOf(run: () => Promise): Promise<{ code?: string; status?: number; statusCode?: number }> { + try { + await run(); + } catch (e) { + return e as { code?: string; status?: number; statusCode?: number }; + } + throw new Error('expected the insert to be refused, but it was admitted'); +} + +describe('[#19886] a row-level check comparing against a list refuses the forbidden insert', () => { + for (const [spelling, check] of [ + ['`!=` against a list', "record.status != ['closed', 'archived']"], + ['a negated `==` against a list', "!(record.status == ['closed', 'archived'])"], + ] as const) { + it(`${spelling}: INVALID_FILTER / 400, and nothing is stored`, async () => { + const { engine, caller, stored } = await bootWithCheck(check); + + const err = await refusalOf(() => + engine.insert(OBJ, { id: 'ins_bad', status: 'closed' }, { context: caller } as never)); + + expect(err.code).toBe('INVALID_FILTER'); + expect(err.statusCode ?? err.status).toBe(400); + expect(await stored()).toEqual([]); + }); + } +}); diff --git a/packages/spec/src/migrations/entries/semantic/18.rls-predicate-array-comparand-refused.ts b/packages/spec/src/migrations/entries/semantic/18.rls-predicate-array-comparand-refused.ts new file mode 100644 index 00000000000..eca68292edd --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.rls-predicate-array-comparand-refused.ts @@ -0,0 +1,54 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// The row-level-security face of ruling A on #19886 (stage 2a): the formula +// evaluator plugin-security runs a policy's check against. Recorded as its own +// entry because the surface an author rewrites is the RLS predicate, a CEL +// string, not a filter object they wrote by hand. +export const entry: SemanticMigration = { + id: 'rls-predicate-array-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[].check (and .using, where the explain engine ' + + 'attributes a record) — a CEL predicate comparing a field with != or == against a list, ' + + 'a list literal or a current_user membership array, and the negation of such an ==. They ' + + 'lower to { field: { $ne: [...] } }, { field: [...] } and { $not: { field: [...] } }, which ' + + 'the @objectstack/formula evaluator matchesFilterCondition now refuses, together with ' + + '{ field: { $eq: [...] } }, at any depth under $and / $or / $not, the empty array included', + replacement: + 'the list operator the comparison was standing in for. "One of these values" is in: ' + + 'record.status in ["open", "pending"]. "None of these values" is the negated in: ' + + '!(record.status in ["closed", "archived"]). Scalar != and ==, null, Date comparands and ' + + '{ $field } references evaluate exactly as before', + reason: + 'Ruling A on #19886 refuses an array comparand under $ne, and the equality slot is ruling ' + + '乙 on #19757; stage 2a of #19886 lands both on the formula face, the evaluator ' + + 'plugin-security runs against the post-image of an insert or update to enforce a ' + + 'row-level check. It compared strictly, and no stored value ever equals an array, so a ' + + 'check written record.status != ["closed", "archived"], or != against a current_user ' + + 'membership array, matched EVERY post-image, and a check written ' + + '!(record.status == ["closed", "archived"]) did the same: every write such a policy was ' + + 'written to refuse was admitted and stored. The positive record.status == ["open", ' + + '"pending"] refused every write (403). All of these shapes now fail the write with ' + + 'INVALID_FILTER / 400 before any record is judged, the envelope driver-sql and ' + + 'driver-memory already give the same shape on the read side, and the explain engine\'s ' + + 'record attribution refuses too. The message withholds the field, the operator and the ' + + 'value, because the filter is usually an access policy the caller did not write and the ' + + 'comparand may be a resolved membership set. Metadata AT REST is not rewritten and this ' + + 'entry adds no D2 conversion: the platform cannot tell which list operator a list ' + + 'comparison was standing in for, and a policy rewritten on the author\'s behalf would ' + + 'change which writes it admits (the negated forms would start refusing writes they ' + + 'admitted, the positive form would start admitting writes it refused), which is the ' + + 'policy author\'s decision. ADR-0058 D4 / ADR-0087 / ADR-0112.', + acceptanceCriteria: + 'Grep the rowLevelSecurity check and using predicates of your permission sets for != or == ' + + 'whose right-hand side is a list literal or a current_user membership array, and for the ' + + 'negation of such an ==, then rewrite each with in or !(... in ...). A check that still ' + + 'carries the shape refuses every write it governs with INVALID_FILTER / 400, allowed ' + + 'values included, so one allowed write under each policy finds every such check left. ' + + 'Then re-check what each policy is supposed to refuse rather than assuming the writes it ' + + 'admitted before were right: before this change a != or a negated == against a list ' + + 'admitted every write.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 259122158e3..bf6f9b06e86 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -11404,6 +11404,56 @@ const step18: MigrationStep = { + 'of the ten keys ever reached it. No code imports `CrudEndpointPattern(Schema)` from ' + '`@objectstack/spec/api` (TS2305 after upgrade).', }, + // The row-level-security face of ruling A on #19886 (stage 2a): the formula + // evaluator plugin-security runs a policy's check against. Recorded as its own + // entry because the surface an author rewrites is the RLS predicate, a CEL + // string, not a filter object they wrote by hand. + { + id: 'rls-predicate-array-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[].check (and .using, where the explain engine ' + + 'attributes a record) — a CEL predicate comparing a field with != or == against a list, ' + + 'a list literal or a current_user membership array, and the negation of such an ==. They ' + + 'lower to { field: { $ne: [...] } }, { field: [...] } and { $not: { field: [...] } }, which ' + + 'the @objectstack/formula evaluator matchesFilterCondition now refuses, together with ' + + '{ field: { $eq: [...] } }, at any depth under $and / $or / $not, the empty array included', + replacement: + 'the list operator the comparison was standing in for. "One of these values" is in: ' + + 'record.status in ["open", "pending"]. "None of these values" is the negated in: ' + + '!(record.status in ["closed", "archived"]). Scalar != and ==, null, Date comparands and ' + + '{ $field } references evaluate exactly as before', + reason: + 'Ruling A on #19886 refuses an array comparand under $ne, and the equality slot is ruling ' + + '乙 on #19757; stage 2a of #19886 lands both on the formula face, the evaluator ' + + 'plugin-security runs against the post-image of an insert or update to enforce a ' + + 'row-level check. It compared strictly, and no stored value ever equals an array, so a ' + + 'check written record.status != ["closed", "archived"], or != against a current_user ' + + 'membership array, matched EVERY post-image, and a check written ' + + '!(record.status == ["closed", "archived"]) did the same: every write such a policy was ' + + 'written to refuse was admitted and stored. The positive record.status == ["open", ' + + '"pending"] refused every write (403). All of these shapes now fail the write with ' + + 'INVALID_FILTER / 400 before any record is judged, the envelope driver-sql and ' + + 'driver-memory already give the same shape on the read side, and the explain engine\'s ' + + 'record attribution refuses too. The message withholds the field, the operator and the ' + + 'value, because the filter is usually an access policy the caller did not write and the ' + + 'comparand may be a resolved membership set. Metadata AT REST is not rewritten and this ' + + 'entry adds no D2 conversion: the platform cannot tell which list operator a list ' + + 'comparison was standing in for, and a policy rewritten on the author\'s behalf would ' + + 'change which writes it admits (the negated forms would start refusing writes they ' + + 'admitted, the positive form would start admitting writes it refused), which is the ' + + 'policy author\'s decision. ADR-0058 D4 / ADR-0087 / ADR-0112.', + acceptanceCriteria: + 'Grep the rowLevelSecurity check and using predicates of your permission sets for != or == ' + + 'whose right-hand side is a list literal or a current_user membership array, and for the ' + + 'negation of such an ==, then rewrite each with in or !(... in ...). A check that still ' + + 'carries the shape refuses every write it governs with INVALID_FILTER / 400, allowed ' + + 'values included, so one allowed write under each policy finds every such check left. ' + + 'Then re-check what each policy is supposed to refuse rather than assuming the writes it ' + + 'admitted before were right: before this change a != or a negated == against a list ' + + 'admitted every write.', + }, { id: 'schedule-flow-acting-organization-required', surface: