Skip to content

Commit 9c5abf4

Browse files
os-zhuangclaude
andauthored
fix(driver-sql,driver-memory,driver-mongodb): refuse out-of-contract filter input at the door (#5347, #5348) (#5368)
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. Claude-Session: https://claude.ai/code/session_01Pbu27iNUfQCHeuS551Rqo7 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2f6516e commit 9c5abf4

10 files changed

Lines changed: 1058 additions & 10 deletions
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
"@objectstack/driver-memory": patch
4+
"@objectstack/driver-mongodb": patch
5+
---
6+
7+
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)
8+
9+
Two shapes the Filter Protocol never declared were reaching the drivers, and
10+
every driver ANSWERED them — with a different answer. Both are now refused with
11+
`INVALID_FILTER` / 400, in the ADR-0112 envelope every sibling filter refusal
12+
already speaks.
13+
14+
## `$null` with a non-boolean comparand — a behaviour change you can observe
15+
16+
`FieldOperatorsSchema` declares `$null: z.boolean()`. A non-boolean was read by
17+
default branches hung on opposite sides, so one filter meant opposite things per
18+
backend. Measured against one row with `stage: 'won'` (id 1) and one with
19+
`stage: null` (id 2), on `{ stage: { $null: 'yes' } }`:
20+
21+
| backend | read as | rows |
22+
|---|---|---|
23+
| driver-sql, driver-sqlite-wasm, Turso local | IS NULL (anything but `false`) | `["2"]` |
24+
| driver-memory query path, driver-mongodb | IS NOT NULL (anything but `true`) | `["1"]` |
25+
| driver-memory reference matcher | no constraint at all | `["1","2"]` |
26+
27+
**What changes for you:** a caller that today gets rows back for
28+
`{ field: { $null: <non-boolean> } }` now gets a `400 INVALID_FILTER` naming the
29+
operator, the field and the position. That includes calls working by truthy /
30+
falsy coincidence — and the sharpest case is the STRING `"false"`, which is
31+
truthy: it compiled to IS NULL on SQL and IS NOT NULL on the JS backends, i.e.
32+
the opposite of what its author wrote it to mean, on at least one of them
33+
whichever they meant. A JSON round-trip or generated metadata produces it
34+
readily.
35+
36+
**The fix:** write the boolean. `{ field: { $null: true } }` for "has no value",
37+
`{ field: { $null: false } }` for "has a value". Both are unchanged, on all four
38+
backends, and so is every other operator. `$exists` is deliberately NOT tightened
39+
here — it diverges on its own axis (what "exists" means for a null-valued key)
40+
and is tracked separately.
41+
42+
## An undeclared `$op` in a document position — silent empty set becomes a 400
43+
44+
`FilterConditionSchema` declares exactly three `$`-keys at a node
45+
(`$and` / `$or` / `$not`); every other key is a field name. `driver-sql`
46+
compiled the rest as COLUMNS, so `{ $where: '…' }`, `{ $nor: […] }`,
47+
`{ $expr: … }` produced a predicate that matched nothing and reported nothing —
48+
a caller could not tell "no rows matched" from "the filter never compiled". The
49+
FIELD position had refused the same class of input since v16, so one driver gave
50+
two answers depending on depth.
51+
52+
**What changes for you:** those filters now raise `400 INVALID_FILTER` instead of
53+
returning `[]`. `driver-memory` already refused them; this brings `driver-sql`
54+
(and `driver-sqlite-wasm`, which inherits it) into line. The three declared
55+
combinators, their boolean identities (`$and: []` is TRUE, `$or: []` is FALSE)
56+
and every legal filter compile byte-identically.
57+
58+
Both refusals are raised on the driver's validating walk rather than in its SQL
59+
emitter, so a malformed node is refused regardless of whether a sibling
60+
disjunct would have short-circuited the compile.

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,52 @@ export function malformedBetweenError(field: string, value: unknown, path: strin
259259
);
260260
}
261261

