From 25a12790dcee02d8daff8cc38af7946519f043bf Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 01:47:52 +0000 Subject: [PATCH] fix(driver-sql,driver-memory,driver-mongodb): refuse out-of-contract filter input at the door (#5347, #5348) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two shapes the Filter Protocol never declared reached the drivers, and every driver ANSWERED them — with a different answer. Both are now refused with INVALID_FILTER / 400, on the validating walk rather than in the emitter. #5347 — `$null` with a non-boolean comparand. `FieldOperatorsSchema` declares `$null: z.boolean()`. Measured against one row with `stage: 'won'` and one with `stage: null`, `{ stage: { $null: 'yes' } }` returned the NULL row on driver-sql / driver-sqlite-wasm / Turso local (IS NULL — anything but `false`), the valued row on driver-memory's query path and driver-mongodb (IS NOT NULL — anything but `true`), and BOTH rows through driver-memory's reference matcher, whose two conditionals a third value satisfies neither of, so the constraint vanished. Three readings of one declared operator; the third is new evidence the issue's own fixture could not show. Refused on all four backends per the ruling. #5348 — an undeclared `$op` in a node position. `FilterConditionSchema` declares three `$`-keys at a node; driver-sql compiled the rest as COLUMNS, so `{ $where: … }` / `{ $nor: … }` produced a predicate matching nothing and reporting nothing. Its FIELD position had refused the same class of input since #3948/#4436, so one driver answered two ways depending on depth. Both gates sit in `reduceFilterKey` / `assertFilterConditionShape`, not in the emitters, because the emitters are skipped wholesale by a boolean identity — `{ $or: [ {}, { $where: … } ] }` would otherwise be refused or ignored depending on its siblings. Same placement argument as #5240/#5327. `nullValueSatisfiesOperator`'s `$null` arm is tightened from `value !== false` to `value === true`: the two are equivalent only while the refusal holds, and the lenient spelling would silently resume answering if the gate ever moved. `$exists` keeps its lenient read deliberately — it has no comparand gate, so tightening it alone would create the divergence rather than close one. driver-sqlite-wasm and cloud's local/replica TursoDriver inherit both refusals from SqlDriver; both verified by execution, not assumed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Pbu27iNUfQCHeuS551Rqo7 --- .../refuse-out-of-contract-filter-input.md | 60 ++++ .../driver-memory/src/filter-refusal.ts | 53 ++++ .../driver-memory/src/memory-driver.ts | 11 + .../driver-memory/src/memory-matcher.ts | 10 + .../src/memory-null-comparand-refusal.test.ts | 186 +++++++++++++ .../driver-mongodb/src/mongodb-filter.ts | 100 ++++++- .../mongodb-null-comparand-refusal.test.ts | 116 ++++++++ ...river-out-of-contract-filter-input.test.ts | 260 ++++++++++++++++++ packages/plugins/driver-sql/src/sql-driver.ts | 158 ++++++++++- ...-wasm-out-of-contract-filter-input.test.ts | 114 ++++++++ 10 files changed, 1058 insertions(+), 10 deletions(-) create mode 100644 .changeset/refuse-out-of-contract-filter-input.md create mode 100644 packages/plugins/driver-memory/src/memory-null-comparand-refusal.test.ts create mode 100644 packages/plugins/driver-mongodb/src/mongodb-null-comparand-refusal.test.ts create mode 100644 packages/plugins/driver-sql/src/sql-driver-out-of-contract-filter-input.test.ts create mode 100644 packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-out-of-contract-filter-input.test.ts diff --git a/.changeset/refuse-out-of-contract-filter-input.md b/.changeset/refuse-out-of-contract-filter-input.md new file mode 100644 index 0000000000..b7fec8567b --- /dev/null +++ b/.changeset/refuse-out-of-contract-filter-input.md @@ -0,0 +1,60 @@ +--- +"@objectstack/driver-sql": patch +"@objectstack/driver-memory": patch +"@objectstack/driver-mongodb": patch +--- + +fix(driver-sql,driver-memory,driver-mongodb): refuse out-of-contract filter input at the door instead of answering it differently per backend (#5347, #5348) + +Two shapes the Filter Protocol never declared were reaching the drivers, and +every driver ANSWERED them — with a different answer. Both are now refused with +`INVALID_FILTER` / 400, in the ADR-0112 envelope every sibling filter refusal +already speaks. + +## `$null` with a non-boolean comparand — a behaviour change you can observe + +`FieldOperatorsSchema` declares `$null: z.boolean()`. A non-boolean was read by +default branches hung on opposite sides, so one filter meant opposite things per +backend. Measured against one row with `stage: 'won'` (id 1) and one with +`stage: null` (id 2), on `{ stage: { $null: 'yes' } }`: + +| backend | read as | rows | +|---|---|---| +| driver-sql, driver-sqlite-wasm, Turso local | IS NULL (anything but `false`) | `["2"]` | +| driver-memory query path, driver-mongodb | IS NOT NULL (anything but `true`) | `["1"]` | +| driver-memory reference matcher | no constraint at all | `["1","2"]` | + +**What changes for you:** a caller that today gets rows back for +`{ field: { $null: } }` now gets a `400 INVALID_FILTER` naming the +operator, the field and the position. That includes calls working by truthy / +falsy coincidence — and the sharpest case is the STRING `"false"`, which is +truthy: it compiled to IS NULL on SQL and IS NOT NULL on the JS backends, i.e. +the opposite of what its author wrote it to mean, on at least one of them +whichever they meant. A JSON round-trip or generated metadata produces it +readily. + +**The fix:** write the boolean. `{ field: { $null: true } }` for "has no value", +`{ field: { $null: false } }` for "has a value". Both are unchanged, on all four +backends, and so is every other operator. `$exists` is deliberately NOT tightened +here — it diverges on its own axis (what "exists" means for a null-valued key) +and is tracked separately. + +## An undeclared `$op` in a document position — silent empty set becomes a 400 + +`FilterConditionSchema` declares exactly three `$`-keys at a node +(`$and` / `$or` / `$not`); every other key is a field name. `driver-sql` +compiled the rest as COLUMNS, so `{ $where: '…' }`, `{ $nor: […] }`, +`{ $expr: … }` produced a predicate that matched nothing and reported nothing — +a caller could not tell "no rows matched" from "the filter never compiled". The +FIELD position had refused the same class of input since v16, so one driver gave +two answers depending on depth. + +**What changes for you:** those filters now raise `400 INVALID_FILTER` instead of +returning `[]`. `driver-memory` already refused them; this brings `driver-sql` +(and `driver-sqlite-wasm`, which inherits it) into line. The three declared +combinators, their boolean identities (`$and: []` is TRUE, `$or: []` is FALSE) +and every legal filter compile byte-identically. + +Both refusals are raised on the driver's validating walk rather than in its SQL +emitter, so a malformed node is refused regardless of whether a sibling +disjunct would have short-circuited the compile. diff --git a/packages/plugins/driver-memory/src/filter-refusal.ts b/packages/plugins/driver-memory/src/filter-refusal.ts index 2f1a173274..bc52ad0c17 100644 --- a/packages/plugins/driver-memory/src/filter-refusal.ts +++ b/packages/plugins/driver-memory/src/filter-refusal.ts @@ -259,6 +259,52 @@ export function malformedBetweenError(field: string, value: unknown, path: strin ); } +/** + * [#5347] `$null` whose comparand is not a boolean. + * + * `FieldOperatorsSchema` declares `$null: z.boolean()`, and nothing between an + * authored `where` and a driver validates against it — so a non-boolean really + * arrives. Every backend then read it, and they did NOT agree; measured on one + * row with `stage: 'won'` and one with `stage: null`, on `{ stage: { $null: 'yes' } }`: + * + * | backend | read as | rows | + * |---|---|---| + * | driver-sql / driver-sqlite-wasm / Turso local | IS NULL (anything but `false`) | the NULL row | + * | THIS driver's live path (mingo), driver-mongodb | IS NOT NULL (anything but `true`) | the valued row | + * | THIS driver's reference matcher | nothing at all — the constraint vanished | BOTH rows | + * + * Note the last line: this package's own two faces disagreed with EACH OTHER, + * which #5347 could not see because it measured a fixture with no null-valued + * row — there the matcher's "match everything" and mingo's "IS NOT NULL" + * coincide. The matcher's `$null` arm is written as two conditionals + * (`target === true && …`, `target === false && …`); a third value satisfies + * neither, so the operator silently stopped constraining anything. That is the + * #5240 / #5328 shape exactly — one filter, one package, two answers, and the + * widening one is a permission bypass on a read scope. + * + * Ruled on #5347: REFUSED everywhere, the same disposition `{ field: {} }` got + * and for the same reason — there is no reading of a non-boolean here that is + * not a guess about the author's intent. The string `"false"` is the sharpest + * case: it is truthy, so it landed on the opposite side from the `false` it was + * written to mean, and it is exactly what an AI-authored or JSON-round-tripped + * scope produces. + * + * The leading sentence is `driver-sql`'s, verbatim — one condition, one wording + * (#5240). + */ +export function nonBooleanNullComparandError(field: string, value: unknown, path: string): Error { + return unsupportedFilterError( + `Operator "$null" on field "${field}" requires a boolean comparand (true or false). ` + + `Received ${describeFilterOperand(value)} (${safeShapePreview(value)}) at ${path}. ` + + `@objectstack/spec FieldOperatorsSchema declares $null as a boolean. It is refused rather ` + + `than coerced because the backends read a non-boolean in OPPOSITE directions — driver-sql ` + + `compiled IS NULL (anything but false), this driver's query path and driver-mongodb ` + + `compiled IS NOT NULL (anything but true), and this driver's matcher dropped the ` + + `constraint entirely. Note "false" the STRING is truthy, so it landed on the side opposite ` + + `the false it was written to mean (#5347).`, + ); +} + /** * [#5324] `$options` without the `$regex` it modifies. * @@ -386,6 +432,13 @@ function assertFieldConstraintShape(field: string, spec: unknown, path: string): if (op === '$between' && !isBetweenComparand(spec[op])) { throw malformedBetweenError(field, spec[op], `${path}.$between`); } + // [#5347] `$null`'s comparand is a boolean by declaration. It joins + // `$between`'s arity as the second COMPARAND-shape check this gate makes, + // and for the identical reason: a shape the operator cannot evaluate was + // being answered silently, differently, by each face. + if (op === '$null' && typeof spec[op] !== 'boolean') { + throw nonBooleanNullComparandError(field, spec[op], `${path}.$null`); + } } // `$options` is the one entry in the vocabulary that is a modifier rather than // a predicate, so it is the one that needs a companion. diff --git a/packages/plugins/driver-memory/src/memory-driver.ts b/packages/plugins/driver-memory/src/memory-driver.ts index df8f57d61b..d3a08a05e3 100644 --- a/packages/plugins/driver-memory/src/memory-driver.ts +++ b/packages/plugins/driver-memory/src/memory-driver.ts @@ -12,6 +12,7 @@ import { filterNodeExpectedError, filterNodeListExpectedError, malformedBetweenError, + nonBooleanNullComparandError, unknownFieldOperatorError, unknownLogicalOperatorError, unsupportedFilterError, @@ -973,6 +974,16 @@ export class InMemoryDriver implements IDataDriver { case '$null': // $null: true → field is null, $null: false → field is not null // Use $eq/$ne null for Mingo compatibility + // + // [#5347] The arm used to be a two-branch `if/else` on `val === true`, + // so EVERY non-boolean comparand fell to the `else` and compiled + // `$ne: null` — IS NOT NULL. `driver-sql` hung its default on the + // opposite side (`opValue === false` → IS NULL) and the reference + // matcher on neither (the constraint vanished), so one declared + // operator had three readings. The shape gate refuses a non-boolean + // now; this throw is the totality floor, the same one `$between` + // keeps beside it. + if (typeof val !== 'boolean') throw nonBooleanNullComparandError(field, val, `${path}.$null`); if (val === true) { result.$eq = null; } else { diff --git a/packages/plugins/driver-memory/src/memory-matcher.ts b/packages/plugins/driver-memory/src/memory-matcher.ts index dc55268453..c6f8509a5f 100644 --- a/packages/plugins/driver-memory/src/memory-matcher.ts +++ b/packages/plugins/driver-memory/src/memory-matcher.ts @@ -193,6 +193,16 @@ function checkCondition(value: any, condition: any): boolean { break; case '$null': // $null: true → value must be null/undefined; $null: false → value must not be null/undefined + // + // [#5347] These two conditionals are EXHAUSTIVE now: the shape + // gate refuses a non-boolean `target` before evaluation starts. + // They were not, and that is what the issue's fixture could not + // see. A third value satisfied neither test, so the operator + // matched EVERY row — while the live query path compiled the + // same filter to IS NOT NULL and driver-sql to IS NULL. This + // face's answer was the widening one, which on an RLS read scope + // is a permission bypass, not a degraded filter (#3948, and the + // identical `$between` note above). if (target === true && value != null) return false; if (target === false && value == null) return false; break; diff --git a/packages/plugins/driver-memory/src/memory-null-comparand-refusal.test.ts b/packages/plugins/driver-memory/src/memory-null-comparand-refusal.test.ts new file mode 100644 index 0000000000..050ed5d285 --- /dev/null +++ b/packages/plugins/driver-memory/src/memory-null-comparand-refusal.test.ts @@ -0,0 +1,186 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5347] `$null` takes a boolean. A non-boolean is refused on BOTH faces. + * + * # What was measured + * + * `FieldOperatorsSchema` declares `$null: z.boolean()`, and nothing between an + * authored `where` and a driver validates against it. Given one row with + * `stage: 'won'` (id 1) and one with `stage: null` (id 2), the filter + * `{ stage: { $null: 'yes' } }` produced THREE answers: + * + * | face | compiled to | rows | + * |---|---|---| + * | `driver-sql` / `driver-sqlite-wasm` / Turso local | `IS NULL` (anything but `false`) | `["2"]` | + * | this driver's live path (mingo), `driver-mongodb` | `IS NOT NULL` (anything but `true`) | `["1"]` | + * | this driver's reference matcher | nothing at all | `["1","2"]` | + * + * The third row is the one #5347 could not see. It measured a fixture with no + * null-valued row, where "matches every row" and "IS NOT NULL" are the same + * answer; adding id 2 separates them. The matcher's arm is two conditionals — + * `target === true && …` and `target === false && …` — and a third value + * satisfies neither, so the constraint silently stopped constraining. That is + * the widening direction, which on an RLS read scope is a permission bypass and + * not a degraded filter (#3948) — and it is precisely the divergence #5324/#5328 + * built the single shape gate to make impossible. + * + * # Why these tests assert through BOTH faces + * + * The rule lives in exactly one function (`assertFilterConditionShape`) and both + * faces call it. A regression that re-forks them fails HERE rather than being + * discovered by a conformance table that only exercises one — the same reason + * `memory-filter-vocabulary-refusal.test.ts` doubles every case. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import type { FilterCondition } from '@objectstack/spec/data'; + +import { InMemoryDriver } from './memory-driver.js'; +import { match } from './memory-matcher.js'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +const ROWS = [ + { id: '1', stage: 'won', score: 10 }, + // The null-valued row that separates "IS NOT NULL" from "no constraint". + { id: '2', stage: null, score: 20 }, +]; + +/** + * The exact leading sentence `driver-sql` produces for this condition, copied + * from `sql-driver.ts`. A literal rather than an import: driver-memory does not + * depend on driver-sql (and must not), so the twin invariant #4436 established + * is held by pinning the other side's wording here. + */ +const DRIVER_SQL_LEADING_SENTENCE = (field: string) => + `Operator "$null" on field "${field}" requires a boolean comparand (true or false).`; + +describe('[#5347] $null requires a boolean comparand, on both filter faces', () => { + let driver: InMemoryDriver; + + beforeEach(async () => { + driver = new InMemoryDriver({ persistence: false }); + await driver.syncSchema('deal', { + fields: { + id: { type: 'text', name: 'id' }, + stage: { type: 'text', name: 'stage' }, + score: { type: 'number', name: 'score' }, + }, + } as any); + for (const row of ROWS) await driver.create('deal', row); + }); + + const findIds = async (where: unknown): Promise => { + const rows = await driver.find('deal', { + object: 'deal', + fields: ['id'], + where: where as FilterCondition, + }); + return (rows as any[]).map((r) => String(r.id)).sort(); + }; + + const matchIds = (where: unknown): string[] => + ROWS.filter((row) => match(row, where as any)).map((r) => r.id).sort(); + + const refusalOfFind = async (where: unknown): Promise => { + try { + await findIds(where); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the live query path to refuse this filter, but it resolved'); + }; + + const refusalOfMatch = (where: unknown): WireBearingError => { + try { + matchIds(where); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the reference matcher to refuse this filter, but it answered'); + }; + + const NON_BOOLEAN: Array<[label: string, value: unknown]> = [ + ["the string 'yes'", 'yes'], + ['the number 1', 1], + ['the number 0', 0], + ['null', null], + ['undefined', undefined], + ['an object', {}], + // The trap: `"false"` is truthy, so it compiled to the OPPOSITE of what its + // author meant on driver-sql — and to the opposite of THAT here. + ["the STRING 'false'", 'false'], + ]; + + for (const [label, value] of NON_BOOLEAN) { + it(`the live query path refuses ${label}`, async () => { + const err = await refusalOfFind({ stage: { $null: value } }); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain(DRIVER_SQL_LEADING_SENTENCE('stage')); + expect(err.message).toContain('filter.stage.$null'); + }); + + it(`the reference matcher refuses ${label}, identically`, () => { + const err = refusalOfMatch({ stage: { $null: value } }); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain(DRIVER_SQL_LEADING_SENTENCE('stage')); + expect(err.message).toContain('filter.stage.$null'); + }); + } + + it('both faces refuse it inside a combinator, at the position that names it', async () => { + for (const [where, path] of [ + [{ $and: [{ stage: { $null: 'yes' } }] }, 'filter.$and[0].stage.$null'], + [{ $or: [{ stage: 'won' }, { stage: { $null: 1 } }] }, 'filter.$or[1].stage.$null'], + [{ $not: { stage: { $null: 'yes' } } }, 'filter.$not.stage.$null'], + ] as Array<[unknown, string]>) { + const findErr = await refusalOfFind(where); + expect(findErr.code).toBe('INVALID_FILTER'); + expect(findErr.message).toContain(path); + const matchErr = refusalOfMatch(where); + expect(matchErr.message).toBe(findErr.message); + } + }); + + it('a satisfiable sibling does not let the malformed one through', async () => { + // The gate is a walk, not an evaluation: `{ stage: 'won' }` matches, and + // `{}` is the TRUE identity, yet neither short-circuits the refusal. + for (const where of [ + { $or: [{ stage: 'won' }, { stage: { $null: 'x' } }] }, + { $or: [{}, { stage: { $null: 'x' } }] }, + ]) { + expect((await refusalOfFind(where)).code).toBe('INVALID_FILTER'); + expect(refusalOfMatch(where).code).toBe('INVALID_FILTER'); + } + }); + + it('true and false are unchanged on both faces, line by line', async () => { + expect(await findIds({ stage: { $null: true } })).toEqual(['2']); + expect(matchIds({ stage: { $null: true } })).toEqual(['2']); + expect(await findIds({ stage: { $null: false } })).toEqual(['1']); + expect(matchIds({ stage: { $null: false } })).toEqual(['1']); + }); + + it('the ordinary vocabulary is untouched on both faces', async () => { + expect(await findIds({ stage: 'won' })).toEqual(['1']); + expect(matchIds({ stage: 'won' })).toEqual(['1']); + expect(await findIds({ score: { $between: [5, 15] } })).toEqual(['1']); + expect(matchIds({ score: { $between: [5, 15] } })).toEqual(['1']); + expect(await findIds({ $or: [{ stage: 'won' }, { score: 20 }] })).toEqual(['1', '2']); + expect(matchIds({ $or: [{ stage: 'won' }, { score: 20 }] })).toEqual(['1', '2']); + expect(await findIds({})).toEqual(['1', '2']); + }); + + it('$exists is deliberately NOT tightened here', () => { + // #5347 ruled on `$null` alone. `$exists` diverges on its own axis (#5299 + // holds the open question of what "exists" means for a null-valued key), so + // it keeps today's answers rather than being settled as a rider. + expect(matchIds({ stage: { $exists: 'yes' } })).toEqual(['1']); + }); +}); diff --git a/packages/plugins/driver-mongodb/src/mongodb-filter.ts b/packages/plugins/driver-mongodb/src/mongodb-filter.ts index 5133671115..fb75c8b8db 100644 --- a/packages/plugins/driver-mongodb/src/mongodb-filter.ts +++ b/packages/plugins/driver-mongodb/src/mongodb-filter.ts @@ -39,7 +39,7 @@ import { * ADR-0112 envelope (`400 INVALID_FILTER`) every sibling filter refusal speaks. */ function filterArrayReachedDriverError(filters: unknown[]): Error { - const err = new Error( + return unsupportedFilterError( `A filter ARRAY reached the driver: ${JSON.stringify(filters)}. ` + `'where' is a FilterCondition object; the array form ('FilterArray') is input-only ` + `authoring sugar and is lowered by @objectstack/spec parseFilterAST() at the engine ` + @@ -47,12 +47,80 @@ function filterArrayReachedDriverError(filters: unknown[]): Error { `second compiler for it — call through ObjectQL, or lower the value yourself with ` + `parseFilterAST(). Note the INFIX join form ([condA, "or", condB]) has no lowering at ` + `all: write the prefix form ["or", condA, condB].`, - ) as Error & { code?: string; status?: number }; + ); +} + +/** + * [#4436 / #5240] A filter this driver cannot evaluate, in the ADR-0112 + * envelope every sibling filter refusal across the backends speaks. + * + * Extracted from {@link filterArrayReachedDriverError}, which built the same + * `INVALID_FILTER` / 400 error inline — it was this package's only refusal + * carrying a wire identity, so there was nothing to share it with until #5347 + * added a second. It is deliberately the SAME envelope as `driver-sql`'s and + * `driver-memory`'s `unsupportedFilterError`, not a third: #3948 made the + * backends agree that an uncompilable filter is a refusal, and a suite that + * swaps one driver for another must see one `400 INVALID_FILTER`, not a coded + * refusal on three backends and a bare `{ error }` on the fourth. + * + * Note what this does NOT do: the `default:` arm of {@link translateFieldOperators} + * still throws a bare `Error` with a `[mongodb]` prefix, outside this envelope. + * That is #5346's, filed and measured separately — converting it here would be + * an unrelated behaviour change riding on #5347. + */ +function unsupportedFilterError(message: string): Error { + const err = new Error(message) as Error & { code?: string; status?: number }; err.code = StandardErrorCode.enum.INVALID_FILTER; err.status = 400; return err; } +/** + * [#5347] `$null` whose comparand is not a boolean. + * + * The leading sentence is `driver-sql`'s, verbatim — one condition, one wording + * (#5240) — and the tail records the measurement that made the refusal the + * ruling: on one row with `stage: 'won'` and one with `stage: null`, + * `{ stage: { $null: 'yes' } }` returned the NULL row on driver-sql / + * driver-sqlite-wasm / Turso local, the VALUED row here and on driver-memory's + * query path, and BOTH rows through driver-memory's reference matcher. Three + * readings of one declared operator, none of them anyone's decision — just what + * a two-branch conditional does with a third value. + */ +function nonBooleanNullComparandError(field: string, value: unknown, path: string): Error { + return unsupportedFilterError( + `Operator "$null" on field "${field}" requires a boolean comparand (true or false). ` + + `Received ${describeFilterOperand(value)} (${safeShapePreview(value)}) at ${path}. ` + + `@objectstack/spec FieldOperatorsSchema declares $null as a boolean. It is refused rather ` + + `than coerced because the backends read a non-boolean in OPPOSITE directions — driver-sql ` + + `compiled IS NULL (anything but false), this driver and driver-memory's query path ` + + `compiled IS NOT NULL (anything but true), and driver-memory's matcher dropped the ` + + `constraint entirely. Note "false" the STRING is truthy, so it landed on the side opposite ` + + `the false it was written to mean (#5347).`, + ); +} + +/** A short type name for an operand a filter refusal has to describe. */ +function describeFilterOperand(value: unknown): string { + if (value === null) return 'null'; + if (Array.isArray(value)) return 'array'; + const kind = typeof value; + if (kind !== 'object') return kind; + const ctor = (value as { constructor?: { name?: string } }).constructor; + return ctor?.name && ctor.name !== 'Object' ? ctor.name : 'object'; +} + +/** A short, non-throwing rendering of an offending value for a message. */ +function safeShapePreview(value: unknown): string { + try { + const json = JSON.stringify(value); + if (typeof json !== 'string') return typeof value; + return json.length > 80 ? `${json.slice(0, 77)}...` : json; + } catch { + return typeof value; + } +} + /** * Translate an ObjectStack `where` clause into a MongoDB filter document. * @@ -82,7 +150,7 @@ export function translateFilter( if (typeof where !== 'object') return {}; - return translateCondition(where as Record, temporalKind); + return translateCondition(where as Record, temporalKind, 'filter'); } /** @@ -91,6 +159,10 @@ export function translateFilter( function translateCondition( condition: Record, temporalKind?: TemporalFieldKindResolver, + // [#5347] Where in the filter tree this node sits, so a refusal can name the + // position it refused — the same `filter.$or[0].stage` spelling driver-sql + // and driver-memory print. + path = 'filter', ): Filter { const mongoFilter: Record = {}; const andClauses: Filter[] = []; @@ -100,7 +172,7 @@ function translateCondition( case '$and': if (Array.isArray(value)) { andClauses.push({ - $and: value.map((sub) => translateCondition(sub as Record, temporalKind)), + $and: value.map((sub, i) => translateCondition(sub as Record, temporalKind, `${path}.$and[${i}]`)), }); } break; @@ -108,14 +180,14 @@ function translateCondition( case '$or': if (Array.isArray(value)) { andClauses.push({ - $or: value.map((sub) => translateCondition(sub as Record, temporalKind)), + $or: value.map((sub, i) => translateCondition(sub as Record, temporalKind, `${path}.$or[${i}]`)), }); } break; case '$not': if (value && typeof value === 'object') { - const inner = translateCondition(value as Record, temporalKind); + const inner = translateCondition(value as Record, temporalKind, `${path}.$not`); // MongoDB $not applies per-field; for top-level negation use $nor andClauses.push({ $nor: [inner] }); } @@ -130,7 +202,7 @@ function translateCondition( const objValue = value as Record; const hasOps = Object.keys(objValue).some((k) => k.startsWith('$')); if (hasOps) { - mongoFilter[key] = translateFieldOperators(objValue, temporalKind?.(key)); + mongoFilter[key] = translateFieldOperators(objValue, temporalKind?.(key), key, `${path}.${key}`); } else { // Nested object — treat as exact match mongoFilter[key] = value; @@ -172,6 +244,10 @@ function translateFieldOperators( // (ADR-0053 D-C1) left the two out of step and the call site stopped // compiling. One definition means the next temporal type is added once. kind?: TemporalFieldKind, + // [#5347] Carried only so a refusal can name the field and the position it + // refused, the way `driver-sql` and `driver-memory` do. + field = '', + path = 'filter', ): Record { const result: Record = {}; const store = (v: unknown) => coerceTemporalValue(v, kind); @@ -238,7 +314,17 @@ function translateFieldOperators( break; // Null check + // + // [#5347] The arm used to be a two-branch `if/else` on `value === true`, + // so EVERY non-boolean comparand fell to the `else` and translated to + // `$ne: null` — IS NOT NULL. `driver-sql` hung its default on the + // opposite side (`opValue === false` → IS NULL) and `driver-memory`'s + // reference matcher on neither (the constraint vanished), so one declared + // operator had three readings across four backends. Ruled REFUSED on + // #5347: `FieldOperatorsSchema` declares `$null: z.boolean()`, and there + // is no reading of a non-boolean here that is not a guess at intent. case '$null': + if (typeof value !== 'boolean') throw nonBooleanNullComparandError(field, value, `${path}.$null`); if (value === true) { result.$eq = null; } else { diff --git a/packages/plugins/driver-mongodb/src/mongodb-null-comparand-refusal.test.ts b/packages/plugins/driver-mongodb/src/mongodb-null-comparand-refusal.test.ts new file mode 100644 index 0000000000..89155ae46c --- /dev/null +++ b/packages/plugins/driver-mongodb/src/mongodb-null-comparand-refusal.test.ts @@ -0,0 +1,116 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5347] `$null` takes a boolean — the FOURTH backend. + * + * This driver's `$null` arm was written `if (value === true) … else …`, i.e. + * identically to `driver-memory`'s, so every non-boolean comparand fell to the + * `else` and translated to `$ne: null` — IS NOT NULL. `driver-sql` hung its + * default on the opposite side (`opValue === false` → IS NULL). #5347 predicted + * this from a code read and asked for a measurement; measured on + * `translateFilter` directly: + * + * ``` + * { stage: { $null: 'yes' } } => {"stage":{"$ne":null}} + * { stage: { $null: 1 } } => {"stage":{"$ne":null}} + * { stage: { $null: 0 } } => {"stage":{"$ne":null}} + * { stage: { $null: 'false'} } => {"stage":{"$ne":null}} // truthy string! + * ``` + * + * — the prediction confirmed, so this driver is refused in step with the other + * three rather than left as the last backend still guessing. + * + * # Why the translator, not a live mongod + * + * `translateFilter` is the whole of the divergence: it is a pure function, it is + * this package's only reader of `$null`, and its output IS the query MongoDB + * receives. This package's live suites need `mongodb-memory-server`, which + * downloads a ~123 MB binary and is skipped whenever that download is blocked + * (see `test-mongod.ts`) — a test of the ruling that can be skipped is not a + * test of the ruling. Asserting on the translated document runs everywhere, and + * is the level `mongodb-filter.test.ts` already pins `$null: true` / `$null: + * false` at. + */ + +import { describe, it, expect } from 'vitest'; +import { translateFilter } from './mongodb-filter.js'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +/** + * The exact leading sentence `driver-sql` produces for this condition. A + * literal, not an import: this package does not depend on driver-sql (and must + * not), so the one-condition-one-wording invariant (#5240) is held by pinning + * the other side's text here — the same discipline + * `memory-filter-vocabulary-refusal.test.ts` uses. + */ +const DRIVER_SQL_LEADING_SENTENCE = (field: string) => + `Operator "$null" on field "${field}" requires a boolean comparand (true or false).`; + +const refusalOf = (where: unknown): WireBearingError => { + try { + translateFilter(where); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the translator to refuse this filter, but it translated'); +}; + +describe('[#5347] driver-mongodb refuses a non-boolean $null comparand', () => { + const NON_BOOLEAN: Array<[label: string, value: unknown]> = [ + ["the string 'yes'", 'yes'], + ['the number 1', 1], + ['the number 0', 0], + ['null', null], + ['undefined', undefined], + ['an object', {}], + ["the STRING 'false'", 'false'], + ]; + + for (const [label, value] of NON_BOOLEAN) { + it(`refuses ${label} with INVALID_FILTER / 400`, () => { + const err = refusalOf({ stage: { $null: value } }); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain(DRIVER_SQL_LEADING_SENTENCE('stage')); + expect(err.message).toContain('filter.stage.$null'); + // #3867 — the `[mongodb]` prefix this package's other refusal still + // carries (#5346) must not appear on a refusal added today. + expect(err.message).not.toContain('[mongodb]'); + }); + } + + it('names the position inside a combinator', () => { + expect(refusalOf({ $and: [{ stage: { $null: 'x' } }] }).message).toContain('filter.$and[0].stage.$null'); + expect(refusalOf({ $or: [{ score: 1 }, { stage: { $null: 'x' } }] }).message).toContain( + 'filter.$or[1].stage.$null', + ); + expect(refusalOf({ $not: { stage: { $null: 'x' } } }).message).toContain('filter.$not.stage.$null'); + }); + + it('true and false translate exactly as before', () => { + expect(translateFilter({ stage: { $null: true } })).toEqual({ stage: { $eq: null } }); + expect(translateFilter({ stage: { $null: false } })).toEqual({ stage: { $ne: null } }); + }); + + it('the surrounding vocabulary is untouched', () => { + expect(translateFilter({ stage: 'won' })).toEqual({ stage: 'won' }); + expect(translateFilter({ stage: { $in: ['won'] } })).toEqual({ stage: { $in: ['won'] } }); + expect(translateFilter({ score: { $between: [1, 2] } })).toEqual({ score: { $gte: 1, $lte: 2 } }); + expect(translateFilter({ stage: { $exists: true } })).toEqual({ stage: { $exists: true } }); + expect(translateFilter({})).toEqual({}); + }); + + it('the FilterArray refusal keeps its envelope through the extracted helper', () => { + // `filterArrayReachedDriverError` was rewritten to build its error through + // the shared `unsupportedFilterError` rather than inline. Same wire + // identity, same text. + const err = refusalOf([['stage', '=', 'won']]); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('A filter ARRAY reached the driver'); + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver-out-of-contract-filter-input.test.ts b/packages/plugins/driver-sql/src/sql-driver-out-of-contract-filter-input.test.ts new file mode 100644 index 0000000000..01d4f4a812 --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-out-of-contract-filter-input.test.ts @@ -0,0 +1,260 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5347 / #5348] Out-of-contract filter input is refused at the door, in both + * positions a filter has. + * + * The two issues are one shape seen twice. `FilterConditionSchema` declares what + * may sit at a NODE (three combinators, then field names) and + * `FieldOperatorsSchema` declares what each operator's COMPARAND may be; this + * driver checked neither, and in both positions the un-refused input produced an + * ANSWER rather than an error. + * + * ## #5348 — the node position + * + * A `$`-key that is not `$and`/`$or`/`$not` was compiled as a COLUMN of that + * name. Measured on better-sqlite3 before the fix, against one row: + * + * ``` + * WHERE {"$where":"return true"} => RESOLVED [] + * WHERE {"$nor":[{"stage":"won"}]} => RESOLVED [] + * WHERE {"$or":[{"$where":"x"}]} => RESOLVED [] + * ``` + * + * SQLite degrades a double-quoted name that resolves to no column into a string + * literal, so the query compiled, ran, and matched nothing — indistinguishable + * from "no rows matched". The FIELD position had answered the same class of + * input with `INVALID_FILTER` / 400 since #3948/#4436, so one driver gave two + * answers depending on depth — the internal contradiction #5240 closed for + * `{ field: {} }`. + * + * ## #5347 — the comparand position + * + * `$null`'s comparand is declared `z.boolean()`. A non-boolean was read by + * DEFAULT BRANCHES hung on opposite sides, so the same filter meant opposite + * things per backend. Measured against one row with `stage: 'won'` (id 1) and + * one with `stage: null` (id 2), on `{ stage: { $null: 'yes' } }`: + * + * | backend | answer | + * |---|---| + * | driver-sql, driver-sqlite-wasm, Turso local | `["2"]` — IS NULL | + * | driver-memory query path, driver-mongodb | `["1"]` — IS NOT NULL | + * | driver-memory reference matcher | `["1","2"]` — no constraint at all | + * + * Ruled REFUSED on all four (#5347): a non-boolean has no reading here that is + * not a guess at intent, and the string `"false"` — truthy — lands on the side + * opposite the `false` it was written to mean. + * + * ## Why both gates sit on the reduction walk + * + * `reduceFilterKey`, not the emitter. The emitter is skipped wholesale by a + * boolean identity, so `{ $or: [ {}, { $where: '…' } ] }` would be refused or + * ignored depending on its SIBLINGS — the "gate conditional on evaluation + * order" `reduceFilterNode`'s own doc comment warns against, and the same + * placement argument #5327 made for `{ field: {} }`. The `$or`/`$and` cases + * below are the ones that would fail if either gate moved into the emitter. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { SqlDriver } from './index.js'; +import type { FilterCondition } from '@objectstack/spec/data'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +describe('[#5347/#5348] SqlDriver refuses out-of-contract filter input', () => { + 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' }, + score: { type: 'number', name: 'score' }, + }, + } as any, + ]); + await driver.create('deal', { id: '1', stage: 'won', score: 10 }); + await driver.create('deal', { id: '2', stage: null, score: 20 }); + }); + + const ids = async (where: unknown): Promise => { + const rows = await driver.find('deal', { + object: 'deal', + fields: ['id'], + where: where as FilterCondition, + }); + return (rows as any[]).map((r) => String(r.id)).sort(); + }; + + const refusalOf = async (where: unknown): Promise => { + try { + await ids(where); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the driver to refuse this filter, but it resolved'); + }; + + // ── #5348: the node position ─────────────────────────────────────────────── + + describe('[#5348] a $-key in a node position that is not a declared combinator', () => { + // `$where` / `$expr` are named rather than invented: driver-mongodb's own + // refusal comment calls them P0 because a backend that EVALUATES them + // bypasses query intent. `$nor` and `$elemMatch` are the shapes a + // Mongo-fluent author reaches for that the Filter Protocol never declared. + const UNDECLARED: Array<[label: string, where: unknown, key: string, path: string]> = [ + ['$where at the top level', { $where: 'return true' }, '$where', 'filter.$where'], + ['$nor at the top level', { $nor: [{ stage: 'won' }] }, '$nor', 'filter.$nor'], + ['$expr at the top level', { $expr: { $eq: ['$stage', 'won'] } }, '$expr', 'filter.$expr'], + ['$elemMatch at the top level', { $elemMatch: { stage: 'won' } }, '$elemMatch', 'filter.$elemMatch'], + ['$where inside $or', { $or: [{ $where: 'x' }] }, '$where', 'filter.$or[0].$where'], + ['$nor inside $and', { $and: [{ $nor: [{ stage: 'won' }] }] }, '$nor', 'filter.$and[0].$nor'], + ['$expr inside $not', { $not: { $expr: 1 } }, '$expr', 'filter.$not.$expr'], + ]; + + for (const [label, where, key, path] of UNDECLARED) { + it(`refuses ${label} with INVALID_FILTER / 400`, async () => { + const err = await refusalOf(where); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + // The caller must be able to see WHICH key, and WHERE. + expect(err.message).toContain(`"${key}"`); + expect(err.message).toContain(path); + expect(err.message).toContain('$and, $or and $not'); + // #3867 — no driver-internal prefix on the wire. + expect(err.message).not.toContain('[sql-driver]'); + }); + } + + // The placement proof. `{ stage: 'won' }` is a satisfiable disjunct, so an + // emitter-side gate would compile the `$or` from that branch alone and + // never look at the malformed sibling. Before the fix this whole filter + // RESOLVED — and to `[]`, not even to the row the good disjunct matches. + it('refuses a malformed disjunct even when a sibling disjunct is satisfiable', async () => { + const err = await refusalOf({ $or: [{ stage: 'won' }, { $where: 'x' }] }); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.message).toContain('filter.$or[1].$where'); + }); + + // The other half of the same argument: an identity that resolves the whole + // node TRUE before the malformed sibling is reached. + it('refuses a malformed disjunct beside the TRUE identity `{}`', async () => { + const err = await refusalOf({ $or: [{}, { $nor: [{ stage: 'won' }] }] }); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.message).toContain('filter.$or[1].$nor'); + }); + + it('the three DECLARED combinators are untouched', async () => { + // Each spelling, alone and nested, still compiles and answers. + expect(await ids({ $and: [{ stage: 'won' }] })).toEqual(['1']); + expect(await ids({ $or: [{ stage: 'won' }, { score: 20 }] })).toEqual(['1', '2']); + expect(await ids({ $not: { stage: 'won' } })).toEqual(['2']); + expect(await ids({ $and: [{ $or: [{ stage: 'won' }] }, { $not: { stage: 'lost' } }] })).toEqual(['1']); + // The boolean identities #5134 settled are unchanged by the new gate. + expect(await ids({ $and: [] })).toEqual(['1', '2']); + expect(await ids({ $or: [] })).toEqual([]); + }); + }); + + // ── #5347: the comparand position ────────────────────────────────────────── + + describe('[#5347] $null with a non-boolean comparand', () => { + // Every non-boolean the issue enumerated, plus the truthy-string trap. + const NON_BOOLEAN: Array<[label: string, value: unknown]> = [ + ["the string 'yes'", 'yes'], + ['the number 1', 1], + ['the number 0', 0], + ['null', null], + ['undefined', undefined], + ['an object', {}], + // The one that matters most in practice: `"false"` is TRUTHY, so under + // the old rule it compiled IS NULL on SQL and IS NOT NULL on the JS + // backends — the exact opposite of what its author wrote it to mean. A + // JSON round-trip or an AI-authored scope produces it readily. + ["the STRING 'false'", 'false'], + ]; + + for (const [label, value] of NON_BOOLEAN) { + it(`refuses ${label} with INVALID_FILTER / 400`, async () => { + const err = await refusalOf({ stage: { $null: value } }); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('Operator "$null" on field "stage" requires a boolean comparand'); + expect(err.message).toContain('filter.stage.$null'); + expect(err.message).not.toContain('[sql-driver]'); + }); + } + + it('refuses inside a combinator, and inside $not', async () => { + // `$not` matters on its own: its operand goes through the #5146 NULL-safe + // rewrite, which SYNTHESISES `{ $null: false }` / `{ $null: true }` nodes. + // The reduction runs on the ORIGINAL operand, so the refusal fires before + // the rewrite and the synthesised booleans are never confused with the + // caller's comparand. + for (const where of [ + { $and: [{ stage: { $null: 'yes' } }] }, + { $or: [{ stage: 'won' }, { stage: { $null: 1 } }] }, + { $not: { stage: { $null: 'yes' } } }, + ]) { + const err = await refusalOf(where); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.message).toContain('requires a boolean comparand'); + } + }); + + it('true and false are unchanged, line by line', async () => { + expect(await ids({ stage: { $null: true } })).toEqual(['2']); + expect(await ids({ stage: { $null: false } })).toEqual(['1']); + // …including under the #5146 NULL-safe negation rewrite, whose synthesised + // guards are themselves `{ $null: }` nodes. + expect(await ids({ $not: { stage: { $null: true } } })).toEqual(['1']); + expect(await ids({ $not: { stage: 'won' } })).toEqual(['2']); + }); + + it('$exists is deliberately NOT tightened here', async () => { + // #5347 ruled on `$null`. `$exists` carries the identical `=== false` + // identity read and diverges too, but on its own axis — what "exists" + // means for a null-valued key is #5299's open question — so it keeps + // today's behaviour rather than being settled as a rider on this fix. + expect(await ids({ stage: { $exists: 'yes' } })).toEqual(['1']); + expect(await ids({ stage: { $exists: 0 } })).toEqual(['1']); + }); + }); + + // ── Both gates: the legal shapes are untouched ───────────────────────────── + + describe('legal filters compile exactly as before', () => { + it('the ordinary vocabulary still answers', async () => { + expect(await ids({ stage: 'won' })).toEqual(['1']); + expect(await ids({ stage: { $in: ['won'] } })).toEqual(['1']); + expect(await ids({ stage: { $ne: 'won' } })).toEqual([]); + expect(await ids({ score: { $between: [5, 15] } })).toEqual(['1']); + expect(await ids({ score: { $gte: 10 } })).toEqual(['1', '2']); + expect(await ids({ stage: { $startsWith: 'w' } })).toEqual(['1']); + expect(await ids({})).toEqual(['1', '2']); + // A `$field` REFERENCE is still refused by #5041's comparand gate, with + // its own wording — the new gates did not swallow it. + const crossField = await refusalOf({ stage: { $eq: { $field: 'score' } } }); + expect(crossField.message).toContain('Cross-field comparison'); + }); + + it('the regex family keeps its non-string comparands (explicitly out of scope)', async () => { + // #5347 measured this family as AGREEING across backends and fail-closed, + // and #5041 left it out of the comparand guard on the same evidence. It + // is not tightened here, and this pins that it was not tightened by + // accident. + expect(await ids({ stage: { $contains: 1 } })).toEqual([]); + expect(await ids({ stage: { $startsWith: {} } })).toEqual([]); + }); + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index eab71a39b7..1ddd569b34 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -740,6 +740,98 @@ function isEmptyFieldConstraint(spec: unknown): boolean { return isFilterNode(spec) && Object.keys(spec).length === 0; } +/** + * [#5348] A `$`-prefixed key in a NODE position that is not a declared + * combinator. + * + * `FilterConditionSchema` declares exactly three (`LOGICAL_OPERATORS`: `$and`, + * `$or`, `$not`); every other key of a node is a FIELD NAME. This driver's + * emitter is written on that assumption and nothing checked it, so `$where`, + * `$nor`, `$expr` and friends fell through to the field arms and were compiled + * as COLUMNS — `remoteColumn(table, '$where', …)`. On SQLite a double-quoted + * name that resolves to no column degrades to a string literal, so the query + * compiled, ran, and returned ZERO ROWS: + * + * ``` + * WHERE {"$where":"return true"} → SELECT … WHERE "$where" = 'return true' → [] + * WHERE {"$nor":[{"stage":"won"}]} → (its array value missed the object arm) → [] + * ``` + * + * Measured on better-sqlite3 in #5348. Other dialects reject the unknown + * identifier instead — a different symptom, the same cause, and neither is an + * answer to the filter that was asked. + * + * The refusal is this driver's FIELD-LEVEL posture (#3948 / #4436) finally + * reaching the node position: `{ stage: { $sounds_like: 'x' } }` has answered + * `INVALID_FILTER` / 400 for two releases while `{ $sounds_like: 'x' }` one + * level up answered "no rows". One driver, two positions, two answers — the + * same internal contradiction #5240 closed for `{ field: {} }`. + * + * The wording is `driver-memory`'s `unknownLogicalOperatorError`, verbatim + * through the vocabulary sentence, because #3948 made the backends AGREE that + * an uncompilable filter is a refusal and #5240 made one condition speak one + * wording. Only the closing clause differs: it names what THIS driver used to + * do with the key. + */ +function unknownLogicalOperatorError(key: string, path: string): Error { + return unsupportedFilterError( + `Unsupported filter combinator "${key}" at ${path}. A filter node's $-prefixed keys are the ` + + `declared logical operators $and, $or and $not (@objectstack/spec LOGICAL_OPERATORS); every ` + + `other key is a field name. It is refused rather than compiled as a COLUMN of that name, ` + + `which is what this driver used to do — producing a predicate that matched no row and ` + + `reported nothing, so a caller could not tell "no rows matched" from "the filter never ` + + `compiled" (#5348).`, + ); +} + +/** + * [#5347] `$null` whose comparand is not a boolean. + * + * `FieldOperatorsSchema` declares `$null: z.boolean()`, and nothing between an + * authored `where` and this driver validates against it — so a non-boolean + * really does arrive here. Every backend then read it, and they did NOT agree; + * measured in #5347 against one row with `stage: 'won'` and one with + * `stage: null`, on `{ stage: { $null: 'yes' } }`: + * + * | backend | compiled to | rows | + * |---|---|---| + * | driver-sql / driver-sqlite-wasm / Turso local | `IS NULL` (anything but `false`) | the NULL row | + * | driver-memory live path (mingo), driver-mongodb | `IS NOT NULL` (anything but `true`) | the valued row | + * | driver-memory reference matcher | nothing at all — the constraint vanished | BOTH rows | + * + * Three readings of one declared operator, and two of them are each other's + * exact complement. The cause is a pair of default branches hung on opposite + * sides: this driver's emitter asked `opValue === false`, the JS drivers asked + * `val === true`. Neither is a rule anyone wrote down; both are what a + * two-branch conditional does with a third value. + * + * Ruled on #5347: REFUSED, in every position, on every backend — the same + * disposition #5240 gave `{ field: {} }` and for the same reason. `$null: 0` + * read as IS NULL (this driver's rule) is almost certainly not what the author + * meant, `$null: 0` read as IS NOT NULL (the JS rule) is no better, and the + * string `"false"` — which an AI-authored or JSON-round-tripped scope produces + * readily — is truthy, so it lands on the opposite side from the `false` it was + * written to mean. There is no reading of a non-boolean here that is not a + * guess about the author's intent, so the driver stops guessing. + * + * `$exists` carries the identical `=== false` identity read one arm below and + * is deliberately NOT touched here: it diverges too, but on its own axis (what + * "exists" means for a null-valued key is #5299's open question), and #5347 + * ruled on `$null`. Filed separately rather than settled as a rider. + */ +function nonBooleanNullComparandError(field: string, value: unknown, path: string): Error { + return unsupportedFilterError( + `Operator "$null" on field "${field}" requires a boolean comparand (true or false). ` + + `Received ${describeFilterOperand(value)} (${safeShapePreview(value)}) at ${path}. ` + + `@objectstack/spec FieldOperatorsSchema declares $null as a boolean. It is refused rather ` + + `than coerced because the backends read a non-boolean in OPPOSITE directions — this driver ` + + `compiled IS NULL (anything but false), driver-memory's query path and driver-mongodb ` + + `compiled IS NOT NULL (anything but true), and driver-memory's matcher dropped the ` + + `constraint entirely. Note "false" the STRING is truthy, so it landed on the side opposite ` + + `the false it was written to mean (#5347).`, + ); +} + /** [#5134] `$and`/`$or` take a list; anything else is refused, never coerced. */ function assertFilterNodeList(value: unknown, key: string, path: string): asserts value is unknown[] { if (Array.isArray(value)) return; @@ -812,6 +904,20 @@ function reduceFilterKey(key: string, value: unknown, path: string): FilterVerdi return inner === 'true' ? 'false' : inner === 'false' ? 'true' : 'clause'; } + // [#5348] Everything still `$`-prefixed at this point is an UNDECLARED + // combinator — the three declared ones each returned above. Refused here and + // not in the emitter for exactly the reason the two lines below are here, and + // the reason #5327 gave for `{ field: {} }`: this walk is exhaustive and does + // not short-circuit, while the emitter is skipped wholesale by a boolean + // identity. `{ $or: [ {}, { $where: '…' } ] }` reduces to TRUE on its first + // disjunct, so an emitter-side gate would refuse the `$where` or ignore it + // depending on its SIBLINGS — "a gate conditional on evaluation order", which + // this function's own doc comment warns against. + // + // It must also come BEFORE the field arms below, because that is precisely + // what those arms did wrong: they accepted `$where` as a field name. + if (key.startsWith('$')) throw unknownLogicalOperatorError(key, here); + // [#5240] `{ field: {} }` is refused HERE — on the validating walk, beside // `assertFilterNode` / `assertFilterNodeList` — rather than in the emitter // below, for the same reason those two sit here: the walk is exhaustive and @@ -829,6 +935,26 @@ function reduceFilterKey(key: string, value: unknown, path: string): FilterVerdi // before compiles byte-identically now. if (isEmptyFieldConstraint(value)) throw emptyFieldConstraintError(key, here); + // [#5347] `$null`'s comparand is a boolean by declaration. Checked on this + // walk rather than in the emitter's `$null` arm for the same + // evaluation-order reason, and checked on the RAW value so the message names + // the shape the caller sent rather than whatever `coerceFilterValue` made of + // it. Only `$null` is inspected: the surrounding operator vocabulary is the + // emitter's `default: throw` to enforce, and widening this walk into a second + // vocabulary gate is how two lists drift apart (#3948). + // `hasOwnProperty` rather than `'$null' in value` so an inherited key can + // never trip the gate, and rather than `Object.hasOwn` because this package + // targets es2020. `{ $null: undefined }` still counts: the key is own and + // enumerable, and `undefined` is exactly one of the comparands the issue + // measured a divergence on. + if ( + isFilterNode(value) && + Object.prototype.hasOwnProperty.call(value, '$null') && + typeof value.$null !== 'boolean' + ) { + throw nonBooleanNullComparandError(key, value.$null, `${here}.$null`); + } + // A field key always contributes a predicate. return 'clause'; } @@ -861,9 +987,28 @@ function nullValueSatisfiesOperator(op: string, value: unknown): boolean { // `$eq: null` IS the null predicate; any other comparand is a value test. case '$eq': return value === null; case '$ne': return value !== null; - // The emitter reads `$null`/`$exists` by identity against `false`, so the - // guard must read them the same way or the two can disagree. - case '$null': return value !== false; + // [#5347] `$null` is now TOTAL over its declared domain: `reduceFilterKey` + // refuses a non-boolean comparand before this table is ever consulted, so + // the only values that reach here are `true` and `false`. The arm was + // `value !== false` — a lenient read written to mirror the emitter's own + // `opValue === false` identity test, because at the time BOTH had to agree + // about a third value that could arrive. Neither does any more, so the arm + // says what it means: a NULL column satisfies `$null` exactly when the + // caller asked for null. + // + // Tightened rather than left alone deliberately. `value !== false` and + // `value === true` are equivalent only while the refusal upstream holds; the + // lenient spelling would keep compiling if that gate were ever moved or + // removed, and would silently resume answering for shapes nobody ruled on. + // The strict spelling cannot — it is the same "declared = enforced" reflex + // the refusal itself is. + case '$null': return value === true; + // `$exists` keeps its lenient identity read: unlike `$null` it has NO + // comparand gate (#5347 ruled on `$null` only), so a non-boolean still + // reaches this table, and the guard must keep answering it the same way the + // emitter's `opValue === false` arm does or the two can disagree about a + // row. Tightening this one without the matching refusal would be the + // divergence, not the fix — filed separately. case '$exists': return value === false; // Negative-polarity set/substring tests: "not among" / "does not contain" // hold vacuously for a value that is absent. @@ -6360,6 +6505,13 @@ export class SqlDriver implements IDataDriver { // (spec `parseFilterAST` maps those to `$null`). Previously this fell // to the equality default and compiled `field = true`, silently // returning the wrong rows (issue #2704). + // + // [#5347] `opValue` is a boolean here — `reduceFilterKey` refused + // anything else while validating the tree, which happens before this + // emitter runs. The `=== false` test is therefore an exhaustive + // two-way choice, not the "anything but false is IS NULL" rule it + // used to be; that rule was this driver's half of a three-way split + // across the backends. See {@link nonBooleanNullComparandError}. case '$null': (builder as any)[opValue === false ? (logicalOp === 'or' ? 'orWhereNotNull' : 'whereNotNull') diff --git a/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-out-of-contract-filter-input.test.ts b/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-out-of-contract-filter-input.test.ts new file mode 100644 index 0000000000..0d58feb9e4 --- /dev/null +++ b/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-out-of-contract-filter-input.test.ts @@ -0,0 +1,114 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5347 / #5348] The wasm driver refuses out-of-contract filter input too. + * + * `SqliteWasmDriver extends SqlDriver`, so both refusals are inherited and + * nothing here re-implements either. What this pins is that the inheritance + * actually DELIVERS them end to end: this driver swaps knex's transport for a + * custom sql.js dialect, and a refusal has to survive that pipeline with its + * ADR-0112 envelope (`code` / `status`) intact rather than being swallowed, + * re-wrapped by the wasm error path, or bypassed by an override. + * + * "It inherits the compiler, therefore it is fine" is the assumption this + * driver's temporal / pagination / filter-logic suites exist to disprove + * (#4405), and #5347 named both drivers as unverified. Measured here before the + * fix, on one row with `stage: 'won'` (id 1) and one with `stage: null` (id 2): + * + * ``` + * { stage: { $null: 'yes' } } => RESOLVED ["2"] // IS NULL, like driver-sql + * { $where: 'return true' } => RESOLVED [] // compiled as a column + * { $or: [{ $where: 'x' }] } => RESOLVED [] + * ``` + * + * — the same two defects as the base class, confirming the inheritance carried + * the bugs as faithfully as it now carries the fixes. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { SqliteWasmDriver } from './index.js'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +describe('[#5347/#5348] driver-sqlite-wasm inherits the out-of-contract filter refusals', () => { + let driver: SqliteWasmDriver; + + beforeAll(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.initObjects([ + { name: 'deal', fields: { stage: { type: 'string' }, score: { type: 'number' } } }, + ]); + await driver.create('deal', { id: '1', stage: 'won', score: 10 }, { bypassTenantAudit: true } as any); + await driver.create('deal', { id: '2', stage: null, score: 20 }, { bypassTenantAudit: true } as any); + }); + + afterAll(async () => { + await driver.disconnect(); + }); + + const ids = async (where: unknown): Promise => { + const rows = await driver.find( + 'deal', + { object: 'deal', fields: ['id'], where } as any, + { bypassTenantAudit: true } as any, + ); + return (rows as any[]).map((r) => String(r.id)).sort(); + }; + + const refusalOf = async (where: unknown): Promise => { + try { + await ids(where); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the driver to refuse this filter, but it resolved'); + }; + + describe('[#5348] undeclared $-keys in a node position', () => { + for (const [label, where, key] of [ + ['$where at the top level', { $where: 'return true' }, '$where'], + ['$nor at the top level', { $nor: [{ stage: 'won' }] }, '$nor'], + ['$where inside $or', { $or: [{ $where: 'x' }] }, '$where'], + ] as Array<[string, unknown, string]>) { + it(`refuses ${label} with the base class's envelope`, async () => { + const err = await refusalOf(where); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain(`Unsupported filter combinator "${key}"`); + }); + } + + it('the three declared combinators still answer', async () => { + expect(await ids({ $and: [{ $or: [{ stage: 'won' }] }, { $not: { stage: 'lost' } }] })).toEqual(['1']); + }); + }); + + describe('[#5347] a non-boolean $null comparand', () => { + for (const [label, value] of [ + ["the string 'yes'", 'yes'], + ['the number 0', 0], + ["the STRING 'false'", 'false'], + ] as Array<[string, unknown]>) { + it(`refuses ${label} with the base class's envelope`, async () => { + const err = await refusalOf({ stage: { $null: value } }); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('requires a boolean comparand'); + }); + } + + it('true and false are unchanged', async () => { + expect(await ids({ stage: { $null: true } })).toEqual(['2']); + expect(await ids({ stage: { $null: false } })).toEqual(['1']); + }); + }); + + it('legal filters are untouched', async () => { + expect(await ids({ stage: 'won' })).toEqual(['1']); + expect(await ids({ score: { $gte: 10 } })).toEqual(['1', '2']); + expect(await ids({})).toEqual(['1', '2']); + }); +});