fix(lint)!: refuse an RLS predicate that compares a field with a json or multiple field when it is authored

.changeset/19886-rls-list-holding-field-comparison-authoring.md

@@ -0,0 +1,24 @@
+---
+"@objectstack/lint": minor
+---
+
+A row-level-security predicate that compares a field with a `json` or `multiple` field is refused when it is authored, at `os validate` / `os build` / `os lint` and at the metadata save door, instead of only when it runs (#19886).
+
+**BREAKING** — an accept-set narrowing, shipped by `@objectstack/lint` as `minor` under the repo's launch-window convention for accept-set narrowings. The hand-migration prescription is already registered under protocol major 18 as `cel-predicate-one-value-comparand-refused`, which names this class.
+
+Clause-②: no (narrowing)
+
+`record.status != record.tags`, with `tags` a `json` field or a `multiple` lookup, lowers to a legal filter shape, because the CEL lowering sees the predicate's text and not the object's field types, and the engine's filter admission does not judge a `{ $field }` reference against the referenced column's type either. Measured before this change: 400 cells (`==`, `!=`, `!(==)`, `>`, `<=`; a `json`, `address`, `multiselect`, `multiple` lookup and `multiple` user field; both operand orders; `using` on `select` / `all` / `update` / `delete` / `insert` and `check` on `insert` / `update` / `all`) were all accepted by the real `os validate` and by the save door. The runtime refused every one of them, measured through the real plugin-security on driver-sql: a read the `using` scopes answered `INVALID_FILTER` / 400, a by-id update or delete it scopes `PERMISSION_DENIED` / 403, and every insert or by-id update judged by the `check` (or by a `using` standing in as the check) `INVALID_FILTER` / 400, with nothing stored.
+
+What changes:
+
+- `@objectstack/lint`: `validateRlsPredicateEnforceability` reports `rls-predicate-unenforceable` for every lowered field-to-field comparison (`==`, `!=`, `>`, `>=`, `<`, `<=`, on either side, under `!` too) in which either column is DECLARED to hold a list or an object. The declaration is read from the stack's own objects through the spec's value-shape classes, the same two driver-sql refuses such a comparison by: a structured JSON type (`json`, `composite`, `repeater`, `record`, `location`, `address`, `vector`), or a multi-valued field (`multiselect`, `checkboxes`, `tags`, or `select` / `radio` / `lookup` / `user` / `file` / `image` with `multiple: true`). It judges `using` and `check` on every operation. The finding names each comparison and the declaration behind it, and states the clause's run-time consequence. A clause it refuses is not also handed to the engine's filter judge, so one defect earns one finding.
+- Both doors run this rule already, so both refuse: `os validate` / `os build` / `os lint` fail, and a publish through the metadata save door answers `422 INVALID_METADATA` with the same sentence in `issues[]`. `OS_ALLOW_UNLINTED_METADATA_WRITES=1` still turns the save-door refusal into a logged warning.
+
+Not changed: a field compared with a single-valued field (`record.status != record.owner`, `record.amount > record.budget`), a `json` or `multiple` field compared with a literal or tested against `null`, and any column the stack does not declare (an object from another package, an external object with no field map), which the rule does not judge. The runtime refusals of stages 2d and 2e stay as the backstop. The stage 2d changeset's sentence that a field compared with a `json` or `multiple` field "is not reported at authoring time" no longer holds: it is now reported at both doors.
+
+No shipped predicate moves: 0 of the 187 `using` / `check` / `condition` strings in this repository's packages and examples, and 0 of the 3 in the cloud repository, compare a field with a `json` or `multiple` field, and the real `os validate` over `app-crm`, `app-multi-package`, `app-showcase` and `app-todo` reports no `rls-predicate-*` finding.
+
+**What to change.** A field compared with a `json` or `multiple` field has no row-filter form: compare with a single-valued column, or with a literal or a `current_user` value ("one of these values" is `record.status in ['open', 'pending']`, or `record.owner in current_user.org_user_ids`), or move the condition into a validation rule or a hook.
+
+<!-- adr-0087: not-required (already-registered cel-predicate-one-value-comparand-refused) The entry already names this exact class in its surface ("a field compared with another field (==, !=, or an ordering operator) where either column holds a list or an object on the record, as a json column or a multiple lookup does") and carries its prescription in its replacement ("A field compared with a json or multiple field has no pushdown form: compare with a single-valued column, or move the condition into a validation rule or hook"). This change moves where that registered class is refused, from run time to authoring time; it adds no class and no prescription the entry does not already hold, and it rewrites no stored metadata. -->

packages/cli/test/rls-policy-authoring-admission.test.ts

@@ -61,25 +61,27 @@ const deal = {
       returnType: 'boolean',
     },
     account: { type: 'lookup', label: 'Account', reference: 'account' },
+    tags: { type: 'json', label: 'Tags' },
+    watchers: { type: 'lookup', label: 'Watchers', reference: 'account', multiple: true },
   },
 };
 const account = { name: 'account', label: 'Account', fields: { region: { type: 'text', label: 'Region' } } };
 
