Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions .changeset/refuse-out-of-contract-filter-input.md
Original file line number Diff line number Diff line change
@@ -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: <non-boolean> } }` 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.
53 changes: 53 additions & 0 deletions packages/plugins/driver-memory/src/filter-refusal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions packages/plugins/driver-memory/src/memory-driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
filterNodeExpectedError,
filterNodeListExpectedError,
malformedBetweenError,
nonBooleanNullComparandError,
unknownFieldOperatorError,
unknownLogicalOperatorError,
unsupportedFilterError,
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions packages/plugins/driver-memory/src/memory-matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<string[]> => {
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<WireBearingError> => {
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']);
});
});
Loading
Loading