diff --git a/.changeset/20347-cross-field-comparison-class-authoring.md b/.changeset/20347-cross-field-comparison-class-authoring.md new file mode 100644 index 00000000000..5338e200e21 --- /dev/null +++ b/.changeset/20347-cross-field-comparison-class-authoring.md @@ -0,0 +1,25 @@ +--- +"@objectstack/spec": minor +"@objectstack/lint": minor +--- + +A row-level-security predicate or a sharing-rule condition that compares two fields of different comparison classes — a text field with a number field, a field with a single image or file field, a field with a formula field — is refused when it is authored, at `os validate` / `os build` / `os lint` and, for a permission set, at the metadata save door (#20347). The classification it is judged by is exported once, from `@objectstack/spec/data`. + +**BREAKING** — an accept-set narrowing in `@objectstack/lint`, shipped as `minor` under the repo's launch-window convention for accept-set narrowings. `@objectstack/spec` gains exports only. + +Clause-②: yes (narrowing) + +`record.status != record.amount` (text vs number) and `record.status != record.photo` (text vs a single image) lower to a legal `{ status: { $ne: { $field: … } } }` filter and hold no list, so no authoring rule refused them. Measured before this change, through the real `os validate` and the real plugin-security and ObjectQL on driver-sql: `os validate` reported both valid; the read a `using` scopes answered `INVALID_FILTER` / 400 and a by-id update or delete it scopes `PERMISSION_DENIED` / 403, because driver-sql compiles a column-to-column comparison only between two columns of one comparison class; and a single-record insert judged by the `check` — or by a `using` standing in as the check — was admitted and stored, because the in-process write check compares the two raw values. A formula field (`record.status != record.is_open`) answered the same three ways. The same-class control (`record.status != record.note`) read, updated, deleted and inserted normally. For a sharing rule, the condition lowers and is seeded, and every criteria query it runs meets the same driver-sql refusal. + +What changes: + +- `@objectstack/spec/data` (`filter-cross-field-comparison-class.ts`): the cross-field comparison classification. `CROSS_FIELD_COMPARISON_CLASSES` names the six classes (`numeric`, `text`, `boolean`, `date`, `datetime`, `time`); `CROSS_FIELD_NO_CLASS_REASONS` the three families with none (`list-or-object`, `file`, `formula`); `CROSS_FIELD_COMPARISON_TYPE_CLASSES` classifies every `FieldType` member exactly once, by reference to the existing value-class sets; `crossFieldColumnVerdict` answers one declared column (a multi-capable type flagged `multiple: true` holds a list); and `crossFieldComparisonVerdict` answers two (`comparable`, `cross-class`, `no-class`, or `unjudged` for a type outside `FieldType`). It is lifted case for case from driver-sql's cross-field boundary, and a pairwise parity test in driver-sql holds the two equal over every declared field type. +- `@objectstack/lint`: `validateRlsPredicateEnforceability` reports `rls-predicate-unenforceable`, and `validateSharingRuleEnforceability` reports `sharing-rule-unlowerable-condition`, for every lowered field-to-field comparison (`==`, `!=`, `>`, `>=`, `<`, `<=`, either side, under `!` too) whose two declared columns are not `comparable`. It judges `using` and `check` on every operation, and sharing-rule conditions. The finding names each comparison, each column's declared type and class, and the clause's run-time consequence; the hint lists every class with the declared types it holds, read from the spec. A comparison either side of which holds a list or an object stays the existing list-holding finding, and a clause either arm refuses is not also handed to the engine's filter judge, so one defect earns one finding. + +Not changed: driver-sql and the in-process write check keep their own behaviour here; moving both onto the exported classification is the engine-lane half. A comparison between two columns of one class (`record.amount > record.budget`, `record.stage == record.account`), a file or formula field compared with a literal or tested against `null`, and any column the stack does not declare or declares with a type outside `FieldType`, are not reported. + +No shipped predicate moves: of the 163 `using` / `check` / `condition` string literals in this repository's packages and examples, the 105 that lower hold two field-to-field comparisons, both same-class (`spent > budget`, a hook condition; `a > b`, a gate fixture), and neither is an RLS predicate or a sharing-rule condition. + +To keep such a rule, compare a field only with a field of the same class, or with a literal or a `current_user` value; test a file field with `!= null`; or store the value the rule keys on in a field of the right type. If the two columns really hold comparable values, one of them is declared with the wrong type, and the declaration is what to fix. + + diff --git a/packages/cli/test/rls-policy-authoring-admission.test.ts b/packages/cli/test/rls-policy-authoring-admission.test.ts index 10bd1331a55..c723c3ae345 100644 --- a/packages/cli/test/rls-policy-authoring-admission.test.ts +++ b/packages/cli/test/rls-policy-authoring-admission.test.ts @@ -63,6 +63,7 @@ const deal = { account: { type: 'lookup', label: 'Account', reference: 'account' }, tags: { type: 'json', label: 'Tags' }, watchers: { type: 'lookup', label: 'Watchers', reference: 'account', multiple: true }, + photo: { type: 'image', label: 'Photo' }, }, }; const account = { name: 'account', label: 'Account', fields: { region: { type: 'text', label: 'Region' } } }; @@ -293,3 +294,60 @@ describe('a field compared with a json / multiple field is refused at both doors }); } }); + +/** + * [#20347] A field compared with a field of ANOTHER comparison class — text vs + * number, text vs a single image, text vs a formula field — is refused when it + * is AUTHORED, at both doors, on every clause. None of these holds a list, so + * the #19886 arm above lets them through; measured before this arm, the real + * `os validate` reported `record.status != record.amount` and + * `record.status != record.photo` valid, while through the real plugin-security + * on driver-sql the read their `using` scopes answered `INVALID_FILTER` / 400 + * and the insert their `check` judges was admitted and stored. The rule judges + * by the spec's classification (`crossFieldComparisonVerdict`); the full + * operator × clause × class × order table is pinned beside the rule in + * `@objectstack/lint`. + */ +describe('a field compared with a field of another comparison class is refused at both doors, on every clause (#20347)', () => { + const ROWS: ReadonlyArray<{ label: string; clause: 'using' | 'check'; operation: string; predicate: string }> = [ + { label: 'using on select, text != number', clause: 'using', operation: 'select', predicate: 'record.region != record.amount' }, + { label: 'using on all, text != a single image', clause: 'using', operation: 'all', predicate: 'record.region != record.photo' }, + { label: 'using on select, text != a formula field', clause: 'using', operation: 'select', predicate: 'record.region != record.is_open' }, + { label: 'using on update, number > date', clause: 'using', operation: 'update', predicate: 'record.amount > record.close_date' }, + { label: 'check on insert, text != number', clause: 'check', operation: 'insert', predicate: 'record.region != record.amount' }, + { label: 'check on insert, the image first', clause: 'check', operation: 'insert', predicate: 'record.photo != record.region' }, + ]; + const CONTROLS: ReadonlyArray<{ label: string; clause: 'using' | 'check'; operation: string; predicate: string }> = [ + { label: 'using on select, text != text', clause: 'using', operation: 'select', predicate: 'record.region != record.owner' }, + { label: 'check on insert, a single lookup == text (both text)', clause: 'check', operation: 'insert', predicate: 'record.account == record.owner' }, + { label: 'using on all, an image null test', clause: 'using', operation: 'all', predicate: 'record.photo != null' }, + ]; + const setFor = (row: { clause: string; operation: string; predicate: string }) => + permissionSet('', { operation: row.operation, [row.clause]: row.predicate }); + + for (const row of ROWS) { + it(`REFUSED at both doors with one sentence — ${row.label}: \`${row.predicate}\``, async () => { + const cli = cliDoor('', setFor(row)); + const saved = await runtimeDoor('', setFor(row)); + + expect(cli.map((f) => ({ severity: f.severity, rule: f.rule, path: f.path }))).toEqual([ + { severity: 'error', rule: UNENFORCEABLE, path: `permissions[0].rowLevelSecurity[0].${row.clause}` }, + ]); + expect(cli[0].message).toContain('lowers, but compares two fields that share no comparison class'); + + expect(saved.accepted).toBe(false); + expect({ code: saved.code, status: saved.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect(saved.issues.map((i) => ({ rule: i.rule, path: i.path }))).toEqual([ + { rule: UNENFORCEABLE, path: `permissions.sales.rowLevelSecurity[0].${row.clause}` }, + ]); + expect(saved.issues[0].message).toBe(cli[0].message); + }); + } + + for (const row of CONTROLS) { + it(`ACCEPTED at both doors — ${row.label}: \`${row.predicate}\``, async () => { + expect(cliDoor('', setFor(row))).toEqual([]); + expect(await runtimeDoor('', setFor(row))).toEqual({ accepted: true, issues: [] }); + }); + } +}); 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 new file mode 100644 index 00000000000..04a10342e29 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-20347-cross-field-class-parity.test.ts @@ -0,0 +1,128 @@ +// 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/lint/src/validate-rls-predicate-enforceability.cross-class-field.test.ts b/packages/lint/src/validate-rls-predicate-enforceability.cross-class-field.test.ts new file mode 100644 index 00000000000..8c34e04d5ad --- /dev/null +++ b/packages/lint/src/validate-rls-predicate-enforceability.cross-class-field.test.ts @@ -0,0 +1,343 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20347] A field compared with a field of ANOTHER comparison class is refused + * at authoring time, judged by the DECLARED types the rule's object graph + * carries and the spec's classification (`crossFieldComparisonVerdict`). + * + * `record.status != record.amount` (text vs number) and + * `record.status != record.photo` (text vs a single image) lower to legal + * `{ status: { $ne: { $field: … } } }` shapes, and before this arm nothing + * refused them when they were written: measured at the real `os validate`, + * both were valid. At run time the same policy gave two more answers: the read + * its `using` scopes answered `INVALID_FILTER` / 400 on driver-sql, and the + * insert its `check` judges was admitted and stored. + * + * One table for the three measured cells (text vs number, text vs image, text + * vs a formula field): operator × clause × operand order. Then every class + * against every other class, the same-class controls, and the seams with the + * neighbouring arms. Both doors are pinned with a real engine in + * `packages/cli/test/rls-policy-authoring-admission.test.ts`. + */ + +import { describe, expect, it } from 'vitest'; +import type { EngineFilterJudgement, EngineFilterJudgementOptions } from '@objectstack/spec/contracts'; + +import { + validateRlsPredicateEnforceability, + RLS_PREDICATE_UNENFORCEABLE, + RLS_PREDICATE_UNKNOWN_FIELD, +} from './validate-rls-predicate-enforceability.js'; +import { runAuthoringRules } from './authoring-rules.js'; +import { runRuntimeAuthoringRules } from './runtime-gate.js'; + +const deal = { + name: 'deal', + label: 'Deal', + fields: { + status: { type: 'text', label: 'Status' }, + owner: { type: 'text', label: 'Owner' }, + stage: { type: 'select', label: 'Stage', options: [{ label: 'Open', value: 'open' }] }, + account: { type: 'lookup', label: 'Account', reference: 'account' }, + amount: { type: 'number', label: 'Amount' }, + budget: { type: 'currency', label: 'Budget' }, + is_won: { type: 'boolean', label: 'Won' }, + is_hot: { type: 'toggle', label: 'Hot' }, + close_date: { type: 'date', label: 'Close date' }, + signed_date: { type: 'date', label: 'Signed date' }, + signed_at: { type: 'datetime', label: 'Signed at' }, + call_time: { type: 'time', label: 'Call time' }, + photo: { type: 'image', label: 'Photo' }, + contract: { type: 'file', label: 'Contract' }, + is_open: { + type: 'formula', + label: 'Is open', + expression: { dialect: 'cel', source: "record.status != 'closed'" }, + returnType: 'boolean', + }, + tags: { type: 'json', label: 'Tags' }, + }, +}; +const account = { name: 'account', label: 'Account', fields: { region: { type: 'text', label: 'Region' } } }; + +/** One permission set, one policy on `deal`. */ +const stackWith = (policy: Record, objects: unknown[] = [deal, account]) => ({ + objects, + permissions: [{ name: 'sales', label: 'Sales', rowLevelSecurity: [{ name: 'p', object: 'deal', ...policy }] }], +}); + +/** Every clause, on every operation it can be authored on. */ +const CLAUSES = [ + { clause: 'using', operation: 'select' }, + { clause: 'using', operation: 'all' }, + { clause: 'using', operation: 'update' }, + { clause: 'using', operation: 'delete' }, + { clause: 'using', operation: 'insert' }, + { clause: 'check', operation: 'insert' }, + { clause: 'check', operation: 'update' }, + { clause: 'check', operation: 'all' }, +] as const; + +/** Each operator an author can write between two fields, and what the finding quotes back. */ +const OPERATORS: ReadonlyArray<{ op: string; spell: (a: string, b: string) => string; quoted: string }> = [ + { op: '!=', spell: (a, b) => `record.${a} != record.${b}`, quoted: '!=' }, + { op: '!(==)', spell: (a, b) => `!(record.${a} == record.${b})`, quoted: '==' }, + { op: '==', spell: (a, b) => `record.${a} == record.${b}`, quoted: '==' }, + { op: '>', spell: (a, b) => `record.${a} > record.${b}`, quoted: '>' }, + { op: '>=', spell: (a, b) => `record.${a} >= record.${b}`, quoted: '>=' }, + { op: '<', spell: (a, b) => `record.${a} < record.${b}`, quoted: '<' }, + { op: '<=', spell: (a, b) => `record.${a} <= record.${b}`, quoted: '<=' }, +]; + +/** + * The three measured cells, each against `status` (text). `declared` is what + * the finding says about the column at fault: for a cross-class pair BOTH + * columns and their classes, for a column with no class that column alone. + */ +const CELLS = [ + { + cell: 'text vs number', + field: 'amount', + declared: (statusFirst: boolean) => + statusFirst + ? "`status` is declared `type: 'text'`, compared as text and `amount` is declared `type: 'number'`, compared as a number" + : "`amount` is declared `type: 'number'`, compared as a number and `status` is declared `type: 'text'`, compared as text", + }, + { + cell: 'text vs a single image', + field: 'photo', + declared: () => "`photo` is declared `type: 'image'`, a file field, which no row filter can compare with another column", + }, + { + cell: 'text vs a formula field', + field: 'is_open', + declared: () => "`is_open` is declared `type: 'formula'`, a formula field, which has no stored column a row filter can read", + }, +] as const; + +const CLASS_SENTENCE = + 'Two columns are compared only within one comparison class — the class decides how their stored values order ' + + 'and equal, and across classes SQL and the in-memory evaluator answer differently — and a file field or a ' + + 'formula field has no class at all, so the platform defines no comparison between these columns'; + +describe('validateRlsPredicateEnforceability — a field compared with a field of another class is REFUSED (#20347)', () => { + for (const { clause, operation } of CLAUSES) { + for (const { op, spell, quoted } of OPERATORS) { + for (const { cell, field, declared } of CELLS) { + for (const order of ['text first', 'text second'] as const) { + const statusFirst = order === 'text first'; + const [left, right] = statusFirst ? ['status', field] : [field, 'status']; + const predicate = spell(left, right); + it(`${clause} on ${operation} · ${op} · ${cell} · ${order}: \`${predicate}\``, () => { + const findings = validateRlsPredicateEnforceability(stackWith({ operation, [clause]: predicate })); + expect(findings.map((f) => ({ severity: f.severity, rule: f.rule, path: f.path }))).toEqual([ + { severity: 'error', rule: RLS_PREDICATE_UNENFORCEABLE, path: `permissions[0].rowLevelSecurity[0].${clause}` }, + ]); + const [finding] = findings; + expect(finding.where).toBe('permission set "sales" policy "p" on object "deal"'); + expect(finding.message).toContain( + `RLS ${clause} \`${predicate}\` lowers, but compares two fields that share no comparison class: ` + + `\`record.${left} ${quoted} record.${right}\`, where ${declared(statusFirst)}. ${CLASS_SENTENCE}: `, + ); + expect(finding.hint).toMatch(/^Compare a field only with a field of the same comparison class: /); + }); + } + } + } + } + + it('the table covers every clause-operation, every field-to-field operator, the three cells, both orders', () => { + expect(CLAUSES.length * OPERATORS.length * CELLS.length * 2).toBe(336); + }); + + it('states the consequence of its own clause — the refused read and the write check for `using`, the write for `check`', () => { + const using = validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.status != record.amount' }))[0]; + expect(using.message).toContain( + '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).', + ); + 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).not.toContain('every read this policy scopes'); + }); + + it('the prescription names every class with the declared types it holds, read from the spec', () => { + const [finding] = validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.status != record.amount' })); + expect(finding.hint).toContain('a number only with a number (`number`, `currency`, `percent`, `rating`, `slider`, `progress`, `summary`)'); + expect(finding.hint).toContain('a boolean only with a boolean (`boolean`, `toggle`)'); + expect(finding.hint).toContain('a date only with a date (`date`)'); + expect(finding.hint).toContain('a datetime only with a datetime (`datetime`)'); + expect(finding.hint).toContain('a time of day only with a time of day (`time`)'); + for (const t of ['text', 'autonumber', 'select', 'radio', 'lookup', 'master_detail', 'user', 'tree']) { + expect(finding.hint).toMatch(new RegExp(`text only with text \\([^)]*\`${t}\``)); + } + }); +}); + +describe('validateRlsPredicateEnforceability — every class against every other class (#20347)', () => { + // One column per class, plus the two single-valued families with none. The + // expectation is written from THIS table's labels — same label, same class — + // never from the verdict function under test. + const REPRESENTATIVES: ReadonlyArray<[label: string, field: string]> = [ + ['text', 'status'], + ['text', 'stage'], + ['text', 'account'], + ['numeric', 'amount'], + ['numeric', 'budget'], + ['boolean', 'is_won'], + ['boolean', 'is_hot'], + ['date', 'close_date'], + ['datetime', 'signed_at'], + ['time', 'call_time'], + ['file', 'photo'], + ['file', 'contract'], + ['formula', 'is_open'], + ]; + const hasClass = (label: string) => label !== 'file' && label !== 'formula'; + + for (const [leftLabel, left] of REPRESENTATIVES) { + for (const [rightLabel, right] of REPRESENTATIVES) { + if (left === right) continue; + const refused = !(hasClass(leftLabel) && leftLabel === rightLabel); + it(`${left} (${leftLabel}) != ${right} (${rightLabel}) → ${refused ? 'refused' : 'clean'}`, () => { + const findings = validateRlsPredicateEnforceability( + stackWith({ operation: 'select', using: `record.${left} != record.${right}` }), + ); + expect(findings.map((f) => f.rule)).toEqual(refused ? [RLS_PREDICATE_UNENFORCEABLE] : []); + }); + } + } + + it('a column compared with itself is judged by its own class: a number is comparable, a file field is not', () => { + expect(validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.amount == record.amount' }))).toEqual([]); + const [finding] = validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.photo == record.photo' })); + expect(finding.message).toContain( + "`record.photo == record.photo`, where `photo` is declared `type: 'image'`, a file field, which no row filter can compare with another column.", + ); + }); + + it('a registry-injected column is judged by the definition the registry provisions: `created_at` is a datetime', () => { + const [finding] = validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.status != record.created_at' })); + expect(finding.rule).toBe(RLS_PREDICATE_UNENFORCEABLE); + expect(finding.message).toContain("`created_at` is declared `type: 'datetime'`, compared as a datetime"); + expect(validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.signed_at > record.created_at' }))).toEqual([]); + }); +}); + +describe('validateRlsPredicateEnforceability — same-class comparisons stay CLEAN (#20347 controls)', () => { + const CONTROLS: ReadonlyArray<[string, string]> = [ + ['text != text', 'record.status != record.owner'], + ['a select == a lookup (both text)', 'record.stage == record.account'], + ['number > currency (both numeric)', 'record.amount > record.budget'], + ['boolean == toggle', 'record.is_won == record.is_hot'], + ['date <= date', 'record.close_date <= record.signed_date'], + ['a file field null test (not a field-to-field comparison)', 'record.photo != null'], + ['a number against a literal', 'record.amount > 5'], + ['text against a current_user value', 'record.owner == current_user.id'], + ]; + for (const { clause, operation } of CLAUSES) { + for (const [label, predicate] of CONTROLS) { + it(`${clause} on ${operation} · ${label}: \`${predicate}\``, () => { + expect(validateRlsPredicateEnforceability(stackWith({ operation, [clause]: predicate }))).toEqual([]); + }); + } + } +}); + +describe('validateRlsPredicateEnforceability — the arm is the graph\'s, and reports once (#20347)', () => { + it('names every offending comparison of one clause in ONE finding', () => { + const findings = validateRlsPredicateEnforceability( + stackWith({ operation: 'select', using: 'record.status != record.amount || record.close_date < record.signed_at' }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain( + "class: `record.status != record.amount`, where `status` is declared `type: 'text'`, compared as text and " + + "`amount` is declared `type: 'number'`, compared as a number; `record.close_date < record.signed_at`, where " + + "`close_date` is declared `type: 'date'`, compared as a date and `signed_at` is declared `type: 'datetime'`, " + + 'compared as a datetime. ', + ); + }); + + it('a comparison against a list or an object stays the list-holding arm\'s — never reported twice', () => { + for (const predicate of ['record.status != record.tags', 'record.photo != record.tags', 'record.is_open == record.tags']) { + const findings = validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: predicate })); + expect(findings, predicate).toHaveLength(1); + expect(findings[0].message, predicate).toContain('compares a field with a field that holds a list or an object'); + expect(findings[0].message, predicate).not.toContain('share no comparison class'); + } + }); + + it('two defects in one clause — a list and a class mismatch — earn one finding each', () => { + const findings = validateRlsPredicateEnforceability( + stackWith({ operation: 'select', using: 'record.status != record.tags && record.status != record.amount' }), + ); + expect(findings.map((f) => f.rule)).toEqual([RLS_PREDICATE_UNENFORCEABLE, RLS_PREDICATE_UNENFORCEABLE]); + expect(findings[0].message).toContain('holds a list or an object: `record.status != record.tags`'); + expect(findings[1].message).toContain('share no comparison class: `record.status != record.amount`'); + }); + + it('judges nothing the graph cannot answer: an object outside the stack, a field map it cannot read, `id`', () => { + expect(validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.status != record.amount' }, [account]))).toEqual([]); + expect( + validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.status != record.amount' }, [{ name: 'deal' }])), + ).toEqual([]); + expect(validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.amount != record.id' }))).toEqual([]); + }); + + it('a declared type outside FieldType is Zod\'s to refuse, not this arm\'s', () => { + const objects = [{ ...deal, fields: { ...deal.fields, subject: { type: 'integer', label: 'Subject' } } }, account]; + expect( + validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.status != record.subject' }, objects)), + ).toEqual([]); + }); + + it('an undeclared column is the unknown-field finding alone — this arm never doubles it', () => { + const findings = validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.amount != record.nope' })); + expect(findings.map((f) => f.rule)).toEqual([RLS_PREDICATE_UNKNOWN_FIELD]); + }); + + it('one defect, one finding: a read scope this arm refused is not handed to the engine judge', () => { + const calls: unknown[] = []; + const judgeFilter = (object: string, where: unknown, options?: EngineFilterJudgementOptions): EngineFilterJudgement => { + calls.push({ object, where, options }); + return { ok: true }; + }; + for (const predicate of ['record.status != record.amount', 'record.status != record.photo', 'record.status != record.is_open']) { + const refused = validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: predicate }), { judgeFilter }); + expect(refused.map((f) => f.rule), predicate).toEqual([RLS_PREDICATE_UNENFORCEABLE]); + } + expect(calls).toEqual([]); + + // CONTROL: the same-class spelling reaches the judge, as every clean read scope does. + expect( + validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.amount > record.budget' }), { judgeFilter }), + ).toEqual([]); + expect(calls).toEqual([ + { object: 'deal', where: { amount: { $gt: { $field: 'budget' } } }, options: { operation: 'find' } }, + ]); + }); + + it('reaches the author through `os validate`\'s rule table and the runtime publish gate alike', () => { + const stack = stackWith({ operation: 'insert', check: 'record.amount > record.status' }); + const cli = runAuthoringRules('validate', { normalized: stack, parsed: stack }).filter((f) => + f.rule.startsWith('rls-predicate-'), + ); + expect(cli.map((f) => ({ rule: f.rule, path: f.path }))).toEqual([ + { rule: RLS_PREDICATE_UNENFORCEABLE, path: 'permissions[0].rowLevelSecurity[0].check' }, + ]); + + const saved = runRuntimeAuthoringRules({ + type: 'permission', + item: stack.permissions[0], + context: { objects: [deal, account], permissions: [] }, + }); + const refused = saved.errors.filter((f) => f.rule.startsWith('rls-predicate-')); + expect(refused.map((f) => ({ rule: f.rule, path: f.path }))).toEqual([ + { rule: RLS_PREDICATE_UNENFORCEABLE, path: 'permissions.sales.rowLevelSecurity[0].check' }, + ]); + expect(refused[0].message).toBe(cli[0].message); + }); +}); diff --git a/packages/lint/src/validate-rls-predicate-enforceability.list-holding-field.test.ts b/packages/lint/src/validate-rls-predicate-enforceability.list-holding-field.test.ts index 1a0cdb43746..c495ca4fd3f 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.list-holding-field.test.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.list-holding-field.test.ts @@ -189,8 +189,8 @@ describe('validateRlsPredicateEnforceability — the one-value spellings stay CL } } - it('a single-valued field of a multi-capable type is one value: `select`, `lookup`, `user`, `file`', () => { - for (const def of [{ type: 'select' }, { type: 'lookup', reference: 'account' }, { type: 'user' }, { type: 'file' }]) { + it('a single-valued field of a multi-capable type is one value: `select`, `lookup`, `user`', () => { + for (const def of [{ type: 'select' }, { type: 'lookup', reference: 'account' }, { type: 'user' }]) { const objects = [{ ...deal, fields: { ...deal.fields, subject: { label: 'Subject', ...def } } }, account]; expect( validateRlsPredicateEnforceability(stackWith({ operation: 'select', using: 'record.status != record.subject' }, objects)), @@ -198,6 +198,18 @@ describe('validateRlsPredicateEnforceability — the one-value spellings stay CL ).toEqual([]); } }); + + it('a single-valued `file` field is one value too — not this arm\'s; the comparison-class arm refuses it (#20347)', () => { + // It holds no list, so this arm stays silent; but the file family has no + // comparison class at all, so the #20347 arm refuses the comparison, once. + const objects = [{ ...deal, fields: { ...deal.fields, subject: { label: 'Subject', type: 'file' } } }, account]; + const findings = validateRlsPredicateEnforceability( + stackWith({ operation: 'select', using: 'record.status != record.subject' }, objects), + ); + expect(findings.map((f) => f.rule)).toEqual([RLS_PREDICATE_UNENFORCEABLE]); + expect(findings[0].message).not.toContain('holds a list or an object'); + expect(findings[0].message).toContain('share no comparison class'); + }); }); describe('validateRlsPredicateEnforceability — the arm is the graph\'s, and reports once (#19886)', () => { diff --git a/packages/lint/src/validate-rls-predicate-enforceability.ts b/packages/lint/src/validate-rls-predicate-enforceability.ts index 6110324886a..700c2d14685 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.ts @@ -225,6 +225,35 @@ * JSON-column set and its multi-valued test from. It runs on every clause, and * before the engine's pass, so the engine never judges a clause this arm * already refused: one defect, one finding. + * + * ## A field compared with a field of another comparison class (#20347) + * + * `record.status != record.amount` (text vs number) and + * `record.status != record.photo` (text vs a single image) lower to legal + * `{ status: { $ne: { $field: … } } }` shapes and hold no list, so the arm above + * lets them through. Measured before this arm, through the real plugin-security + * and ObjectQL on driver-sql, for those two and for `record.status != + * record.is_open` (a formula field): `os validate` reported them valid; the + * read their `using` scopes answered `INVALID_FILTER` / 400 (driver-sql + * compiles a column-to-column comparison only between two columns of ONE + * 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. + * + * 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 + * 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 + * reported twice. It runs on every clause, beside the arm above and ahead of + * the engine's pass: one defect, one finding. */ import type { EngineFilterJudgement, IObjectQLEngine } from '@objectstack/spec/contracts'; @@ -238,10 +267,15 @@ import { import type { CelBoundsOverrun } from '@objectstack/formula'; import { RESERVED_RLS_MEMBERSHIP_KEYS } from '@objectstack/spec/contracts'; import { + CROSS_FIELD_COMPARISON_CLASSES, + CROSS_FIELD_COMPARISON_TYPE_CLASSES, STRUCTURED_JSON_TYPES, assertListComparandShapes, + crossFieldComparisonVerdict, isMultiValueField, normalizeFilterComparandTypes, + type CrossFieldColumnVerdict, + type CrossFieldComparisonClass, } from '@objectstack/spec/data'; import { ExecutionContextSchema } from '@objectstack/spec/kernel'; import { @@ -1066,6 +1100,156 @@ function listHoldingConsequence(clause: 'using' | 'check'): string { 'or an object in that column.'; } +/* ──────────────────────────────────────────────────────────────────────────── + * #20347 — a field compared with a field of ANOTHER comparison class (see the + * header). The classification is the spec's; this block only names it. + * ──────────────────────────────────────────────────────────────────────────── */ + +/** How one comparison class reads in a sentence. */ +const CLASS_PHRASE: Readonly> = { + numeric: 'a number', + text: 'text', + boolean: 'a boolean', + date: 'a date', + datetime: 'a datetime', + time: 'a time of day', +}; + +/** Why a single-valued column has no class, as a sentence (a list or an object is the arm above's). */ +const NO_CLASS_PHRASE: Readonly> = { + file: 'a file field, which no row filter can compare with another column', + formula: 'a formula field, which has no stored column a row filter can read', +}; + +/** + * Every comparison class with the declared types it holds, read off the spec's + * class table — so a type added to one of its value-class sets later is listed + * here without a change to this file. + */ +export const CROSS_FIELD_CLASS_LISTING: string = CROSS_FIELD_COMPARISON_CLASSES.map((cls) => { + const types = CROSS_FIELD_COMPARISON_TYPE_CLASSES + .filter((row) => row.verdict.kind === 'class' && row.verdict.class === cls) + .flatMap((row) => [...row.types]); + return `${CLASS_PHRASE[cls]} only with ${CLASS_PHRASE[cls]} (${types.map((t) => `\`${t}\``).join(', ')})`; +}).join('; '); + +/** A declared column the graph answers for, with the slice the classification reads. */ +function declaredColumn( + graph: ObjectGraph, + object: string, + name: string, +): { name: string; type: string; multiple?: boolean; meta: GraphField } | null { + const verdict = resolveFieldPath(graph, object, name); + if (verdict?.kind !== 'ok' || !verdict.meta || typeof verdict.meta.type !== 'string') return null; + return { name, type: verdict.meta.type, multiple: verdict.meta.multiple === true, meta: verdict.meta }; +} + +/** One column's part in a refused comparison, as the author declared it. */ +function describeColumn(column: { name: string; type: string }, verdict: CrossFieldColumnVerdict): string { + const declared = `\`${column.name}\` is declared \`type: '${column.type}'\``; + if (verdict.kind === 'class') return `${declared}, compared as ${CLASS_PHRASE[verdict.class]}`; + return verdict.reason === 'list-or-object' ? declared : `${declared}, ${NO_CLASS_PHRASE[verdict.reason]}`; +} + +/** One lowered comparison between two columns that share no comparison class. */ +export interface CrossClassComparison { + /** The comparison as the author wrote it, back in CEL. */ + written: string; + /** The column(s) at fault, with what each is declared as. */ + columns: string[]; +} + +/** + * Every lowered `{ $field }` comparison whose two declared columns share no + * comparison class — two classes (text vs number), or a column with none (a + * file field, a formula field) — judged by {@link crossFieldComparisonVerdict}. + * Read off the COMPILER'S OUTPUT and resolved against the object graph, like + * {@link listHoldingComparisons}; a comparison either side of which holds a list + * or an object is that function's and is skipped here, and a column the graph + * cannot answer for, or whose declared type is outside `FieldType`, is not + * judged. + * + * Exported for `validate-sharing-rule-enforceability.ts`, which judges the same + * class on a sharing rule's lowered `condition`: one classification, two rules. + */ +export function crossClassComparisons( + graph: ObjectGraph, + object: string, + filter: Record, +): CrossClassComparison[] { + const found = new Map(); + const sites = loweredSites(filter, (op, operand) => + FIELD_COMPARISON_SYMBOL.has(op) && + !!operand && typeof operand === 'object' && !Array.isArray(operand) && + typeof (operand as Record).$field === 'string'); + for (const site of sites) { + const referenced = (site.operand as { $field: string }).$field; + const target = declaredColumn(graph, object, site.field); + const ref = declaredColumn(graph, object, referenced); + if (!target || !ref) continue; + if (listHoldingDeclaration(target.meta) || listHoldingDeclaration(ref.meta)) continue; + const verdict = crossFieldComparisonVerdict(target, ref); + let columns: string[]; + if (verdict.verdict === 'cross-class') { + columns = [ + describeColumn(target, { kind: 'class', class: verdict.left }), + describeColumn(ref, { kind: 'class', class: verdict.right }), + ]; + } else if (verdict.verdict === 'no-class') { + columns = [ + ...(verdict.left.kind === 'no-class' ? [describeColumn(target, verdict.left)] : []), + ...(verdict.right.kind === 'no-class' && ref.name !== target.name ? [describeColumn(ref, verdict.right)] : []), + ]; + } else { + continue; + } + const written = `record.${site.field} ${FIELD_COMPARISON_SYMBOL.get(site.op)} record.${referenced}`; + found.set(written, { written, columns }); + } + return [...found.values()]; +} + +/** The comparisons, quoted back with the declarations behind them. */ +export function describeCrossClassComparisons(comparisons: readonly CrossClassComparison[]): string { + return comparisons.map((c) => `\`${c.written}\`, where ${c.columns.join(' and ')}`).join('; '); +} + +/** The sentence both rules state before their per-surface consequence. */ +export const CROSS_CLASS_SENTENCE = + 'Two columns are compared only within one comparison class — the class decides how their stored values ' + + 'order and equal, and across classes SQL and the in-memory evaluator answer differently — and a file field ' + + 'or a formula field has no class at all, so the platform defines no comparison between these columns'; + +/** + * What a cross-class comparison does at request time, per clause. Measured + * 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. + */ +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'; + 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.'; +} + +/** The prescription both rules share, ahead of their per-surface alternatives. */ +export const CROSS_CLASS_REMEDY = + `Compare a field only with a field of the same comparison class: ${CROSS_FIELD_CLASS_LISTING}. ` + + 'A file field and a formula field cannot be compared with another column at all. If the two columns do ' + + 'hold comparable values, one of them is declared with the wrong type: fix the declaration, not the predicate.'; + /** * What a reference miss costs at request time, per clause. Measured, not inferred. * @@ -1382,6 +1566,27 @@ function referenceFindings( }); } + // [#20347] A field compared with a field of ANOTHER comparison class — two + // classes, or a file / formula field with none — by the spec's classification + // (this file's header). Every clause, beside the arm above (which keeps every + // comparison against a list or an object), and ahead of the engine's pass. + const crossClass = filter ? crossClassComparisons(graph, object, filter) : []; + if (crossClass.length > 0) { + findings.push({ + severity: 'error', + rule: RLS_PREDICATE_UNENFORCEABLE, + where, + path, + message: + `RLS ${clause} \`${quote(source)}\` lowers, but compares two fields that share no comparison class: ` + + `${describeCrossClassComparisons(crossClass)}. ${CROSS_CLASS_SENTENCE}: ${crossClassConsequence(clause)}`, + hint: + `${CROSS_CLASS_REMEDY} To keep the rule without a second column, compare with a literal or a ` + + '`current_user` value, test a file field with `!= null`, or store the value you mean in a field of the ' + + 'right type and compare that — or move the condition into a validation rule or a hook.', + }); + } + // [#20158] The engine's own admission of the lowered read scope — run only // on a clause every pass above left clean, so one defect earns one finding. // The verdict and its sentence are the engine's; this rule adds the clause, diff --git a/packages/lint/src/validate-sharing-rule-enforceability.cross-class-field.test.ts b/packages/lint/src/validate-sharing-rule-enforceability.cross-class-field.test.ts new file mode 100644 index 00000000000..b2ff2b0d86f --- /dev/null +++ b/packages/lint/src/validate-sharing-rule-enforceability.cross-class-field.test.ts @@ -0,0 +1,151 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20347] A sharing rule whose `condition` compares two fields that share no + * comparison class — text vs number, text vs a single image, text vs a formula + * field — is refused at authoring time: the sharing-rule twin of the RLS arm + * pinned in `validate-rls-predicate-enforceability.cross-class-field.test.ts`, + * through the same classification (`crossClassComparisons`, which reads the + * spec's `crossFieldComparisonVerdict`). + * + * `record.status != record.amount` lowers to a legal + * `{ status: { $ne: { $field: 'amount' } } }`, so the seeder seeds the rule — + * and every criteria query it runs meets driver-sql's cross-field check, which + * refuses a comparison across classes (or against the file family, or a + * formula field) with the same `INVALID_FILTER` / 400 it gives a list-holding + * column. Before this arm every cell below was clean at `os validate`. + */ + +import { describe, expect, it } from 'vitest'; + +import { + validateSharingRuleEnforceability, + SHARING_RULE_UNLOWERABLE_CONDITION, +} from './validate-sharing-rule-enforceability.js'; +import { runAuthoringRules } from './authoring-rules.js'; + +const deal = { + name: 'deal', + label: 'Deal', + sharingModel: 'private', + fields: { + status: { type: 'text', label: 'Status' }, + owner_name: { type: 'text', label: 'Owner name' }, + amount: { type: 'number', label: 'Amount' }, + budget: { type: 'number', label: 'Budget' }, + close_date: { type: 'date', label: 'Close date' }, + signed_at: { type: 'datetime', label: 'Signed at' }, + photo: { type: 'image', label: 'Photo' }, + is_open: { + type: 'formula', + label: 'Is open', + expression: { dialect: 'cel', source: "record.status != 'closed'" }, + returnType: 'boolean', + }, + tags: { type: 'json', label: 'Tags' }, + }, +}; +const account = { name: 'account', label: 'Account', sharingModel: 'private', fields: { region: { type: 'text', label: 'Region' } } }; + +/** One criteria sharing rule on `deal`, the condition swapped in. */ +const stackWith = (condition: unknown, objects: unknown[] = [deal, account]) => ({ + objects, + sharingRules: [ + { + name: 'deal_desk_share', + type: 'criteria', + object: 'deal', + accessLevel: 'read', + sharedWith: { type: 'team', value: 'deal_desk' }, + condition, + }, + ], +}); + +const OPERATORS: ReadonlyArray<{ op: string; spell: (a: string, b: string) => string; quoted: string }> = [ + { op: '!=', spell: (a, b) => `record.${a} != record.${b}`, quoted: '!=' }, + { op: '!(==)', spell: (a, b) => `!(record.${a} == record.${b})`, quoted: '==' }, + { op: '==', spell: (a, b) => `record.${a} == record.${b}`, quoted: '==' }, + { op: '>', spell: (a, b) => `record.${a} > record.${b}`, quoted: '>' }, + { op: '<=', spell: (a, b) => `record.${a} <= record.${b}`, quoted: '<=' }, +]; + +const CELLS = [ + { cell: 'text vs number', field: 'amount', names: ["`status` is declared `type: 'text'`, compared as text", "`amount` is declared `type: 'number'`, compared as a number"] }, + { cell: 'text vs a single image', field: 'photo', names: ["`photo` is declared `type: 'image'`, a file field"] }, + { cell: 'text vs a formula field', field: 'is_open', names: ["`is_open` is declared `type: 'formula'`, a formula field"] }, +] as const; + +describe('validateSharingRuleEnforceability — a field compared with a field of another class is REFUSED (#20347)', () => { + for (const { op, spell, quoted } of OPERATORS) { + for (const { cell, field, names } of CELLS) { + for (const order of ['text first', 'text second'] as const) { + const [left, right] = order === 'text first' ? ['status', field] : [field, 'status']; + const condition = spell(left, right); + it(`${op} · ${cell} · ${order}: \`${condition}\``, () => { + const findings = validateSharingRuleEnforceability(stackWith(condition)); + expect(findings.map((f) => ({ severity: f.severity, rule: f.rule, path: f.path }))).toEqual([ + { severity: 'error', rule: SHARING_RULE_UNLOWERABLE_CONDITION, path: 'sharingRules[0].condition' }, + ]); + const [finding] = findings; + expect(finding.where).toBe('sharing rule "deal_desk_share" on object "deal"'); + expect(finding.message).toContain( + `Sharing-rule condition \`${condition}\` lowers, but compares two fields that share no comparison ` + + `class: \`record.${left} ${quoted} record.${right}\`, where `, + ); + for (const name of names) expect(finding.message).toContain(name); + expect(finding.message).toContain('The rule is declared and grants nothing.'); + expect(finding.hint).toMatch(/^Compare a field only with a field of the same comparison class: /); + }); + } + } + } + + it('the envelope form `os validate` hands in — `{ dialect, source }` — is judged the same', () => { + const findings = validateSharingRuleEnforceability(stackWith({ dialect: 'cel', source: 'record.status != record.amount' })); + expect(findings.map((f) => f.rule)).toEqual([SHARING_RULE_UNLOWERABLE_CONDITION]); + }); +}); + +describe('validateSharingRuleEnforceability — same-class comparisons stay CLEAN (#20347 controls)', () => { + const CONTROLS: ReadonlyArray<[string, string]> = [ + ['text != text', 'record.status != record.owner_name'], + ['number > number', 'record.amount > record.budget'], + ['a file field null test', 'record.photo != null'], + ['a number against a literal', 'record.amount > 1000'], + ]; + for (const [label, condition] of CONTROLS) { + it(`${label}: \`${condition}\``, () => { + expect(validateSharingRuleEnforceability(stackWith(condition))).toEqual([]); + }); + } +}); + +describe('validateSharingRuleEnforceability — one comparison, one finding (#20347)', () => { + it('a comparison against a list or an object stays the list-holding arm\'s', () => { + const findings = validateSharingRuleEnforceability(stackWith('record.photo != record.tags')); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('holds a list or an object'); + expect(findings[0].message).not.toContain('share no comparison class'); + }); + + it('two defects in one condition — a list and a class mismatch — earn one finding each', () => { + const findings = validateSharingRuleEnforceability(stackWith('record.status != record.tags && record.close_date < record.signed_at')); + expect(findings.map((f) => f.rule)).toEqual([SHARING_RULE_UNLOWERABLE_CONDITION, SHARING_RULE_UNLOWERABLE_CONDITION]); + expect(findings[0].message).toContain('holds a list or an object'); + expect(findings[1].message).toContain('share no comparison class: `record.close_date < record.signed_at`'); + }); + + it('judges nothing the graph cannot answer: an anchor outside the stack, a field map it cannot read', () => { + expect(validateSharingRuleEnforceability(stackWith('record.status != record.amount', [account]))).toEqual([]); + expect(validateSharingRuleEnforceability(stackWith('record.status != record.amount', [{ name: 'deal', sharingModel: 'private' }]))).toEqual([]); + }); + + it('reaches the author through `os validate`\'s rule table', () => { + const stack = stackWith('record.amount > record.status'); + const cli = runAuthoringRules('validate', { normalized: stack, parsed: stack }).filter((f) => f.rule.startsWith('sharing-rule-')); + expect(cli.map((f) => ({ rule: f.rule, path: f.path }))).toEqual([ + { rule: SHARING_RULE_UNLOWERABLE_CONDITION, path: 'sharingRules[0].condition' }, + ]); + }); +}); diff --git a/packages/lint/src/validate-sharing-rule-enforceability.list-holding-field.test.ts b/packages/lint/src/validate-sharing-rule-enforceability.list-holding-field.test.ts index 05a76548e66..19d894174db 100644 --- a/packages/lint/src/validate-sharing-rule-enforceability.list-holding-field.test.ts +++ b/packages/lint/src/validate-sharing-rule-enforceability.list-holding-field.test.ts @@ -209,12 +209,22 @@ describe('validateSharingRuleEnforceability — the one-value spellings stay CLE }); } - it('a single-valued field of a multi-capable type is one value: `select`, `lookup`, `user`, `file`', () => { - for (const def of [{ type: 'select' }, { type: 'lookup', reference: 'account' }, { type: 'user' }, { type: 'file' }]) { + it('a single-valued field of a multi-capable type is one value: `select`, `lookup`, `user`', () => { + for (const def of [{ type: 'select' }, { type: 'lookup', reference: 'account' }, { type: 'user' }]) { const objects = [{ ...deal, fields: { ...deal.fields, subject: { label: 'Subject', ...def } } }, account]; expect(validateSharingRuleEnforceability(stackWith('record.status != record.subject', objects)), def.type).toEqual([]); } }); + + it('a single-valued `file` field is one value too — not this arm\'s; the comparison-class arm refuses it (#20347)', () => { + // It holds no list, so this arm stays silent; but the file family has no + // comparison class at all, so the #20347 arm refuses the comparison, once. + const objects = [{ ...deal, fields: { ...deal.fields, subject: { label: 'Subject', type: 'file' } } }, account]; + const findings = validateSharingRuleEnforceability(stackWith('record.status != record.subject', objects)); + expect(findings.map((f) => f.rule)).toEqual([SHARING_RULE_UNLOWERABLE_CONDITION]); + expect(findings[0].message).not.toContain('holds a list or'); + expect(findings[0].message).toContain('share no comparison class'); + }); }); describe('validateSharingRuleEnforceability — the arm is the graph\'s, and reports once (#19886)', () => { diff --git a/packages/lint/src/validate-sharing-rule-enforceability.ts b/packages/lint/src/validate-sharing-rule-enforceability.ts index a95f00af123..355c46e3bf5 100644 --- a/packages/lint/src/validate-sharing-rule-enforceability.ts +++ b/packages/lint/src/validate-sharing-rule-enforceability.ts @@ -136,6 +136,21 @@ * the graph cannot answer for — an object this stack does not declare, a field * map it cannot read, a name it does not declare — is not judged here. * + * ## A field compared with a field of another comparison class (#20347) + * + * `record.status != record.amount` (text vs number) and + * `record.status != record.photo` (text vs a single image) hold no list, so the + * arm above lets them through, and they lower and are seeded just the same. The + * criteria query they run meets the same driver-sql check one clause later: + * driver-sql compiles a column-to-column comparison only between two columns of + * ONE comparison class and refuses the file family and formula fields outright, + * with the same `INVALID_FILTER` / 400 the list-holding class gets — so the + * consequence above holds word for word. The classification is the spec's + * (`crossFieldComparisonVerdict`, `@objectstack/spec/data`), read through the + * RLS rule's `crossClassComparisons`, which leaves every comparison against a + * list or an object to the arm above: one comparison, one finding. Same id, for + * the same reason the arm above keeps it. + * * ## What this rule deliberately does NOT do * * - **It does not re-implement `isMatchAllCriteria`.** The seeder's second @@ -167,12 +182,19 @@ import { compileCelToFilter } from '@objectstack/formula'; import { referenceCarrierOf } from '@objectstack/spec/data'; import { indexObjectGraph, recordsOf, type ObjectGraph } from './object-graph.js'; -import { listHoldingComparisons } from './validate-rls-predicate-enforceability.js'; +import { + CROSS_CLASS_REMEDY, + CROSS_CLASS_SENTENCE, + crossClassComparisons, + describeCrossClassComparisons, + listHoldingComparisons, +} from './validate-rls-predicate-enforceability.js'; /** * A `condition` the runtime cannot evaluate as written: outside the pushdown * subset (the rule is never seeded), or a lowered comparison with a field that - * holds a list or an object (seeded, and it grants nothing). + * holds a list or an object, or between two fields that share no comparison + * class (seeded, and it grants nothing). */ export const SHARING_RULE_UNLOWERABLE_CONDITION = 'sharing-rule-unlowerable-condition'; /** A `condition` reading `current_user.*` — unresolvable when grants are materialized. */ @@ -276,6 +298,41 @@ function listHoldingFinding( }; } +/** + * The finding for a lowered `condition` that compares two fields sharing no + * comparison class — text vs number, or a file / formula field — or `null` + * when it compares none (see this file's header). The classification is the + * spec's, read through the RLS rule's `crossClassComparisons`; the consequence + * is the list-holding arm's, because driver-sql refuses both classes with the + * same check on the same criteria query. + */ +function crossClassFinding( + graph: ObjectGraph, + object: string, + filter: Record, + at: { where: string; path: string; source: string }, +): SharingRuleEnforceabilityFinding | null { + const comparisons = crossClassComparisons(graph, object, filter); + if (comparisons.length === 0) return null; + return { + severity: 'error', + rule: SHARING_RULE_UNLOWERABLE_CONDITION, + where: at.where, + path: at.path, + message: + `Sharing-rule condition \`${at.source}\` lowers, but compares two fields that share no comparison class: ` + + `${describeCrossClassComparisons(comparisons)}. ${CROSS_CLASS_SENTENCE}: the rule is seeded into ` + + '`sys_sharing_rule`, but every criteria query it runs is refused on the SQL drivers (`INVALID_FILTER` / ' + + "400: driver-sql refuses the comparison by the two columns' declared types), and `SharingRuleService` " + + 'reads a refused query as matching no record. No `sys_record_share` grant is ever materialised, at boot ' + + 'or on any later write, and the only signal is a WARN line in the server log. The rule is declared and ' + + 'grants nothing.', + hint: + `${CROSS_CLASS_REMEDY} To keep the rule without a second column, compare with a literal — "one of these ` + + "values\" is `record.status in ['open', 'pending']` — or test a file field with `!= null`.", + }; +} + /** * The object's effective sharing model, as `SharingService` computes it. * @@ -543,6 +600,12 @@ export function validateSharingRuleEnforceability(stack: unknown): SharingRuleEn ? listHoldingFinding(graph, object, result.filter as Record, { where, path, source }) : null; if (listHolding) findings.push(listHolding); + // [#20347] …and when it compares two fields that share no comparison + // class — refused by the same driver check, the same way (file header). + const crossClass = graph + ? crossClassFinding(graph, object, result.filter as Record, { where, path, source }) + : null; + if (crossClass) findings.push(crossClass); return; } // Syntax belongs to `validateStackExpressions`, which already gates this diff --git a/packages/spec/api-surface/data.json b/packages/spec/api-surface/data.json index bffec749ceb..abf488818c9 100644 --- a/packages/spec/api-surface/data.json +++ b/packages/spec/api-surface/data.json @@ -74,6 +74,9 @@ "CREDENTIAL_KEY_SPELLINGS (const)", "CREDENTIAL_URL_QUERY_PARAMS (const)", "CREDENTIAL_URL_QUERY_PARAM_NAMES (const)", + "CROSS_FIELD_COMPARISON_CLASSES (const)", + "CROSS_FIELD_COMPARISON_TYPE_CLASSES (const)", + "CROSS_FIELD_NO_CLASS_REASONS (const)", "CalendarDateValue (type)", "CalendarDateValueSchema (const)", "ClockTimeValue (type)", @@ -95,6 +98,12 @@ "ContextTokenPlaceholder (type)", "ContextTokenPlaceholderSchema (const)", "ContextTokenSchema (const)", + "CrossFieldColumnVerdict (type)", + "CrossFieldComparisonClass (type)", + "CrossFieldComparisonFieldMeta (interface)", + "CrossFieldComparisonTypeClass (interface)", + "CrossFieldComparisonVerdict (type)", + "CrossFieldNoClassReason (type)", "CrossFieldValidation (type)", "CrossFieldValidationParsed (type)", "CrossFieldValidationSchema (const)", @@ -738,6 +747,8 @@ "credentialFreeMongoOptions (function)", "credentialFreeUrl (function)", "credentialQueryParamOf (function)", + "crossFieldColumnVerdict (function)", + "crossFieldComparisonVerdict (function)", "defaultAggregateFor (function)", "defaultValueTokenIssue (function)", "defineCube (function)", diff --git a/packages/spec/export-origins/data.json b/packages/spec/export-origins/data.json index 8617d4da90c..51c0b58ed7a 100644 --- a/packages/spec/export-origins/data.json +++ b/packages/spec/export-origins/data.json @@ -71,6 +71,9 @@ "CREDENTIAL_KEY_SPELLINGS": "src/data/driver/common.zod.ts#CREDENTIAL_KEY_SPELLINGS (const)", "CREDENTIAL_URL_QUERY_PARAMS": "src/data/driver/common.zod.ts#CREDENTIAL_URL_QUERY_PARAMS (const)", "CREDENTIAL_URL_QUERY_PARAM_NAMES": "src/data/driver/common.zod.ts#CREDENTIAL_URL_QUERY_PARAM_NAMES (const)", + "CROSS_FIELD_COMPARISON_CLASSES": "src/data/filter-cross-field-comparison-class.ts#CROSS_FIELD_COMPARISON_CLASSES (const)", + "CROSS_FIELD_COMPARISON_TYPE_CLASSES": "src/data/filter-cross-field-comparison-class.ts#CROSS_FIELD_COMPARISON_TYPE_CLASSES (const)", + "CROSS_FIELD_NO_CLASS_REASONS": "src/data/filter-cross-field-comparison-class.ts#CROSS_FIELD_NO_CLASS_REASONS (const)", "CalendarDateValue": "src/data/field-value.zod.ts#CalendarDateValue (type)", "CalendarDateValueSchema": "src/data/field-value.zod.ts#CalendarDateValueSchema (const)", "ClockTimeValue": "src/data/field-value.zod.ts#ClockTimeValue (type)", @@ -92,6 +95,12 @@ "ContextTokenPlaceholder": "src/data/context-tokens.zod.ts#ContextTokenPlaceholder (type)", "ContextTokenPlaceholderSchema": "src/data/context-tokens.zod.ts#ContextTokenPlaceholderSchema (const)", "ContextTokenSchema": "src/data/context-tokens.zod.ts#ContextTokenSchema (const)", + "CrossFieldColumnVerdict": "src/data/filter-cross-field-comparison-class.ts#CrossFieldColumnVerdict (type)", + "CrossFieldComparisonClass": "src/data/filter-cross-field-comparison-class.ts#CrossFieldComparisonClass (type)", + "CrossFieldComparisonFieldMeta": "src/data/filter-cross-field-comparison-class.ts#CrossFieldComparisonFieldMeta (interface)", + "CrossFieldComparisonTypeClass": "src/data/filter-cross-field-comparison-class.ts#CrossFieldComparisonTypeClass (interface)", + "CrossFieldComparisonVerdict": "src/data/filter-cross-field-comparison-class.ts#CrossFieldComparisonVerdict (type)", + "CrossFieldNoClassReason": "src/data/filter-cross-field-comparison-class.ts#CrossFieldNoClassReason (type)", "CrossFieldValidation": "src/data/validation.zod.ts#CrossFieldValidation (type)", "CrossFieldValidationParsed": "src/data/validation.zod.ts#CrossFieldValidationParsed (type)", "CrossFieldValidationSchema": "src/data/validation.zod.ts#CrossFieldValidationSchema (const)", @@ -725,6 +734,8 @@ "credentialFreeMongoOptions": "src/data/driver/common.zod.ts#credentialFreeMongoOptions (function)", "credentialFreeUrl": "src/data/driver/common.zod.ts#credentialFreeUrl (function)", "credentialQueryParamOf": "src/data/driver/common.zod.ts#credentialQueryParamOf (function)", + "crossFieldColumnVerdict": "src/data/filter-cross-field-comparison-class.ts#crossFieldColumnVerdict (function)", + "crossFieldComparisonVerdict": "src/data/filter-cross-field-comparison-class.ts#crossFieldComparisonVerdict (function)", "defaultAggregateFor": "src/data/aggregation-policy.ts#defaultAggregateFor (function)", "defaultValueTokenIssue": "src/data/default-value-shape.ts#defaultValueTokenIssue (function)", "defineCube": "src/data/analytics.zod.ts#defineCube (function)", 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 new file mode 100644 index 00000000000..175574b0a02 --- /dev/null +++ b/packages/spec/src/data/filter-cross-field-comparison-class.test.ts @@ -0,0 +1,213 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20347] Pins for the cross-field comparison-class CONTRACT — the classes, + * 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. + */ + +import { describe, it, expect } from 'vitest'; +import { + BOOLEAN_VALUE_TYPES, + CALENDAR_DATE_TYPES, + CLOCK_TIME_TYPES, + FILE_REFERENCE_TYPES, + INSTANT_TYPES, + MULTI_CAPABLE_TYPES, + MULTI_OPTION_TYPES, + NUMERIC_VALUE_TYPES, + REFERENCE_VALUE_TYPES, + SINGLE_OPTION_TYPES, + STRING_VALUE_TYPES, + STRUCTURED_JSON_TYPES, +} from './field-value.zod'; +import { FieldType } from './field.zod'; +import { + CROSS_FIELD_COMPARISON_CLASSES, + CROSS_FIELD_COMPARISON_TYPE_CLASSES, + CROSS_FIELD_NO_CLASS_REASONS, + crossFieldColumnVerdict, + crossFieldComparisonVerdict, + type CrossFieldColumnVerdict, +} from './filter-cross-field-comparison-class'; + +const sorted = (s: Iterable) => [...s].sort(); +const union = (...sets: ReadonlySet[]) => new Set(sets.flatMap((s) => [...s])); +const cls = (c: string): CrossFieldColumnVerdict => ({ kind: 'class', class: c } as CrossFieldColumnVerdict); +const none = (r: string): CrossFieldColumnVerdict => ({ kind: 'no-class', reason: r } as CrossFieldColumnVerdict); + +describe('[#20347] the class table', () => { + it('classifies every FieldType member exactly once — rows pairwise disjoint, union exactly FieldType', () => { + const seen = new Map(); + for (const row of CROSS_FIELD_COMPARISON_TYPE_CLASSES) { + for (const t of row.types) { + expect(seen.get(t), `${t} is in both ${seen.get(t)} and ${row.name}`).toBeUndefined(); + seen.set(t, row.name); + } + } + expect(sorted(seen.keys())).toEqual(sorted(FieldType.options)); + }); + + it('spells each class by REFERENCE to the existing value-class sets — nothing re-listed', () => { + const byName = new Map(CROSS_FIELD_COMPARISON_TYPE_CLASSES.map((row) => [row.name, row.types])); + expect(byName.get('NUMERIC_VALUE_TYPES')).toBe(NUMERIC_VALUE_TYPES); + expect(byName.get('STRING_VALUE_TYPES')).toBe(STRING_VALUE_TYPES); + expect(byName.get('SINGLE_OPTION_TYPES')).toBe(SINGLE_OPTION_TYPES); + expect(byName.get('REFERENCE_VALUE_TYPES')).toBe(REFERENCE_VALUE_TYPES); + expect(byName.get('BOOLEAN_VALUE_TYPES')).toBe(BOOLEAN_VALUE_TYPES); + expect(byName.get('CALENDAR_DATE_TYPES')).toBe(CALENDAR_DATE_TYPES); + expect(byName.get('INSTANT_TYPES')).toBe(INSTANT_TYPES); + expect(byName.get('CLOCK_TIME_TYPES')).toBe(CLOCK_TIME_TYPES); + expect(byName.get('STRUCTURED_JSON_TYPES')).toBe(STRUCTURED_JSON_TYPES); + expect(byName.get('MULTI_OPTION_TYPES')).toBe(MULTI_OPTION_TYPES); + expect(byName.get('FILE_REFERENCE_TYPES')).toBe(FILE_REFERENCE_TYPES); + }); + + it('the six classes, by the value classes each one holds', () => { + const typesOf = (c: string) => union( + ...CROSS_FIELD_COMPARISON_TYPE_CLASSES + .filter((row) => row.verdict.kind === 'class' && row.verdict.class === c) + .map((row) => row.types), + ); + expect(sorted(typesOf('numeric'))).toEqual(sorted(NUMERIC_VALUE_TYPES)); + expect(sorted(typesOf('text'))).toEqual(sorted(union( + STRING_VALUE_TYPES, new Set(['autonumber']), SINGLE_OPTION_TYPES, REFERENCE_VALUE_TYPES, + ))); + expect(sorted(typesOf('boolean'))).toEqual(sorted(BOOLEAN_VALUE_TYPES)); + expect(sorted(typesOf('date'))).toEqual(sorted(CALENDAR_DATE_TYPES)); + expect(sorted(typesOf('datetime'))).toEqual(sorted(INSTANT_TYPES)); + expect(sorted(typesOf('time'))).toEqual(sorted(CLOCK_TIME_TYPES)); + }); + + it('the three families with no class: a list or an object, the file family, formula', () => { + const typesOf = (r: string) => union( + ...CROSS_FIELD_COMPARISON_TYPE_CLASSES + .filter((row) => row.verdict.kind === 'no-class' && row.verdict.reason === r) + .map((row) => row.types), + ); + expect(sorted(typesOf('list-or-object'))).toEqual(sorted(union(STRUCTURED_JSON_TYPES, MULTI_OPTION_TYPES))); + expect(sorted(typesOf('file'))).toEqual(sorted(FILE_REFERENCE_TYPES)); + expect(sorted(typesOf('formula'))).toEqual(['formula']); + }); + + it('declares no phantom class or reason: every one named is used by a row, and every row uses a named one', () => { + const usedClasses = new Set(); + const usedReasons = new Set(); + for (const row of CROSS_FIELD_COMPARISON_TYPE_CLASSES) { + if (row.verdict.kind === 'class') usedClasses.add(row.verdict.class); + else usedReasons.add(row.verdict.reason); + } + expect(sorted(usedClasses)).toEqual(sorted(CROSS_FIELD_COMPARISON_CLASSES)); + expect(sorted(usedReasons)).toEqual(sorted(CROSS_FIELD_NO_CLASS_REASONS)); + }); +}); + +describe('[#20347] crossFieldColumnVerdict — one declared column', () => { + it('answers each FieldType member its row\'s verdict when declared single-valued', () => { + for (const row of CROSS_FIELD_COMPARISON_TYPE_CLASSES) { + for (const type of row.types) { + expect(crossFieldColumnVerdict({ type }), type).toEqual(row.verdict); + expect(crossFieldColumnVerdict({ type, multiple: false }), `${type} multiple:false`).toEqual(row.verdict); + } + } + }); + + it('`multiple: true` on a multi-capable type holds a list — whatever its row says', () => { + for (const type of MULTI_CAPABLE_TYPES) { + expect(crossFieldColumnVerdict({ type, multiple: true }), type).toEqual(none('list-or-object')); + } + }); + + it('`multiple: true` on any other type moves nothing — isMultiValueField\'s own reading', () => { + for (const type of FieldType.options.filter((t) => !MULTI_CAPABLE_TYPES.has(t))) { + expect(crossFieldColumnVerdict({ type, multiple: true }), type).toEqual(crossFieldColumnVerdict({ type })); + } + }); + + it('formula has no class whatever its declared returnType — no stored column to reference', () => { + expect(crossFieldColumnVerdict({ type: 'formula' })).toEqual(none('formula')); + }); + + it('a type outside FieldType is not judged: driver aliases, the absent-type default, garbage', () => { + for (const type of ['integer', 'int', 'float', 'object', 'array', 'string', 'reference', '', 'Text']) { + expect(crossFieldColumnVerdict({ type }), JSON.stringify(type)).toBeUndefined(); + } + }); +}); + +describe('[#20347] crossFieldComparisonVerdict — the measured cells', () => { + it('text vs number is cross-class, in both orders', () => { + expect(crossFieldComparisonVerdict({ type: 'text' }, { type: 'number' })) + .toEqual({ verdict: 'cross-class', left: 'text', right: 'numeric' }); + expect(crossFieldComparisonVerdict({ type: 'number' }, { type: 'text' })) + .toEqual({ verdict: 'cross-class', left: 'numeric', right: 'text' }); + }); + + it('text vs a single image has no class on the image side, in both orders', () => { + expect(crossFieldComparisonVerdict({ type: 'text' }, { type: 'image' })) + .toEqual({ verdict: 'no-class', left: cls('text'), right: none('file') }); + expect(crossFieldComparisonVerdict({ type: 'image' }, { type: 'text' })) + .toEqual({ verdict: 'no-class', left: none('file'), right: cls('text') }); + }); + + it('text vs a formula field has no class on the formula side', () => { + expect(crossFieldComparisonVerdict({ type: 'text' }, { type: 'formula' })) + .toEqual({ verdict: 'no-class', left: cls('text'), right: none('formula') }); + }); + + it('two columns of one class are comparable — the control', () => { + expect(crossFieldComparisonVerdict({ type: 'text' }, { type: 'text' })).toEqual({ verdict: 'comparable', class: 'text' }); + expect(crossFieldComparisonVerdict({ type: 'select' }, { type: 'lookup' })).toEqual({ verdict: 'comparable', class: 'text' }); + expect(crossFieldComparisonVerdict({ type: 'currency' }, { type: 'number' })).toEqual({ verdict: 'comparable', class: 'numeric' }); + }); + + it('the three temporal classes do not mix, and a boolean is not a number', () => { + expect(crossFieldComparisonVerdict({ type: 'date' }, { type: 'datetime' }).verdict).toBe('cross-class'); + expect(crossFieldComparisonVerdict({ type: 'datetime' }, { type: 'time' }).verdict).toBe('cross-class'); + expect(crossFieldComparisonVerdict({ type: 'boolean' }, { type: 'number' }).verdict).toBe('cross-class'); + }); + + it('two file fields are not comparable either — the family has no class at all', () => { + expect(crossFieldComparisonVerdict({ type: 'image' }, { type: 'image' })) + .toEqual({ verdict: 'no-class', left: none('file'), right: none('file') }); + }); + + it('an undeclared type on either side is unjudged', () => { + expect(crossFieldComparisonVerdict({ type: 'integer' }, { type: 'number' })).toEqual({ verdict: 'unjudged' }); + expect(crossFieldComparisonVerdict({ type: 'formula' }, { type: 'string' })).toEqual({ verdict: 'unjudged' }); + }); +}); + +describe('[#20347] crossFieldComparisonVerdict — every pair of declared columns', () => { + // Every FieldType member single-valued, plus every multi-capable member + // flagged `multiple: true`. + const columns = [ + ...FieldType.options.map((type) => ({ type })), + ...[...MULTI_CAPABLE_TYPES].map((type) => ({ type, multiple: true })), + ]; + const label = (c: { type: string; multiple?: boolean }) => (c.multiple ? `${c.type}[]` : c.type); + + it('is comparable exactly when both columns have a class and it is the same one', () => { + for (const a of columns) { + for (const b of columns) { + const va = crossFieldColumnVerdict(a)!; + const vb = crossFieldColumnVerdict(b)!; + const same = va.kind === 'class' && vb.kind === 'class' && va.class === vb.class; + expect(crossFieldComparisonVerdict(a, b).verdict === 'comparable', `${label(a)} vs ${label(b)}`).toBe(same); + } + } + }); + + it('is symmetric: swapping the sides never changes the verdict kind', () => { + for (const a of columns) { + for (const b of columns) { + expect(crossFieldComparisonVerdict(a, b).verdict, `${label(a)} vs ${label(b)}`) + .toBe(crossFieldComparisonVerdict(b, a).verdict); + } + } + }); +}); diff --git a/packages/spec/src/data/filter-cross-field-comparison-class.ts b/packages/spec/src/data/filter-cross-field-comparison-class.ts new file mode 100644 index 00000000000..c6006a234ec --- /dev/null +++ b/packages/spec/src/data/filter-cross-field-comparison-class.ts @@ -0,0 +1,339 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20347] The cross-field **comparison class** — which two DECLARED columns a + * field-to-field comparison may put on its two sides: + * `{ a: { $eq: { $field: 'b' } } }`, and the same shape under `$ne` / `$gt` / + * `$gte` / `$lt` / `$lte` (the lowering of `record.a == record.b`, `!=`, `>`, + * `>=`, `<`, `<=` in an RLS predicate or a sharing-rule condition). + * + * ## Why one classification, exported once + * + * Three judges ask this question of one comparison: + * + * - **driver-sql**, when it compiles a read — the #5222 validation boundary + * refuses a comparison between two columns of different classes, or against + * a column with no class, with `INVALID_FILTER` / 400; + * - **the in-process write check** (`matches-filter`), when an RLS `check` is + * evaluated against a post-image; + * - **the authoring door** — `@objectstack/lint`'s RLS and sharing-rule + * enforceability rules, which hold the declared field map and can say so + * before anything runs. + * + * Until this module the rule lived in driver-sql alone, as a module-private + * function (`crossFieldComparisonClass`), so the authoring door had nothing to + * read and the write check had no rule. Measured on one RLS policy, + * `record.status != record.amount` (text vs number) or + * `record.status != record.photo` (text vs image): `os validate` said valid, + * the `find` its `using` scopes answered `INVALID_FILTER` / 400 on driver-sql, + * and an `insert` its `check` judges was admitted and stored — one policy, + * three answers. This module is the one definition every judge reads. + * + * ## The rule, lifted case for case from driver-sql + * + * A column has a comparison CLASS when its stored value is one scalar of a + * shape SQL and the in-memory evaluator read the same way. Six classes, each + * spelled by REFERENCE to the existing `field-value.zod.ts` value classes, so a + * member added to one of those sets later is classified without a change here: + * + * | class | declared types | + * |:-----------|:---------------------------------------------------------------------------------| + * | `numeric` | `NUMERIC_VALUE_TYPES` | + * | `text` | `STRING_VALUE_TYPES`, `autonumber`, `SINGLE_OPTION_TYPES`, `REFERENCE_VALUE_TYPES` | + * | `boolean` | `BOOLEAN_VALUE_TYPES` | + * | `date` | `CALENDAR_DATE_TYPES` | + * | `datetime` | `INSTANT_TYPES` | + * | `time` | `CLOCK_TIME_TYPES` | + * + * Three families have NO class, and no comparison against them is compiled: + * + * - **`list-or-object`** — `STRUCTURED_JSON_TYPES`, `MULTI_OPTION_TYPES`, and + * any multi-capable type flagged `multiple: true` (`isMultiValueField`): + * element-wise semantics SQL comparison operators do not have. + * - **`file`** — `FILE_REFERENCE_TYPES`, refused BY NAME and independent of + * the deployment. 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. + * - **`formula`** — virtual: there is no stored column to reference. This + * holds whatever the formula's `returnType` — unlike the text-operator door + * (`filter-text-operator-declared-type.ts`), which judges a formula by its + * return type, a column-to-column comparison needs a COLUMN on both sides. + * + * A comparison is **comparable** only when both columns have a class and it is + * the same class. The rule is symmetric and deliberately covers pairings that + * happen to agree on one backend: across classes SQLite orders by storage class + * (every TEXT above every INTEGER) while JS relational operators coerce, so + * `{ stage: { $gt: { $field: 'amount' } } }` returned four rows on SQLite and + * none in memory when driver-sql measured it; a `boolean` column holds `0/1` + * where a record holds `true/false`; and the three temporal classes store three + * different text shapes. Refusing a pairing that would have agreed costs an + * 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. + * + * ## Declared types only + * + * The table is over `FieldType` members, exactly once each (pinned). A type + * outside `FieldType` — a driver-internal alias (`integer` / `int` / `float` + * are numeric columns, `object` / `array` JSON columns, `string` is driver-sql's + * default for an absent type), an introspected column — is not a declaration + * this table judges: {@link crossFieldColumnVerdict} answers `undefined` and + * {@link crossFieldComparisonVerdict} `unjudged`. A driver layers its own + * aliases above this table, as `field-value.zod.ts`'s header says every + * driver-internal alias does. + * + * ## What this module is + * + * The contract only — the classes, the class table, and two pure verdict + * functions. It writes no door and carries no runtime logic (Prime Directive + * #2). The authoring door reads it in `@objectstack/lint` + * (`validate-rls-predicate-enforceability.ts`, `validate-sharing-rule-enforceability.ts`); + * the engine lane rewires driver-sql and the write check onto it. + * + * @see FILE_REFERENCE_TYPES / STRUCTURED_JSON_TYPES / isMultiValueField — the no-class families. + * @see TEXT_OPERATOR_DOOR_TYPE_CLASSES — the neighbouring declared-type table this one is shaped after. + */ + +import { + BOOLEAN_VALUE_TYPES, + CALENDAR_DATE_TYPES, + CLOCK_TIME_TYPES, + FILE_REFERENCE_TYPES, + INSTANT_TYPES, + MULTI_OPTION_TYPES, + NUMERIC_VALUE_TYPES, + REFERENCE_VALUE_TYPES, + SINGLE_OPTION_TYPES, + STRING_VALUE_TYPES, + STRUCTURED_JSON_TYPES, + isMultiValueField, +} from './field-value.zod'; + +/* ──────────────────────────────────────────────────────────────────────────── + * The classes, and the families with none + * ──────────────────────────────────────────────────────────────────────────── */ + +/** + * The six comparison classes. Two columns are comparable only within one of + * them. The names are driver-sql's, so a refusal it logs ("stored as numeric") + * and a verdict read here name a class the same way. + */ +export const CROSS_FIELD_COMPARISON_CLASSES = [ + 'numeric', + 'text', + 'boolean', + 'date', + 'datetime', + 'time', +] as const; + +export type CrossFieldComparisonClass = (typeof CROSS_FIELD_COMPARISON_CLASSES)[number]; + +/** + * Why a declared column has NO comparison class — the three families no + * column-to-column comparison is compiled against. + * + * - `list-or-object` — the value is a list or an object (a structured JSON + * type, an inherently-multi option type, or `multiple: true`). + * - `file` — a media / attachment type, refused by name (see the module header). + * - `formula` — virtual: no stored column. + */ +export const CROSS_FIELD_NO_CLASS_REASONS = ['list-or-object', 'file', 'formula'] as const; + +export type CrossFieldNoClassReason = (typeof CROSS_FIELD_NO_CLASS_REASONS)[number]; + +/** One declared column's standing in a field-to-field comparison. */ +export type CrossFieldColumnVerdict = + | { readonly kind: 'class'; readonly class: CrossFieldComparisonClass } + | { readonly kind: 'no-class'; readonly reason: CrossFieldNoClassReason }; + +/** The slice of a field definition the classification reads. */ +export interface CrossFieldComparisonFieldMeta { + /** The declared `type` — a `FieldType` member, or the verdict is `undefined`. */ + type: string; + /** + * The declared `multiple` flag. Only `true` moves a verdict, and only on a + * multi-capable type (`isMultiValueField`'s own reading). + */ + multiple?: boolean | undefined; +} + +/* ──────────────────────────────────────────────────────────────────────────── + * The class table — every FieldType member, exactly once + * ──────────────────────────────────────────────────────────────────────────── */ + +/** One row of {@link CROSS_FIELD_COMPARISON_TYPE_CLASSES}. */ +export interface CrossFieldComparisonTypeClass { + /** The row, named after the `field-value.zod.ts` set it references. */ + readonly name: string; + /** Its members — the existing export, never a re-listing. */ + readonly types: ReadonlySet; + /** Every member's verdict when it is declared single-valued. */ + readonly verdict: CrossFieldColumnVerdict; + /** Why — surfaced in failure output. */ + readonly note: string; +} + +const classOf = (cls: CrossFieldComparisonClass): CrossFieldColumnVerdict => ({ kind: 'class', class: cls }); +const noClass = (reason: CrossFieldNoClassReason): CrossFieldColumnVerdict => ({ kind: 'no-class', reason }); + +/** + * The classification, one row per value class. Its test pins that the rows' + * members are pairwise disjoint and that their union is EXACTLY `FieldType`: + * no member may be silently absent, and none may be classified twice. + * + * A row states the verdict for a member declared single-valued; `multiple: + * true` on a multi-capable member (`select` / `radio` / `lookup` / `user` / + * `file` / `image`) moves it to `list-or-object` — see + * {@link crossFieldColumnVerdict}. + */ +export const CROSS_FIELD_COMPARISON_TYPE_CLASSES: readonly CrossFieldComparisonTypeClass[] = [ + { + name: 'NUMERIC_VALUE_TYPES', + types: NUMERIC_VALUE_TYPES, + verdict: classOf('numeric'), + note: 'A finite number, stored in a numeric column on every SQL dialect. `summary` is a member (verified at the set, not the name).', + }, + { + name: 'STRING_VALUE_TYPES', + types: STRING_VALUE_TYPES, + verdict: classOf('text'), + note: 'A plain string, stored as text.', + }, + { + name: 'autonumber', + types: new Set(['autonumber']), + verdict: classOf('text'), + note: 'The stored value is the formatted number, a string.', + }, + { + name: 'SINGLE_OPTION_TYPES', + types: SINGLE_OPTION_TYPES, + verdict: classOf('text'), + note: 'One option code, a string. `multiple: true` on `select` / `radio` makes it a list.', + }, + { + name: 'REFERENCE_VALUE_TYPES', + types: REFERENCE_VALUE_TYPES, + verdict: classOf('text'), + note: 'A record id, a string. `multiple: true` on `lookup` / `user` makes it a list.', + }, + { + name: 'BOOLEAN_VALUE_TYPES', + types: BOOLEAN_VALUE_TYPES, + verdict: classOf('boolean'), + note: 'A boolean — `0/1` in a SQL column, `true/false` on a record, so it is its own class.', + }, + { + name: 'CALENDAR_DATE_TYPES', + types: CALENDAR_DATE_TYPES, + verdict: classOf('date'), + note: 'A calendar day (`YYYY-MM-DD`) — not an instant, so not comparable with a `datetime`.', + }, + { + name: 'INSTANT_TYPES', + types: INSTANT_TYPES, + verdict: classOf('datetime'), + note: 'A UTC instant (canonical ISO text on SQLite) — its own class.', + }, + { + name: 'CLOCK_TIME_TYPES', + types: CLOCK_TIME_TYPES, + verdict: classOf('time'), + note: 'A wall-clock time of day (`HH:MM:SS`) — its own class.', + }, + { + name: 'STRUCTURED_JSON_TYPES', + types: STRUCTURED_JSON_TYPES, + verdict: noClass('list-or-object'), + note: 'A structured JSON payload in a JSON column — not one comparable value.', + }, + { + name: 'MULTI_OPTION_TYPES', + types: MULTI_OPTION_TYPES, + verdict: noClass('list-or-object'), + note: 'An array of option codes in a JSON column — not one comparable value.', + }, + { + name: 'FILE_REFERENCE_TYPES', + types: FILE_REFERENCE_TYPES, + verdict: noClass('file'), + note: '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.', + }, + { + name: 'formula', + types: new Set(['formula']), + verdict: noClass('formula'), + note: 'Virtual — no stored column to reference, whatever the declared `returnType`.', + }, +]; + +/** `type` → its row's verdict, built once from the table above. */ +const VERDICT_OF_TYPE: ReadonlyMap = new Map( + CROSS_FIELD_COMPARISON_TYPE_CLASSES.flatMap((row) => [...row.types].map((type) => [type, row.verdict] as const)), +); + +const LIST_OR_OBJECT: CrossFieldColumnVerdict = noClass('list-or-object'); + +/* ──────────────────────────────────────────────────────────────────────────── + * The verdicts + * ──────────────────────────────────────────────────────────────────────────── */ + +/** + * One declared column's comparison class, or the reason it has none. + * `undefined` for a `type` outside `FieldType` — not a declaration this table + * judges (see the module header). Pure: one input, no I/O. + */ +export function crossFieldColumnVerdict(field: CrossFieldComparisonFieldMeta): CrossFieldColumnVerdict | undefined { + const row = VERDICT_OF_TYPE.get(field.type); + if (row === undefined) return undefined; + // The same question driver-sql asks first, through the same predicate: a + // multi-capable type flagged `multiple: true` holds a list, whatever its row. + if (isMultiValueField({ type: field.type, multiple: field.multiple === true })) return LIST_OR_OBJECT; + return row; +} + +/** + * The verdict on one field-to-field comparison between two declared columns. + * + * - `comparable` — both columns have a class, and it is the same one. + * - `cross-class` — both have a class, and they differ (text vs number). + * - `no-class` — at least one column has no class (a list or an object, a + * file field, a formula); both columns' verdicts are carried so a caller can + * name the one at fault. + * - `unjudged` — a declared type is outside `FieldType`; no verdict. + * + * Symmetric in its answer: swapping the two sides never changes the verdict + * kind (pinned over every pair). + */ +export type CrossFieldComparisonVerdict = + | { readonly verdict: 'comparable'; readonly class: CrossFieldComparisonClass } + | { + readonly verdict: 'cross-class'; + readonly left: CrossFieldComparisonClass; + readonly right: CrossFieldComparisonClass; + } + | { readonly verdict: 'no-class'; readonly left: CrossFieldColumnVerdict; readonly right: CrossFieldColumnVerdict } + | { readonly verdict: 'unjudged' }; + +/** + * May `left` and `right` be compared column to column? Pure: two declared + * columns in, one of four answers out, no I/O. Only `comparable` is a + * comparison the platform defines; every other judged answer is one driver-sql + * refuses to compile. + */ +export function crossFieldComparisonVerdict( + left: CrossFieldComparisonFieldMeta, + right: CrossFieldComparisonFieldMeta, +): CrossFieldComparisonVerdict { + const l = crossFieldColumnVerdict(left); + const r = crossFieldColumnVerdict(right); + if (l === undefined || r === undefined) return { verdict: 'unjudged' }; + if (l.kind === 'no-class' || r.kind === 'no-class') return { verdict: 'no-class', left: l, right: r }; + if (l.class !== r.class) return { verdict: 'cross-class', left: l.class, right: r.class }; + return { verdict: 'comparable', class: l.class }; +} diff --git a/packages/spec/src/data/index.ts b/packages/spec/src/data/index.ts index 46cf2bf868c..8dd5158d5a0 100644 --- a/packages/spec/src/data/index.ts +++ b/packages/spec/src/data/index.ts @@ -82,6 +82,14 @@ export * from './filter-comparand-type-conformance'; // door it declares, like `filter-comparand-type`, not as a driver case-set: // drivers sit beneath this door and keep answering FILTER_TEXT_CASES' row. export * from './filter-text-operator-declared-type'; +// [#20347] The cross-field COMPARISON CLASS — which two declared columns a +// field-to-field comparison (`{ a: { $eq: { $field: 'b' } } }` and its five +// sibling operators) may put on either side: six classes over the existing +// value-class sets, three families with none (a list or an object, the file +// family, formula), and a pure verdict over two declared types. Lifted case +// for case from driver-sql's #5222 boundary so every judge — the authoring +// door, driver-sql, the write check — reads one definition. +export * from './filter-cross-field-comparison-class'; export * from './temporal-conformance'; // Canonical conformance cases for deterministic paged reads — the standard // every driver's `find()` is held to whenever `limit`/`offset` slice the result