fix(driver-sql,driver-memory): an uncompilable filter throws instead of matching everything (#3948)

.changeset/filter-no-silent-drop.md

@@ -0,0 +1,42 @@
+---
+"@objectstack/driver-memory": minor
+"@objectstack/driver-sql": patch
+---
+
+fix(driver-sql,driver-memory): an uncompilable filter now throws instead of matching everything (#3948)
+
+A filter the driver could not compile was **skipped**, not rejected. No predicate
+was emitted and the query returned every row — the caller asked to filter and
+silently received the unfiltered set.
+
+The reachable shape is a bare comparison triple. `['close_date','before','2024-01-01']`
+arrives at a driver only when `isFilterAST()` refused it — its operator is outside
+`VALID_AST_OPERATORS`, so `parseFilterAST()` never converted it and the raw array
+was assigned to `where`. `driver-sql`'s loop then saw three *strings*, matched
+neither `and` nor `or`, and `continue`d past all three. `driver-memory` was worse:
+it cast every string to a logic keyword, opening three empty groups and returning
+`{}` — a filter matching every record.
+
+This is reachable from ordinary authoring, not just malformed input: `before` and
+`after` are canonical `VIEW_FILTER_OPERATORS` members that `VALID_AST_OPERATORS`
+does not accept. Eight of the nineteen canonical view operators are in that
+position, including `equals`; the others were masked only because ObjectUI's
+adapter alias table happened to cover them.
+
+**Behaviour change.** Both drivers now throw on a filter element that is neither a
+logical keyword (`and`/`or`) nor a condition array, and `driver-memory` throws on
+an operator it cannot express rather than dropping the condition. The nested and
+`$`-object paths already threw on the same input, so this makes the three paths
+agree. A caller that was relying on the old silence was receiving wrong results;
+the error names the operator and the offending filter.
+
+**`driver-memory` also gains seven operators it silently ignored:** `not_in`,
+`is_null`, `is_not_null`, `isnull`, `isnotnull`, `is_empty`, `is_not_empty` — all
+members of `VALID_AST_OPERATORS`, all previously falling through to
+`default: return null`. `is_null` narrowed nothing instead of matching null rows.
+Alias sets and semantics mirror `driver-sql`'s `whereNull`/`whereNotNull` arms so
+the two backends accept one vocabulary.
+
+Migration: none for well-formed filters. If a query now throws, the filter was
+never being applied — fix the operator (the message names it), or lower it to an
+AST spelling. `before` → `<`, `after` → `>`, `'not in'` → `nin`.

packages/plugins/driver-memory/src/memory-driver.ts

@@ -703,15 +703,40 @@ export class InMemoryDriver implements IDataDriver {
 
     for (const item of filters) {
       if (typeof item === 'string') {
-        const newLogic = item.toLowerCase() as 'and' | 'or';
-        if (newLogic !== currentLogic) {
-          currentLogic = newLogic;
+        const lower = item.toLowerCase();
+        // Previously this cast ANY string to 'and' | 'or'. A bare comparison
+        // triple — which reaches a driver only when `isFilterAST()` refused its
+        // operator, leaving the array unparsed — therefore opened three empty
+        // logic groups, produced no conditions, and returned `{}`: a filter that
+        // matches EVERY record. An unapplied filter must not look like a
+        // satisfied one. #3948.
+        if (lower !== 'and' && lower !== 'or') {
+          throw new Error(
+            `[driver-memory] Unrecognized filter operator "${item}" in a comparison triple. ` +
+              `A filter array is either a logical node (["and"|"or", …]) or nested ` +
+              `conditions ([[field, op, value], …]); a bare [field, op, value] only ` +
+              `reaches the driver when its operator is outside @objectstack/spec ` +
+              `VALID_AST_OPERATORS, which leaves the filter unparsed. ` +
+              `Filter was: ${JSON.stringify(filters)}`,
+          );
+        }
+        if (lower !== currentLogic) {
+          currentLogic = lower;
           logicGroups.push({ logic: currentLogic, conditions: [] });
         }
       } else if (Array.isArray(item)) {
         const [field, operator, value] = item;
+        // `convertConditionToMongo` now throws rather than returning null for an
+        // operator it cannot express, so a dropped condition can no longer
+        // silently widen the result set.
         const cond = this.convertConditionToMongo(field, operator, value);
         if (cond) logicGroups[logicGroups.length - 1].conditions.push(cond);
+      } else {
+        throw new Error(
+          `[driver-memory] Unrecognized filter element of type ` +
+            `"${item === null ? 'null' : typeof item}" — expected a logical keyword ` +
+            `("and"/"or") or a condition array. Filter was: ${JSON.stringify(filters)}`,
+        );
       }
     }
 
@@ -750,23 +775,46 @@ export class InMemoryDriver implements IDataDriver {
         return { [field]: { $lte: value } };
       case 'in':
         return { [field]: { $in: value } };
-      case 'nin': case 'not in':
+      case 'nin': case 'not_in': case 'notin': case 'not in':
         return { [field]: { $nin: value } };
-      case 'contains': case 'like':
+      case 'contains': case 'like': case 'ilike':
         return { [field]: { $regex: new RegExp(this.escapeRegex(value), 'i') } };
       case 'notcontains': case 'not_contains':
         return { [field]: { $not: { $regex: new RegExp(this.escapeRegex(value), 'i') } } };
       case 'startswith': case 'starts_with':
         return { [field]: { $regex: new RegExp(`^${this.escapeRegex(value)}`, 'i') } };
       case 'endswith': case 'ends_with':
         return { [field]: { $regex: new RegExp(`${this.escapeRegex(value)}$`, 'i') } };
+      // Null / empty predicates. These are in `VALID_AST_OPERATORS` and were
+      // absent here, so every one of them fell to `default: return null` and was
+      // dropped — `is_null` narrowed nothing instead of matching null rows.
+      // Alias sets and semantics mirror driver-sql's `whereNull`/`whereNotNull`
+      // arms so both backends accept the same vocabulary. In a document store
+      // `{field: null}` matches null AND missing, and `$ne: null` excludes both,
+      // which is the right analogue of SQL IS [NOT] NULL. #3948.
+      case 'is_null': case 'isnull': case 'is_empty': case 'isempty': case 'empty':
+        return { [field]: null };
+      case 'is_not_null': case 'isnotnull':
+      case 'is_not_empty': case 'isnotempty': case 'not_empty': case 'notempty':
+      case 'is_set': case 'set':
+        return { [field]: { $ne: null } };
       case 'between':
         if (Array.isArray(value) && value.length === 2) {
           return { [field]: { $gte: value[0], $lte: value[1] } };
         }
-        return null;
+        throw new Error(
+          `[driver-memory] "between" on field "${field}" needs a two-element array, got ` +
+            `${JSON.stringify(value)}. Returning no predicate would silently match every record.`,
+        );
       default:
-        return null;
+        // Was `return null`, which the caller dropped — so an operator this
+        // driver cannot express narrowed nothing instead of erroring. driver-sql
+        // already threw on the same input; the two backends disagreed. #3948.
+        throw new Error(
+          `[driver-memory] Unsupported filter operator "${operator}" on field "${field}". ` +
+            `Supported operators: =, !=, <, <=, >, >=, in, nin, between, contains, ` +
+            `not_contains, starts_with, ends_with (see @objectstack/spec VALID_AST_OPERATORS).`,
+        );
     }
   }
 

packages/plugins/driver-memory/src/memory-filter-ast-vocabulary.test.ts

@@ -0,0 +1,102 @@
+/**
+ * Filter-AST vocabulary parity, and no-silent-drop. (#3948)
+ *
+ * Two invariants, both of which this driver used to break in the same direction:
+ *
+ *  1. Every operator in `VALID_AST_OPERATORS` must be expressible. That set gates
+ *     `isFilterAST()`, so anything in it is an operator the protocol will happily
+ *     parse and hand to a driver. Seven of them — `not_in`, `is_null`,
+ *     `is_not_null`, `isnull`, `isnotnull`, `is_empty`, `is_not_empty` — fell to
+ *     `default: return null` and the caller dropped the condition, so e.g.
+ *     `is_null` narrowed nothing instead of matching null rows.
+ *
+ *  2. An operator this driver cannot express must THROW, never be skipped. A
+ *     dropped condition widens the result set: the caller asked to filter and
+ *     silently received more rows than it asked for. driver-sql already threw on
+ *     the same input, so the two backends disagreed about the same query.
+ *
+ * The nastiest case is the bare comparison triple. `['x','before',1]` reaches a
+ * driver only when `isFilterAST()` refused it, leaving the array unparsed — and
+ * the old loop cast each string element to a logic keyword, opening empty logic
+ * groups and returning `{}`: a filter matching EVERY record.
+ */
+import { describe, it, expect, beforeEach } from 'vitest';
+import { VALID_AST_OPERATORS } from '@objectstack/spec/data';
+import { InMemoryDriver } from './memory-driver.js';
+
+const TABLE = 'vocab_probe';
+
+describe('InMemoryDriver filter vocabulary ↔ VALID_AST_OPERATORS', () => {
+  let driver: InMemoryDriver;
+
+  beforeEach(async () => {
+    driver = new InMemoryDriver({ persistence: false });
+    await driver.connect();
+    await driver.create(TABLE, { id: '1', name: 'alpha', score: 10, note: null });
+    await driver.create(TABLE, { id: '2', name: 'beta', score: 20, note: 'set' });
+  });
+
+  /** Operators are exercised through `find`, the path a real query takes. */
+  const find = (where: unknown) =>
+    driver.find(TABLE, { object: TABLE, fields: ['id'], where } as any);
+
+  it('reads a non-empty operator set from the spec', () => {
+    // Guards every assertion below from passing vacuously.
+    expect(VALID_AST_OPERATORS.size).toBeGreaterThan(0);
+  });
+
+  /** A representative value per operator, so each one is actually exercised. */
+  const valueFor = (op: string): unknown => {
+    if (op === 'in' || op === 'nin' || op === 'not_in') return ['alpha'];
+    if (op === 'between') return [0, 100];
+    if (/null|empty/.test(op)) return true;
+    if (/contains|like|startswith|starts_with|endswith|ends_with/.test(op)) return 'alp';
+    return 'alpha';
+  };
+
+  it.each([...VALID_AST_OPERATORS])('expresses %s without dropping it', async (op) => {
+    const field = /^[<>=!]/.test(op) || op === 'between' ? 'score' : 'name';
+    const value = field === 'score' && !Array.isArray(valueFor(op)) ? 10 : valueFor(op);
+    // The assertion is that this does not throw and does not silently degrade to
+    // "no predicate". An operator the driver cannot express now throws, so any
+    // rejection here means the spec accepts a name this driver cannot honour.
+    await expect(
+      find([[field, op, value]]),
+      `VALID_AST_OPERATORS accepts "${op}" but InMemoryDriver cannot express it`,
+    ).resolves.toBeDefined();
+  });
+
+  it('matches null rows for is_null instead of dropping the predicate', async () => {
+    // The regression this pins: `is_null` used to return null from the converter,
+    // the condition was dropped, and the query returned BOTH rows.
+    const rows = await find([['note', 'is_null', true]]);
+    expect(rows.map((r: any) => r.id)).toEqual(['1']);
+  });
+
+  it('matches non-null rows for is_not_null', async () => {
+    const rows = await find([['note', 'is_not_null', true]]);
+    expect(rows.map((r: any) => r.id)).toEqual(['2']);
+  });
+
+  it('throws on an operator it cannot express, rather than matching everything', async () => {
+    await expect(find([['name', 'sounds_like', 'alpha']]))
+      .rejects.toThrow(/Unsupported filter operator "sounds_like"/);
+  });
+
+  it('throws on a bare comparison triple instead of returning every record', async () => {
+    // `before` is a canonical VIEW_FILTER_OPERATORS member that VALID_AST_OPERATORS
+    // does not accept, so this is the exact shape that reached drivers unparsed.
+    await expect(find(['created_at', 'before', '2024-01-01']))
+      .rejects.toThrow(/Unrecognized filter operator "created_at"/);
+  });
+
+  it('throws on a malformed between rather than emitting no predicate', async () => {
+    await expect(find([['score', 'between', 5]]))
+      .rejects.toThrow(/needs a two-element array/);
+  });
+
+  it('still honours a well-formed logical node', async () => {
+    const rows = await find(['or', ['name', '=', 'alpha'], ['name', '=', 'beta']]);
+    expect(rows.map((r: any) => r.id).sort()).toEqual(['1', '2']);
+  });
+});

packages/plugins/driver-sql/src/sql-driver-filter-no-silent-drop.test.ts

@@ -0,0 +1,99 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * A filter that cannot be compiled must THROW, never be skipped. (#3948)
+ *
+ * `applyFilters` walked the filter array and `continue`d past anything that was
+ * not a join keyword or a condition array. For a BARE comparison triple —
+ * `['close_date', 'before', '2024-01-01']` — the three elements are all strings,
+ * so every one was skipped and **no WHERE clause was emitted at all**: the caller
+ * asked to filter and silently received every row.
+ *
+ * A bare triple reaches a driver only when `isFilterAST()` refused it, i.e. its
+ * operator is outside `VALID_AST_OPERATORS`, so `parseFilterAST()` never
+ * converted it and the raw array arrived as `where`. That is reachable from
+ * ordinary authoring: `before`/`after` are canonical `VIEW_FILTER_OPERATORS`
+ * members which `VALID_AST_OPERATORS` does not accept.
+ *
+ * The nested and `$`-object paths already threw on the same class of input, so
+ * the three code paths disagreed about one query. These tests pin the loud
+ * behaviour, and pin that well-formed filters still compile.
+ */
+
+import { describe, it, expect, beforeEach } from 'vitest';
+import { SqlDriver } from '../src/index.js';
+
+describe('SqlDriver rejects an uncompilable filter instead of dropping it', () => {
+  let driver: SqlDriver;
+
+  beforeEach(async () => {
+    driver = new SqlDriver({
+      client: 'better-sqlite3',
+      connection: { filename: ':memory:' },
+      useNullAsDefault: true,
+    });
+
+    await driver.initObjects([
+      {
+        name: 'deal',
+        fields: {
+          id: { type: 'text', name: 'id' },
+          stage: { type: 'text', name: 'stage' },
+          amount: { type: 'number', name: 'amount' },
+        },
+      } as any,
+    ]);
+
+    await driver.create('deal', { id: '1', stage: 'won', amount: 10 });
+    await driver.create('deal', { id: '2', stage: 'lost', amount: 20 });
+  });
+
+  const find = (where: unknown) =>
+    driver.find('deal', { object: 'deal', fields: ['id'], where } as any);
+
+  it('throws on a bare triple whose operator the AST gate refused', async () => {
+    // The exact shape a stored single-condition `before` view produced.
+    await expect(find(['close_date', 'before', '2024-01-01']))
+      .rejects.toThrow(/Unrecognized filter operator "close_date"/);
+  });
+
+  it('does not silently return every row for that filter', async () => {
+    // The regression itself: before the fix this resolved with BOTH rows.
+    await expect(find(['stage', 'sounds_like', 'won'])).rejects.toThrow();
+  });
+
+  it('throws on a filter element that is neither a keyword nor a condition', async () => {
+    await expect(find([42 as any])).rejects.toThrow(/Unrecognized filter element of type "number"/);
+    await expect(find([null as any])).rejects.toThrow(/Unrecognized filter element of type "null"/);
+  });
+
+  it('still throws on an unsupported operator inside a well-formed condition', async () => {
+    // Pre-existing behaviour, pinned so the two paths cannot diverge again.
+    await expect(find([['stage', 'sounds_like', 'won']]))
+      .rejects.toThrow(/Unsupported filter operator "sounds_like"/);
+  });
+
+  it('compiles a nested condition array', async () => {
+    const rows = await find([['stage', '=', 'won']]);
+    expect(rows.map((r: any) => r.id)).toEqual(['1']);
+  });
+
+  it('compiles an infix logical join', async () => {
+    // This path's legacy array form is INFIX — `[condA, 'or', condB]`. The
+    // prefix spec-AST form `['or', condA, condB]` reaches the driver already
+    // converted to `{$or: […]}` by `parseFilterAST()`, so it takes the
+    // object branch instead and never lands here.
+    const rows = await find([['stage', '=', 'won'], 'or', ['stage', '=', 'lost']]);
+    expect(rows.map((r: any) => r.id).sort()).toEqual(['1', '2']);
+  });
+
+  it('compiles an object-form filter', async () => {
+    const rows = await find({ stage: 'lost' });
+    expect(rows.map((r: any) => r.id)).toEqual(['2']);
+  });
+
+  it('leaves an empty filter alone (no filter is not a failed filter)', async () => {
+    const rows = await find([]);
+    expect(rows).toHaveLength(2);
+  });
+});

packages/plugins/driver-sql/src/sql-driver.ts

@@ -4641,9 +4641,23 @@ export class SqlDriver implements IDataDriver {
 
     for (const item of filters) {
       if (typeof item === 'string') {
-        if (item.toLowerCase() === 'or') nextJoin = 'or';
-        else if (item.toLowerCase() === 'and') nextJoin = 'and';
-        continue;
+        const lower = item.toLowerCase();
+        if (lower === 'or') { nextJoin = 'or'; continue; }
+        if (lower === 'and') { nextJoin = 'and'; continue; }
+        // Anything else is not a join keyword, and the only way a bare string
+        // reaches here is a comparison triple that `isFilterAST()` refused —
+        // its operator is outside `VALID_AST_OPERATORS`, so `parseFilterAST()`
+        // never converted it and the raw array arrived as `where`. Skipping it
+        // (the old behaviour) emitted NO predicate at all: the caller asked to
+        // filter and silently got every row. Fail loudly instead. #3948.
+        throw new Error(
+          `[sql-driver] Unrecognized filter operator "${item}" in a comparison triple. ` +
+            `A filter array is either a logical node (["and"|"or", …]) or nested ` +
+            `conditions ([[field, op, value], …]); a bare [field, op, value] only ` +
+            `reaches the driver when its operator is outside @objectstack/spec ` +
+            `VALID_AST_OPERATORS, which leaves the filter unparsed. ` +
+            `Filter was: ${JSON.stringify(filters)}`,
+        );
       }
 
       if (Array.isArray(item)) {
@@ -4666,7 +4680,18 @@ export class SqlDriver implements IDataDriver {
         }
 
         nextJoin = 'and';
+        continue;
       }
+
+      // Neither a join keyword nor a condition. Previously fell out of both
+      // branches and was dropped, so a malformed element silently narrowed
+      // nothing. Same reasoning as above: an unapplied filter must not look
+      // like a satisfied one. #3948.
+      throw new Error(
+        `[sql-driver] Unrecognized filter element of type "${item === null ? 'null' : typeof item}" — ` +
+          `expected a logical keyword ("and"/"or") or a condition array. ` +
+          `Filter was: ${JSON.stringify(filters)}`,
+      );
     }
   }