262+
/**
263+
* [#5347] `$null` whose comparand is not a boolean.
264+
*
265+
* `FieldOperatorsSchema` declares `$null: z.boolean()`, and nothing between an
266+
* authored `where` and a driver validates against it — so a non-boolean really
267+
* arrives. Every backend then read it, and they did NOT agree; measured on one
268+
* row with `stage: 'won'` and one with `stage: null`, on `{ stage: { $null: 'yes' } }`:
269+
*
270+
* | backend | read as | rows |
271+
* |---|---|---|
272+
* | driver-sql / driver-sqlite-wasm / Turso local | IS NULL (anything but `false`) | the NULL row |
273+
* | THIS driver's live path (mingo), driver-mongodb | IS NOT NULL (anything but `true`) | the valued row |
274+
* | THIS driver's reference matcher | nothing at all — the constraint vanished | BOTH rows |
275+
*
276+
* Note the last line: this package's own two faces disagreed with EACH OTHER,
277+
* which #5347 could not see because it measured a fixture with no null-valued
278+
* row — there the matcher's "match everything" and mingo's "IS NOT NULL"
279+
* coincide. The matcher's `$null` arm is written as two conditionals
280+
* (`target === true && …`, `target === false && …`); a third value satisfies
281+
* neither, so the operator silently stopped constraining anything. That is the
282+
* #5240 / #5328 shape exactly — one filter, one package, two answers, and the
283+
* widening one is a permission bypass on a read scope.
284+
*
285+
* Ruled on #5347: REFUSED everywhere, the same disposition `{ field: {} }` got
286+
* and for the same reason — there is no reading of a non-boolean here that is
287+
* not a guess about the author's intent. The string `"false"` is the sharpest
288+
* case: it is truthy, so it landed on the opposite side from the `false` it was
289+
* written to mean, and it is exactly what an AI-authored or JSON-round-tripped
290+
* scope produces.
291+
*
292+
* The leading sentence is `driver-sql`'s, verbatim — one condition, one wording
293+
* (#5240).
294+
*/
295+
export function nonBooleanNullComparandError(field: string, value: unknown, path: string): Error {
296+
return unsupportedFilterError(
297+
`Operator "$null" on field "${field}" requires a boolean comparand (true or false). ` +
298+
`Received ${describeFilterOperand(value)} (${safeShapePreview(value)}) at ${path}. ` +
299+
`@objectstack/spec FieldOperatorsSchema declares $null as a boolean. It is refused rather ` +
300+
`than coerced because the backends read a non-boolean in OPPOSITE directions — driver-sql ` +
301+
`compiled IS NULL (anything but false), this driver's query path and driver-mongodb ` +
302+
`compiled IS NOT NULL (anything but true), and this driver's matcher dropped the ` +
303+
`constraint entirely. Note "false" the STRING is truthy, so it landed on the side opposite ` +
304+
`the false it was written to mean (#5347).`,
305+
);
306+
}
307+
262308
/**
263309
* [#5324] `$options` without the `$regex` it modifies.
264310
*
@@ -386,6 +432,13 @@ function assertFieldConstraintShape(field: string, spec: unknown, path: string):
386432
if (op === '$between' && !isBetweenComparand(spec[op])) {
387433
throw malformedBetweenError(field, spec[op], `${path}.$between`);
388434
}
435+
// [#5347] `$null`'s comparand is a boolean by declaration. It joins
436+
// `$between`'s arity as the second COMPARAND-shape check this gate makes,
437+
// and for the identical reason: a shape the operator cannot evaluate was
438+
// being answered silently, differently, by each face.
439+
if (op === '$null' && typeof spec[op] !== 'boolean') {
440+
throw nonBooleanNullComparandError(field, spec[op], `${path}.$null`);
441+
}
389442
}
390443
// `$options` is the one entry in the vocabulary that is a modifier rather than
391444
// a predicate, so it is the one that needs a companion.

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
filterNodeExpectedError,
1313
filterNodeListExpectedError,
1414
malformedBetweenError,
15+
nonBooleanNullComparandError,
1516
unknownFieldOperatorError,
1617
unknownLogicalOperatorError,
1718
unsupportedFilterError,
@@ -973,6 +974,16 @@ export class InMemoryDriver implements IDataDriver {
973974
case '$null':
974975
// $null: true → field is null, $null: false → field is not null
975976
// Use $eq/$ne null for Mingo compatibility
977+
//
978+
// [#5347] The arm used to be a two-branch `if/else` on `val === true`,
979+
// so EVERY non-boolean comparand fell to the `else` and compiled
980+
// `$ne: null` — IS NOT NULL. `driver-sql` hung its default on the
981+
// opposite side (`opValue === false` → IS NULL) and the reference
982+
// matcher on neither (the constraint vanished), so one declared
983+
// operator had three readings. The shape gate refuses a non-boolean
984+
// now; this throw is the totality floor, the same one `$between`
985+
// keeps beside it.
986+
if (typeof val !== 'boolean') throw nonBooleanNullComparandError(field, val, `${path}.$null`);
976987
if (val === true) {
977988
result.$eq = null;
978989
} else {

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,16 @@ function checkCondition(value: any, condition: any): boolean {
193193
break;
194194
case '$null':
195195
// $null: true → value must be null/undefined; $null: false → value must not be null/undefined
196+
//
197+
// [#5347] These two conditionals are EXHAUSTIVE now: the shape
198+
// gate refuses a non-boolean `target` before evaluation starts.
199+
// They were not, and that is what the issue's fixture could not
200+
// see. A third value satisfied neither test, so the operator
201+
// matched EVERY row — while the live query path compiled the
202+
// same filter to IS NOT NULL and driver-sql to IS NULL. This
203+
// face's answer was the widening one, which on an RLS read scope
204+
// is a permission bypass, not a degraded filter (#3948, and the
205+
// identical `$between` note above).
196206
if (target === true && value != null) return false;
197207
if (target === false && value == null) return false;
198208
break;
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#5347] `$null` takes a boolean. A non-boolean is refused on BOTH faces.
5+
*
6+
* # What was measured
7+
*
8+
* `FieldOperatorsSchema` declares `$null: z.boolean()`, and nothing between an
9+
* authored `where` and a driver validates against it. Given one row with
10+
* `stage: 'won'` (id 1) and one with `stage: null` (id 2), the filter
11+
* `{ stage: { $null: 'yes' } }` produced THREE answers:
12+
*
13+
* | face | compiled to | rows |
14+
* |---|---|---|
15+
* | `driver-sql` / `driver-sqlite-wasm` / Turso local | `IS NULL` (anything but `false`) | `["2"]` |
16+
* | this driver's live path (mingo), `driver-mongodb` | `IS NOT NULL` (anything but `true`) | `["1"]` |
17+
* | this driver's reference matcher | nothing at all | `["1","2"]` |
18+
*
19+
* The third row is the one #5347 could not see. It measured a fixture with no
20+
* null-valued row, where "matches every row" and "IS NOT NULL" are the same
21+
* answer; adding id 2 separates them. The matcher's arm is two conditionals —
22+
* `target === true && …` and `target === false && …` — and a third value
23+
* satisfies neither, so the constraint silently stopped constraining. That is
24+
* the widening direction, which on an RLS read scope is a permission bypass and
25+
* not a degraded filter (#3948) — and it is precisely the divergence #5324/#5328
26+
* built the single shape gate to make impossible.
27+
*
28+
* # Why these tests assert through BOTH faces
29+
*
30+
* The rule lives in exactly one function (`assertFilterConditionShape`) and both
31+
* faces call it. A regression that re-forks them fails HERE rather than being
32+
* discovered by a conformance table that only exercises one — the same reason
33+
* `memory-filter-vocabulary-refusal.test.ts` doubles every case.
34+
*/
35+
36+
import { describe, it, expect, beforeEach } from 'vitest';
37+
import type { FilterCondition } from '@objectstack/spec/data';
38+
39+
import { InMemoryDriver } from './memory-driver.js';
40+
import { match } from './memory-matcher.js';
41+
42+
interface WireBearingError extends Error {
43+
code?: string;
44+
status?: number;
45+
}
46+
47+
const ROWS = [
48+
{ id: '1', stage: 'won', score: 10 },
49+
// The null-valued row that separates "IS NOT NULL" from "no constraint".
50+
{ id: '2', stage: null, score: 20 },
51+
];
52+
53+
/**
54+
* The exact leading sentence `driver-sql` produces for this condition, copied
55+
* from `sql-driver.ts`. A literal rather than an import: driver-memory does not
56+
* depend on driver-sql (and must not), so the twin invariant #4436 established
57+
* is held by pinning the other side's wording here.
58+
*/
59+
const DRIVER_SQL_LEADING_SENTENCE = (field: string) =>
60+
`Operator "$null" on field "${field}" requires a boolean comparand (true or false).`;
61+
62+
describe('[#5347] $null requires a boolean comparand, on both filter faces', () => {
63+
let driver: InMemoryDriver;
64+
65+
beforeEach(async () => {
66+
driver = new InMemoryDriver({ persistence: false });
67+
await driver.syncSchema('deal', {
68+
fields: {
69+
id: { type: 'text', name: 'id' },
70+
stage: { type: 'text', name: 'stage' },
71+
score: { type: 'number', name: 'score' },
72+
},
73+
} as any);
74+
for (const row of ROWS) await driver.create('deal', row);
75+
});
76+
77+
const findIds = async (where: unknown): Promise<string[]> => {
78+
const rows = await driver.find('deal', {
79+
object: 'deal',
80+
fields: ['id'],
81+
where: where as FilterCondition,
82+
});
83+
return (rows as any[]).map((r) => String(r.id)).sort();
84+
};
85+
86+
const matchIds = (where: unknown): string[] =>
87+
ROWS.filter((row) => match(row, where as any)).map((r) => r.id).sort();
88+
89+
const refusalOfFind = async (where: unknown): Promise<WireBearingError> => {
90+
try {
91+
await findIds(where);
92+
} catch (e) {
93+
return e as WireBearingError;
94+
}
95+
throw new Error('expected the live query path to refuse this filter, but it resolved');
96+
};
97+
98+
const refusalOfMatch = (where: unknown): WireBearingError => {
99+
try {
100+
matchIds(where);
101+
} catch (e) {
102+
return e as WireBearingError;
103+
}
104+
throw new Error('expected the reference matcher to refuse this filter, but it answered');
105+
};
106+
107+
const NON_BOOLEAN: Array<[label: string, value: unknown]> = [
108+
["the string 'yes'", 'yes'],
109+
['the number 1', 1],
110+
['the number 0', 0],
111+
['null', null],
112+
['undefined', undefined],
113+
['an object', {}],
114+
// The trap: `"false"` is truthy, so it compiled to the OPPOSITE of what its
115+
// author meant on driver-sql — and to the opposite of THAT here.
116+
["the STRING 'false'", 'false'],
117+
];
118+
119+
for (const [label, value] of NON_BOOLEAN) {
120+
it(`the live query path refuses ${label}`, async () => {
121+
const err = await refusalOfFind({ stage: { $null: value } });
122+
expect(err.code).toBe('INVALID_FILTER');
123+
expect(err.status).toBe(400);
124+
expect(err.message).toContain(DRIVER_SQL_LEADING_SENTENCE('stage'));
125+
expect(err.message).toContain('filter.stage.$null');
126+
});
127+
128+
it(`the reference matcher refuses ${label}, identically`, () => {
129+
const err = refusalOfMatch({ stage: { $null: value } });
130+
expect(err.code).toBe('INVALID_FILTER');
131+
expect(err.status).toBe(400);
132+
expect(err.message).toContain(DRIVER_SQL_LEADING_SENTENCE('stage'));
133+
expect(err.message).toContain('filter.stage.$null');
134+
});
135+
}
136+
137+
it('both faces refuse it inside a combinator, at the position that names it', async () => {
138+
for (const [where, path] of [
139+
[{ $and: [{ stage: { $null: 'yes' } }] }, 'filter.$and[0].stage.$null'],
140+
[{ $or: [{ stage: 'won' }, { stage: { $null: 1 } }] }, 'filter.$or[1].stage.$null'],
141+
[{ $not: { stage: { $null: 'yes' } } }, 'filter.$not.stage.$null'],
142+
] as Array<[unknown, string]>) {
143+
const findErr = await refusalOfFind(where);
144+
expect(findErr.code).toBe('INVALID_FILTER');
145+
expect(findErr.message).toContain(path);
146+
const matchErr = refusalOfMatch(where);
147+
expect(matchErr.message).toBe(findErr.message);
148+
}
149+
});
150+
151+
it('a satisfiable sibling does not let the malformed one through', async () => {
152+
// The gate is a walk, not an evaluation: `{ stage: 'won' }` matches, and
153+
// `{}` is the TRUE identity, yet neither short-circuits the refusal.
154+
for (const where of [
155+
{ $or: [{ stage: 'won' }, { stage: { $null: 'x' } }] },
156+
{ $or: [{}, { stage: { $null: 'x' } }] },
157+
]) {
158+
expect((await refusalOfFind(where)).code).toBe('INVALID_FILTER');
159+
expect(refusalOfMatch(where).code).toBe('INVALID_FILTER');
160+
}
161+
});
162+
163+
it('true and false are unchanged on both faces, line by line', async () => {
164+
expect(await findIds({ stage: { $null: true } })).toEqual(['2']);
165+
expect(matchIds({ stage: { $null: true } })).toEqual(['2']);
166+
expect(await findIds({ stage: { $null: false } })).toEqual(['1']);
167+
expect(matchIds({ stage: { $null: false } })).toEqual(['1']);
168+
});
169+
170+
it('the ordinary vocabulary is untouched on both faces', async () => {
171+
expect(await findIds({ stage: 'won' })).toEqual(['1']);
172+
expect(matchIds({ stage: 'won' })).toEqual(['1']);
173+
expect(await findIds({ score: { $between: [5, 15] } })).toEqual(['1']);
174+
expect(matchIds({ score: { $between: [5, 15] } })).toEqual(['1']);
175+
expect(await findIds({ $or: [{ stage: 'won' }, { score: 20 }] })).toEqual(['1', '2']);
176+
expect(matchIds({ $or: [{ stage: 'won' }, { score: 20 }] })).toEqual(['1', '2']);
177+
expect(await findIds({})).toEqual(['1', '2']);
178+
});
179+
180+
it('$exists is deliberately NOT tightened here', () => {
181+
// #5347 ruled on `$null` alone. `$exists` diverges on its own axis (#5299
182+
// holds the open question of what "exists" means for a null-valued key), so
183+
// it keeps today's answers rather than being settled as a rider.
184+
expect(matchIds({ stage: { $exists: 'yes' } })).toEqual(['1']);
185+
});
186+
});

0 commit comments

Comments
 (0)