fix(formula): refuse an array comparand under $ne and in the equality slot (write-check bypass)

.changeset/19886-formula-array-comparand-refused.md

@@ -0,0 +1,24 @@
+---
+"@objectstack/formula": minor
+"@objectstack/plugin-security": minor
+"@objectstack/spec": patch
+---
+
+`matchesFilterCondition` refuses an array comparand under `$ne` and in the equality position (`{ field: [...] }`, `$eq: [...]`) with `INVALID_FILTER` / 400, before any record is judged (#19886).
+
+**BREAKING** — an accept-set narrowing, shipped by `@objectstack/formula` and `@objectstack/plugin-security` as `minor` under the repo's launch-window convention for accept-set narrowings. The hand-migration prescription is registered under protocol major 18 as `rls-predicate-array-comparand-refused`.
+
+Clause-②: no (narrowing)
+
+**Security fix for row-level write checks.** This evaluator is what `@objectstack/plugin-security` runs against the post-image of an insert or update to enforce a row-level `check`. It compared strictly, and no stored value ever equals an array, so:
+
+- a `check` written `record.status != ['closed', 'archived']`, or `!=` against a `current_user` membership array, lowered to `{ status: { $ne: [...] } }` and matched **every** post-image;
+- a `check` written `!(record.status == ['closed', 'archived'])` lowered to `{ $not: { status: [...] } }` and did the same.
+
+Every write such a policy was written to refuse was admitted and stored. Both shapes now fail the write with `INVALID_FILTER` / 400, the envelope driver-sql and driver-memory already give the same shape on the read side. The explain engine's record attribution reads the same evaluator, so it refuses too, instead of reporting a row as admitted that the enforcing read refuses. The positive `record.status == ['open', 'pending']` already refused every write (403); it now refuses with the 400 instead.
+
+The message withholds the field, the operator and the value, because the filter is usually an access policy the caller did not write, and the comparand may be a resolved membership set.
+
+**What to change.** A `check` or `using` predicate that means "one of these values" or "none of these values" is spelled with `in`: `record.status in ['open', 'pending']`, or `!(record.status in ['closed', 'archived'])`. Those, scalar `!=` / `==`, `null`, `Date` comparands and `{ $field }` references evaluate exactly as before.
+
+<!-- adr-0087: registered rls-predicate-array-comparand-refused -->

packages/formula/src/matches-filter-array-comparand.test.ts

@@ -0,0 +1,161 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#19886] An ARRAY where one comparable value is expected — under `$ne`, or in
+ * the equality position (a bare-array field spec, or `$eq`) — is REFUSED by
+ * `matchesFilterCondition` with the `INVALID_FILTER` / 400 envelope driver-sql
+ * and driver-memory already raise for the same shape.
+ *
+ * # Why this face's old answer was a write-gate bypass
+ *
+ * This evaluator IS the enforcement of a row-level `check` (plugin-security
+ * matches it against the post-image; there is no query to push down to). It
+ * compares strictly, and no stored scalar is `===` an array:
+ *
+ * | `check` policy (the CEL it lowers from)                  | before                           | after            |
+ * |---|---|---|
+ * | `{ s: { $ne: ['a','b'] } }` (`record.s != ['a','b']`)     | `true` → every write **ALLOWED** | throws → 400     |
+ * | `{ $not: { s: ['a','b'] } }` (`!(record.s == ['a','b'])`) | `true` → every write **ALLOWED** | throws → 400     |
+ * | `{ s: ['a','b'] }` (`record.s == ['a','b']`)              | `false` → every write DENIED     | throws → 400     |
+ *
+ * The first two rows are the bypass. The third failed closed only by accident
+ * of polarity, which is why the equality position is refused with it.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { matchesFilterCondition } from './matches-filter';
+
+interface WireBearingError extends Error {
+  code?: string;
+  status?: number;
+}
+
+const RECORD = { id: '1', status: 'closed', owner: 'u1', tags: ['a', 'b'], other: 'x' };
+
+const refusalOf = (filter: unknown, record: Record<string, unknown> = RECORD): WireBearingError => {
+  try {
+    matchesFilterCondition(record, filter as never);
+  } catch (e) {
+    return e as WireBearingError;
+  }
+  throw new Error('expected the evaluator to refuse this filter, but it answered');
+};
+
+const LIST = ['closed', 'archived'];
+
+/** Each refused shape, spelled once per operator position. */
+const SHAPES: Array<[string, (list: unknown[]) => Record<string, unknown>]> = [
+  ['$ne with an array', (list) => ({ status: { $ne: list } })],
+  ['a bare-array field spec (implicit equality)', (list) => ({ status: list })],
+  ['$eq with an array', (list) => ({ status: { $eq: list } })],
+];
+
+/** Every depth the refusal must reach — and the ones where the old answer was absorbed or inverted. */
+const POSITIONS: Array<[string, (leaf: Record<string, unknown>) => Record<string, unknown>]> = [
+  ['top level', (leaf) => leaf],
+  ['inside $and', (leaf) => ({ $and: [{ owner: 'u1' }, leaf] })],
+  ['inside $or, beside a satisfied branch', (leaf) => ({ $or: [{ owner: 'u1' }, leaf] })],
+  ['inside $not', (leaf) => ({ $not: leaf })],
+  ['nested two combinators deep', (leaf) => ({ $and: [{ $or: [{ $not: leaf }] }] })],
+  ['after a sibling that already fails for this record', (leaf) => ({ owner: 'nobody', ...leaf })],
+];
+
+describe('[#19886] matchesFilterCondition refuses an array comparand in a single-value position', () => {
+  for (const [shapeName, shape] of SHAPES) {
+    for (const [positionName, position] of POSITIONS) {
+      it(`${shapeName} — ${positionName}: INVALID_FILTER / 400`, () => {
+        const err = refusalOf(position(shape(LIST)));
+        expect(err.code).toBe('INVALID_FILTER');
+        expect(err.status).toBe(400);
+      });
+    }
+
+    it(`${shapeName} — an EMPTY array is refused too`, () => {
+      const err = refusalOf(shape([]));
+      expect(err.code).toBe('INVALID_FILTER');
+      expect(err.status).toBe(400);
+    });
+
+    it(`${shapeName} — refused for EVERY record, before any row is judged`, () => {
+      // An empty record, a record whose value is a member of the list, a record
+      // whose value is the very same array reference, and a record that would
+      // have failed an earlier sibling: the verdict may not depend on the row.
+      const records: Array<Record<string, unknown>> = [
+        {},
+        { status: 'closed' },
+        { status: LIST },
+        { status: 'open', owner: 'nobody' },
+      ];
+      for (const record of records) {
+        const err = refusalOf(shape(LIST), record);
+        expect(err.code).toBe('INVALID_FILTER');
+        expect(err.status).toBe(400);
+      }
+    });
+  }
+
+  it('the remedy names the list operators by their FieldOperatorsSchema spelling', () => {
+    const err = refusalOf({ status: { $ne: LIST } });
+    expect(err.message).toContain('"$in"');
+    expect(err.message).toContain('"$nin"');
+  });
+
+  it('the message withholds the field and the comparand — the filter may be a policy the caller did not write', () => {
+    const secretField = 'secret_scope_column';
+    const secretIds = ['usr_member_one', 'usr_member_two'];
+    for (const filter of [
+      { [secretField]: { $ne: secretIds } },
+      { [secretField]: secretIds },
+      { [secretField]: { $eq: secretIds } },
+    ]) {
+      const err = refusalOf(filter, { [secretField]: 'x' });
+      expect(err.message).not.toContain(secretField);
+      for (const id of secretIds) expect(err.message).not.toContain(id);
+    }
+  });
+});
+
+describe('[#19886] every neighbouring shape answers exactly as before', () => {
+  const m = matchesFilterCondition;
+
+  it('$in / $nin — the list operators — still evaluate their array', () => {
+    expect(m({ status: 'closed' }, { status: { $in: LIST } })).toBe(true);
+    expect(m({ status: 'open' }, { status: { $in: LIST } })).toBe(false);
+    expect(m({ status: 'closed' }, { status: { $nin: LIST } })).toBe(false);
+    expect(m({ status: 'open' }, { status: { $nin: LIST } })).toBe(true);
+    expect(m({ status: 'open' }, { $not: { status: { $in: LIST } } })).toBe(true);
+    expect(m({ status: 'closed' }, { $not: { status: { $in: LIST } } })).toBe(false);
+  });
+
+  it('scalar $ne / $eq / implicit equality are untouched', () => {
+    expect(m({ status: 'closed' }, { status: { $ne: 'closed' } })).toBe(false);
+    expect(m({ status: 'open' }, { status: { $ne: 'closed' } })).toBe(true);
+    expect(m({ status: 'closed' }, { status: { $eq: 'closed' } })).toBe(true);
+    expect(m({ status: 'closed' }, { status: 'closed' })).toBe(true);
+    expect(m({ status: 'open' }, { $not: { status: 'closed' } })).toBe(true);
+  });
+
+  it('null comparands are untouched', () => {
+    expect(m({ status: null }, { status: null })).toBe(true);
+    expect(m({ status: 'x' }, { status: { $ne: null } })).toBe(true);
+    expect(m({ status: null }, { status: { $eq: null } })).toBe(true);
+  });
+
+  it('a Date comparand is still a comparand, not an operator map', () => {
+    const d = new Date('2026-01-01T00:00:00.000Z');
+    expect(m({ at: new Date(d.getTime()) }, { at: d })).toBe(true);
+    expect(m({ at: new Date(d.getTime()) }, { at: { $ne: d } })).toBe(false);
+  });
+
+  it('a { $field } reference is judged by what was AUTHORED, not by what it resolves to', () => {
+    // `tags` holds an array on this record; the reference itself is not one.
+    expect(m(RECORD, { tags: { $eq: { $field: 'tags' } } })).toBe(true);
+    expect(m(RECORD, { other: { $ne: { $field: 'status' } } })).toBe(true);
+    expect(m(RECORD, { status: { $ne: { $field: 'status' } } })).toBe(false);
+  });
+
+  it('a scalar comparand against a STORED array is untouched — the refusal is about the comparand', () => {
+    expect(m(RECORD, { tags: { $ne: 'a' } })).toBe(true);
+    expect(m(RECORD, { tags: 'a' })).toBe(false);
+  });
+});

packages/formula/src/matches-filter-empty-field-constraint.test.ts

@@ -109,9 +109,9 @@ describe('[#5240] matchesFilterCondition refuses a zero-operator field constrain
       expect(matchesFilterCondition(RECORD, { stage: { nested: 'won' } } as never)).toBe(false);
     });
 
