diff --git a/.changeset/20355-rls-write-check-cross-class.md b/.changeset/20355-rls-write-check-cross-class.md new file mode 100644 index 00000000000..58ae899c58c --- /dev/null +++ b/.changeset/20355-rls-write-check-cross-class.md @@ -0,0 +1,63 @@ +--- +'@objectstack/plugin-security': minor +'@objectstack/formula': minor +'@objectstack/driver-sql': patch +'@objectstack/lint': patch +'@objectstack/spec': patch +--- + +fix(security)!: the RLS write check refuses a field-to-field comparison the read refuses — one comparison class, one answer per policy (#20355) + +Clause-②: yes (narrowing) + + + +**BREAKING** — an accept-set narrowing on the row-level write check, shipped as `minor` +under the launch-window convention (`check-changeset-no-major` refuses `major` until GA; +breaking-ness is carried by this banner and the ADR-0087 disposition above, not by the +level). The hand-migration prescription is registered under protocol major 18 as +`rls-predicate-cross-class-field-comparison-refused`, one ADR-0087 D3 entry for the whole +family: the authoring arm `os validate` gained in #20347 and this write-check arm. + +**What changed.** A row-level policy that compares two fields of no shared comparison +class — `record.status != record.amount` (text and a number), `record.status != +record.photo` (text and a file field), `record.status != record.is_open` (text and a +formula field), `record.status != record.meta` (text and a json field) — already had +every read it scopes refused with `INVALID_FILTER` / 400 on the SQL drivers, because +driver-sql compiles a column-to-column comparison only within one class. The write +check did not know the rule: it compared the two raw values in-process, so an insert +or update the policy's `check` judges (or its `using`, standing in as the check) was +admitted and stored whenever that comparison happened to hold. Measured through +plugin-security and ObjectQL on SQLite, sqlite-wasm and PostgreSQL. The write check +now refuses the comparison too, with the read's envelope, `INVALID_FILTER` / 400, for +every insert (single or array), by-id update and predicate update it judges, and +nothing is stored. The same-class comparisons it always compared are compared as +before. The 400 names no column of the policy; the server log names the policy and +both columns. A comparison against a json or `multiple` field is refused by its +declared type now, where it used to be judged by the value each record held. + +**`@objectstack/formula`.** `matchesFilterCondition(record, filter, options?)` takes an +optional third argument: `options.fields`, the object's declared columns (`type` and +`multiple` per field name). Given it, every `{ $field }` comparison between two +declared columns is judged by `crossFieldComparisonVerdict` from +`@objectstack/spec/data` before any record is read, and one the platform defines no +answer for throws `INVALID_FILTER` / 400. Without it the evaluator behaves exactly as +before. Two new exports go with it: `findCrossFieldClassRefusal(filter, fields)`, the +pure judgement, and `crossFieldClassRefusalCarriedBy(error)`, which reads the refused +comparison off the error for a server-side log. + +**`@objectstack/driver-sql`.** `crossFieldComparisonClass` reads the same export +(`crossFieldColumnVerdict`) instead of keeping its own copy of the classification, and +layers above it only its internal type aliases. Every read answers as before. + +**`@objectstack/lint`.** The `rls-predicate-unenforceable` finding for such a +comparison now states the write answer the runtime gives: the in-process write check +refuses it by the same classification and stores nothing. + +**If a policy of yours is refused.** The platform defines no comparison between those +two columns on any path, so the policy never protected a read either. Compare a field +only with a field of the same class — a number with a number, text with text, a +boolean with a boolean, a date with a date, a datetime with a datetime, a time of day +with a time of day — or, if the two columns do hold comparable values, correct the +declaration of the one declared with the wrong type. `os validate` names every such +comparison. diff --git a/packages/drivers/driver-sql/src/sql-driver-20347-cross-field-class-parity.test.ts b/packages/drivers/driver-sql/src/sql-driver-20347-cross-field-class-parity.test.ts deleted file mode 100644 index 04a10342e29..00000000000 --- a/packages/drivers/driver-sql/src/sql-driver-20347-cross-field-class-parity.test.ts +++ /dev/null @@ -1,128 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * [#20347] PARITY: this driver's cross-field comparison boundary answers - * exactly what `crossFieldComparisonVerdict` (`@objectstack/spec/data`) answers, - * on every pair of declared columns. - * - * The spec's classification was LIFTED from this driver's module-private - * `crossFieldComparisonClass` (the #5222 boundary) so the authoring door and - * the write check could read one definition. Lifting it made a second copy - * for as long as the driver keeps its own, so this file holds the two equal: - * one object declaring every `FieldType` member once (`f_`) plus every - * multi-capable member flagged `multiple: true` (`m_`), and every - * ordered pair of those columns compiled as `{ a: { $eq: { $field: b } } }`. - * - * The driver's verdict is read from what it DOES, never from its prose: the - * pair compiles and runs (admitted), or it is refused in the withheld - * `INVALID_FILTER` / 400 envelope the cross-field boundary raises (#7929 — - * `withheldFilterDiagnosticOf` answers non-null only for that family). Any - * other outcome fails the case: it means the pair never reached the class - * question, and a parity claim over it would be a claim about nothing. The - * fixture keeps the boundary's other refusals out by construction — every - * column is declared, no reference is dotted, and no tenant-isolation column - * is compared. - * - * Only `$eq` is driven: the class question is asked once per comparison, - * before the operator is read, for all six operators the boundary compiles - * (`sql-driver-cross-field-reference.test.ts` pins the operator matrix). - * - * The engine lane's rewire of this driver onto the spec export keeps this file - * green by construction; until then it is the proof the lift changed nothing. - */ - -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { SqlDriver, withheldFilterDiagnosticOf } from './index.js'; -import { - FieldType, - MULTI_CAPABLE_TYPES, - REFERENCE_VALUE_TYPES, - crossFieldComparisonVerdict, - type FilterCondition, -} from '@objectstack/spec/data'; - -const OBJ = 'cfc_parity_probe'; - -interface ProbeColumn { - name: string; - type: string; - multiple?: boolean; -} - -const columns: ProbeColumn[] = [ - ...FieldType.options.map((type) => ({ name: `f_${type}`, type })), - ...[...MULTI_CAPABLE_TYPES].map((type) => ({ name: `m_${type}`, type, multiple: true })), -]; - -/** A declaration the driver's DDL accepts for each probe column. */ -function declarationOf(c: ProbeColumn): Record { - const decl: Record = { name: c.name, type: c.type }; - if (c.multiple) decl.multiple = true; - if (REFERENCE_VALUE_TYPES.has(c.type)) decl.reference = OBJ; - if (c.type === 'formula') decl.expression = '1'; - return decl; -} - -type Observed = 'admitted' | 'refused'; - -describe('[#20347] driver-sql cross-field boundary ⇔ crossFieldComparisonVerdict, every declared pair', () => { - let driver: SqlDriver; - - beforeAll(async () => { - driver = new SqlDriver({ - client: 'better-sqlite3', - connection: { filename: ':memory:' }, - useNullAsDefault: true, - }); - await driver.initObjects([ - { - name: OBJ, - fields: Object.fromEntries([ - ['id', { name: 'id', type: 'text' }], - ...columns.map((c) => [c.name, declarationOf(c)] as const), - ]), - } as never, - ]); - }); - - afterAll(async () => { - await driver.disconnect(); - }); - - async function observe(target: string, ref: string): Promise { - const where = { [target]: { $eq: { $field: ref } } } as FilterCondition; - try { - await driver.find(OBJ, { fields: ['id'], where }); - return 'admitted'; - } catch (e) { - const err = e as { code?: unknown; status?: unknown }; - // The ADR-0112 envelope AND the cross-field boundary's own withheld form: - // anything else never reached the class question. - expect({ code: err.code, status: err.status }, `${target} vs ${ref}: ${String(e)}`) - .toEqual({ code: 'INVALID_FILTER', status: 400 }); - expect(withheldFilterDiagnosticOf(e), `${target} vs ${ref}: not the cross-field boundary's refusal`) - .not.toBeNull(); - return 'refused'; - } - } - - it('the probe covers every FieldType member and every multi-capable member flagged multiple', () => { - expect(columns.filter((c) => !c.multiple).map((c) => c.type).sort()).toEqual([...FieldType.options].sort()); - expect(columns.filter((c) => c.multiple).map((c) => c.type).sort()).toEqual([...MULTI_CAPABLE_TYPES].sort()); - }); - - for (const target of columns) { - it(`${target.name} against every declared column`, async () => { - const mismatches: string[] = []; - for (const ref of columns) { - const verdict = crossFieldComparisonVerdict(target, ref).verdict; - const expected: Observed = verdict === 'comparable' ? 'admitted' : 'refused'; - const observed = await observe(target.name, ref.name); - if (observed !== expected) { - mismatches.push(`${target.name} vs ${ref.name}: spec says ${verdict}, driver ${observed}`); - } - } - expect(mismatches).toEqual([]); - }); - } -}); diff --git a/packages/drivers/driver-sql/src/sql-driver-20355-cross-field-class-driver-aliases.test.ts b/packages/drivers/driver-sql/src/sql-driver-20355-cross-field-class-driver-aliases.test.ts new file mode 100644 index 00000000000..2c29cebd8f5 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-20355-cross-field-class-driver-aliases.test.ts @@ -0,0 +1,94 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20355] This driver's cross-field comparison class READS the spec's + * classification (`crossFieldColumnVerdict`, `@objectstack/spec/data`) for + * every declared `FieldType`, and layers above it only what the spec leaves to + * a driver: its internal aliases, which are not `FieldType` members and which + * the spec answers `undefined` for. + * + * The `FieldType` half needs no pin of its own any more — it IS the export, + * which `filter-cross-field-comparison-class.test.ts` pins in the spec. The + * #20347 parity test that held this driver's private copy equal to the export + * over every declared pair retired with the copy. What this file pins is the + * alias layer the rewire kept, read off the driver's own column sets: + * + * | declared `type` | class | why | + * |---|---|---| + * | `integer` / `int` / `float` | numeric | `NUMERIC_SCALAR_TYPES`' driver-internal aliases | + * | `object` / `array` | none — refused | `JSON_COLUMN_TYPES`' driver-internal aliases | + * + * (The absent-type `string` default is not pinned: `createColumn` refuses a + * field that declares no `type`, so no managed table carries one to compare.) + * + * Each cell is read from what the driver DOES — the comparison compiles and + * runs, or it is refused in the `INVALID_FILTER` / 400 envelope — so an alias + * the rewire reclassified shows up as an admitted refusal or a refused + * admission, never as prose. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { SqlDriver, withheldFilterDiagnosticOf } from './index.js'; +import type { FilterCondition } from '@objectstack/spec/data'; + +const OBJ = 'cfc_alias_probe'; + +const FIELDS: Record> = { + id: { name: 'id', type: 'text' }, + f_number: { name: 'f_number', type: 'number' }, + f_integer: { name: 'f_integer', type: 'integer' }, + f_int: { name: 'f_int', type: 'int' }, + f_float: { name: 'f_float', type: 'float' }, + f_text: { name: 'f_text', type: 'text' }, + f_object: { name: 'f_object', type: 'object' }, + f_array: { name: 'f_array', type: 'array' }, + f_json: { name: 'f_json', type: 'json' }, +}; + +type Observed = 'admitted' | 'refused'; + +const CELLS: ReadonlyArray<[target: string, ref: string, expected: Observed]> = [ + ['f_integer', 'f_number', 'admitted'], + ['f_int', 'f_float', 'admitted'], + ['f_float', 'f_number', 'admitted'], + ['f_integer', 'f_text', 'refused'], + ['f_object', 'f_text', 'refused'], + ['f_text', 'f_array', 'refused'], + ['f_object', 'f_object', 'refused'], + ['f_object', 'f_json', 'refused'], +]; + +describe('[#20355] driver-sql cross-field class — the driver-internal aliases above the spec classification', () => { + let driver: SqlDriver; + + beforeAll(async () => { + driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + await driver.initObjects([{ name: OBJ, fields: FIELDS } as never]); + }); + + afterAll(async () => { + await driver.disconnect(); + }); + + async function observe(target: string, ref: string): Promise { + const where = { [target]: { $eq: { $field: ref } } } as FilterCondition; + try { + await driver.find(OBJ, { fields: ['id'], where }); + return 'admitted'; + } catch (e) { + const err = e as { code?: unknown; status?: unknown }; + expect({ code: err.code, status: err.status }, `${target} vs ${ref}: ${String(e)}`) + .toEqual({ code: 'INVALID_FILTER', status: 400 }); + expect(withheldFilterDiagnosticOf(e), `${target} vs ${ref}: not the cross-field boundary's refusal`) + .toMatch(/stored as|no scalar stored column/); + return 'refused'; + } + } + + for (const [target, ref, expected] of CELLS) { + it(`${target} vs ${ref}: ${expected}, in both orders`, async () => { + expect(await observe(target, ref)).toBe(expected); + expect(await observe(ref, target)).toBe(expected); + }); + } +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index c5ff707b489..c28afb3c645 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -26,6 +26,12 @@ import { STRUCTURED_JSON_TYPES, FILE_REFERENCE_TYPES, MULTI_OPTION_TYPES, NUMERI // `os generate migration` reads the SAME table, in both of its formats — that // shared table IS the repair, so ⛔ never restate one of its numbers here. import { numericColumnFor } from '@objectstack/spec/data'; +// [#20355] The cross-field comparison class, defined once in the spec (#20347, +// lifted case for case from this driver's #5222 boundary). This driver READS it +// — `crossFieldComparisonClass` below delegates — so the read it compiles and +// the write check `@objectstack/formula` evaluates judge one comparison by one +// rule. +import { crossFieldColumnVerdict, type CrossFieldComparisonClass } from '@objectstack/spec/data'; // [#5659] The Filter Protocol's boolean identity reduction — `$and: []` is TRUE, // `$or: []` is FALSE, `{}` is a TRUE disjunct, `$not: {}` is FALSE. One // implementation for all four consumers, proven against the same @@ -2729,6 +2735,16 @@ const CROSS_FIELD_COMPARISON_OPERATORS: ReadonlySet = new Set([ * [#5222] The comparison class a declared field's stored column belongs to, or * `null` for a field no column-to-column comparison can be compiled against. * + * [#20355] The classification is the SPEC'S now — `crossFieldColumnVerdict` + * (`@objectstack/spec/data`), lifted case for case from this function by + * #20347 — and this function is its reader for the one thing the spec leaves + * to a driver: its internal aliases. The write check (`@objectstack/formula`'s + * `matchesFilterCondition`, handed the object's declared columns by the RLS + * write gate) and the authoring door (`@objectstack/lint`) read the same + * export, so a policy's comparison has one answer on the read, on the write + * and at `os validate`. The reasoning below is the classification's, kept + * here because this driver is where it was measured. + * * Cross-field comparison is only emitted between two columns of the SAME * class. One class = one storage shape on both sides of one row, which is what * makes the SQL answer provably the memory evaluator's answer (the cross-path @@ -2771,19 +2787,22 @@ const CROSS_FIELD_COMPARISON_OPERATORS: ReadonlySet = new Set([ */ function crossFieldComparisonClass( decl: Record, -): 'numeric' | 'text' | 'boolean' | 'date' | 'datetime' | 'time' | null { +): CrossFieldComparisonClass | null { const type = String((decl as { type?: unknown }).type || 'string'); - if (isMultiValuedColumn(type, decl)) return null; - if (type === 'formula') return null; - if (JSON_COLUMN_TYPES.has(type) || FILE_REFERENCE_TYPES.has(type)) return null; + // [#20355] A declared `FieldType` is the spec's to classify — the one + // classification the write check and the authoring door read too. Its + // `multiple` reading is `isMultiValueField`'s, the same predicate + // `isMultiValuedColumn` asks. + const verdict = crossFieldColumnVerdict({ type, multiple: (decl as { multiple?: unknown }).multiple === true }); + if (verdict !== undefined) return verdict.kind === 'class' ? verdict.class : null; + // A type outside `FieldType` is a driver-internal alias the spec does not + // judge (its module header: "a driver layers its own aliases above this + // table"). This driver's are read off its own column sets, never restated: + // `object` / `array` are JSON columns ({@link JSON_COLUMN_TYPES}), `integer` / + // `int` / `float` numeric ones ({@link NUMERIC_SCALAR_TYPES}), and everything + // else — the absent-type default `string` included — is stored as TEXT. + if (JSON_COLUMN_TYPES.has(type)) return null; if (NUMERIC_SCALAR_TYPES.has(type)) return 'numeric'; - if (type === 'boolean' || type === 'toggle') return 'boolean'; - if (type === 'date') return 'date'; - if (type === 'datetime') return 'datetime'; - if (type === 'time') return 'time'; - // Everything else `createColumn` stores as TEXT: string/text/textarea/html/ - // markdown/email/url/phone/password, select, lookup/user (row ids), - // autonumber, and the unknown-type default. return 'text'; } diff --git a/packages/formula/src/index.ts b/packages/formula/src/index.ts index 38034fafa0f..70ac210ea91 100644 --- a/packages/formula/src/index.ts +++ b/packages/formula/src/index.ts @@ -116,6 +116,11 @@ export { __resetPushdownLimitWarnings } from './cel-to-filter'; // conditions ARE the red/green line. export { isSupportedRlsExpression, sqlPredicateToCel } from './rls-predicate'; export { matchesFilterCondition } from './matches-filter'; +// #20355 — the write check's comparison-class rule: the spec's cross-field +// classification, judged over a filter against the object's declared columns, +// and the reader a caller uses to log the refused comparison server-side. +export { crossFieldClassRefusalCarriedBy, findCrossFieldClassRefusal } from './matches-filter'; +export type { CrossFieldClassRefusal, MatchesFilterOptions } from './matches-filter'; // #13594 — the function-EXISTENCE verdict, isolated from the rest of what // cel-js's `check()` has an opinion about. Published for the same reason as // `firstUndeclaredReference` above and under the same discipline: the answer to diff --git a/packages/formula/src/matches-filter-cross-field-class.test.ts b/packages/formula/src/matches-filter-cross-field-class.test.ts new file mode 100644 index 00000000000..e8710fdbc68 --- /dev/null +++ b/packages/formula/src/matches-filter-cross-field-class.test.ts @@ -0,0 +1,173 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20355] Given the object's declared columns, `matchesFilterCondition` + * refuses a `{ $field }` comparison between two columns that share no + * comparison class — the spec's `crossFieldComparisonVerdict`, the rule + * driver-sql's read applies to the same comparison — with `INVALID_FILTER` / + * 400, before any record is read. + * + * This evaluator IS the RLS write check (plugin-security matches a policy's + * `check`, or its `using` standing in, against the post-image). Measured + * through the real plugin-security on driver-sql before this rule, on SQLite + * and PostgreSQL: + * + * | policy | read it scopes | insert its `check` judges, before | now | + * |---|---|---|---| + * | `record.status != record.amount` | `INVALID_FILTER` / 400 | admitted, stored (`'open' !== 5`) | 400, nothing stored | + * | `record.status != record.photo` | `INVALID_FILTER` / 400 | admitted, stored | 400, nothing stored | + * | `record.status != record.is_open` | `INVALID_FILTER` / 400 | admitted, stored | 400, nothing stored | + * | `record.status != record.title` | admitted | admitted | admitted (control) | + * + * The expectations below are written from each column's LABEL in this file's + * own table (same class label ⇒ comparable), never from the verdict function + * under test, so the pin is a second statement of the rule rather than an + * echo of it. + */ + +import { describe, it, expect } from 'vitest'; +import { + crossFieldClassRefusalCarriedBy, + findCrossFieldClassRefusal, + matchesFilterCondition, + type MatchesFilterOptions, +} from './matches-filter'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +/** + * One declared column per comparison class, and one per family with none. The + * label is the expectation: two columns are comparable iff both carry the same + * CLASS label (`none:*` never compares, not even with itself). + */ +const COLUMNS: ReadonlyArray<{ name: string; type: string; multiple?: boolean; label: string }> = [ + { name: 'status', type: 'text', label: 'text' }, + { name: 'title', type: 'textarea', label: 'text' }, + { name: 'stage', type: 'select', label: 'text' }, + { name: 'account', type: 'lookup', label: 'text' }, + { name: 'amount', type: 'number', label: 'numeric' }, + { name: 'budget', type: 'currency', label: 'numeric' }, + { name: 'is_won', type: 'boolean', label: 'boolean' }, + { name: 'close_date', type: 'date', label: 'date' }, + { name: 'closed_at', type: 'datetime', label: 'datetime' }, + { name: 'call_at', type: 'time', label: 'time' }, + { name: 'photo', type: 'image', label: 'none:file' }, + { name: 'is_open', type: 'formula', label: 'none:formula' }, + { name: 'meta', type: 'json', label: 'none:list-or-object' }, + { name: 'watchers', type: 'lookup', multiple: true, label: 'none:list-or-object' }, +]; + +const FIELDS: NonNullable = Object.fromEntries( + COLUMNS.map((c) => [c.name, c.multiple ? { type: c.type, multiple: true } : { type: c.type }]), +); +const OPTIONS: MatchesFilterOptions = { fields: FIELDS }; +const OPERATORS = ['$eq', '$ne', '$gt', '$gte', '$lt', '$lte'] as const; + +const comparable = (a: string, b: string): boolean => { + const la = COLUMNS.find((c) => c.name === a)!.label; + const lb = COLUMNS.find((c) => c.name === b)!.label; + return la === lb && !la.startsWith('none:'); +}; + +const refusalOf = (filter: unknown, record: Record = {}): WireBearingError | null => { + try { + matchesFilterCondition(record, filter as never, OPTIONS); + return null; + } catch (e) { + return e as WireBearingError; + } +}; + +describe('matchesFilterCondition — a field compared with a field of no shared comparison class, given the declared columns (#20355)', () => { + for (const target of COLUMNS) { + it(`${target.name} (${target.label}) against every declared column, all six operators`, () => { + const wrong: string[] = []; + for (const ref of COLUMNS) { + for (const op of OPERATORS) { + const err = refusalOf({ [target.name]: { [op]: { $field: ref.name } } }); + const refused = err !== null; + if (refused === comparable(target.name, ref.name)) { + wrong.push(`${target.name} ${op} ${ref.name}: ${refused ? 'refused' : 'admitted'}`); + } + if (err) { + expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_FILTER', status: 400 }); + } + } + } + expect(wrong).toEqual([]); + }); + } + + it('the card\'s cells: text vs number, text vs a file field, text vs a formula field are refused; text vs text is compared', () => { + for (const ref of ['amount', 'photo', 'is_open']) { + const err = refusalOf({ status: { $ne: { $field: ref } } }, { status: 'open', amount: 5, photo: 'f1' }); + expect({ code: err?.code, status: err?.status }).toEqual({ code: 'INVALID_FILTER', status: 400 }); + } + expect(matchesFilterCondition({ status: 'open', title: 'x' }, { status: { $ne: { $field: 'title' } } } as never, OPTIONS)).toBe(true); + expect(matchesFilterCondition({ status: 'x', title: 'x' }, { status: { $ne: { $field: 'title' } } } as never, OPTIONS)).toBe(false); + }); + + it('is refused for every record or for none — the record is never read', () => { + const filter = { status: { $ne: { $field: 'amount' } } }; + for (const record of [{}, { status: 'open', amount: 5 }, { status: 'open', amount: 'open' }, { status: null, amount: null }]) { + expect(refusalOf(filter, record)?.code).toBe('INVALID_FILTER'); + } + }); + + it('is found at any depth under $and / $or / $not, beside a satisfied branch', () => { + const record = { status: 'open', title: 'open', amount: 5 }; + for (const filter of [ + { $and: [{ status: { $eq: 'open' } }, { status: { $ne: { $field: 'amount' } } }] }, + { $or: [{ status: { $eq: 'open' } }, { status: { $ne: { $field: 'amount' } } }] }, + { $not: { amount: { $gt: { $field: 'status' } } } }, + { $or: [{ $and: [{ $not: { status: { $lte: { $field: 'closed_at' } } } }] }] }, + ]) { + expect(refusalOf(filter, record)?.code).toBe('INVALID_FILTER'); + } + }); + + it('judges an offset reference on its two columns, as driver-sql asks the class before it reads the offset', () => { + expect(refusalOf({ close_date: { $lte: { $field: 'closed_at', addDays: 3 } } })?.code).toBe('INVALID_FILTER'); + expect(refusalOf({ close_date: { $lte: { $field: 'close_date', addDays: 3 } } })).toBeNull(); + }); + + it('leaves a comparison it cannot judge from the declarations to the rules that own it', () => { + // An undeclared column, a dotted path, a type outside FieldType: not this rule's question. + expect(refusalOf({ status: { $ne: { $field: 'nope' } } })).toBeNull(); + expect(refusalOf({ nope: { $ne: { $field: 'amount' } } })).toBeNull(); + expect(refusalOf({ status: { $ne: { $field: 'account.amount' } } })).toBeNull(); + expect(matchesFilterCondition({ a: 'x', b: 1 }, { a: { $eq: { $field: 'b' } } } as never, { fields: { a: { type: 'string' }, b: { type: 'number' } } })).toBe(false); + // A literal comparand and the list operators are not field-to-field comparisons. + expect(refusalOf({ amount: { $gt: 5 }, status: { $in: ['open'] } }, { amount: 6, status: 'open' })).toBeNull(); + }); + + it('without the declared columns it judges values only, exactly as before', () => { + expect(matchesFilterCondition({ status: 'open', amount: 5 }, { status: { $ne: { $field: 'amount' } } } as never)).toBe(true); + expect(matchesFilterCondition({ status: 'open', amount: 5 }, { status: { $ne: { $field: 'amount' } } } as never, {})).toBe(true); + }); + + it('names nothing from the filter in the message, and carries the comparison for the server log', () => { + const err = refusalOf({ status: { $ne: { $field: 'photo' } } })!; + for (const name of ['status', 'photo', '$ne', 'image']) expect(err.message).not.toContain(`"${name}"`); + expect(err.message).not.toMatch(/\bstatus\b|\bphoto\b/); + expect(JSON.parse(JSON.stringify(err))).not.toHaveProperty('field'); + const carried = crossFieldClassRefusalCarriedBy(err); + expect(carried).toMatchObject({ field: 'status', operator: '$ne', reference: 'photo' }); + expect(carried?.verdict.verdict).toBe('no-class'); + expect(carried?.diagnostic).toContain('"photo" (type \'image\') is a file field'); + expect(crossFieldClassRefusalCarriedBy(new Error('x'))).toBeNull(); + }); + + it('findCrossFieldClassRefusal answers null for a filter whose comparisons all compare', () => { + expect(findCrossFieldClassRefusal({ $and: [{ status: { $eq: { $field: 'title' } } }, { amount: { $lt: { $field: 'budget' } } }] }, FIELDS)).toBeNull(); + expect(findCrossFieldClassRefusal({ amount: { $lt: { $field: 'status' } } }, FIELDS)).toMatchObject({ + field: 'amount', + operator: '$lt', + reference: 'status', + verdict: { verdict: 'cross-class', left: 'numeric', right: 'text' }, + }); + }); +}); diff --git a/packages/formula/src/matches-filter.ts b/packages/formula/src/matches-filter.ts index 6e061b4c8c8..8e920434e12 100644 --- a/packages/formula/src/matches-filter.ts +++ b/packages/formula/src/matches-filter.ts @@ -76,9 +76,26 @@ * 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}. + * + * [#20355] A third shape is refused when the caller hands over the object's + * declared columns ({@link MatchesFilterOptions.fields}): a `{ $field }` + * comparison between two columns that share no COMPARISON CLASS — text against + * a number, text against a file field, anything against a formula field. The + * rule is the spec's `crossFieldComparisonVerdict`, the same one driver-sql's + * read applies, so one access policy gets one answer on both sides of the + * write. See {@link findCrossFieldClassRefusal}. */ import type { FilterCondition } from '@objectstack/spec/data'; +// [#20355] The cross-field comparison class, defined once in the spec (#20347) +// and read by every judge of a column-to-column comparison: driver-sql's read, +// the authoring door in `@objectstack/lint`, and this evaluator's write check. +import { + crossFieldComparisonVerdict, + type CrossFieldColumnVerdict, + type CrossFieldComparisonFieldMeta, + type CrossFieldComparisonVerdict, +} from '@objectstack/spec/data'; // [#6520] `asciiCaseInsensitiveContains` is `$icontains`' fold, defined once in // the spec and shared by every JS evaluation face — so a `check` evaluated here // and the same predicate compiled to SQL by `read-scope-sql.ts` fold the same @@ -265,8 +282,31 @@ function isOperatorMap(spec: unknown): spec is Record { return proto === Object.prototype || proto === null; } +/** + * [#20355] What a caller that knows the record's object may tell the evaluator. + */ +export interface MatchesFilterOptions { + /** + * The declared columns of the object `record` belongs to, keyed by field + * name — each column's declared `type` and `multiple`, the slice the spec's + * cross-field classification reads. + * + * Supplied, every `{ $field }` comparison between two columns named here is + * judged by that classification before any record is read, and one the + * platform defines no answer for is refused ({@link findCrossFieldClassRefusal}). + * Omitted, the evaluator judges values only, as it always has: it has no + * schema of its own, and a caller without one (an aggregated row, a probe + * record) is not asked for one. + */ + readonly fields?: Readonly>; +} + /** True iff `record` satisfies `filter`. A null/empty filter matches everything. */ -export function matchesFilterCondition(record: Record, filter: FilterCondition | null | undefined): boolean { +export function matchesFilterCondition( + record: Record, + filter: FilterCondition | null | undefined, + options?: MatchesFilterOptions, +): boolean { if (filter == null) return true; if (typeof filter !== 'object' || Array.isArray(filter)) return false; // [#5240] Shape first, then evaluate. The refusal is raised by a walk of the @@ -276,9 +316,212 @@ export function matchesFilterCondition(record: Record, filter: // depending on the RECORD being tested. A malformed permission rule must be // refused for every record or none. Evaluation below is untouched. assertFilterShape(filter as Record, 'filter'); + // [#20355] The comparison-class rule is judged the same way, and for the same + // reason: it reads the declared columns, never the record, so a policy is + // refused for every record or for none. + if (options?.fields) { + const refusal = findCrossFieldClassRefusal(filter, options.fields); + if (refusal) throw crossFieldClassError(refusal); + } return evalNode(record, filter as Record); } +/** + * [#20355] One `{ $field }` comparison between two declared columns that share + * no comparison class, as {@link findCrossFieldClassRefusal} found it. + */ +export interface CrossFieldClassRefusal { + /** The constrained column — the key the comparison sits under. */ + readonly field: string; + /** The comparison operator (`$eq`, `$ne`, `$gt`, `$gte`, `$lt`, `$lte`). */ + readonly operator: string; + /** The referenced column — the `{ $field }` value. */ + readonly reference: string; + /** The classification's answer: two classes, or a column with none. */ + readonly verdict: Extract; + /** + * The comparison and both declarations in words. SERVER-SIDE ONLY: it names + * the columns of a predicate the caller may not have written, so it goes to + * a log, never into an error message (see {@link crossFieldClassError}). + */ + readonly diagnostic: string; +} + +/** + * [#20355] The first `{ $field }` comparison in `filter` whose two columns are + * both declared in `fields` and share no comparison class — `null` when there + * is none. Pure: it reads the filter and the declarations, never a record. + * + * ## The rule, and why it is the spec's + * + * A column-to-column comparison has one meaning only between two columns of + * ONE comparison class, and a file field, a formula field or a column holding + * a list or an object has no class at all (`@objectstack/spec/data` + * `crossFieldComparisonVerdict`, lifted from driver-sql's #5222 boundary by + * #20347). Across classes the backends answer differently: SQLite orders + * every TEXT above every INTEGER, while this evaluator's JS comparison coerces + * (`'open' > 5` is false), so a comparison that is well defined nowhere gets a + * different answer on each path. + * + * Measured through the real plugin-security and ObjectQL on driver-sql, on + * SQLite and on PostgreSQL, before this rule: an RLS policy + * `record.status != record.amount` (text vs number), `record.status != + * record.photo` (text vs image), `record.status != record.is_open` (text vs a + * formula field) or `record.status != record.meta` (text vs json) answered the + * read it scopes with `INVALID_FILTER` / 400, because driver-sql refuses to + * compile the comparison; and the insert its `check` judges — or its `using`, + * standing in as the check — was ADMITTED and stored, because this evaluator + * compared the two raw values (`'open' !== 5`). One policy, two answers, the + * permissive one on the write side. The write check now refuses the + * comparison exactly where the read does, by the same classification. + * + * ## What it judges, and what it leaves + * + * The six operators a `{ $field }` comparand is declared for + * ({@link ARRAY_REFUSED_OPERATORS}: the ones that compare ONE value), at any + * depth under `$and` / `$or` / `$not`, whether or not the reference carries an + * `addDays` offset — driver-sql asks the class question before it reads the + * offset. Only a comparison whose BOTH columns are keys of `fields` is judged: + * a dotted path, or a column the object does not declare, is a different + * question with its own answer elsewhere (the RLS compiler's field guard + * refuses an undeclared column before a filter ever reaches this evaluator). + * A declared type outside `FieldType` is not judged either + * (`unjudged`) — it is not a declaration the classification covers, and the + * metadata schema refuses it at authoring. + */ +export function findCrossFieldClassRefusal( + filter: FilterCondition | Record | null | undefined, + fields: Readonly>, +): CrossFieldClassRefusal | null { + const declared = (name: string): CrossFieldComparisonFieldMeta | undefined => + Object.prototype.hasOwnProperty.call(fields, name) ? fields[name] : undefined; + const walk = (node: unknown): CrossFieldClassRefusal | null => { + if (node == null || typeof node !== 'object' || Array.isArray(node)) return null; + for (const [key, val] of Object.entries(node as Record)) { + if (key === '$and' || key === '$or') { + if (!Array.isArray(val)) continue; + for (const child of val) { + const found = walk(child); + if (found) return found; + } + continue; + } + if (key === '$not') { + const found = walk(val); + if (found) return found; + continue; + } + if (key.startsWith('$') || !isOperatorMap(val)) continue; + const target = declared(key); + if (!target) continue; + for (const op of ARRAY_REFUSED_OPERATORS) { + const raw = val[op]; + if (!isFieldReference(raw) || typeof raw.$field !== 'string') continue; + const ref = declared(raw.$field); + if (!ref) continue; + const verdict = crossFieldComparisonVerdict(target, ref); + if (verdict.verdict !== 'cross-class' && verdict.verdict !== 'no-class') continue; + return { + field: key, + operator: op, + reference: raw.$field, + verdict, + diagnostic: describeCrossFieldClassRefusal(key, target, op, raw.$field, ref, verdict), + }; + } + } + return null; + }; + return walk(filter); +} + +/** How one declared column stands in a refused comparison, in words. */ +function describeColumn(name: string, meta: CrossFieldComparisonFieldMeta, verdict: CrossFieldColumnVerdict): string { + const declared = `"${name}" (type '${meta.type}'${meta.multiple === true ? ', multiple' : ''})`; + if (verdict.kind === 'class') return `${declared} is compared as ${verdict.class}`; + if (verdict.reason === 'file') return `${declared} is a file field, which has no comparison class`; + if (verdict.reason === 'formula') return `${declared} is a formula field, which has no stored column to compare`; + return `${declared} holds a list or an object, which has no comparison class`; +} + +function describeCrossFieldClassRefusal( + field: string, + target: CrossFieldComparisonFieldMeta, + op: string, + reference: string, + ref: CrossFieldComparisonFieldMeta, + verdict: CrossFieldClassRefusal['verdict'], +): string { + const parts = + verdict.verdict === 'cross-class' + ? [ + describeColumn(field, target, { kind: 'class', class: verdict.left }), + describeColumn(reference, ref, { kind: 'class', class: verdict.right }), + ] + : [ + ...(verdict.left.kind === 'no-class' ? [describeColumn(field, target, verdict.left)] : []), + ...(verdict.right.kind === 'no-class' && reference !== field + ? [describeColumn(reference, ref, verdict.right)] + : []), + ]; + return ( + `the comparison { "${field}": { "${op}": { "$field": "${reference}" } } } compares two columns that ` + + `share no comparison class: ${parts.join(', and ')}` + ); +} + +/** + * [#20355] The refusal carried on the error, under a SYMBOL key — the same + * non-travel reason driver-sql's withheld diagnostic uses one: `JSON.stringify`, + * a spread, `Object.keys` and the structured-clone boundary all skip it, so no + * error mapper can put the column names back on the wire. `Symbol.for` so a + * duplicated copy of this package resolves the same key. + */ +const CROSS_FIELD_CLASS_REFUSAL = Symbol.for('objectstack.formula.crossFieldClassRefusal'); + +/** + * [#20355] A `{ $field }` comparison between two columns of no shared + * comparison class is REFUSED, with the envelope the read gives the same + * comparison: `INVALID_FILTER` / 400. + * + * The message names nothing from the filter, for the reason + * {@link arrayComparandError} names nothing: on the write gate the filter is an + * access policy, and the caller who receives the 400 is usually not its author + * — driver-sql withholds the same comparison's columns on the read for the + * same reason (#7929). The columns, the operator and both declarations travel + * on the error for the server log ({@link crossFieldClassRefusalCarriedBy}). + */ +function crossFieldClassError(refusal: CrossFieldClassRefusal): Error { + const err = new Error( + 'A field-to-field comparison ({ "$field": … }) in this filter compares two columns that share no ' + + 'comparison class. Two columns are compared only within one class — a number with a number, text ' + + 'with text, a boolean with a boolean, a date with a date, a datetime with a datetime, a time of day ' + + 'with a time of day — and a file field, a formula field, or a column that holds a list or an object ' + + 'has no class at all, so the platform defines no answer for this comparison. It is refused rather ' + + 'than evaluated: across classes SQL and this evaluator answer differently, and the read path refuses ' + + 'the same comparison, so an answer here would give one access policy two meanings. The columns and ' + + 'the operator are withheld from this message because the filter may be an access policy the caller ' + + 'did not write; the server log names them. In a row-level policy, compare a field only with a field ' + + 'of the same class, or fix the declaration of the one that is declared with the wrong type.', + ) as Error & { code?: string; status?: number }; + err.code = StandardErrorCode.enum.INVALID_FILTER; + err.status = 400; + Object.defineProperty(err, CROSS_FIELD_CLASS_REFUSAL, { value: refusal, enumerable: false }); + return err; +} + +/** + * [#20355] The comparison-class refusal an error carries, or `null` for any + * other error — the read half of {@link crossFieldClassError}, for a caller that + * logs the refused comparison server-side (the RLS write gate names the policy + * beside it). + */ +export function crossFieldClassRefusalCarriedBy(err: unknown): CrossFieldClassRefusal | null { + if (err === null || (typeof err !== 'object' && typeof err !== 'function')) return null; + const refusal = (err as Record)[CROSS_FIELD_CLASS_REFUSAL]; + return refusal && typeof refusal === 'object' ? (refusal as CrossFieldClassRefusal) : null; +} + /** * [#5240] Walk the whole condition tree and refuse any zero-operator field * constraint. [#19886] The same walk refuses an array comparand under `$ne` diff --git a/packages/lint/src/validate-rls-predicate-enforceability.cross-class-field.test.ts b/packages/lint/src/validate-rls-predicate-enforceability.cross-class-field.test.ts index 8c34e04d5ad..8d80f1a3f70 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.cross-class-field.test.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.cross-class-field.test.ts @@ -160,7 +160,7 @@ describe('validateRlsPredicateEnforceability — a field compared with a field o expect(using.message).toContain('the same `using` is also the write check whenever no applicable policy'); const check = validateRlsPredicateEnforceability(stackWith({ operation: 'insert', check: 'record.status != record.amount' }))[0]; - expect(check.message).toContain('the write is admitted and stored whenever that comparison happens to hold'); + expect(check.message).toContain('every insert or update it judges is refused and nothing is stored'); expect(check.message).not.toContain('every read this policy scopes'); }); diff --git a/packages/lint/src/validate-rls-predicate-enforceability.ts b/packages/lint/src/validate-rls-predicate-enforceability.ts index 700c2d14685..bd1c7348ac9 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.ts @@ -239,16 +239,15 @@ * comparison class, and refuses the file family and formula fields outright) * and a by-id update or delete it scopes `PERMISSION_DENIED` / 403; and an * insert their `check` judges — or their `using`, standing in as the check — - * was ADMITTED and stored. The in-process write check has no class rule and - * compares the two raw values, so the write answer is whatever that comparison - * happens to give (`record.amount > record.status` was refused 403, because - * `5 > 'open'` is false in JS): the permissive answer sits on the write side of - * an access policy. One policy, three answers. + * was ADMITTED and stored. The in-process write check had no class rule then + * and compared the two raw values; since #20355 it refuses the comparison by + * the same classification (`INVALID_FILTER` / 400), so every insert or update + * it judges is refused and nothing is stored. One policy, three answers. * * This arm refuses the comparison where it is written, by the same rule the * read applies: {@link crossFieldComparisonVerdict} (`@objectstack/spec/data`), - * the classification lifted from driver-sql and held to it by a pairwise parity - * test there. Only `comparable` passes; `cross-class` and `no-class` (a file + * the classification lifted from driver-sql, which now delegates to it + * through `crossFieldColumnVerdict`. Only `comparable` passes; `cross-class` and `no-class` (a file * field, a formula field) are refused, and `unjudged` — a declared type outside * `FieldType` — is Zod's to reject, not this arm's. A comparison either side of * which holds a list or an object stays the arm above's, so no comparison is @@ -1225,23 +1224,21 @@ export const CROSS_CLASS_SENTENCE = * through the real plugin-security and ObjectQL on driver-sql (see this file's * header). * - * ⚠️ The WRITE half is the in-process write check's behaviour, which the engine - * lane moves onto the same classification: when it does, this sentence changes - * in the same change. + * The WRITE half is the in-process write check's behaviour, which reads the + * same classification since the engine lane moved it onto + * `crossFieldComparisonVerdict` (#20355): it refuses where the read refuses. */ function crossClassConsequence(clause: 'using' | 'check'): string { const write = - 'the in-process write check has no class rule of its own, so a single-record insert or by-id update it ' + - 'judges compares the two raw values instead, and the write is admitted and stored whenever that ' + - 'comparison happens to hold — an answer the read path refuses to give'; + 'the in-process write check refuses the comparison by the same classification (`INVALID_FILTER` / 400), ' + + 'so every insert or update it judges is refused and nothing is stored'; return clause === 'using' ? 'every read this policy scopes is refused on the SQL drivers (`INVALID_FILTER` / 400: driver-sql refuses ' + 'the comparison by the two columns\' declared types), and every by-id update or delete it scopes fails ' + 'closed (`PERMISSION_DENIED` / 403). On an `insert`, `update` or `all` policy the same `using` is also ' + 'the write check whenever no applicable policy for that operation declares a `check` (ADR-0058 D4), and ' + `there ${write}.` - : `${write[0].toUpperCase()}${write.slice(1)}. The policy reads as a write rule and is enforced by an ` + - 'accident of the two values.'; + : `${write[0].toUpperCase()}${write.slice(1)}. The policy reads as a write rule and admits no write at all.`; } /** The prescription both rules share, ahead of their per-surface alternatives. */ diff --git a/packages/plugins/plugin-security/src/rls-check-cross-class-field-refused.test.ts b/packages/plugins/plugin-security/src/rls-check-cross-class-field-refused.test.ts new file mode 100644 index 00000000000..5e8b02ddb9a --- /dev/null +++ b/packages/plugins/plugin-security/src/rls-check-cross-class-field-refused.test.ts @@ -0,0 +1,233 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20355] A row-level policy that compares two fields of no shared comparison + * class gets ONE answer on both sides of a write: the read it scopes and the + * write its `check` judges are both refused with `INVALID_FILTER` / 400, + * through the real plugin, the real engine and the SQL drivers. + * + * `record.status != record.amount` lowers to `{ status: { $ne: { $field: + * 'amount' } } }`, a legal shape. driver-sql refuses to compile it (a + * column-to-column comparison has one meaning only within one comparison + * class), so the read answered 400. The write check evaluated the same filter + * in-process against the post-image and compared the two raw values, so the + * write was ADMITTED and stored. Both sides now read the spec's + * `crossFieldComparisonVerdict`. Measured through this same stack on + * better-sqlite3, sqlite-wasm and PostgreSQL 16: + * + * | policy | read (`using`) | by-id update / delete (`using`) | insert, `using` as the check | `check` insert | `check` by-id update | + * |---|---|---|---|---|---| + * | `record.status != record.amount` (text vs number) | 400 | 403 | admitted, stored → **400** | admitted, stored → **400** | admitted → **400** | + * | `record.status != record.photo` (text vs image) | 400 | 403 | admitted, stored → **400** | admitted, stored → **400** | admitted → **400** | + * | `record.status != record.is_open` (text vs formula) | 400 | 403 | admitted, stored → **400** | admitted, stored → **400** | admitted → **400** | + * | `record.status != record.meta` (text vs json holding one value) | 400 | 403 | admitted, stored → **400** | admitted, stored → **400** | admitted → **400** | + * | `record.amount > record.status` (number vs text) | 400 | 403 | 403 (`5 > 'open'` is false) → **400** | 403 → **400** | 403 → **400** | + * | `record.status != record.title` (text vs text, the control) | rows | admitted | admitted | admitted | admitted | + * + * The read and the by-id update / delete columns did not move: the read is + * driver-sql's refusal, and a by-id update or delete it scopes fails closed at + * the pre-image gate (403) before any check runs. + * + * PostgreSQL runs when `OS_TEST_POSTGRES_URL` names a server (CI's live-dialect + * job does not run this package, so there it is skipped; it was measured + * locally on PostgreSQL 16). driver-memory's cells were measured out of tree — + * this package cannot declare `@objectstack/driver-memory` without a + * `driver-memory-census` disposition — and are reported on the PR: its write + * answers as here (the check is evaluated in-process, before any driver), and + * its read, which has no `{ $field }` arm, is unchanged. + */ + +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 SYS_CTX = { isSystem: true, userId: 'usr_system' }; +const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; +const PG_URL = process.env.OS_TEST_POSTGRES_URL; + +type Driver = { disconnect?: () => Promise }; +const DRIVERS: Array<[name: string, make: () => Driver, available: boolean]> = [ + ['driver-sql (better-sqlite3)', () => + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), true], + ['driver-sqlite-wasm', () => new SqliteWasmDriver({ filename: ':memory:' }), true], + ['driver-sql (PostgreSQL)', () => new SqlDriver({ client: 'pg', connection: PG_URL! }), !!PG_URL], +]; + +const booted: Array<{ engine: ObjectQL; driver: Driver; table: string }> = []; +afterEach(async () => { + while (booted.length) { + const { engine, driver, table } = booted.pop()!; + // A live server outlives the run: drop what this file created. + const knex = (driver as { knex?: { schema: { dropTableIfExists: (t: string) => Promise } } }).knex; + if (PG_URL && knex && driver instanceof SqlDriver) { + try { await knex.schema.dropTableIfExists(table); } catch { /* noop */ } + } + try { await engine.destroy(); } catch { /* noop */ } + } +}); + +let seq = 0; +async function boot(makeDriver: () => Driver, clause: 'using' | 'check', predicate: string) { + const OBJ = `qa_deal_20355_${process.pid}_${++seq}`; + const driver = makeDriver(); + const engine = new ObjectQL(); + engine.registerDriver(driver as never, true); + await engine.init(); + engine.registerApp({ + id: `com.objectstack.qa.rls-cross-class-20355-${seq}`, + name: 'RLS cross-class field comparison', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + { + name: OBJ, + label: 'Deal', + sharingModel: 'public_read_write', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + status: { name: 'status', type: 'text' }, + title: { name: 'title', type: 'text' }, + amount: { name: 'amount', type: 'number' }, + photo: { name: 'photo', type: 'image' }, + meta: { name: 'meta', type: 'json' }, + is_open: { + name: 'is_open', + type: 'formula', + expression: { dialect: 'cel', source: "record.status != 'closed'" }, + returnType: 'boolean', + }, + }, + }, + ], + } as never); + await engine.syncSchemas(); + booted.push({ engine, driver, table: OBJ }); + + const set = PermissionSetSchema.parse({ + name: 'qa_deal_guard', + objects: { [OBJ]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }, + rowLevelSecurity: [{ name: 'deal_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 logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + const ctx = { + logger, + 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) => ({ id: r.id, status: r.status, amount: r.amount })) + .sort((a, b) => String(a.id).localeCompare(String(b.id))); + const refusalLines = () => + logger.warn.mock.calls.map((c) => String(c[0])).filter((line) => line.includes('RLS check REFUSED')); + return { OBJ, engine, caller, stored, refusalLines }; +} + +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)); +const messageOf = (p: Promise): Promise => p.then(() => '', (e: unknown) => String((e as Error)?.message)); + +const ROW = { id: 'r1', status: 'open', title: 'x', amount: 5 }; +const NEW = { id: 'n1', status: 'open', title: 'x', amount: 5 }; + +interface Case { + id: string; + predicate: string; + /** The two columns the refusal must name in the server log, and never on the wire. */ + columns: [string, string]; +} + +const REFUSED: Case[] = [ + { id: 'X1 text vs number', predicate: 'record.status != record.amount', columns: ['status', 'amount'] }, + { id: 'X2 text vs image', predicate: 'record.status != record.photo', columns: ['status', 'photo'] }, + { id: 'X3 text vs formula', predicate: 'record.status != record.is_open', columns: ['status', 'is_open'] }, + { id: 'X4 text vs json', predicate: 'record.status != record.meta', columns: ['status', 'meta'] }, + { id: 'X5 number vs text', predicate: 'record.amount > record.status', columns: ['amount', 'status'] }, +]; + +for (const [driverName, makeDriver, available] of DRIVERS) { + describe.skipIf(!available)(`[#20355] ${driverName}: a policy comparing two fields of no shared class is refused on read and write`, () => { + for (const c of REFUSED) { + it(`${c.id} \`${c.predicate}\` — using: the read 400, by-id update and delete 403, the insert it stands in as the check for 400 with nothing stored`, async () => { + const w = await boot(makeDriver, 'using', c.predicate); + await w.engine.insert(w.OBJ, [ROW], { context: SYS_CTX } as never); + const before = await w.stored(); + + expect(await outcome(w.engine.find(w.OBJ, { context: w.caller } as never))).toEqual(INVALID); + expect(await outcome(w.engine.update(w.OBJ, { title: 'y' }, { where: { id: 'r1' }, context: w.caller } as never))).toEqual(DENIED); + expect(await outcome(w.engine.delete(w.OBJ, { where: { id: 'r1' }, context: w.caller } as never))).toEqual(DENIED); + expect(await outcome(w.engine.insert(w.OBJ, NEW, { context: w.caller } as never))).toEqual(INVALID); + expect(await w.stored()).toEqual(before); + }); + + it(`${c.id} \`${c.predicate}\` — check: the insert and the by-id update are refused 400, nothing stored or changed`, async () => { + const w = await boot(makeDriver, 'check', c.predicate); + await w.engine.insert(w.OBJ, [ROW], { context: SYS_CTX } as never); + const before = await w.stored(); + + expect(await outcome(w.engine.insert(w.OBJ, NEW, { context: w.caller } as never))).toEqual(INVALID); + expect(await outcome(w.engine.insert(w.OBJ, [{ ...NEW, id: 'n2' }], { context: w.caller } as never))).toEqual(INVALID); + expect(await outcome(w.engine.update(w.OBJ, { amount: 6 }, { where: { id: 'r1' }, context: w.caller } as never))).toEqual(INVALID); + expect(await w.stored()).toEqual(before); + }); + + it(`${c.id} \`${c.predicate}\` — the 400 names neither column; the server log names the policy and both`, async () => { + const w = await boot(makeDriver, 'check', c.predicate); + const message = await messageOf(w.engine.insert(w.OBJ, NEW, { context: w.caller } as never)); + expect(message).toMatch(/^A field-to-field comparison/); + for (const column of c.columns) expect(message).not.toContain(column); + + const lines = w.refusalLines(); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain("policy 'deal_guard'"); + for (const column of c.columns) expect(lines[0]).toContain(`"${column}"`); + }); + } + + it('the control `record.status != record.title` (text vs text) is compared on both sides: the read returns the row, the writes are admitted and stored', async () => { + const r = await boot(makeDriver, 'using', 'record.status != record.title'); + await r.engine.insert(r.OBJ, [ROW, { ...ROW, id: 'r2', title: 'open' }], { context: SYS_CTX } as never); + const rows = (await r.engine.find(r.OBJ, { context: r.caller } as never)) as Array<{ id: string }>; + expect(rows.map((x) => x.id)).toEqual(['r1']); + expect(await outcome(r.engine.insert(r.OBJ, NEW, { context: r.caller } as never))).toBe('admitted'); + + const w = await boot(makeDriver, 'check', 'record.status != record.title'); + await w.engine.insert(w.OBJ, [ROW], { context: SYS_CTX } as never); + expect(await outcome(w.engine.insert(w.OBJ, NEW, { context: w.caller } as never))).toBe('admitted'); + expect(await outcome(w.engine.update(w.OBJ, { amount: 6 }, { where: { id: 'r1' }, context: w.caller } as never))).toBe('admitted'); + expect(await outcome(w.engine.insert(w.OBJ, { ...NEW, id: 'n2', title: 'open' }, { context: w.caller } as never))).toEqual(DENIED); + expect((await w.stored()).map((x) => x.id)).toEqual(['n1', 'r1']); + expect(w.refusalLines()).toEqual([]); + }); + }); +} diff --git a/packages/plugins/plugin-security/src/rls-compiler.ts b/packages/plugins/plugin-security/src/rls-compiler.ts index 2438357788d..9fcb366d8b1 100644 --- a/packages/plugins/plugin-security/src/rls-compiler.ts +++ b/packages/plugins/plugin-security/src/rls-compiler.ts @@ -320,6 +320,29 @@ export const RLS_DENY_FILTER: Record = Object.freeze({ id: '__rls_deny__:00000000-0000-0000-0000-000000000000', }); +/** + * [#20355] Which policy each compiled policy filter came from — recorded by + * {@link RLSCompiler.compileFilter} as it keeps a policy's filter, read by + * {@link compiledPolicyNameOf}. + * + * The composed answer is either one policy's filter itself or `{ $or: [...] }` + * of them, and both are built fresh by every compile, so the identity of a + * member IS its policy. A WeakMap rather than a property on the filter: the + * filter is handed to drivers and evaluators that walk its own keys, and a + * record kept beside it can neither leak into a query nor outlive it. + */ +const POLICY_OF_COMPILED_FILTER = new WeakMap(); + +/** + * [#20355] The name of the policy whose compiled filter `node` is, or + * `undefined` for any other node (a composed `$or`, a sub-condition, the deny + * sentinel). Lets a caller that refuses a compiled filter name the policy it + * refused, which the composed filter no longer says. + */ +export function compiledPolicyNameOf(node: unknown): string | undefined { + return node !== null && typeof node === 'object' ? POLICY_OF_COMPILED_FILTER.get(node) : undefined; +} + /** * Is this field constraint an emptied membership, and what CONSTANT does it * evaluate to? `{ $in: [] }` is constant FALSE on every backend ("IN () @@ -624,6 +647,7 @@ export class RLSCompiler { const comparands = judgeCompiledComparands(outcome.filter); if (comparands.ok) { filters.push(comparands.filter); + POLICY_OF_COMPILED_FILTER.set(comparands.filter, (policy as { name?: string }).name ?? '(unnamed)'); } else { deniedBy.push({ policy, cause: { reason: 'refused-comparand', detail: comparands.detail } }); } diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index a57b30ca417..af0a956396d 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -72,7 +72,7 @@ import { bootstrapSystemCapabilities } from './bootstrap-system-capabilities.js' import { normalizeManagedByVocab } from './normalize-managed-by.js'; import { bootstrapDeclaredCapabilities } from './bootstrap-declared-capabilities.js'; import { readDeclaredCapabilityContext } from './declared-capability-context.js'; -import { RLSCompiler, RLS_DENY_FILTER, policyDeclaresClause } from './rls-compiler.js'; +import { RLSCompiler, RLS_DENY_FILTER, compiledPolicyNameOf, policyDeclaresClause } from './rls-compiler.js'; import { computeTenantLayer0Verdict, tenantLayer0FilterOf, @@ -119,7 +119,12 @@ import { type ISeedSettlementService, type SeedSettlementSnapshot, } from '@objectstack/spec/contracts'; -import { matchesFilterCondition } from '@objectstack/formula'; +import { + crossFieldClassRefusalCarriedBy, + findCrossFieldClassRefusal, + matchesFilterCondition, + type MatchesFilterOptions, +} from '@objectstack/formula'; import { FieldMasker } from './field-masker.js'; import { assertReadableQueryFields } from './predicate-guard.js'; import { @@ -3165,8 +3170,51 @@ export class SecurityPlugin implements Plugin { developerMessage, ); }; - const satisfiesCheck = (image: Record): boolean => - checkParts.every((f) => matchesFilterCondition(image as any, f as any)); + // [#20355] The object's declared columns go with every judgement, so + // the evaluator applies the spec's cross-field comparison class — the + // rule driver-sql's read applies to the same policy. A comparison + // between two columns of no shared class (text vs number, text vs a + // file field, anything vs a formula field) is refused `INVALID_FILTER` + // / 400 for every image, where it used to be answered by comparing + // the two raw values and the write admitted and stored. No map (a + // schema that cannot be loaded) judges values only, as before. + const checkFieldOptions = await this.writeCheckFieldOptions(opCtx.object); + const satisfiesCheck = (image: Record): boolean => { + try { + return checkParts.every((f) => matchesFilterCondition(image as any, f as any, checkFieldOptions)); + } catch (e) { + const refusal = crossFieldClassRefusalCarriedBy(e); + if (refusal && checkFieldOptions?.fields) { + // The caller's 400 names nothing from the policy; the operator + // reading this line is told which policy and which columns. + const fields = checkFieldOptions.fields; + const policies = checkParts.flatMap((f) => { + const members = compiledPolicyNameOf(f) === undefined && Array.isArray((f as { $or?: unknown }).$or) + ? ((f as { $or: unknown[] }).$or) + : [f]; + return members + .filter((m) => findCrossFieldClassRefusal(m as Record, fields) !== null) + .map((m) => compiledPolicyNameOf(m) ?? '(unnamed)'); + }); + ctx.logger.warn( + `[Security] RLS check REFUSED on ${opCtx.operation} '${opCtx.object}' (INVALID_FILTER): ` + + `policy ${[...new Set(policies)].map((p) => `'${p}'`).join(', ') || '(unattributed)'} — ` + + `${refusal.diagnostic}. Two columns are compared only within one comparison class; the ` + + `read this policy scopes is refused for the same reason.`, + { + operation: opCtx.operation, + object: opCtx.object, + policies: [...new Set(policies)], + field: refusal.field, + operator: refusal.operator, + reference: refusal.reference, + userId: opCtx.context?.userId ?? 'unknown', + }, + ); + } + throw e; + } + }; // [insert-check commit a016f08b8a] (the original card no longer resolves) // The judgement the engine runs: every image it hands over // must pass, and the first that fails refuses the whole write. The @@ -8725,6 +8773,38 @@ export class SecurityPlugin implements Plugin { return result; } + /** + * [#20355] The object's declared columns as the write check's evaluator reads + * them — each field's `type` and `multiple` — from the sources + * {@link loadObjectFieldNames} reads, in its order: ObjectQL's live + * SchemaRegistry first, then the metadata service. + * + * `undefined` when neither answers with a field map: the evaluator then + * judges values only, exactly as it did before it could judge declarations — + * a schema that cannot be loaded must not manufacture refusals (the field + * guard's rule in `rls-compiler.ts`, `RlsFieldGuard`). A field without a string `type` is + * left out, so it is never judged. + */ + private async writeCheckFieldOptions(object: string): Promise { + let obj: any; + try { + obj = typeof this.ql?.getSchema === 'function' ? this.ql.getSchema(object) : null; + if (!obj || !obj.fields) obj = await this.metadata?.get?.('object', object); + } catch { + return undefined; + } + if (!obj || !obj.fields || typeof obj.fields !== 'object') return undefined; + const fields: Record = {}; + const entries: Array<[string, any]> = Array.isArray(obj.fields) + ? (obj.fields as any[]).filter((f) => f?.name).map((f) => [String(f.name), f]) + : Object.entries(obj.fields as Record); + for (const [name, decl] of entries) { + if (!decl || typeof decl !== 'object' || typeof decl.type !== 'string') continue; + fields[name] = { type: decl.type, multiple: decl.multiple === true }; + } + return { fields }; + } + private async loadObjectFieldNames( metadata: any, objectName: string, diff --git a/packages/spec/src/data/filter-cross-field-comparison-class.test.ts b/packages/spec/src/data/filter-cross-field-comparison-class.test.ts index 175574b0a02..00d5acb797b 100644 --- a/packages/spec/src/data/filter-cross-field-comparison-class.test.ts +++ b/packages/spec/src/data/filter-cross-field-comparison-class.test.ts @@ -5,9 +5,10 @@ * the class table and the two verdicts. What this file keeps honest is that * every `FieldType` member is classified exactly once, by reference to the * existing value-class sets, and that the pairwise verdict is the rule the - * module header states. That driver-sql answers the same on every pair is - * pinned in driver-sql (`sql-driver-20347-cross-field-class-parity.test.ts`), - * which is the only place both can be run. + * module header states. driver-sql answers the same on every pair because it + * delegates to `crossFieldColumnVerdict`; the driver-internal aliases it keeps + * above this table are pinned in driver-sql + * (`sql-driver-20355-cross-field-class-driver-aliases.test.ts`). */ import { describe, it, expect } from 'vitest'; diff --git a/packages/spec/src/data/filter-cross-field-comparison-class.ts b/packages/spec/src/data/filter-cross-field-comparison-class.ts index c6006a234ec..ae1b66c4c9d 100644 --- a/packages/spec/src/data/filter-cross-field-comparison-class.ts +++ b/packages/spec/src/data/filter-cross-field-comparison-class.ts @@ -70,10 +70,10 @@ * author a message; admitting one that diverges costs a permission rule its * meaning. * - * `crossFieldComparisonClass` in `@objectstack/driver-sql` is held to this - * table by a pairwise parity test over every declared field type - * (`sql-driver-20347-cross-field-class-parity.test.ts`), so the two cannot - * disagree on a single pair while both exist. + * `crossFieldComparisonClass` in `@objectstack/driver-sql` delegates to + * {@link crossFieldColumnVerdict} for every declared field type, so the two + * cannot disagree on a single pair; the driver-internal aliases it keeps above + * this table are pinned by `sql-driver-20355-cross-field-class-driver-aliases.test.ts`. * * ## Declared types only * diff --git a/packages/spec/src/migrations/entries/semantic/18.rls-predicate-cross-class-field-comparison-refused.ts b/packages/spec/src/migrations/entries/semantic/18.rls-predicate-cross-class-field-comparison-refused.ts new file mode 100644 index 00000000000..1b75f4b0110 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.rls-predicate-cross-class-field-comparison-refused.ts @@ -0,0 +1,75 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// The cross-field comparison-class family, both arms in one entry: #20347 +// refuses the comparison where it is authored (the lint rules behind +// os validate, and the permission save door), and #20355 refuses it where the +// row-level write check evaluates it. One classification decides both — +// crossFieldComparisonVerdict in @objectstack/spec/data, lifted from +// driver-sql's #5222 read boundary, which driver-sql now reads too. Recorded as +// its own entry because the surface an author rewrites is a CEL predicate string. +export const entry: SemanticMigration = { + id: 'rls-predicate-cross-class-field-comparison-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 comparing a field with another field (==, !=, >, >=, <, <=) where the two ' + + 'declared columns share no comparison class: text against a number, a date against a ' + + 'datetime, a boolean against text, and any column against a file field (file, image, avatar, ' + + 'video, audio) or a formula field. In a filter passed to matchesFilterCondition together with ' + + 'the object\'s declared columns (options.fields), a { $field } comparison under $eq, $ne, $gt, ' + + '$gte, $lt or $lte between two such columns, and between a column and a json or multiple ' + + 'field', + replacement: + 'a comparison between two columns of one comparison class: a number with a number (number, ' + + 'currency, percent, rating, slider, progress, summary), text with text (the string types, ' + + 'autonumber, a single select or radio, a single lookup or user, a master_detail or a tree), a ' + + 'boolean with a boolean, a date with a date, a datetime with a datetime, a time of day with a ' + + 'time of day. ' + + 'A file field and a formula field cannot be compared with another column at all: compare the ' + + 'field with a literal or test it for null. If the two columns do hold comparable values, one ' + + 'of them is declared with the wrong type, so correct that declaration rather than the ' + + 'predicate. Comparisons between two columns of one class lower and evaluate exactly as before', + reason: + 'A column-to-column comparison has one meaning only within one comparison class: across ' + + 'classes SQLite orders every TEXT above every INTEGER while the in-process evaluator coerces ' + + '("open" > 5 is false). A formula field is virtual, with no stored column to reference. The ' + + 'file family is refused by name, whatever the deployment stores: during the ADR-0104 ' + + 'dual-encoding window one media column can hold a bare id and another the JSON-quoted form of ' + + 'the same id, so no comparison against the family is provably one answer on every path. ' + + 'driver-sql has refused such a comparison on the read since #5222, so a policy written ' + + 'record.status != record.amount (text and a number), record.status != record.photo (text and ' + + 'an image) or record.status != record.is_open (text and a formula field) got three answers, ' + + 'measured through the real plugin-security on driver-sql, on SQLite and PostgreSQL: ' + + 'os validate called it valid, every read it scoped answered INVALID_FILTER / 400 and every ' + + 'by-id update or delete it scoped 403, and an insert or update its check judged, or its using ' + + 'standing in as the check, was admitted and stored, because the write check compared the two ' + + 'raw values. The classification is now exported once from @objectstack/spec/data ' + + '(crossFieldComparisonVerdict) and read by every judge. The authoring arm (#20347): the ' + + 'rls-predicate-unenforceable rule refuses the comparison in using and check, on every ' + + 'operation, at os validate, build and lint and at the metadata save door for a permission ' + + 'set, and the sharing-rule-unlowerable-condition rule refuses it in a sharing-rule condition ' + + 'at os validate, build and lint. The write-check arm (#20355): the row-level write gate hands ' + + 'matchesFilterCondition the object\'s declared columns, and a comparison the classification ' + + 'does not define is refused INVALID_FILTER / 400 for every insert and update the check judges, ' + + 'before any record is read, with nothing stored; the message withholds the columns and the ' + + 'server log names the policy and both. A comparison against a json or multiple field is now ' + + 'refused by its declared type on the write too, where #19886 judged it by the value each ' + + 'record held. driver-memory, a test driver with no field-reference arm, still reads such a ' + + 'comparison as a literal. Shipped producers were counted before the change: no shipped ' + + 'row-level policy or sharing-rule condition compares two fields of different classes. ' + + 'Metadata AT REST is not rewritten and this entry adds no D2 conversion: the platform cannot ' + + 'tell which comparison the author meant, and rewriting it on the author\'s behalf would change ' + + 'which rows and writes the policy admits, which is the policy author\'s decision. ' + + 'ADR-0058 D4 / ADR-0087 / ADR-0112.', + acceptanceCriteria: + 'Run os validate over your stack: it names every row-level or sharing-rule predicate that ' + + 'compares two fields of different comparison classes, with both declarations. Rewrite each as ' + + 'the replacement says. A policy that never passed os validate (stored before the authoring ' + + 'arm, or written by another path) is refused at request time instead: every read it scopes ' + + 'answers 400 on the SQL drivers, and so does every insert or update its check judges, so ' + + 're-check what each such policy is meant to admit rather than assuming the writes it admitted ' + + 'before were right.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 6095aa93471..26dccad9f86 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -14237,6 +14237,77 @@ const step18: MigrationStep = { + 'admitted before were right: before this change a != or a negated == against a list ' + 'admitted every write.', }, + // The cross-field comparison-class family, both arms in one entry: #20347 + // refuses the comparison where it is authored (the lint rules behind + // os validate, and the permission save door), and #20355 refuses it where the + // row-level write check evaluates it. One classification decides both — + // crossFieldComparisonVerdict in @objectstack/spec/data, lifted from + // driver-sql's #5222 read boundary, which driver-sql now reads too. Recorded as + // its own entry because the surface an author rewrites is a CEL predicate string. + { + id: 'rls-predicate-cross-class-field-comparison-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 comparing a field with another field (==, !=, >, >=, <, <=) where the two ' + + 'declared columns share no comparison class: text against a number, a date against a ' + + 'datetime, a boolean against text, and any column against a file field (file, image, avatar, ' + + 'video, audio) or a formula field. In a filter passed to matchesFilterCondition together with ' + + 'the object\'s declared columns (options.fields), a { $field } comparison under $eq, $ne, $gt, ' + + '$gte, $lt or $lte between two such columns, and between a column and a json or multiple ' + + 'field', + replacement: + 'a comparison between two columns of one comparison class: a number with a number (number, ' + + 'currency, percent, rating, slider, progress, summary), text with text (the string types, ' + + 'autonumber, a single select or radio, a single lookup or user, a master_detail or a tree), a ' + + 'boolean with a boolean, a date with a date, a datetime with a datetime, a time of day with a ' + + 'time of day. ' + + 'A file field and a formula field cannot be compared with another column at all: compare the ' + + 'field with a literal or test it for null. If the two columns do hold comparable values, one ' + + 'of them is declared with the wrong type, so correct that declaration rather than the ' + + 'predicate. Comparisons between two columns of one class lower and evaluate exactly as before', + reason: + 'A column-to-column comparison has one meaning only within one comparison class: across ' + + 'classes SQLite orders every TEXT above every INTEGER while the in-process evaluator coerces ' + + '("open" > 5 is false). A formula field is virtual, with no stored column to reference. The ' + + 'file family is refused by name, whatever the deployment stores: during the ADR-0104 ' + + 'dual-encoding window one media column can hold a bare id and another the JSON-quoted form of ' + + 'the same id, so no comparison against the family is provably one answer on every path. ' + + 'driver-sql has refused such a comparison on the read since #5222, so a policy written ' + + 'record.status != record.amount (text and a number), record.status != record.photo (text and ' + + 'an image) or record.status != record.is_open (text and a formula field) got three answers, ' + + 'measured through the real plugin-security on driver-sql, on SQLite and PostgreSQL: ' + + 'os validate called it valid, every read it scoped answered INVALID_FILTER / 400 and every ' + + 'by-id update or delete it scoped 403, and an insert or update its check judged, or its using ' + + 'standing in as the check, was admitted and stored, because the write check compared the two ' + + 'raw values. The classification is now exported once from @objectstack/spec/data ' + + '(crossFieldComparisonVerdict) and read by every judge. The authoring arm (#20347): the ' + + 'rls-predicate-unenforceable rule refuses the comparison in using and check, on every ' + + 'operation, at os validate, build and lint and at the metadata save door for a permission ' + + 'set, and the sharing-rule-unlowerable-condition rule refuses it in a sharing-rule condition ' + + 'at os validate, build and lint. The write-check arm (#20355): the row-level write gate hands ' + + 'matchesFilterCondition the object\'s declared columns, and a comparison the classification ' + + 'does not define is refused INVALID_FILTER / 400 for every insert and update the check judges, ' + + 'before any record is read, with nothing stored; the message withholds the columns and the ' + + 'server log names the policy and both. A comparison against a json or multiple field is now ' + + 'refused by its declared type on the write too, where #19886 judged it by the value each ' + + 'record held. driver-memory, a test driver with no field-reference arm, still reads such a ' + + 'comparison as a literal. Shipped producers were counted before the change: no shipped ' + + 'row-level policy or sharing-rule condition compares two fields of different classes. ' + + 'Metadata AT REST is not rewritten and this entry adds no D2 conversion: the platform cannot ' + + 'tell which comparison the author meant, and rewriting it on the author\'s behalf would change ' + + 'which rows and writes the policy admits, which is the policy author\'s decision. ' + + 'ADR-0058 D4 / ADR-0087 / ADR-0112.', + acceptanceCriteria: + 'Run os validate over your stack: it names every row-level or sharing-rule predicate that ' + + 'compares two fields of different comparison classes, with both declarations. Rewrite each as ' + + 'the replacement says. A policy that never passed os validate (stored before the authoring ' + + 'arm, or written by another path) is refused at request time instead: every read it scopes ' + + 'answers 400 on the SQL drivers, and so does every insert or update its check judges, so ' + + 're-check what each such policy is meant to admit rather than assuming the writes it admitted ' + + 'before were right.', + }, // 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