-const permissionSet = (using: string) => ({
+const permissionSet = (using: string, policy: Record<string, unknown> = { operation: 'select', using }) => ({
   name: 'sales',
   label: 'Sales',
   objects: { deal: { allowRead: true } },
-  rowLevelSecurity: [{ name: 'p', label: 'P', object: 'deal', operation: 'select' as const, using }],
+  rowLevelSecurity: [{ name: 'p', label: 'P', object: 'deal', ...policy }],
 });
 
 const RLS = (f: { rule: string }) => f.rule.startsWith('rls-predicate-');
 
 /** `os validate` step 3, in process — see the file header. */
-function cliDoor(using: string): AuthoringFinding[] {
+function cliDoor(using: string, set = permissionSet(using)): AuthoringFinding[] {
   const config = {
     manifest: { id: 'com.example.rls', namespace: 'rls', version: '1.0.0', name: 'RLS', type: 'app' },
     objects: [deal, account],
-    permissions: [permissionSet(using)],
+    permissions: [set],
   };
   const normalized = normalizeStackInput(config as Record<string, unknown>);
   const lowering = lowerCallables(normalized as Record<string, unknown>);
@@ -126,10 +128,10 @@ interface SaveOutcome {
   issues: Array<{ rule: string; path: string; message: string }>;
 }
 
-async function runtimeDoor(using: string): Promise<SaveOutcome> {
+async function runtimeDoor(using: string, set = permissionSet(using)): Promise<SaveOutcome> {
   const { protocol } = await runtimeHost();
   try {
-    await protocol.saveMetaItem({ type: 'permission', name: 'sales', item: permissionSet(using) });
+    await protocol.saveMetaItem({ type: 'permission', name: 'sales', item: set });
     return { accepted: true, issues: [] };
   } catch (err) {
     const e = err as { code?: string; status?: number; issues?: SaveOutcome['issues'] };
@@ -235,3 +237,59 @@ describe('the judge pass binds an app-staged membership key to [] (#20158)', ()
     expect(saved.issues.map((i) => i.message)).toEqual([cli[0].message]);
   });
 });
+
+/**
+ * [#19886] A field compared with a field that holds a list or an object — a
+ * `json` field or a `multiple` lookup — is refused when it is AUTHORED, at both
+ * doors, on every clause. The lowering sees the predicate's text and the
+ * engine's admission does not judge a `{ $field }` reference against the
+ * referenced column's type, so before this arm every row below was ACCEPTED at
+ * both doors (measured) while the runtime refused it: the write check per
+ * record (400), driver-sql on the read by declared type (400), and the by-id
+ * update or delete a `using` scopes fails closed (403). The rule judges by the
+ * DECLARED type its object graph carries; the full operator × clause × class ×
+ * order table is pinned beside the rule in `@objectstack/lint`.
+ */
+describe('a field compared with a json / multiple field is refused at both doors, on every clause (#19886)', () => {
+  const ROWS: ReadonlyArray<{ label: string; clause: 'using' | 'check'; operation: string; predicate: string }> = [
+    { label: 'using on select, != a json field', clause: 'using', operation: 'select', predicate: 'record.region != record.tags' },
+    { label: 'using on all, the json field first', clause: 'using', operation: 'all', predicate: 'record.tags != record.region' },
+    { label: 'using on update, a negated == a multiple lookup', clause: 'using', operation: 'update', predicate: '!(record.owner == record.watchers)' },
+    { label: 'using on delete, == a multiple lookup', clause: 'using', operation: 'delete', predicate: 'record.owner == record.watchers' },
+    { label: 'check on insert, != a json field', clause: 'check', operation: 'insert', predicate: 'record.region != record.tags' },
+    { label: 'check on update, > a multiple lookup', clause: 'check', operation: 'update', predicate: 'record.watchers > record.owner' },
+  ];
+  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, number > number', clause: 'check', operation: 'insert', predicate: 'record.amount > record.amount' },
+    { label: 'using on all, a json field null test', clause: 'using', operation: 'all', predicate: 'record.tags != 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 a field with a field that holds a list or an object');
+
+      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: [] });
+    });
+  }
+});