-    it('a bare array field spec', () => {
-      expect(matchesFilterCondition(RECORD, { stage: ['won'] } as never)).toBe(false);
-    });
+    // [#19886] A bare array field spec is no longer in this group: it is
+    // REFUSED (INVALID_FILTER / 400), with `$eq` / `$ne` carrying an array —
+    // pinned in `matches-filter-array-comparand.test.ts`.
 
     it('an unknown top-level operator', () => {
       expect(matchesFilterCondition(RECORD, { $nope: 1 } as never)).toBe(false);

packages/formula/src/matches-filter.test.ts

@@ -125,8 +125,11 @@ describe('matchesFilterCondition — FAIL CLOSED', () => {
   it('nested relation object (non-$ key) → false', () => {
     expect(m(rec, { account: { region: 'EMEA' } } as never)).toBe(false);
   });
-  it('bare array value → false', () => {
-    expect(m(rec, { stage: ['won'] } as never)).toBe(false);
+  it('bare array value → REFUSED, not answered (#19886; the full pins are matches-filter-array-comparand.test.ts)', () => {
+    let err: { code?: string; status?: number } | undefined;
+    try { m(rec, { stage: ['won'] } as never); } catch (e) { err = e as { code?: string; status?: number }; }
+    expect(err?.code).toBe('INVALID_FILTER');
+    expect(err?.status).toBe(400);
   });
   it('malformed (array/scalar) filter → false', () => {
     expect(m(rec, [] as never)).toBe(false);

packages/formula/src/matches-filter.ts

@@ -63,6 +63,19 @@
  * where such a constraint sat under an `$or` beside a satisfied branch, or under
  * a `$not`, the old `false` was ABSORBED and the write was allowed. Those writes
  * now fail. See {@link emptyFieldConstraintError}.
+ *
+ * [#19886] A second shape is refused the same way: an ARRAY where a single
+ * comparable value is expected — under `$ne`, or in the equality position
+ * (`{ field: [...] }`, `{ field: { $eq: [...] } }`). It is the one position
+ * where this face's answer was not merely silent but the WRONG way round for a
+ * write gate: a strict comparison never equals an array, so `$ne` matched EVERY
+ * record, and a negated equality (`$not` around `{ field: [...] }`, which is
+ * what `!(record.f == [...])` lowers to) did too. On the ADR-0058 D4 `check`
+ * that admitted every write the policy was written to refuse, measured through
+ * the real `plugin-security` on three drivers. The query faces the read side
+ * runs on — driver-sql's unbindable-comparand refusal and driver-memory's
+ * array-comparand refusal — already refuse the shape with `INVALID_FILTER` /
+ * 400; this face now gives the same envelope. See {@link arrayComparandError}.
  */
 
 import type { FilterCondition } from '@objectstack/spec/data';
@@ -118,6 +131,83 @@ function emptyFieldConstraintError(field: string, path: string): Error {
   return err;
 }
 
+/**
+ * [#19886] An ARRAY where one comparable value is expected — under `$ne`, or in
+ * the equality position (a bare-array field spec, or `$eq`) — is REFUSED, not
+ * evaluated, with the envelope the other faces already give the same shape
+ * (`INVALID_FILTER` / 400).
+ *
+ * # Why refused rather than answered
+ *
+ * The answer this face used to give was unsafe on the surface it exists for.
+ * `looseEq` is a strict comparison, and no stored scalar is ever `===` an array,
+ * so:
+ *
+ * | shape                                   | old answer, every record |
+ * |:----------------------------------------|:-------------------------|
+ * | `{ f: { $ne: [...] } }`                 | `true`                   |
+ * | `{ f: [...] }` / `{ f: { $eq: [...] } }`| `false`                  |
+ * | `{ $not: { f: [...] } }`                | `true`                   |
+ *
+ * An RLS `check` is authored as CEL, and `record.f != ['a', 'b']` (or `!=`
+ * against a `current_user` membership array) lowers to the first row, so every
+ * write the policy was written to refuse was admitted and stored. The positive
+ * equality row only failed closed by accident, and inverted the moment it was
+ * negated. There is no answer here that is right in every polarity — which is
+ * the #5240 argument for refusing rather than choosing.
+ *
+ * # What stays exactly as it was
+ *
+ * `$in` / `$nin` (the list operators — this is what they are FOR), scalars,
+ * `null`, `Date`, and `{ $field }` references. The refusal reads the AUTHORED
+ * comparand, never a resolved one: a `{ $field }` reference whose column
+ * happens to hold an array is untouched.
+ *
+ * # Why the message names nothing from the filter
+ *
+ * This face's callers evaluate access policies — the write gate's `check`, and
+ * the explain engine's record attribution — and the caller who receives the
+ * 400 is usually not the author of the predicate. The comparand may be a
+ * resolved membership set (other users' ids), which must not be echoed to them.
+ * So the field, the operator and the value are withheld, the posture
+ * driver-sql's withheld-diagnostic ruling took for the same reason, and the
+ * message carries the refusal's identity and the remedy only.
+ */
+function arrayComparandError(): Error {
+  const err = new Error(
+    'A single-value comparison in this filter received an array as its comparand: an array ' +
+      'under "$ne", or an array in the equality position ({ "field": [ ... ] } or "$eq"). A list ' +
+      'is not one comparable value. For "one of these values" use "$in", and for "none of these ' +
+      'values" use "$nin" — the list operators the filter protocol declares. It is refused before ' +
+      'any record is judged rather than evaluated, because this evaluator compares strictly and ' +
+      'no stored value ever equals an array: "$ne" matched EVERY record, and so did a negated ' +
+      'equality, which on a row-level write check admitted every write the check was written to ' +
+      'refuse. The field, the operator and the value are withheld from this message because the ' +
+      'filter may be an access policy the caller did not write; in a row-level policy, look for ' +
+      'a "!=" or "==" compared against a list literal or a current_user membership key, and ' +
+      'rewrite it with "in" (for example "!(record.status in [\'closed\', \'archived\'])").',
+  ) as Error & { code?: string; status?: number };
+  err.code = StandardErrorCode.enum.INVALID_FILTER;
+  err.status = 400;
+  return err;
+}
+
+/**
+ * [#19886] The operators whose array comparand this face refuses: exactly the
+ * two the refusal was ruled for — the equality slot and its negation. The
+ * ordering operators (`$gt` / `$gte` / `$lt` / `$lte`) are deliberately NOT in
+ * this list; what they do with an array is a separate question this refusal
+ * does not answer.
+ */
+const ARRAY_REFUSED_OPERATORS = ['$eq', '$ne'] as const;
+
+/** A plain object — an operator map rather than a comparand (`Date` is a comparand). */
+function isOperatorMap(spec: unknown): spec is Record<string, unknown> {
+  if (spec === null || typeof spec !== 'object' || Array.isArray(spec) || spec instanceof Date) return false;
+  const proto = Object.getPrototypeOf(spec);
+  return proto === Object.prototype || proto === null;
+}
+
 /** True iff `record` satisfies `filter`. A null/empty filter matches everything. */
 export function matchesFilterCondition(record: Record<string, unknown>, filter: FilterCondition | null | undefined): boolean {
   if (filter == null) return true;
@@ -134,9 +224,11 @@ export function matchesFilterCondition(record: Record<string, unknown>, filter:
 
 /**
  * [#5240] Walk the whole condition tree and refuse any zero-operator field
- * constraint. Shapes this evaluator already answers fail-closed (a non-node
- * `$and` element, an unknown `$`-operator, a bare array field spec) are left to
- * it — this walk adds exactly one refusal and changes nothing else.
+ * constraint. [#19886] The same walk refuses an array comparand under `$ne`
+ * or in the equality position ({@link arrayComparandError}), at any depth under
+ * `$and` / `$or` / `$not`. Shapes this evaluator already answers fail-closed in
+ * every polarity (a non-node `$and` element, an unknown `$`-operator) are left
+ * to it — this walk adds those two refusals and changes nothing else.
  */
 function assertFilterShape(node: unknown, path: string): void {
   if (node == null || typeof node !== 'object' || Array.isArray(node)) return;
@@ -152,6 +244,14 @@ function assertFilterShape(node: unknown, path: string): void {
     }
     if (key.startsWith('$')) continue;
     if (isEmptyFieldConstraint(val)) throw emptyFieldConstraintError(key, here);
+    // [#19886] The equality position, spelled bare: `{ field: [...] }`.
+    if (Array.isArray(val)) throw arrayComparandError();
+    // [#19886] …and spelled with an operator: `$eq` / `$ne` carrying an array.
+    if (isOperatorMap(val)) {
+      for (const op of ARRAY_REFUSED_OPERATORS) {
+        if (Array.isArray(val[op])) throw arrayComparandError();
+      }
+    }
   }
 }
 
@@ -195,6 +295,9 @@ function evalField(record: Record<string, unknown>, field: string, spec: unknown
   // Scalar / Date → implicit equality.
   if (typeof spec !== 'object' || spec instanceof Date) return looseEq(actual, spec);
   // A bare array value is not a valid field spec (must be `{ $in: [...] }`).
+  // [#19886] Refused up front by `assertFilterShape` on the public entry point,
+  // so this arm is a floor for a recursive call on a subtree, not this face's
+  // answer to the shape — the same standing as the `keys.length === 0` arm below.
   if (Array.isArray(spec)) return false;
 
   const ops = spec as Record<string, unknown>;