From 4605cc785301d1ba39dafa026b323f3755c71a1a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 09:43:51 +0000 Subject: [PATCH 1/6] fix(security): the RLS write check refuses a cross-class field comparison the read refuses The write check's evaluator (formula matchesFilterCondition) now takes the object's declared columns and judges every { $field } comparison by the spec's crossFieldComparisonVerdict, refusing a non-comparable one with INVALID_FILTER / 400 before any record is read. plugin-security's write gate hands it the declared columns and logs the refused policy and columns server-side. driver-sql's crossFieldComparisonClass delegates to crossFieldColumnVerdict, layering only its own aliases. lint's check-clause consequence sentence states the new write answer. Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN Co-authored-by: Claude --- packages/drivers/driver-sql/src/sql-driver.ts | 41 ++- packages/formula/src/index.ts | 5 + packages/formula/src/matches-filter.ts | 245 +++++++++++++++++- ...e-enforceability.cross-class-field.test.ts | 2 +- .../validate-rls-predicate-enforceability.ts | 14 +- .../plugin-security/src/rls-compiler.ts | 24 ++ .../plugin-security/src/security-plugin.ts | 88 ++++++- 7 files changed, 394 insertions(+), 25 deletions(-) diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 48253517659..4760990eed5 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.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..1355721ca16 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.ts @@ -1225,23 +1225,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-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, From 9bb93b1b944daf331dd977a96cdbe77f262dd83f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 10:01:20 +0000 Subject: [PATCH 2/6] test(security): pin the write check's comparison-class refusal on read and write; retire the lift parity test formula: every declared class against every other, all six operators, record independence, nesting, the offset form, the withheld message and the carried diagnostic. plugin-security: through the real engine on better-sqlite3, sqlite-wasm and (opt-in) PostgreSQL, the read, by-id update/delete, the using- as-check insert, the check insert and by-id update, the named server log, and the same-class control. driver-sql: the alias layer crossFieldComparisonClass keeps above the spec classification. The #20347 parity test retires with the private copy it held equal to the export. Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN Co-authored-by: Claude --- ...ver-20347-cross-field-class-parity.test.ts | 128 ---------- ...5-cross-field-class-driver-aliases.test.ts | 94 +++++++ .../matches-filter-cross-field-class.test.ts | 173 +++++++++++++ ...ls-check-cross-class-field-refused.test.ts | 233 ++++++++++++++++++ 4 files changed, 500 insertions(+), 128 deletions(-) delete mode 100644 packages/drivers/driver-sql/src/sql-driver-20347-cross-field-class-parity.test.ts create mode 100644 packages/drivers/driver-sql/src/sql-driver-20355-cross-field-class-driver-aliases.test.ts create mode 100644 packages/formula/src/matches-filter-cross-field-class.test.ts create mode 100644 packages/plugins/plugin-security/src/rls-check-cross-class-field-refused.test.ts 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/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/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([]); + }); + }); +} From 0ee6f4c20494a46570c550c3412730fc049f999f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 10:25:22 +0000 Subject: [PATCH 3/6] chore(changeset): the RLS write check's comparison-class refusal Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN Co-authored-by: Claude --- .../20355-rls-write-check-cross-class.md | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 .changeset/20355-rls-write-check-cross-class.md 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..9e042440cbf --- /dev/null +++ b/.changeset/20355-rls-write-check-cross-class.md @@ -0,0 +1,60 @@ +--- +'@objectstack/plugin-security': minor +'@objectstack/formula': minor +'@objectstack/driver-sql': patch +'@objectstack/lint': 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-②: no (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). + +**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. From e72518ad7627262706569daa57ad9b25dcd934af Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 11:09:12 +0000 Subject: [PATCH 4/6] =?UTF-8?q?chore(changeset):=20declare=20Clause-?= =?UTF-8?q?=E2=91=A1=20yes=20(narrowing)=20=E2=80=94=20formula's=20public?= =?UTF-8?q?=20surface=20grows?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @objectstack/formula's root entry gains findCrossFieldClassRefusal, crossFieldClassRefusalCarriedBy, their two types and an optional third argument on matchesFilterCondition, so the widening question answers yes; the narrowing arm stays for the write check's new refusal. Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN Co-authored-by: Claude --- .changeset/20355-rls-write-check-cross-class.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/20355-rls-write-check-cross-class.md b/.changeset/20355-rls-write-check-cross-class.md index 9e042440cbf..b4b243aa0d9 100644 --- a/.changeset/20355-rls-write-check-cross-class.md +++ b/.changeset/20355-rls-write-check-cross-class.md @@ -7,7 +7,7 @@ fix(security)!: the RLS write check refuses a field-to-field comparison the read refuses — one comparison class, one answer per policy (#20355) -Clause-②: no (narrowing) +Clause-②: yes (narrowing) From 7b0c82e2bdbc284eb5c6fda2412c56d5966fe47b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 11:34:39 +0000 Subject: [PATCH 5/6] feat(spec): register the cross-class field-comparison refusal family under protocol major 18 One ADR-0087 D3 semantic entry, rls-predicate-cross-class-field-comparison-refused, names both arms: the authoring arm os validate gained and this write-check arm; registry.ts regenerated by gen:migration-registry. The changeset moves to `registered` and adds @objectstack/spec at patch. The two spec comments and the one lint header sentence this PR had made stale now say what holds. Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN Co-authored-by: Claude --- .../20355-rls-write-check-cross-class.md | 7 +- .../validate-rls-predicate-enforceability.ts | 9 ++- ...ilter-cross-field-comparison-class.test.ts | 7 +- .../filter-cross-field-comparison-class.ts | 8 +-- ...te-cross-class-field-comparison-refused.ts | 70 +++++++++++++++++++ packages/spec/src/migrations/registry.ts | 66 +++++++++++++++++ 6 files changed, 153 insertions(+), 14 deletions(-) create mode 100644 packages/spec/src/migrations/entries/semantic/18.rls-predicate-cross-class-field-comparison-refused.ts diff --git a/.changeset/20355-rls-write-check-cross-class.md b/.changeset/20355-rls-write-check-cross-class.md index b4b243aa0d9..58ae899c58c 100644 --- a/.changeset/20355-rls-write-check-cross-class.md +++ b/.changeset/20355-rls-write-check-cross-class.md @@ -3,18 +3,21 @@ '@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). +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 != diff --git a/packages/lint/src/validate-rls-predicate-enforceability.ts b/packages/lint/src/validate-rls-predicate-enforceability.ts index 1355721ca16..c12e076d2bd 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.ts @@ -239,11 +239,10 @@ * 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`), 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..cc32d41f5e6 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.rls-predicate-cross-class-field-comparison-refused.ts @@ -0,0 +1,70 @@ +// 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, master_detail or user), 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), and a file or formula field has no stored column value to compare. ' + + '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, 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..9923a11bb92 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -14237,6 +14237,72 @@ 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, master_detail or user), 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), and a file or formula field has no stored column value to compare. ' + + '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, 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 From 859dfa7e95e3b6ddcf5a07eabd74a5343460de08 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 12:40:34 +0000 Subject: [PATCH 6/6] docs(spec): correct three statements in the cross-class D3 entry; lint header names the delegation The entry's reason no longer says a file field has no stored column: the file family is refused by name for the ADR-0104 dual-encoding reason the spec module gives. acceptanceCriteria qualifies the 400 read with "on the SQL drivers", and the replacement's text class lists all four reference types (tree included). registry.ts regenerated by gen:migration-registry. The lint header's #20347 paragraph says driver-sql now delegates through crossFieldColumnVerdict instead of naming the retired parity test. Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN Co-authored-by: Claude --- .../src/validate-rls-predicate-enforceability.ts | 4 ++-- ...dicate-cross-class-field-comparison-refused.ts | 15 ++++++++++----- packages/spec/src/migrations/registry.ts | 15 ++++++++++----- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/packages/lint/src/validate-rls-predicate-enforceability.ts b/packages/lint/src/validate-rls-predicate-enforceability.ts index c12e076d2bd..bd1c7348ac9 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.ts @@ -246,8 +246,8 @@ * * 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 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 index cc32d41f5e6..1b75f4b0110 100644 --- 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 @@ -25,8 +25,9 @@ export const entry: SemanticMigration = { 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, master_detail or user), a boolean with ' - + 'a boolean, a date with a date, a datetime with a datetime, a time of day with a time of day. ' + + '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 ' @@ -34,7 +35,10 @@ export const entry: SemanticMigration = { 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), and a file or formula field has no stored column value to compare. ' + + '("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, ' @@ -65,6 +69,7 @@ export const entry: SemanticMigration = { + '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, 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.', + + '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 9923a11bb92..26dccad9f86 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -14260,8 +14260,9 @@ const step18: MigrationStep = { 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, master_detail or user), a boolean with ' - + 'a boolean, a date with a date, a datetime with a datetime, a time of day with a time of day. ' + + '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 ' @@ -14269,7 +14270,10 @@ const step18: MigrationStep = { 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), and a file or formula field has no stored column value to compare. ' + + '("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, ' @@ -14300,8 +14304,9 @@ const step18: MigrationStep = { + '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, 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.', + + '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