From 18e5a78b636f91fb71786be80c6c9cea1784092a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 18:18:40 +0000 Subject: [PATCH] fix(formula)!: refuse an ordering comparison whose stored operand holds a list or an object The write-check evaluator compared a list's JavaScript string form under $gt / $gte / $lt / $lte and $between, so a row-level check such as record.tags > 'a' admitted and stored a write whose json column held ['m']. It now refuses, per record, with the stage 2d envelope and sentence (INVALID_FILTER / 400), following driver-sql's declared refusal of an ordering comparison on a JSON-stored column. null, Date and equality are untouched. Adds the semantic migration entry and corrects the stage 2a entry's replacement sentence. Claude-Session: https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN Co-authored-by: Claude --- .../19886-stored-list-ordering-refused.md | 40 +++ .../matches-filter-array-comparand.test.ts | 130 ++++++++ packages/formula/src/matches-filter.ts | 53 +++- ...-stored-list-ordering-fails-closed.test.ts | 293 ++++++++++++++++++ ...8.rls-predicate-array-comparand-refused.ts | 4 +- ...-predicate-stored-list-ordering-refused.ts | 54 ++++ packages/spec/src/migrations/registry.ts | 54 +++- 7 files changed, 619 insertions(+), 9 deletions(-) create mode 100644 .changeset/19886-stored-list-ordering-refused.md create mode 100644 packages/plugins/plugin-security/src/rls-stored-list-ordering-fails-closed.test.ts create mode 100644 packages/spec/src/migrations/entries/semantic/18.rls-predicate-stored-list-ordering-refused.ts diff --git a/.changeset/19886-stored-list-ordering-refused.md b/.changeset/19886-stored-list-ordering-refused.md new file mode 100644 index 00000000000..a06285eb232 --- /dev/null +++ b/.changeset/19886-stored-list-ordering-refused.md @@ -0,0 +1,40 @@ +--- +"@objectstack/formula": minor +"@objectstack/plugin-security": minor +"@objectstack/spec": patch +--- + +A row-level write check that orders a field against a bound (`>`, `>=`, `<`, `<=`) is refused with `INVALID_FILTER` / 400 when that field holds a list or an object on the record being written, instead of comparing the list's string form and admitting the write (#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-stored-list-ordering-refused`. + +Clause-②: no (narrowing) + +**Security fix for RLS write checks.** Measured through the real plugin-security on driver-sql and driver-memory: `record.tags > 'a'`, with `tags` a `json` field holding `['m']`, compared `'m' > 'a'` and admitted and stored the write. `record.meta < 'a'` with `meta` holding `{ a: 1 }` compared `'[object Object]' < 'a'` and did the same, and so did a `multiple` lookup. `using` stands in as the check for a policy that declares no `check`, so the same predicate in `using` was enforced the same way on writes. No shipped row-level or sharing-rule predicate orders a field at all. + +What changes: + +- `@objectstack/formula`: `matchesFilterCondition` refuses `$gt` / `$gte` / `$lt` / `$lte` and `$between` on a field whose value on the record is a list or a plain object, whatever the comparand, with the same `INVALID_FILTER` / 400 and the same message as the stage 2d refusals. The refusal is per record: a record whose `json` field holds one scalar is compared as before. `null` and `Date` values are unchanged, and so is every equality against a stored list (`$eq`, `$ne`, implicit equality, `$in`, `$nin`). `$between` is not produced by the CEL lowering, so it reaches this only through a filter passed to `matchesFilterCondition` directly. +- `@objectstack/plugin-security`: a check insert or by-id update whose post-image holds a list or an object in an ordered field is refused 400 and stores nothing. That includes a by-id update that edits another field of a row whose stored `json` column holds a list, because the post-image merges the stored row. +- `@objectstack/spec`: the migration registry carries the entry. The stage 2a entry `rls-predicate-array-comparand-refused` now ends "Scalar != and ==, null, Date comparands, and { $field } references between single-valued columns evaluate exactly as before", which is true since stage 2d. + +Three moves, named: + +1. **The write check now matches driver-sql's read.** driver-sql refuses every ordering comparison, and `$between`, on a column it stores as JSON text, by declared type (400, #7398). The in-process check now refuses the same predicate on the same row (400). +2. **driver-memory's read parts from the write check.** driver-memory, a test driver, compares a stored list element by element on a read and keeps returning those rows (`record.tags > 'a'` reads a row holding `['m']`), while the check now refuses writing it. This is declared on #15104, as for stage 2d's `{ $field }` half. +3. **A list written into a scalar field under an ordering check now answers 400.** `status: ['m']` into a `text` field under `record.status > 'a'`, or `amount: [500]` into a `number` field under `record.amount > 10`, was admitted, and driver-sql stored it as the text `'["m"]'` / `'[500]'`. It is now refused before anything is stored. + +**The explain answer.** `security/explain` evaluates the business RLS predicate in-process on the fetched record, so it now answers `INVALID_FILTER` / 400 where the record holds a list or an object under an ordering predicate (this stage). It already answered 400 for a `{ $field }` comparison against a list-holding column (stage 2d). For both, per operation: + +| explain operation | driver | the enforced operation answers | same as explain's 400? | +|---|---|---|---| +| `read` | driver-sql | 400 `INVALID_FILTER` (the driver's refusal) | yes | +| `update` | driver-memory | 400 `INVALID_FILTER` (the post-image check) | yes | +| `update` | driver-sql | 403 `PERMISSION_DENIED`: the pre-image gate fails closed on the driver's 400 | no — both deny | +| `read` | driver-memory | the rows its element-wise read admits | no — the test driver's read | + +Explain itself is unchanged. + +**What to change.** Order a single-valued column (`record.priority > 2`), or test membership in the list with `in` (`record.status in ['open', 'pending']`). A `json` or `multiple` field has no ordering. + + diff --git a/packages/formula/src/matches-filter-array-comparand.test.ts b/packages/formula/src/matches-filter-array-comparand.test.ts index f094daba896..26d5103f037 100644 --- a/packages/formula/src/matches-filter-array-comparand.test.ts +++ b/packages/formula/src/matches-filter-array-comparand.test.ts @@ -34,6 +34,15 @@ * 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. + * + * [#19886 stage 2e] The mirror of the first 2d row, with the list on the + * RECORD's side and a literal bound — also judged on the record: + * + * | shape | before | after | + * |---|---|---| + * | `{ tags: { $gt: 'a' } }`, `tags` holding `['m']` | `'m' > 'a'` → `true`, the write **ALLOWED** | throws → 400 | + * | `{ meta: { $lt: 'a' } }`, `meta` holding `{ a: 1 }` | `'[object Object]' < 'a'` → `true`, **ALLOWED** | throws → 400 | + * | `{ tags: { $between: ['a', 'z'] } }`, `['m']` | `true` | throws → 400 | */ import { describe, it, expect } from 'vitest'; @@ -235,3 +244,124 @@ describe('[#19886 stage 2d] a { $field } comparison whose column holds a list is } }); }); + +describe('[#19886 stage 2e] an ordering comparison whose STORED operand holds a list or an object is refused on that record', () => { + const m = matchesFilterCondition; + + /** What a `json` column or a `multiple` lookup holds on a post-image. */ + const STORED: Array<[string, unknown]> = [ + ['a one-element list', ['m']], + ['a multi-element list', ['a', 'z']], + ['an empty list', []], + ['a list of lists', [['m']]], + ['a plain object', { a: 1 }], + ['an empty object', {}], + ]; + + /** Every ordering operator, each with a bound the coerced string form used to satisfy or not. */ + const ORDERINGS: Array<[string, Record]> = [ + ['$gt', { $gt: 'a' }], + ['$gte', { $gte: 'a' }], + ['$lt', { $lt: 'z' }], + ['$lte', { $lte: 'z' }], + ['$between', { $between: ['a', 'z'] }], + ]; + + for (const [storedName, stored] of STORED) { + for (const [opName, spec] of ORDERINGS) { + it(`${opName} on a field holding ${storedName}: INVALID_FILTER / 400`, () => { + const err = refusalOf({ tags: spec }, { tags: stored }); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + }); + } + } + + it('the refusal reaches every depth the evaluation reaches, and a negation cannot turn it into an answer', () => { + const record = { owner: 'u1', tags: ['m'] }; + const leaf = { tags: { $gt: 'a' } }; + for (const filter of [ + { $and: [{ owner: 'u1' }, leaf] }, + { $or: [{ owner: 'nobody' }, leaf] }, + { $not: leaf }, + { $and: [{ $or: [{ $not: leaf }] }] }, + ]) { + const err = refusalOf(filter, record); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + } + }); + + it('a verdict already decided without the field does not reach it — judged per record, like the stage 2d refusal', () => { + // Unlike the shape refusals, which judge the authored filter before any + // record, this one judges a VALUE, so it fires where evaluation reads it. + // Where it is not read, the answer does not depend on it either way. + const record = { owner: 'u1', tags: ['m'] }; + expect(m(record, { $or: [{ owner: 'u1' }, { tags: { $gt: 'a' } }] })).toBe(true); + expect(m(record, { owner: 'nobody', tags: { $gt: 'a' } })).toBe(false); + }); + + it('whatever the comparand: a number, a Date, null or a { $field } reference', () => { + const record = { tags: ['m'], status: 'a' }; + for (const spec of [ + { $gt: 5 }, + { $lt: new Date('2026-01-01T00:00:00.000Z') }, + { $gte: null }, + { $lte: { $field: 'status' } }, + ]) { + const err = refusalOf({ tags: spec }, record); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + } + }); + + it('a list written into a scalar field is judged the same way (a text or number field under an ordering check)', () => { + for (const [record, filter] of [ + [{ status: ['m'] }, { status: { $gt: 'a' } }], + [{ amount: [500] }, { amount: { $gt: 10 } }], + ] as const) { + const err = refusalOf(filter, record); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + } + }); + + it('the SAME filter over a record holding one scalar is compared, not refused — the refusal is per record', () => { + expect(m({ tags: 'm' }, { tags: { $gt: 'a' } })).toBe(true); + expect(m({ tags: 'a' }, { tags: { $gt: 'a' } })).toBe(false); + expect(m({ tags: 'm' }, { tags: { $between: ['a', 'z'] } })).toBe(true); + expect(m({ tags: 5 }, { tags: { $lte: 10 } })).toBe(true); + }); + + it('null, a missing field and a Date are untouched: no value is false, and a Date is a value', () => { + for (const record of [{ tags: null }, {}]) { + expect(m(record, { tags: { $gt: 'a' } })).toBe(false); + expect(m(record, { tags: { $lt: 'z' } })).toBe(false); + expect(m(record, { tags: { $between: ['a', 'z'] } })).toBe(false); + } + const at = new Date('2026-06-01T00:00:00.000Z'); + expect(m({ at }, { at: { $gt: '2026-01-01T00:00:00.000Z' } })).toBe(true); + expect(m({ at }, { at: { $lt: '2026-01-01T00:00:00.000Z' } })).toBe(false); + expect(m({ at }, { at: { $between: ['2026-01-01T00:00:00.000Z', '2026-12-31T00:00:00.000Z'] } })).toBe(true); + }); + + it('equality against a stored list keeps the answer stage 2a pinned — only ordering moved', () => { + const record = { tags: ['m'] }; + expect(m(record, { tags: 'm' })).toBe(false); + expect(m(record, { tags: { $eq: 'm' } })).toBe(false); + expect(m(record, { tags: { $ne: 'm' } })).toBe(true); + expect(m(record, { tags: { $in: ['m'] } })).toBe(false); + expect(m(record, { tags: { $nin: ['m'] } })).toBe(true); + expect(m(record, { tags: { $exists: true } })).toBe(true); + expect(m(record, { tags: { $null: false } })).toBe(true); + }); + + it('the message withholds the field and the stored value', () => { + const err = refusalOf({ secret_scope_column: { $gt: 'a' } }, { + secret_scope_column: ['usr_member_one', 'usr_member_two'], + }); + for (const secret of ['secret_scope_column', '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 1fb6f28c22e..6e061b4c8c8 100644 --- a/packages/formula/src/matches-filter.ts +++ b/packages/formula/src/matches-filter.ts @@ -156,9 +156,12 @@ function emptyFieldConstraintError(field: string, path: string): Error { * # 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. + * `null`, `Date`, and `{ $field }` references between single-valued columns. + * The shape refusal reads the AUTHORED comparand, never a resolved one. The + * two record-side refusals that reuse this error are judged per record in + * {@link evalOp}: [stage 2d] a `{ $field }` column holding a list or an object + * ({@link assertComparableReference}), and [stage 2e] a list or an object stored + * under an ordering operator ({@link ORDERING_OPERATORS}). * * # Why the message names nothing from the filter * @@ -175,8 +178,9 @@ function arrayComparandError(): 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"), 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 ' + + '"$nin" list, or a column that holds a list or an object on either side of a { "$field" } ' + + 'comparison or on the compared side of an ordering operator or "$between". 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 ' + @@ -211,6 +215,39 @@ const ARRAY_REFUSED_OPERATORS = ['$eq', '$ne', '$gt', '$gte', '$lt', '$lte'] as */ const LIST_MEMBER_OPERATORS = ['$in', '$nin'] as const; +/** + * [#19886 stage 2e] The ordering operators, whose STORED operand is judged on + * the record: a list or a plain object there is refused with + * {@link arrayComparandError}, per record, whatever the comparand — the mirror + * of stage 2d's array-comparand refusal with the list on the record's side. + * + * `record.tags > 'a'` on a `json` column or a `multiple` lookup lowers to + * `{ tags: { $gt: 'a' } }`, a legal shape; the list arrives on the post-image. + * `order` then compared the list's JavaScript string form: `['m'] > 'a'` is + * `'m' > 'a'`, and `{ a: 1 } < 'a'` is `'[object Object]' < 'a'` — both `true`. + * Measured through the real plugin-security write check on driver-sql and + * driver-memory, such a `check` admitted and stored the list-holding write. + * `$between` is `>=` and `<=` over the same `order`, and coerced the same way. + * + * The norm it follows is the production read driver's: driver-sql refuses every + * ordering comparison, and `$between`, against a column it stores as JSON text, + * by DECLARED type (#7398, `JSON_COLUMN_INCOMPATIBLE_OPERATORS`), because such a + * comparison "can never mean what the caller wrote". This evaluator has no + * schema, so it judges the VALUE: a record whose json column holds one scalar is + * compared as before. `null` (no value; `order` is never reached) and `Date` (a + * comparand, not an object map) are untouched, and so is equality — a scalar + * `$eq` / `$ne` / implicit equality against a stored list keeps the answer stage + * 2a pinned. + * + * What this does NOT align: driver-memory's read, a frozen test driver, compares + * a stored list element by element and keeps returning those rows, so its write + * check and its read part here (declared on #15104, as for stage 2d's `$field` + * half). And a list written into a scalar column (`amount: [500]` into a + * `number`) under an ordering check is refused here too, where it was admitted + * and stored stringified. + */ +const ORDERING_OPERATORS = ['$gt', '$gte', '$lt', '$lte', '$between'] 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); @@ -347,6 +384,12 @@ function evalOp(actual: unknown, op: string, raw: unknown, record: Record 'a'` lowers to `{ tags: { $gt: 'a' } }` — a legal shape. The + * list arrives on the post-image, and the write-check evaluator used to compare + * its JavaScript string form: `['m'] > 'a'` is `'m' > 'a'`, and `{ a: 1 } < 'a'` + * is `'[object Object]' < 'a'`. Measured before the fix through this same stack + * on driver-sql and on driver-memory: + * + * | predicate | list on the post-image | check insert, before | now | + * |---|---|---|---| + * | `record.tags > 'a'` / `< 'z'` / `>= 'a'` / `<= 'z'` (`tags` json) | `['m']` | admitted, stored | 400 | + * | `record.tags > 'n'` | `['a', 'z']` | 403 (`'a,z' > 'n'` is false) | 400 | + * | `record.meta < 'a'` (`meta` json) | `{ a: 1 }` | admitted, stored | 400 | + * | `record.meta > 'a'` | `{ a: 1 }` | 403 | 400 | + * | `record.watchers > 'a'` / `< 'p2'` (`multiple` lookup) | `['p1']` | admitted, stored | 400 | + * | `record.status > 'a'` (`text`) | `['m']` | admitted, driver-sql stored the text `'["m"]'` | 400 | + * | `record.amount > 10` (`number`) | `[500]` | admitted, driver-sql stored `'[500]'` | 400 | + * + * The by-id update door answers the same: a change set carrying the list, and + * an edit of another field on a row whose stored json column holds one (the + * post-image merges the stored row). The `using` read is the norm this follows: + * driver-sql refuses an ordering comparison on a column it stores as JSON text, + * by declared type (`INVALID_FILTER` / 400), and did so before this change. + * + * Not pinned here, and why: driver-memory's read compares a stored list element + * by element and keeps returning those rows, so its write and read part (the + * test driver's half, declared on #15104). `@objectstack/driver-memory` cannot + * be declared by this package without a `driver-memory-census` ledger + * disposition, so its cells were measured out of tree and reported on the PR. + * + * The controls — the same predicate over a record holding one scalar, `null`, + * and a `Date`; and equality against a stored list (stage 2a) — answer exactly + * as before. 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 { 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-stored-list-ordering-19886', + name: 'RLS stored-list ordering', + 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' }, + amount: { name: 'amount', type: 'number' }, + due: { name: 'due', type: 'datetime' }, + tags: { name: 'tags', type: 'json' }, + meta: { name: 'meta', 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 }], + }); + let security: { explain: (req: unknown, ctx: unknown) => Promise } | undefined; + 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: (name: string, impl: unknown) => { + if (name === 'security') security = impl as typeof security; + }, + 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); + if (!security) throw new Error('SecurityPlugin did not register the security service'); + + 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>) + .sort((a, b) => String(a.id).localeCompare(String(b.id))); + return { engine, caller, stored, explain: security.explain }; +} + +type Envelope = { code: string; status: number }; +const DENIED: Envelope = { code: 'PERMISSION_DENIED', status: 403 }; +const INVALID: Envelope = { code: 'INVALID_FILTER', status: 400 }; + +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) }; +}; +const outcome = (p: Promise): Promise<'admitted' | Envelope> => + p.then(() => 'admitted' as const, (e: unknown) => envelopeOf(e)); + +interface Case { + id: string; + predicate: string; + /** The column the predicate orders. */ + field: string; + /** A post-image value that is a list or an object — refused. */ + list: unknown; + /** The same field holding one value (or none) — compared as before. */ + scalar: unknown; + scalarVerdict: 'admitted' | Envelope; + /** Whether the driver reads this column back as the list it was given (json / multiple). */ + readsBackAsList: boolean; +} + +const CASES: Case[] = [ + { id: 'O1', predicate: "record.tags > 'a'", field: 'tags', list: ['m'], scalar: 'm', scalarVerdict: 'admitted', readsBackAsList: true }, + { id: 'O2', predicate: "record.tags < 'z'", field: 'tags', list: ['m'], scalar: 'm', scalarVerdict: 'admitted', readsBackAsList: true }, + { id: 'O3', predicate: "record.tags >= 'a'", field: 'tags', list: ['m'], scalar: 'm', scalarVerdict: 'admitted', readsBackAsList: true }, + { id: 'O4', predicate: "record.tags <= 'z'", field: 'tags', list: ['m'], scalar: 'm', scalarVerdict: 'admitted', readsBackAsList: true }, + { id: 'O5', predicate: "record.tags > 'n'", field: 'tags', list: ['a', 'z'], scalar: 'm', scalarVerdict: DENIED, readsBackAsList: true }, + { id: 'O6', predicate: "record.meta < 'a'", field: 'meta', list: { a: 1 }, scalar: 'm', scalarVerdict: DENIED, readsBackAsList: true }, + { id: 'O7', predicate: "record.meta > 'a'", field: 'meta', list: { a: 1 }, scalar: 'm', scalarVerdict: 'admitted', readsBackAsList: true }, + { id: 'O8', predicate: "record.watchers > 'a'", field: 'watchers', list: ['p1'], scalar: null, scalarVerdict: DENIED, readsBackAsList: true }, + { id: 'O9', predicate: "record.watchers < 'p2'", field: 'watchers', list: ['p1'], scalar: null, scalarVerdict: DENIED, readsBackAsList: true }, + { id: 'C1', predicate: "record.status > 'a'", field: 'status', list: ['m'], scalar: 'm', scalarVerdict: 'admitted', readsBackAsList: false }, + { id: 'C2', predicate: 'record.amount > 10', field: 'amount', list: [500], scalar: 500, scalarVerdict: 'admitted', readsBackAsList: false }, +]; + +/** Stored by the system for the `using` reads. */ +const ROWS = [ + { id: 'r_list', status: 's', amount: 50, tags: ['m'], meta: null, watchers: ['p1'] }, + { id: 'r_obj', status: 's', amount: 50, tags: null, meta: { a: 1 }, watchers: null }, + { id: 'r_scalar', status: 's', amount: 5, tags: 'm', meta: 'm', watchers: null }, +]; + +/** The `using` read: the json-column cases are refused by the driver itself; the scalar columns compare. */ +const READ: Record = { + C1: ['r_list', 'r_obj', 'r_scalar'], + C2: ['r_list', 'r_obj'], +}; + +for (const [driverName, makeDriver] of DRIVERS) { + describe(`[#19886 stage 2e] ${driverName}: an ordering check over a field holding a list or an object fails closed`, () => { + for (const c of CASES) { + it(`${c.id} \`${c.predicate}\` — check insert: the list-holding write is refused 400 and nothing is stored; the scalar control ${c.scalarVerdict === 'admitted' ? 'is admitted' : 'is denied 403'}`, async () => { + const w1 = await boot(makeDriver, 'check', c.predicate); + expect(await outcome(w1.engine.insert(OBJ, { id: 'ins_list', status: 's', [c.field]: c.list }, { context: w1.caller } as never))) + .toEqual(INVALID); + expect(await w1.stored()).toEqual([]); + + const w2 = await boot(makeDriver, 'check', c.predicate); + expect(await outcome(w2.engine.insert(OBJ, { id: 'ins_scalar', status: 's', [c.field]: c.scalar }, { context: w2.caller } as never))) + .toEqual(c.scalarVerdict); + expect((await w2.stored()).map((r) => r.id)).toEqual(c.scalarVerdict === 'admitted' ? ['ins_scalar'] : []); + }); + + it(`${c.id} \`${c.predicate}\` — by-id update: a change set carrying the list is refused 400 and the row is unchanged`, async () => { + const w = await boot(makeDriver, 'check', c.predicate); + await w.engine.insert(OBJ, { id: 'u_base', status: 's', [c.field]: c.scalar }, { context: SYS_CTX } as never); + const before = await w.stored(); + expect(await outcome(w.engine.update(OBJ, { [c.field]: c.list }, { where: { id: 'u_base' }, context: w.caller } as never))) + .toEqual(INVALID); + expect(await w.stored()).toEqual(before); + }); + + if (c.readsBackAsList) { + it(`${c.id} \`${c.predicate}\` — by-id update of ANOTHER field on a row whose stored column holds the list: refused 400, row unchanged`, async () => { + const w = await boot(makeDriver, 'check', c.predicate); + await w.engine.insert(OBJ, { id: 'u_stored', status: 's', [c.field]: c.list }, { context: SYS_CTX } as never); + const before = await w.stored(); + expect(await outcome(w.engine.update(OBJ, { amount: 7 }, { where: { id: 'u_stored' }, context: w.caller } as never))) + .toEqual(INVALID); + expect(await w.stored()).toEqual(before); + }); + } + + const read = READ[c.id] ?? 'driver refuses'; + it(`${c.id} \`${c.predicate}\` — the using read ${read === 'driver refuses' ? 'is refused by the driver itself (400, json column)' : `returns ${JSON.stringify(read)}`}`, async () => { + const r = await boot(makeDriver, 'using', c.predicate); + await r.engine.insert(OBJ, ROWS, { context: SYS_CTX } as never); + const got = await r.engine.find(OBJ, { context: r.caller } as never).then( + (rows) => ({ rows: (rows as Array<{ id: string }>).map((x) => x.id).sort() }), + (e: unknown) => ({ error: envelopeOf(e) }), + ); + expect(got).toEqual(read === 'driver refuses' ? { error: INVALID } : { rows: read }); + expect((await r.stored()).map((x) => x.id)).toEqual(['r_list', 'r_obj', 'r_scalar']); + }); + } + }); + + describe(`[#19886 stage 2e] ${driverName}: the controls answer exactly as before`, () => { + it('null in the ordered field is no value — compared, denied 403, not refused', async () => { + const w = await boot(makeDriver, 'check', "record.tags > 'a'"); + expect(await outcome(w.engine.insert(OBJ, { id: 'ins_null', status: 's', tags: null }, { context: w.caller } as never))) + .toEqual(DENIED); + expect(await w.stored()).toEqual([]); + }); + + it('a Date in the ordered field is a value — compared against the bound as an instant', async () => { + const w1 = await boot(makeDriver, 'check', "record.due > '2026-01-01T00:00:00.000Z'"); + expect(await outcome(w1.engine.insert(OBJ, { id: 'ins_late', status: 's', due: new Date('2026-06-01T00:00:00.000Z') }, { context: w1.caller } as never))) + .toBe('admitted'); + const w2 = await boot(makeDriver, 'check', "record.due > '2026-01-01T00:00:00.000Z'"); + expect(await outcome(w2.engine.insert(OBJ, { id: 'ins_early', status: 's', due: new Date('2025-06-01T00:00:00.000Z') }, { context: w2.caller } as never))) + .toEqual(DENIED); + expect(await w2.stored()).toEqual([]); + }); + + it("C3 `record.tags == 'm'` — equality against a stored list is untouched (stage 2a): the list is denied 403, the scalar admitted", async () => { + const w1 = await boot(makeDriver, 'check', "record.tags == 'm'"); + expect(await outcome(w1.engine.insert(OBJ, { id: 'ins_list', status: 's', tags: ['m'] }, { context: w1.caller } as never))) + .toEqual(DENIED); + expect(await w1.stored()).toEqual([]); + const w2 = await boot(makeDriver, 'check', "record.tags == 'm'"); + expect(await outcome(w2.engine.insert(OBJ, { id: 'ins_scalar', status: 's', tags: 'm' }, { context: w2.caller } as never))) + .toBe('admitted'); + }); + }); + + describe(`[#19886 stage 2e] ${driverName}: security/explain answers 400 on a list-holding record, beside what enforcement answers`, () => { + // Explain evaluates the business predicate in-process on the fetched row, + // so both 400 paths reach it: this stage's stored list under an ordering + // operator, and stage 2d's `{ $field }` comparison against a list-holding + // column. On this driver family the enforced read refuses with the same + // envelope; the enforced by-id update is refused 403 by its pre-image gate, + // which fails closed on the driver's 400. Both deny. + for (const predicate of ["record.tags > 'a'", 'record.status != record.tags']) { + it(`\`${predicate}\`: explain read 400 = find 400; explain update 400, the by-id update 403`, async () => { + const r = await boot(makeDriver, 'using', predicate); + await r.engine.insert(OBJ, ROWS, { context: SYS_CTX } as never); + + expect(await outcome(r.explain({ object: OBJ, operation: 'read', recordId: 'r_list' }, { ...r.caller }))).toEqual(INVALID); + expect(await outcome(r.engine.find(OBJ, { where: { id: 'r_list' }, context: { ...r.caller } } as never))).toEqual(INVALID); + + expect(await outcome(r.explain({ object: OBJ, operation: 'update', recordId: 'r_list' }, { ...r.caller }))).toEqual(INVALID); + const before = await r.stored(); + expect(await outcome(r.engine.update(OBJ, { amount: 7 }, { where: { id: 'r_list' }, context: { ...r.caller } } as never))) + .toEqual(DENIED); + expect(await r.stored()).toEqual(before); + }); + } + }); +} 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 index 8023aa9edc5..fa3207a898b 100644 --- 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 @@ -20,8 +20,8 @@ export const entry: SemanticMigration = { 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', + + '!(record.status in ["closed", "archived"]). Scalar != and ==, null, Date comparands, and ' + + '{ $field } references between single-valued columns 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 ' diff --git a/packages/spec/src/migrations/entries/semantic/18.rls-predicate-stored-list-ordering-refused.ts b/packages/spec/src/migrations/entries/semantic/18.rls-predicate-stored-list-ordering-refused.ts new file mode 100644 index 00000000000..96fd4f706a7 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.rls-predicate-stored-list-ordering-refused.ts @@ -0,0 +1,54 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// Stage 2e of #19886: the mirror of stage 2d's ordering refusal, with the list on +// the RECORD's side. The shape is legal, a field ordered against one bound; what +// the write-check evaluator now refuses is the VALUE the record holds there, a +// list or an object, the way driver-sql's read already refuses the same +// comparison on a column it stores as JSON text. Recorded as its own entry +// because the surface an author rewrites is a CEL predicate string. +export const entry: SemanticMigration = { + id: 'rls-predicate-stored-list-ordering-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 it stands in as the check — ' + + 'a CEL predicate ordering a field against a bound (>, >=, <, <=) where the field holds a list ' + + 'or an object on the record being written, as a json column or a multiple lookup does, or as ' + + 'a list written into a text or number field does. In a filter passed to matchesFilterCondition, ' + + '$gt / $gte / $lt / $lte and $between on a field whose value on the record is a list or a plain ' + + 'object, whatever the comparand', + replacement: + 'a comparison that names one value. Order a single-valued column (record.priority > 2), or test ' + + 'membership in the list with in (record.status in ["open", "pending"]); a json or multiple ' + + 'field has no ordering. A record whose json column holds one scalar is compared exactly as ' + + 'before, and so are null and Date values, and every equality (==, !=, in) against a stored ' + + 'list', + reason: + 'Stage 2e of #19886, the mirror of stage 2d with the list on the record\'s side, measured ' + + 'through the real plugin-security on driver-sql and driver-memory. record.tags > "a", with ' + + 'tags a json column holding ["m"], lowered to { tags: { $gt: "a" } }, and the write-check ' + + 'evaluator compared the list\'s JavaScript string form ("m" > "a"), so the check admitted and ' + + 'stored the write; record.meta < "a" with meta holding { a: 1 } compared "[object Object]" and ' + + 'did the same, and so did a multiple lookup. driver-sql\'s read refuses every ordering ' + + 'comparison, and $between, on a column it stores as JSON text, by declared type (400), because ' + + 'such a comparison can never mean what the caller wrote; the in-process write check now ' + + 'follows it, per record: INVALID_FILTER / 400 and nothing stored, on an insert and on a by-id ' + + 'update, including one that edits another field of a row whose stored column holds a list. ' + + 'A list written into a text or number field under an ordering check, admitted before and ' + + 'stored as the text "[500]" by driver-sql, is refused the same way. driver-memory, a test ' + + 'driver, still compares a stored list element by element on a read, so there the write and ' + + 'the read part. Shipped producers were counted before the change: no shipped row-level or ' + + 'sharing-rule predicate orders a field at all. ' + + 'Metadata AT REST is not rewritten and this entry adds no D2 conversion: the platform cannot ' + + 'tell which comparison an ordering over a list was standing in for, and rewriting it on the ' + + 'author\'s behalf would change which writes it admits, which is the policy author\'s decision. ' + + 'ADR-0058 D4 / ADR-0087 / ADR-0112.', + acceptanceCriteria: + 'Grep the rowLevelSecurity check predicates of your permission sets, and the using predicates ' + + 'of policies that declare no check, for >, >=, < or <= whose field is a json field or a ' + + 'multiple lookup, and rewrite each as the replacement says. Then write a record through each ' + + 'such policy: a write whose compared field holds a list now answers 400 rather than being ' + + 'admitted by string comparison, so re-check what the policy is supposed to admit.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 59304d18f0f..e28ca07f6f1 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -12719,8 +12719,8 @@ const step18: MigrationStep = { 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', + + '!(record.status in ["closed", "archived"]). Scalar != and ==, null, Date comparands, and ' + + '{ $field } references between single-valued columns 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 ' @@ -12746,6 +12746,56 @@ const step18: MigrationStep = { + 'admitted before were right: before this change a != or a negated == against a list ' + 'admitted every write.', }, + // Stage 2e of #19886: the mirror of stage 2d's ordering refusal, with the list on + // the RECORD's side. The shape is legal, a field ordered against one bound; what + // the write-check evaluator now refuses is the VALUE the record holds there, a + // list or an object, the way driver-sql's read already refuses the same + // comparison on a column it stores as JSON text. Recorded as its own entry + // because the surface an author rewrites is a CEL predicate string. + { + id: 'rls-predicate-stored-list-ordering-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 it stands in as the check — ' + + 'a CEL predicate ordering a field against a bound (>, >=, <, <=) where the field holds a list ' + + 'or an object on the record being written, as a json column or a multiple lookup does, or as ' + + 'a list written into a text or number field does. In a filter passed to matchesFilterCondition, ' + + '$gt / $gte / $lt / $lte and $between on a field whose value on the record is a list or a plain ' + + 'object, whatever the comparand', + replacement: + 'a comparison that names one value. Order a single-valued column (record.priority > 2), or test ' + + 'membership in the list with in (record.status in ["open", "pending"]); a json or multiple ' + + 'field has no ordering. A record whose json column holds one scalar is compared exactly as ' + + 'before, and so are null and Date values, and every equality (==, !=, in) against a stored ' + + 'list', + reason: + 'Stage 2e of #19886, the mirror of stage 2d with the list on the record\'s side, measured ' + + 'through the real plugin-security on driver-sql and driver-memory. record.tags > "a", with ' + + 'tags a json column holding ["m"], lowered to { tags: { $gt: "a" } }, and the write-check ' + + 'evaluator compared the list\'s JavaScript string form ("m" > "a"), so the check admitted and ' + + 'stored the write; record.meta < "a" with meta holding { a: 1 } compared "[object Object]" and ' + + 'did the same, and so did a multiple lookup. driver-sql\'s read refuses every ordering ' + + 'comparison, and $between, on a column it stores as JSON text, by declared type (400), because ' + + 'such a comparison can never mean what the caller wrote; the in-process write check now ' + + 'follows it, per record: INVALID_FILTER / 400 and nothing stored, on an insert and on a by-id ' + + 'update, including one that edits another field of a row whose stored column holds a list. ' + + 'A list written into a text or number field under an ordering check, admitted before and ' + + 'stored as the text "[500]" by driver-sql, is refused the same way. driver-memory, a test ' + + 'driver, still compares a stored list element by element on a read, so there the write and ' + + 'the read part. Shipped producers were counted before the change: no shipped row-level or ' + + 'sharing-rule predicate orders a field at all. ' + + 'Metadata AT REST is not rewritten and this entry adds no D2 conversion: the platform cannot ' + + 'tell which comparison an ordering over a list was standing in for, and rewriting it on the ' + + 'author\'s behalf would change which writes it admits, which is the policy author\'s decision. ' + + 'ADR-0058 D4 / ADR-0087 / ADR-0112.', + acceptanceCriteria: + 'Grep the rowLevelSecurity check predicates of your permission sets, and the using predicates ' + + 'of policies that declare no check, for >, >=, < or <= whose field is a json field or a ' + + 'multiple lookup, and rewrite each as the replacement says. Then write a record through each ' + + 'such policy: a write whose compared field holds a list now answers 400 rather than being ' + + 'admitted by string comparison, so re-check what the policy is supposed to admit.', + }, // The saved-report stack and the `report` metadata kind shared a word and // nothing else; this retires the stack and leaves the kind untouched. {