feat(spec,lint)!: refuse an RLS or sharing-rule comparison between two fields of different comparison classes when it is authored (#20347)

.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.
+
+<!-- adr-0087: not-required (no-migration-prescription) nothing is renamed, retired or respelled: no metadata key, export or operator changes shape and no stored metadata is rewritten, so `objectstack migrate meta` has nothing to do; the author's remedy is to change which two columns a predicate compares, which is a change to the policy they meant, not to a spelling. -->

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: [] });
+    });
+  }
+});

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_<type>`) plus every
+ * multi-capable member flagged `multiple: true` (`m_<type>`), 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<string, unknown> {
+  const decl: Record<string, unknown> = { 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<Observed> {
+    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([]);
+    });
+  }
+});