From 261f893d956c65b446e80a2a0641d239c02066c7 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Tue, 4 Aug 2026 20:44:19 +0000 Subject: [PATCH] fix(driver-sql,driver-memory,formula)!: `{ field: {} }` is refused by all four backends (#5240) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A field constrained by ZERO operators is a shape `FilterConditionSchema` still declares legal, and one filter carrying it had three answers in this repo: - driver-sql refused it at the top level (the #5041 comparand gate) but DROPPED it inside `$and`/`$or`/`$not`, where a predicate that emits nothing means "matches every row" — so `{ $or: [{ a: {} }, { b: 2 } ] }` compiled to `(b = 2)` by losing a clause, and the same `{ a: {} }` was a 400 at the top level and a silent match-all one combinator deep; - driver-memory answered "matches nothing" incidentally, and did so through TWO independent paths that had never been compared: the live query path (mingo reads `{ a: {} }` as "deep-equals the empty document") and the reference matcher (`JSON.stringify` structural equality); - formula answered `false` from an explicit fail-closed arm. Ruled on #5240: refuse it everywhere, with one `INVALID_FILTER` / 400 envelope and a message naming the position (`filter.$or[0].stage`). The shape is almost always an authoring accident — a filter builder that recorded a field and never its operator — and both silent readings answer it with a row count the author never asked for. The gate sits on the #5134 validation walk, beside `assertFilterNode`, not in the emitter: the walk is exhaustive, while the emitter returns early whenever an identity settles a node, so an emitter-side gate would let `{ $or: [{ a: {} }, {} ] }` through and make the refusal depend on the shape's siblings. The reduction's VERDICT is unchanged (a field key still contributes `'clause'`), so every filter that compiled before compiles byte-identically. `nullGuardForFieldSpec`'s `entries.length === 0` escape — added by #5146 so the NULL-safe rewrite would not rule on #5240 from there — is removed with the ambiguity it protected: the refusal now fires before that rewrite runs. BREAKING: `matchesFilterCondition` is the RLS `check` evaluation path, so a `check` policy carrying `{ field: {} }` now fails the operation (#4775 posture) instead of evaluating to `false`. Where such a constraint sat under an `$or` beside a satisfied branch, or under a `$not`, the old `false` was absorbed and the write was ALLOWED; those writes now fail. Implementation is stricter than the declared contract: narrowing `FilterConditionSchema` and adding the case to `FILTER_LOGIC_CASES` is the spec lane's half of #5240. Fixes #5240 --- .changeset/empty-field-constraint-refused.md | 56 +++++ ...ches-filter-empty-field-constraint.test.ts | 146 ++++++++++++ packages/formula/src/matches-filter.ts | 100 ++++++++ .../driver-memory/src/filter-refusal.ts | 75 ++++++ .../driver-memory/src/memory-driver.ts | 41 ++-- .../src/memory-empty-field-constraint.test.ts | 181 ++++++++++++++ .../driver-memory/src/memory-matcher.ts | 61 ++++- .../sql-driver-empty-field-constraint.test.ts | 221 ++++++++++++++++++ .../src/sql-driver-not-null-safe.test.ts | 17 +- packages/plugins/driver-sql/src/sql-driver.ts | 88 +++++-- ...sqlite-wasm-empty-field-constraint.test.ts | 83 +++++++ 11 files changed, 1020 insertions(+), 49 deletions(-) create mode 100644 .changeset/empty-field-constraint-refused.md create mode 100644 packages/formula/src/matches-filter-empty-field-constraint.test.ts create mode 100644 packages/plugins/driver-memory/src/filter-refusal.ts create mode 100644 packages/plugins/driver-memory/src/memory-empty-field-constraint.test.ts create mode 100644 packages/plugins/driver-sql/src/sql-driver-empty-field-constraint.test.ts create mode 100644 packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-empty-field-constraint.test.ts diff --git a/.changeset/empty-field-constraint-refused.md b/.changeset/empty-field-constraint-refused.md new file mode 100644 index 0000000000..7d604ee585 --- /dev/null +++ b/.changeset/empty-field-constraint-refused.md @@ -0,0 +1,56 @@ +--- +"@objectstack/driver-sql": minor +"@objectstack/driver-memory": minor +"@objectstack/formula": minor +--- + +fix(driver-sql,driver-memory,formula)!: `{ field: {} }` 一律拒收 —— 零个操作符的字段约束不再在四个后端有三个答案 (#5240) + +`{ a: {} }`(一个字段,后面跟零个操作符)是 `FilterConditionSchema` 今天**声明合法**的形状, +而同一个 filter 在同仓四条路径上有三个答案: + +| 路径 | 改前 | 改后 | +|---|---|---| +| `driver-sql`,顶层 plain map | 抛 `INVALID_FILTER`(#5041 的比较数闸门) | 抛 `INVALID_FILTER`(专用消息) | +| `driver-sql`,`$and`/`$or`/`$not` 内 | 遍历零个操作符 → 不产出任何 SQL → **TRUE(匹配全表)** | 抛 `INVALID_FILTER` | +| `driver-memory` | 实时路径经 mingo 变成「字段深等于空文档」;参考匹配器落到 `JSON.stringify` 结构相等 → 顺带 FALSE | 抛 `INVALID_FILTER` | +| `@objectstack/formula` | `keys.length === 0` 显式 fail-closed → FALSE | 抛 `INVALID_FILTER` | + +于是 `{ $or: [ { a: {} }, { b: 2 } ] }` 在 SQL 上编译成 `(b = 2)` —— 既不是「零约束即 TRUE」 +该给的全表,也不是两个 JS 后端给的 FALSE,而是**子句被 knex 连同空分组一起丢掉**的结果; +而 `driver-sql` 自己内部就不自洽:同一个 `{ a: {} }` 写在顶层被响亮拒收,包进一层 `$or` +就变成静默的 TRUE。 + +维护者拍板取**拒收**(不取 TRUE、不取 FALSE):这个形状几乎必然是编写期事故 —— +筛选器记下了字段却没记下操作符,或生成的元数据把操作符弄丢了 —— 让它在编写期就炸, +好过在某个后端上安静地多返回或少返回几行。与 #5041 已在 driver-sql 顶层建立的先例一致, +本次只是把同一道闸门补进组合子内部。四个后端(第四个是继承 `SqlDriver` 的 +`driver-sqlite-wasm`)现在给出同一个 `INVALID_FILTER` / 400,消息里指名出事的位置 +(如 `filter.$or[0].stage`)。 + +**⚠️ 可观察的行为变更 —— RLS `check` 求值路径。** `@objectstack/formula` 的 +`matchesFilterCondition` 是 `plugin-security` 对 insert/update **后像**执行行级 `check` +的那条路径(没有查询可下推,这个求值器就是执行本身)。它改为抛出后,落在 #4775 +「求不出值 = 该次操作失败」的既定姿态上。这不只是「拒绝得更响」——有一类结果直接翻转: + +| `check` 策略 | 改前 | 改后 | +|---|---|---| +| `{ a: {} }` | FALSE → 写入被拒(403) | 抛出 → 该次写入失败(400) | +| `{ $or: [ { a: {} }, { owner: '{userId}' } ] }` | FALSE 被另一析取项吸收 → 写入**放行** | 抛出 → 该次写入失败 | +| `{ $not: { a: {} } }` | `!false` → 写入**放行** | 抛出 → 该次写入失败 | + +后两行是**原本能成功、现在会失败**的写入。这是拍板的目的而非副作用:一条含 +`{ field: {} }` 的权限规则,是一条作者弄丢了操作符的规则,它的含义不该取决于四个后端里 +哪一个在求值。升级后请检查 `check`/`using` 策略里是否存在零操作符的字段约束—— +错误消息会指名位置。 + +同一条改动也让 `@objectstack/driver-memory` 的两个过滤面(经 mingo 的实时查询路径, +与跨后端一致性套件所用的 `memory-matcher` 参考匹配器)第一次对这个形状给出同一个答案。 + +非空形状**逐字符不变**:普通比较、`$in`、`$or`/`$and` 组合、`$not` 的 #5146 NULL-safe 改写, +编译出的 SQL 文本与匹配结果都与改前相同;`{}`(零个键的**节点**,#5134 的布尔单位元) +与 `{ field: {} }` 是两个不同形状,前者的语义不受本次影响。 + +注:本次收紧的是**实现**。`packages/spec` 的 `FilterConditionSchema` 仍然声明这个形状合法 +(非递归半边是 `z.record(z.string(), z.unknown())`),即实现现在比已声明的契约更严; +契约收窄与 `FILTER_LOGIC_CASES` 补条归 spec 车道另行处理。 diff --git a/packages/formula/src/matches-filter-empty-field-constraint.test.ts b/packages/formula/src/matches-filter-empty-field-constraint.test.ts new file mode 100644 index 0000000000..77c01fbe8f --- /dev/null +++ b/packages/formula/src/matches-filter-empty-field-constraint.test.ts @@ -0,0 +1,146 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5240] `{ field: {} }` — a field constrained by ZERO operators — is REFUSED + * by `matchesFilterCondition`, with the same `INVALID_FILTER` envelope + * `driver-sql` and `driver-memory` raise. + * + * # This is the RLS `check` evaluation path, so the change is observable + * + * `matchesFilterCondition` is what `plugin-security` runs against the POST-IMAGE + * of an insert/update to enforce a row-level `check` — there is no query to push + * down to, so this evaluator IS the enforcement. It answered `false` for the + * shape from an explicit fail-closed arm (`keys.length === 0 || …`), which is + * why the divergence never surfaced as an error: the write was simply denied. + * + * Refusing instead lands on the #4775 posture (a `check` that cannot be + * evaluated fails the operation), and the direction is honest about ONE case + * that flips outright: + * + * | `check` policy | before | after | + * |---|---|---| + * | `{ a: {} }` | `false` → write DENIED (403) | throws → write FAILS (400) | + * | `{ $or: [ { a: {} }, { owner: '{userId}' } ] }` | the `false` disjunct was absorbed → write **ALLOWED** | throws → write FAILS | + * | `{ $not: { a: {} } }` | `!false` → write **ALLOWED** | throws → write FAILS | + * + * The last two rows are writes that used to succeed and now do not. That is the + * point of the ruling rather than a side effect of it: a permission rule whose + * meaning depends on which of four backends evaluated it is the defect, and a + * rule carrying `{ field: {} }` is a rule whose author lost an operator. + */ + +import { describe, it, expect } from 'vitest'; +import { matchesFilterCondition } from './matches-filter'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +const RECORD = { id: '1', stage: 'won', owner: 'u1', amount: 10 }; + +const refusalOf = (filter: unknown): WireBearingError => { + try { + matchesFilterCondition(RECORD, filter as never); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the evaluator to refuse this filter, but it answered'); +}; + +describe('[#5240] matchesFilterCondition refuses a zero-operator field constraint', () => { + const positions: Array<[string, unknown, string]> = [ + ['top level', { stage: {} }, 'filter.stage'], + ['inside $or', { $or: [{ stage: {} }, { owner: 'u2' }] }, 'filter.$or[0].stage'], + ['inside $and', { $and: [{ stage: 'won' }, { owner: {} }] }, 'filter.$and[1].owner'], + ['inside $not', { $not: { stage: {} } }, 'filter.$not.stage'], + ['nested two combinators deep', { $and: [{ $or: [{ stage: {} }] }] }, 'filter.$and[0].$or[0].stage'], + ]; + + for (const [name, filter, position] of positions) { + it(`${name} → INVALID_FILTER naming ${position}`, () => { + const err = refusalOf(filter); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain(position); + expect(err.message).toContain('zero operators'); + }); + } + + // ── The behaviour change, stated as tests ───────────────────────────────── + + describe('the RLS `check` consequence, pinned explicitly', () => { + it('a check that used to DENY now fails the operation instead', () => { + // Same outcome for the caller's data (the write does not land), different + // failure: INVALID_FILTER names the broken policy instead of blaming the + // writer with a permission denial. + expect(() => matchesFilterCondition(RECORD, { stage: {} } as never)).toThrow(/zero operators/); + }); + + it('a check that used to ALLOW (the false disjunct was absorbed) now fails', () => { + // Pre-fix: `{ a: {} }` → false, `{ owner: 'u1' }` → true, `$or` → true → + // the write was permitted by a policy half of which was meaningless. + expect(() => matchesFilterCondition(RECORD, { $or: [{ stage: {} }, { owner: 'u1' }] } as never)) + .toThrow(/zero operators/); + }); + + it('a check under $not that used to ALLOW now fails', () => { + // Pre-fix: `!false` → true → permitted. + expect(() => matchesFilterCondition(RECORD, { $not: { stage: {} } } as never)).toThrow(/zero operators/); + }); + + it('the refusal does not depend on the RECORD, so a policy is not row-dependent', () => { + const rows = [RECORD, { id: '2', stage: 'lost', owner: 'u2', amount: 20 }, {}]; + for (const row of rows) { + expect(() => matchesFilterCondition(row as never, { stage: 'no-match', owner: {} } as never)) + .toThrow(/zero operators/); + } + }); + }); + + // ── The fail-closed posture is otherwise untouched ──────────────────────── + + describe('every other unevaluable shape still fails CLOSED (returns false)', () => { + it('an unknown operator', () => { + expect(matchesFilterCondition(RECORD, { stage: { $sounds_like: 'won' } } as never)).toBe(false); + }); + + it('a nested relation object a flat record cannot satisfy', () => { + expect(matchesFilterCondition(RECORD, { stage: { nested: 'won' } } as never)).toBe(false); + }); + + it('a bare array field spec', () => { + expect(matchesFilterCondition(RECORD, { stage: ['won'] } as never)).toBe(false); + }); + + it('an unknown top-level operator', () => { + expect(matchesFilterCondition(RECORD, { $nope: 1 } as never)).toBe(false); + }); + + it('a non-object filter', () => { + expect(matchesFilterCondition(RECORD, 'x' as never)).toBe(false); + }); + }); + + describe('ordinary evaluation is byte-identical', () => { + it('answers the same booleans as before', () => { + expect(matchesFilterCondition(RECORD, { stage: 'won' } as never)).toBe(true); + expect(matchesFilterCondition(RECORD, { stage: 'lost' } as never)).toBe(false); + expect(matchesFilterCondition(RECORD, { amount: { $gt: 5 } } as never)).toBe(true); + expect(matchesFilterCondition(RECORD, { $or: [{ stage: 'lost' }, { owner: 'u1' }] } as never)).toBe(true); + expect(matchesFilterCondition(RECORD, { $not: { stage: 'won' } } as never)).toBe(false); + expect(matchesFilterCondition(RECORD, {} as never)).toBe(true); + expect(matchesFilterCondition(RECORD, null)).toBe(true); + }); + + it('an EMPTY NODE keeps its #5134 identity meaning, it is not this shape', () => { + expect(matchesFilterCondition(RECORD, { $or: [{ stage: 'lost' }, {}] } as never)).toBe(true); + expect(matchesFilterCondition(RECORD, { $not: {} } as never)).toBe(false); + }); + + it('a Date comparand enumerates to nothing but is NOT a zero-operator constraint', () => { + const d = new Date('2026-01-01T00:00:00Z'); + expect(matchesFilterCondition({ due: d }, { due: d } as never)).toBe(true); + }); + }); +}); diff --git a/packages/formula/src/matches-filter.ts b/packages/formula/src/matches-filter.ts index 78fad38984..d89188f45e 100644 --- a/packages/formula/src/matches-filter.ts +++ b/packages/formula/src/matches-filter.ts @@ -14,18 +14,112 @@ * node, an unknown operator, a nested relation object a flat record can't * satisfy — returns `false` (the write is denied), never `true`. The operator * vocabulary mirrors `read-scope-sql.ts` so the in-memory and SQL backends agree. + * + * ONE shape is refused instead of answered (#5240): `{ field: {} }`, a field + * constrained by zero operators, throws `INVALID_FILTER` rather than returning + * `false`. It is the shape the four backends could not agree on, so no answer + * here is defensible; the operation fails, which is the #4775 posture for a + * `check` that cannot be evaluated. Note this is not merely a louder denial: + * where such a constraint sat under an `$or` beside a satisfied branch, or under + * a `$not`, the old `false` was ABSORBED and the write was allowed. Those writes + * now fail. See {@link emptyFieldConstraintError}. */ import type { FilterCondition } from '@objectstack/spec/data'; import { nextUtcCalendarDay, utcInstantMs } from '@objectstack/spec/data'; +import { StandardErrorCode } from '@objectstack/spec/api'; + +/** + * [#5240] `{ field: {} }` — a field constrained by ZERO operators — is REFUSED, + * not evaluated, and this is the ONE place this fail-closed evaluator throws. + * + * The shape had three answers in the repo: `driver-sql` refused it at the top + * level but dropped it inside `$and`/`$or`/`$not` (a predicate that emits + * nothing matches every row), `driver-memory` answered "matches nothing" by + * accident of structural equality, and THIS evaluator answered `false` from the + * explicit `keys.length === 0` arm below. Ruled on #5240: refused in all four + * backends, with the same `INVALID_FILTER` code, so an authoring accident — a + * filter builder that recorded a field and never its operator — fails loudly at + * the producer instead of quietly changing a row count per backend. + * + * # Why this one throw does not weaken the fail-closed posture + * + * "Fail closed" is about what an UNEVALUABLE condition does to an ANSWER: it + * must never widen access. Throwing is the strongest form of that — there is no + * answer to widen — and it lands on the posture #4775 already settled for this + * surface: a `check` that cannot be evaluated fails the operation. What changes + * is the shape of the failure, and one case where the outcome flips outright: + * a `check` whose broken constraint sat under an `$or` beside a satisfied + * branch, or under a `$not`, used to evaluate to ALLOW. Those writes now fail. + * That is a real, observable behaviour change and it is the point of the ruling + * — the alternative is a permission rule whose meaning depends on which of four + * backends evaluated it. + */ +function emptyFieldConstraintError(field: string, path: string): Error { + const err = new Error( + `Field constraint at ${path} carries zero operators ({ "${field}": {} }). A field constraint ` + + `must name at least one operator (e.g. { "${field}": { "$eq": "value" } }) or be a direct ` + + `comparand (e.g. { "${field}": "value" }). It is refused rather than evaluated because the ` + + `backends disagreed on what it means — driver-sql dropped it inside $and/$or/$not (matching ` + + `EVERY row) while refusing it at the top level, and driver-memory / this evaluator ` + + `answered "matches nothing". #5240.`, + ) as Error & { code?: string; status?: number }; + err.code = StandardErrorCode.enum.INVALID_FILTER; + err.status = 400; + return err; +} /** True iff `record` satisfies `filter`. A null/empty filter matches everything. */ export function matchesFilterCondition(record: Record, filter: FilterCondition | null | undefined): boolean { if (filter == null) return true; if (typeof filter !== 'object' || Array.isArray(filter)) return false; + // [#5240] Shape first, then evaluate. The refusal is raised by a walk of the + // WHOLE tree, up front, rather than from inside `evalField` — because the + // evaluator short-circuits (`every`/`some`, and a node returns on its first + // false entry), so a refusal raised mid-evaluation would fire or not fire + // depending on the RECORD being tested. A malformed permission rule must be + // refused for every record or none. Evaluation below is untouched. + assertFilterShape(filter as Record, 'filter'); return evalNode(record, filter as Record); } +/** + * [#5240] Walk the whole condition tree and refuse any zero-operator field + * constraint. Shapes this evaluator already answers fail-closed (a non-node + * `$and` element, an unknown `$`-operator, a bare array field spec) are left to + * it — this walk adds exactly one refusal and changes nothing else. + */ +function assertFilterShape(node: unknown, path: string): void { + if (node == null || typeof node !== 'object' || Array.isArray(node)) return; + for (const [key, val] of Object.entries(node as Record)) { + const here = `${path}.${key}`; + if (key === '$and' || key === '$or') { + if (Array.isArray(val)) val.forEach((child, i) => assertFilterShape(child, `${here}[${i}]`)); + continue; + } + if (key === '$not') { + assertFilterShape(val, here); + continue; + } + if (key.startsWith('$')) continue; + if (isEmptyFieldConstraint(val)) throw emptyFieldConstraintError(key, here); + } +} + +/** + * [#5240] Is this field spec `{}` — a field constrained by ZERO operators? + * + * A plain object with no own enumerable keys, and nothing else: a `Date` also + * enumerates to nothing but is a COMPARAND (`evalField` treats it as implicit + * equality), not a constraint. + */ +function isEmptyFieldConstraint(spec: unknown): boolean { + if (spec === null || typeof spec !== 'object' || Array.isArray(spec) || spec instanceof Date) return false; + const proto = Object.getPrototypeOf(spec); + if (proto !== Object.prototype && proto !== null) return false; + return Object.keys(spec as Record).length === 0; +} + function evalNode(record: Record, node: Record): boolean { // A node is the AND of all its entries. for (const [key, val] of Object.entries(node)) { @@ -58,6 +152,12 @@ function evalField(record: Record, field: string, spec: unknown const keys = Object.keys(ops); // Must be all-operators; a non-`$` key means a nested relation a flat record // cannot satisfy → fail closed. + // + // [#5240] `keys.length === 0` no longer reaches this arm on the public entry + // point: `assertFilterShape` refuses `{ field: {} }` before evaluation starts. + // The clause stays because this function is also reachable from a recursive + // `evalNode` on a subtree, and a total function must stay total — but it is a + // floor, no longer this backend's ANSWER to the shape. if (keys.length === 0 || keys.some((k) => !k.startsWith('$'))) return false; for (const op of keys) { if (!evalOp(actual, op, ops[op], record)) return false; diff --git a/packages/plugins/driver-memory/src/filter-refusal.ts b/packages/plugins/driver-memory/src/filter-refusal.ts new file mode 100644 index 0000000000..740a2f43ca --- /dev/null +++ b/packages/plugins/driver-memory/src/filter-refusal.ts @@ -0,0 +1,75 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The filter refusals this driver raises, in ONE place. + * + * Both of this package's filter surfaces refuse the same shapes with the same + * wire envelope: the live query path (`memory-driver.ts` → mingo) and the + * reference matcher (`memory-matcher.ts`, the record-at-a-time evaluator the + * conformance suites hold against `driver-sql` and `@objectstack/formula`). They + * were two independent code paths with two independent notions of what a filter + * may be, which is exactly how #5240's divergence survived unnoticed in-package. + */ + +import { StandardErrorCode } from '@objectstack/spec/api'; + +/** + * [#4436] A filter this driver cannot evaluate — see the twin in `driver-sql`'s + * `unsupportedFilterError`, which carries the full rationale. + * + * Kept in lockstep with driver-sql deliberately: #3948 made the two backends + * AGREE that an uncompilable filter is a refusal rather than a silent + * match-everything, and the refusal's wire envelope has to agree too. A test + * suite that swaps the memory driver for SQL must see the same `400 + * INVALID_FILTER`, not a coded refusal on one backend and a bare `{error}` on + * the other. + */ +export 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; +} + +/** + * [#5240] Is this field spec `{}` — a field constrained by ZERO operators? + * + * A plain object with no own enumerable keys, and nothing else: a `Date`, a + * `RegExp` or a class instance also enumerates to nothing but is a COMPARAND, + * not a constraint, and is left to the paths that already handle it. + */ +export function isEmptyFieldConstraint(spec: unknown): boolean { + if (spec === null || typeof spec !== 'object' || Array.isArray(spec)) return false; + const proto = Object.getPrototypeOf(spec); + if (proto !== Object.prototype && proto !== null) return false; + return Object.keys(spec as Record).length === 0; +} + +/** + * [#5240] `{ field: {} }` — a field constrained by ZERO operators. + * + * One declared shape, three answers across the repo: `driver-sql` refused it at + * the top level but DROPPED it inside `$and`/`$or`/`$not` (a predicate that + * emits nothing matches every row), while this driver and `@objectstack/formula` + * answered "matches nothing" — this one only incidentally, by falling through to + * a structural-equality comparison against the empty object. + * + * Ruled on #5240: refused everywhere, in the ADR-0112 envelope every sibling + * filter refusal speaks. The shape is almost always an authoring accident (a + * filter builder that recorded a field and never its operator, or generated + * metadata that lost one), and both silent readings answer it with a row count + * the author never asked for. This driver's incidental FALSE is the clearest + * case of that: `{ status: {} }` did not mean "no rows", it meant "rows whose + * `status` is literally the empty object" — a different filter that HAPPENS to + * match nothing in ordinary data. + */ +export function emptyFieldConstraintError(field: string, path: string): Error { + return unsupportedFilterError( + `Field constraint at ${path} carries zero operators ({ "${field}": {} }). A field constraint ` + + `must name at least one operator (e.g. { "${field}": { "$eq": "value" } }) or be a direct ` + + `comparand (e.g. { "${field}": "value" }). It is refused rather than evaluated because the ` + + `backends disagreed on what it means — driver-sql dropped it inside $and/$or/$not (matching ` + + `EVERY row) while refusing it at the top level, and this driver / @objectstack/formula ` + + `answered "matches nothing". #5240.`, + ); +} diff --git a/packages/plugins/driver-memory/src/memory-driver.ts b/packages/plugins/driver-memory/src/memory-driver.ts index 6c690a860b..3b9b159b7c 100644 --- a/packages/plugins/driver-memory/src/memory-driver.ts +++ b/packages/plugins/driver-memory/src/memory-driver.ts @@ -3,34 +3,20 @@ import type { QueryAST, QueryInput, DriverOptions } from '@objectstack/spec/data'; import { canonicalAstOperator } from '@objectstack/spec/data'; import type { IDataDriver } from '@objectstack/spec/contracts'; -import { StandardErrorCode } from '@objectstack/spec/api'; import { Logger, createLogger, nextUtcCalendarDay } from '@objectstack/core'; import { Query, Aggregator } from 'mingo'; import { getValueByPath } from './memory-matcher.js'; +import { + emptyFieldConstraintError, + isEmptyFieldConstraint, + unsupportedFilterError, +} from './filter-refusal.js'; import { coerceTemporalValue, indexTemporalFields, type TemporalFieldKind, } from './memory-temporal.js'; -/** - * [#4436] A filter this driver cannot COMPILE — see the twin in - * `driver-sql`'s `unsupportedFilterError`, which carries the full rationale. - * - * Kept in lockstep with driver-sql deliberately: #3948 made the two backends - * AGREE that an uncompilable filter is a refusal rather than a silent - * match-everything, and the refusal's wire envelope has to agree too. A test - * suite that swaps the memory driver for SQL must see the same `400 - * INVALID_FILTER`, not a coded refusal on one backend and a bare `{error}` on - * the other. - */ -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; -} - /** * Persistence adapter interface. * Matches the PersistenceAdapterSchema contract from @objectstack/spec. @@ -875,22 +861,33 @@ export class InMemoryDriver implements IDataDriver { * operators ($contains, $notContains, $startsWith, $endsWith, $between, $null) * to Mingo-compatible equivalents ($regex, $gte/$lte, null checks). */ - private normalizeFilterCondition(filter: Record, object?: string): Record { + private normalizeFilterCondition(filter: Record, object?: string, path = 'filter'): Record { const result: Record = {}; const extraAndConditions: Record[] = []; for (const key of Object.keys(filter)) { const value = filter[key]; + const here = `${path}.${key}`; + // [#5240] `{ field: {} }` is refused in EVERY position, before mingo sees + // it. Left alone it normalises to `{ field: {} }`, which mingo reads as + // "the field deep-equals the empty document" — a filter that matches + // nothing in ordinary data and is therefore indistinguishable, from the + // outside, from the FALSE the reference matcher answered. Neither is what + // the author meant, and driver-sql read the same shape as TRUE inside a + // combinator. Refused rather than reinterpreted; see the ruling on #5240. + if (isEmptyFieldConstraint(value) && !key.startsWith('$')) { + throw emptyFieldConstraintError(key, here); + } // Recurse into logical operators if (key === '$and' || key === '$or') { result[key] = Array.isArray(value) - ? value.map((child: any) => this.normalizeFilterCondition(child, object)) + ? value.map((child: any, i: number) => this.normalizeFilterCondition(child, object, `${here}[${i}]`)) : value; continue; } if (key === '$not') { result[key] = value && typeof value === 'object' - ? this.normalizeFilterCondition(value, object) + ? this.normalizeFilterCondition(value, object, here) : value; continue; } diff --git a/packages/plugins/driver-memory/src/memory-empty-field-constraint.test.ts b/packages/plugins/driver-memory/src/memory-empty-field-constraint.test.ts new file mode 100644 index 0000000000..7ad35d3410 --- /dev/null +++ b/packages/plugins/driver-memory/src/memory-empty-field-constraint.test.ts @@ -0,0 +1,181 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5240] `{ field: {} }` — a field constrained by ZERO operators — is REFUSED + * by BOTH of this package's filter surfaces, with the same `INVALID_FILTER` + * envelope `driver-sql` and `@objectstack/formula` now raise. + * + * # Why two surfaces + * + * This package evaluates filters twice, through code that never meets: + * + * - `InMemoryDriver.find` (the LIVE query path) normalises the condition and + * hands it to **mingo**. The issue's table attributed this driver's answer to + * `memory-matcher`, but the driver does not call it — it imports only + * `getValueByPath`. Measured here rather than assumed: mingo reads a bare + * `{ a: {} }` as "the field deep-equals the empty document", so the shape + * selected rows whose `a` is literally `{}` and looked like "matches nothing" + * on ordinary data. Right answer for the wrong reason, and a DIFFERENT filter + * from the FALSE everyone believed was being computed. + * - `memory-matcher.match` (the reference matcher the cross-backend conformance + * suites hold against driver-sql and formula) fell through to + * `JSON.stringify(value) === JSON.stringify(condition)` — structural equality + * against `{}` — and answered `false` incidentally. + * + * Neither was a ruling, and driver-sql read the same shape as TRUE inside a + * combinator. #5240 ruled REFUSE; a backend whose two halves disagree about + * what a filter MEANS is exactly the divergence the ruling closes, so both + * halves are pinned here. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { InMemoryDriver } from './memory-driver.js'; +import { match } from './memory-matcher.js'; +import type { FilterCondition } from '@objectstack/spec/data'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +const ROWS = [ + { id: '1', stage: 'won', owner: 'u1', amount: 10 }, + { id: '2', stage: 'lost', owner: 'u2', amount: 20 }, + { id: '3', stage: 'open', owner: 'u1', amount: 30 }, +]; + +/** The positions every backend pins, in the wording of the issue's table. */ +const POSITIONS: Array<[string, unknown, string]> = [ + ['top level', { stage: {} }, 'filter.stage'], + ['inside $or', { $or: [{ stage: {} }, { owner: 'u2' }] }, 'filter.$or[0].stage'], + ['inside $and', { $and: [{ stage: 'won' }, { owner: {} }] }, 'filter.$and[1].owner'], + ['inside $not', { $not: { stage: {} } }, 'filter.$not.stage'], + ['nested two combinators deep', { $and: [{ $or: [{ stage: {} }] }] }, 'filter.$and[0].$or[0].stage'], +]; + +describe('[#5240] InMemoryDriver (live mingo path) refuses a zero-operator field constraint', () => { + let driver: InMemoryDriver; + + beforeEach(async () => { + driver = new InMemoryDriver(); + await driver.syncSchema('deal', { + fields: { + id: { type: 'text', name: 'id' }, + stage: { type: 'text', name: 'stage' }, + owner: { type: 'text', name: 'owner' }, + amount: { type: 'number', name: 'amount' }, + }, + }); + for (const row of ROWS) await driver.create('deal', { ...row }); + }); + + const ids = async (where: unknown): Promise => { + const rows = await driver.find('deal', { object: 'deal', fields: ['id'], where: where as FilterCondition }); + return rows.map((r: any) => 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'); + }; + + for (const [name, where, position] of POSITIONS) { + it(`${name} → 400 INVALID_FILTER naming ${position}`, async () => { + const err = await refusalOf(where); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain(position); + expect(err.message).toContain('zero operators'); + expect(err.message).not.toContain('[driver-memory]'); + }); + } + + it('the refusal replaces a filter that was silently something ELSE', async () => { + // Pre-fix this resolved: mingo compared `stage` against the empty document, + // returning no rows — indistinguishable from a deliberate FALSE, and not + // what the author asked for either way. + await expect(ids({ stage: {} })).rejects.toThrow(/zero operators/); + }); + + describe('everything else on this path is unchanged', () => { + it('ordinary filters still select the same rows', async () => { + expect(await ids({ stage: 'won' })).toEqual(['1']); + expect(await ids({ amount: { $gt: 15 } })).toEqual(['2', '3']); + expect(await ids({ $or: [{ stage: 'won' }, { owner: 'u2' }] })).toEqual(['1', '2']); + expect(await ids({ $and: [{ owner: 'u1' }, { amount: { $gt: 15 } }] })).toEqual(['3']); + expect(await ids({})).toEqual(['1', '2', '3']); + }); + + it('an EMPTY NODE is not a zero-operator constraint and is still accepted', async () => { + expect(await ids({ $or: [{ stage: 'won' }, {}] })).toEqual(['1', '2', '3']); + }); + + it('a $-key whose value is an empty object is NOT treated as a field constraint', async () => { + // `{ $not: {} }` is the #5134 identity (NOT TRUE ≡ FALSE), not this shape, + // so the #5240 gate must not claim it. On this path it still fails — but + // for a pre-existing and entirely separate reason: the live query path + // hands `$not` straight to mingo, which has no document-level `$not` at + // all (measured while verifying this issue's premise; filed as #5324). + // Pinned as the CURRENT truth, so #5324's fix is the thing that changes + // it and this suite is not silently asserting a behaviour nobody has. + await expect(ids({ $not: {} })).rejects.toThrow(/unknown top level operator/); + await expect(ids({ $not: {} })).rejects.not.toThrow(/zero operators/); + }); + }); +}); + +describe('[#5240] memory-matcher (reference matcher) refuses the same shape', () => { + const matched = (filter: unknown): string[] => + ROWS.filter((r) => match(r, filter)).map((r) => r.id); + + const refusalOf = (filter: unknown): WireBearingError => { + try { + matched(filter); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the matcher to refuse this filter, but it answered'); + }; + + for (const [name, filter, position] of POSITIONS) { + it(`${name} → INVALID_FILTER naming ${position}`, () => { + const err = refusalOf(filter); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain(position); + expect(err.message).toContain('zero operators'); + }); + } + + it('the refusal does not depend on the RECORD being tested', () => { + // The evaluator short-circuits (`every`/`some`, and a node returns on its + // first failing key), so a gate inside evaluation would fire for some rows + // and not others. The walk runs before evaluation, so every row refuses. + for (const row of ROWS) { + expect(() => match(row, { stage: 'nothing-matches-this', owner: {} })).toThrow(/zero operators/); + expect(() => match(row, { $or: [{ stage: 'won' }, { owner: {} }] })).toThrow(/zero operators/); + } + }); + + describe('evaluation is otherwise byte-identical', () => { + it('ordinary filters answer exactly as before', () => { + expect(matched({ stage: 'won' })).toEqual(['1']); + expect(matched({ amount: { $gt: 15 } })).toEqual(['2', '3']); + expect(matched({ $or: [{ stage: 'won' }, { owner: 'u2' }] })).toEqual(['1', '2']); + expect(matched({ $and: [{ owner: 'u1' }, { amount: { $gt: 15 } }] })).toEqual(['3']); + expect(matched({ $not: { stage: 'won' } })).toEqual(['2', '3']); + expect(matched({})).toEqual(['1', '2', '3']); + }); + + it('a nested object comparison (a NON-empty plain object) still compares structurally', () => { + // The arm `{ field: {} }` used to fall into. It keeps its behaviour for + // every shape that actually carries keys. + expect(match({ meta: { a: 1 } }, { meta: { a: 1 } })).toBe(true); + expect(match({ meta: { a: 2 } }, { meta: { a: 1 } })).toBe(false); + }); + }); +}); diff --git a/packages/plugins/driver-memory/src/memory-matcher.ts b/packages/plugins/driver-memory/src/memory-matcher.ts index 3957e33662..9ae41dd68a 100644 --- a/packages/plugins/driver-memory/src/memory-matcher.ts +++ b/packages/plugins/driver-memory/src/memory-matcher.ts @@ -3,11 +3,17 @@ /** * Simple In-Memory Query Matcher - * + * * Implements a subset of the ObjectStack Filter Protocol (MongoDB-compatible) * for evaluating conditions against in-memory JavaScript objects. + * + * It answers a boolean for every filter it accepts; the one shape it REFUSES + * (throwing `INVALID_FILTER` instead of answering) is `{ field: {} }` — see + * `filter-refusal.ts` and the ruling on #5240. */ +import { emptyFieldConstraintError, isEmptyFieldConstraint } from './filter-refusal.js'; + type RecordType = Record; /** @@ -16,6 +22,38 @@ type RecordType = Record; * @param filter The filter condition (where clause) */ export function match(record: RecordType, filter: any): boolean { + // [#5240] Shape first, then evaluate. The refusal is raised by a walk of the + // WHOLE tree rather than from inside the field loop below, because that loop + // short-circuits (`every`/`some`, and the loop returns on its first failing + // key) — a refusal raised mid-evaluation would fire or not fire depending on + // the RECORD being tested. Evaluation below is untouched. + assertFilterShape(filter, 'filter'); + return evaluate(record, filter); +} + +/** + * [#5240] Walk the whole condition tree and refuse any zero-operator field + * constraint. Every other malformed shape keeps whatever this matcher does with + * it today — this walk adds exactly one refusal. + */ +function assertFilterShape(node: unknown, path: string): void { + if (node == null || typeof node !== 'object' || Array.isArray(node)) return; + for (const [key, val] of Object.entries(node as Record)) { + const here = `${path}.${key}`; + if (key === '$and' || key === '$or') { + if (Array.isArray(val)) val.forEach((child, i) => assertFilterShape(child, `${here}[${i}]`)); + continue; + } + if (key === '$not') { + assertFilterShape(val, here); + continue; + } + if (key.startsWith('$')) continue; + if (isEmptyFieldConstraint(val)) throw emptyFieldConstraintError(key, here); + } +} + +function evaluate(record: RecordType, filter: any): boolean { if (!filter || Object.keys(filter).length === 0) return true; // 1. Handle Top-Level Logical Operators ($and, $or, $not) @@ -23,33 +61,33 @@ export function match(record: RecordType, filter: any): boolean { // $and: [ { ... }, { ... } ] if (Array.isArray(filter.$and)) { - if (!filter.$and.every((f: any) => match(record, f))) { + if (!filter.$and.every((f: any) => evaluate(record, f))) { return false; } } - + // $or: [ { ... }, { ... } ] if (Array.isArray(filter.$or)) { - if (!filter.$or.some((f: any) => match(record, f))) { + if (!filter.$or.some((f: any) => evaluate(record, f))) { return false; } } - + // $not: { ... } if (filter.$not) { - if (match(record, filter.$not)) { + if (evaluate(record, filter.$not)) { return false; } } - + // 2. Iterate over field constraints for (const key of Object.keys(filter)) { // Skip logical operators we already handled (or future ones) if (key.startsWith('$')) continue; - + const condition = filter[key]; const value = getValueByPath(record, key); - + if (!checkCondition(value, condition)) { return false; } @@ -90,6 +128,11 @@ function checkCondition(value: any, condition: any): boolean { if (!isOperatorObject) { // It's just a nested object comparison or implicit equality against an object // Simplistic check: + // [#5240] `condition` is never `{}` here — the caller refuses the + // zero-operator constraint before this point. That matters, because + // this arm answering `false` for `{}` was the whole of this backend's + // "FALSE" verdict on the shape: an accident of structural equality, not + // a semantic ruling anyone had made. return JSON.stringify(value) === JSON.stringify(condition); } diff --git a/packages/plugins/driver-sql/src/sql-driver-empty-field-constraint.test.ts b/packages/plugins/driver-sql/src/sql-driver-empty-field-constraint.test.ts new file mode 100644 index 0000000000..1178c8072b --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-empty-field-constraint.test.ts @@ -0,0 +1,221 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5240] `{ field: {} }` — a field constrained by ZERO operators — is REFUSED, + * in every position, with the ADR-0112 `INVALID_FILTER` envelope. + * + * # The three answers this replaces + * + * | path | before | after | + * |---|---|---| + * | `applyFilters` top-level plain map | refused (#5041 comparand gate) | refused (#5240 message) | + * | `applyFilterCondition`, inside `$and`/`$or`/`$not` | zero operators → no SQL → **TRUE** | refused | + * | `driver-memory` | FALSE (structural equality) | refused | + * | `@objectstack/formula` | FALSE (explicit fail-closed arm) | refused | + * + * This driver contradicted ITSELF: the same `{ a: {} }` was a 400 at the top + * level and a silent match-everything one combinator deep, so + * `{ $or: [ { a: {} }, { b: 2 } ] }` compiled to `(b = 2)` while the algorithm + * that treats a zero-constraint node as TRUE says it should match every row and + * the two JS backends say it should match only `b = 2`. Three readings of one + * declared shape; the maintainer ruled REFUSE (#5240) — an authoring accident + * (a filter builder that recorded a field and never its operator) must fail at + * the producer, not change a row count per backend. + * + * # Where the gate sits, and why + * + * On the `reduceFilterNode` / `reduceFilterKey` VALIDATION WALK (#5134), beside + * `assertFilterNode`, not in the emitter. The walk is exhaustive and does not + * short-circuit; the emitter returns early whenever an identity settles the + * node. A gate in the emitter would let `{ $or: [ { a: {} }, {} ] }` through + * untouched — the `{}` disjunct reduces the `$or` to TRUE and the emitter never + * reaches `{ a: {} }` — making the refusal depend on the shape's SIBLINGS. The + * verdict function is otherwise unchanged: a field key still contributes + * `'clause'`, so every filter that compiled before compiles identically. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { SqlDriver } from '../src/index.js'; +import type { FilterCondition } from '@objectstack/spec/data'; + +const FIXTURE = [ + { id: '1', stage: 'won', owner: 'u1', amount: 10 }, + { id: '2', stage: 'lost', owner: 'u2', amount: 20 }, + { id: '3', stage: 'open', owner: 'u1', amount: 30 }, +]; + +const ALL = ['1', '2', '3']; + +/** The shape `mapDataError` / `sendError` read off a thrown driver error. */ +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +describe('[#5240] SqlDriver refuses a field constrained by zero operators', () => { + let driver: SqlDriver; + let knex: any; + + beforeEach(async () => { + driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + knex = (driver as any).knex; + await knex.schema.createTable('deal', (t: any) => { + t.string('id').primary(); + t.string('stage'); + t.string('owner'); + t.float('amount'); + }); + await knex('deal').insert(FIXTURE); + }); + + afterEach(async () => { + await knex.destroy(); + }); + + // The casts are deliberate: `{ field: {} }` is a shape `FilterConditionSchema` + // still DECLARES as legal (its non-recursive half is `z.record(z.string(), + // z.unknown())`), which is the whole point — the implementation is now + // stricter than the declared contract, and narrowing the schema is the spec + // lane's half of #5240 (#5239 / #5146 batch). + const ids = async (where: unknown): Promise => { + const rows = await driver.find('deal', { + object: 'deal', + fields: ['id'], + where: where as FilterCondition, + }); + return rows.map((r: any) => 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'); + }; + + const sqlFor = (where: unknown): string => { + const qb = knex('deal').select('id'); + (driver as any).applyFilters(qb, where); + return qb.toString(); + }; + + // ── The pinned positions ────────────────────────────────────────────────── + + const positions: Array<[string, unknown, string]> = [ + ['top level', { stage: {} }, 'filter.stage'], + ['top level beside a real operator constraint', { stage: {}, amount: { $gt: 1 } }, 'filter.stage'], + ['inside $or', { $or: [{ stage: {} }, { owner: 'u2' }] }, 'filter.$or[0].stage'], + ['inside $and', { $and: [{ stage: 'won' }, { owner: {} }] }, 'filter.$and[1].owner'], + ['inside $not', { $not: { stage: {} } }, 'filter.$not.stage'], + ['nested two combinators deep', { $and: [{ $or: [{ stage: {} }] }] }, 'filter.$and[0].$or[0].stage'], + ['beside a sibling that would settle the node first', { $or: [{ stage: {} }, {} ] }, 'filter.$or[0].stage'], + ['under a $not whose operand also has a real constraint', { $not: { stage: {}, owner: 'u1' } }, 'filter.$not.stage'], + ]; + + for (const [name, where, position] of positions) { + it(`${name} → 400 INVALID_FILTER naming ${position}`, async () => { + const err = await refusalOf(where); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain(position); + expect(err.message).toContain('zero operators'); + // #3867 — driver-internal wording never reaches the wire. + expect(err.message).not.toContain('[sql-driver]'); + }); + } + + // ── The internal inconsistency this closes ──────────────────────────────── + + describe('the top level and a combinator now give the SAME answer', () => { + it('`{stage:{}}` alone and `{$or:[{stage:{}},…]}` are both refused, with the same code', async () => { + const top = await refusalOf({ stage: {} }); + const nested = await refusalOf({ $or: [{ stage: {} }, { owner: 'u2' }] }); + expect(top.code).toBe(nested.code); + expect(top.status).toBe(nested.status); + expect(top.code).toBe('INVALID_FILTER'); + }); + + it('wrapping the refused shape in a combinator no longer turns it into match-all', async () => { + // The reported defect, verbatim: `{$or:[{a:{}},{b:2}]}` compiled to + // `(b = 2)` — neither the TRUE that "zero constraints" implies nor the + // FALSE the JS backends answered, but a DROPPED clause that coincided + // with one of them. It is now refused instead of quietly picking a side. + await expect(ids({ $or: [{ stage: {} }, { owner: 'u2' }] })).rejects.toThrow(/zero operators/); + }); + + it('and `$not` does not swallow it into the #5146 NULL-safe rewrite either', async () => { + // `nullGuardForFieldSpec` used to answer `'none'` for an empty spec + // precisely so it would not rule on #5240 from there. The refusal now + // fires on the validation walk, which runs BEFORE that rewrite. + await expect(ids({ $not: { stage: {} } })).rejects.toThrow(/zero operators/); + }); + }); + + // ── The guard must not touch anything else ──────────────────────────────── + + describe('non-empty shapes compile byte-identically', () => { + it('a plain equality map is unchanged', () => { + expect(sqlFor({ stage: 'won' })).toBe("select `id` from `deal` where `stage` = 'won'"); + }); + + it('an operator constraint is unchanged', () => { + expect(sqlFor({ amount: { $gt: 15 } })).toBe('select `id` from `deal` where `amount` > 15'); + }); + + it('a $or of plain comparisons is unchanged', () => { + expect(sqlFor({ $or: [{ stage: 'won' }, { owner: 'u2' }] })).toBe( + "select `id` from `deal` where ((`stage` = 'won') or (`owner` = 'u2'))", + ); + }); + + it('a $not of a plain comparison keeps its #5146 NULL guard', () => { + expect(sqlFor({ $not: { stage: 'won' } })).toBe( + "select `id` from `deal` where not (((`stage` is not null) and (`stage` = 'won')))", + ); + }); + + it('a $in list is unchanged', () => { + expect(sqlFor({ stage: { $in: ['won', 'open'] } })).toBe( + "select `id` from `deal` where `stage` in ('won', 'open')", + ); + }); + + it('and they still select the same rows', async () => { + expect(await ids({ stage: 'won' })).toEqual(['1']); + expect(await ids({ amount: { $gt: 15 } })).toEqual(['2', '3']); + expect(await ids({ $or: [{ stage: 'won' }, { owner: 'u2' }] })).toEqual(['1', '2']); + expect(await ids({ $not: { stage: 'won' } })).toEqual(['2', '3']); + expect(await ids({})).toEqual(ALL); + }); + }); + + describe('the #5134 boolean identities are untouched', () => { + it('an EMPTY NODE is still TRUE — `{}` is not `{ field: {} }`', async () => { + expect(await ids({ $or: [{ stage: 'won' }, {}] })).toEqual(ALL); + expect(await ids({ $and: [] })).toEqual(ALL); + }); + + it('an empty $or is still FALSE and an empty $not still zero rows', async () => { + expect(await ids({ $or: [] })).toEqual([]); + expect(await ids({ $not: {} })).toEqual([]); + }); + }); + + describe('shapes that merely LOOK empty are not caught by the gate', () => { + it('a Date comparand is a comparand, not a zero-operator constraint', async () => { + // `Object.keys(new Date())` is also `[]`; the gate requires a PLAIN object, + // so a temporal comparand keeps whatever the compiler does with it. + await expect(ids({ stage: new Date('2026-01-01') })).resolves.toBeDefined(); + }); + + it('an empty $in list is a list operator, not an empty constraint', async () => { + expect(await ids({ stage: { $in: [] } })).toEqual([]); + }); + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver-not-null-safe.test.ts b/packages/plugins/driver-sql/src/sql-driver-not-null-safe.test.ts index 711d802f90..137cfa276d 100644 --- a/packages/plugins/driver-sql/src/sql-driver-not-null-safe.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-not-null-safe.test.ts @@ -266,11 +266,18 @@ describe('[#5146] SqlDriver compiles $not NULL-safely', () => { await expect(ids({ $not: [] })).rejects.toThrow(/filter\.\$not/); }); - it('a field constrained by zero operators is still not ruled on (#5240)', async () => { - // `{ stage: {} }` compiles to no SQL; guarding it would have turned that - // into a live `IS NULL` and decided #5240 from here. - expect(await ids({ $not: { stage: {} } })).toEqual(ALL); - expect(sqlFor({ $not: { stage: {} } })).toBe('select `id` from `deal`'); + it('a field constrained by zero operators is REFUSED, not rewritten (#5240)', async () => { + // Was: `{ stage: {} }` compiled to no SQL, and this suite pinned that it + // stayed that way — `nullGuardForFieldSpec` deliberately answered `'none'` + // for an empty spec so the NULL-safe rewrite would not RULE on #5240 by + // turning a shape that emits nothing into a live `IS NULL`. + // + // #5240 has since been ruled: the shape is refused in all four backends. + // The refusal fires on the #5134 validation walk, which runs BEFORE this + // rewrite, so the empty spec no longer reaches `nullGuardForFieldSpec` at + // all and the branch that protected it is gone. + await expect(ids({ $not: { stage: {} } })).rejects.toThrow(/zero operators/); + expect(() => sqlFor({ $not: { stage: {} } })).toThrow(/zero operators/); }); }); }); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index f15759e247..3f61c454b0 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -671,6 +671,41 @@ function assertFilterNode(value: unknown, path: string): asserts value is Record ); } +/** + * [#5240] `{ field: {} }` — a field constrained by ZERO operators. + * + * One declared shape, three answers across this repo: THIS driver refused it at + * the top level (the #5041 comparand gate) while DROPPING it inside + * `$and`/`$or`/`$not` — where a predicate that emits nothing means "matches + * every row" — and `driver-memory` / `@objectstack/formula` both answered + * "matches nothing". So `{ $or: [ { a: {} }, { b: 2 } ] }` returned a different + * row set per backend, and this driver contradicted ITSELF depending on whether + * the same constraint sat at the top level or one combinator deep. + * + * Ruled on #5240: refuse it everywhere, in the ADR-0112 envelope every sibling + * filter refusal here speaks. Not TRUE and not FALSE — because the shape is + * almost always an authoring accident (a filter builder that recorded a field + * and no operator, or generated metadata that lost its operator), and both + * silent readings answer it with a row count the author never asked for. The + * same reasoning #5041 applied one position over: a filter that cannot be given + * one meaning is refused at the producer, loudly, at authoring time. + */ +function emptyFieldConstraintError(field: string, path: string): Error { + return unsupportedFilterError( + `Field constraint at ${path} carries zero operators ({ "${field}": {} }). A field constraint ` + + `must name at least one operator (e.g. { "${field}": { "$eq": "value" } }) or be a direct ` + + `comparand (e.g. { "${field}": "value" }). It is refused rather than ignored because the ` + + `backends disagreed on what it means — this driver dropped it inside $and/$or/$not (matching ` + + `EVERY row) while refusing it at the top level, and driver-memory / @objectstack/formula ` + + `answered "matches nothing". #5240.`, + ); +} + +/** [#5240] Is this field spec the zero-operator constraint refused above? */ +function isEmptyFieldConstraint(spec: unknown): boolean { + return isFilterNode(spec) && Object.keys(spec).length === 0; +} + /** [#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; @@ -743,15 +778,24 @@ function reduceFilterKey(key: string, value: unknown, path: string): FilterVerdi return inner === 'true' ? 'false' : inner === 'false' ? 'true' : 'clause'; } - // A field key always contributes a predicate. Note this stays `'clause'` even - // for `{ field: {} }` (a field constrained by zero operators), which compiles - // to no SQL today. That shape is a SEPARATE divergence tracked in #5240 — - // `matchesFilter` and `driver-memory` both answer FALSE for it, this driver's - // combinator path answers TRUE, and its own top-level `applyFilters` path - // refuses it outright via `assertCompilableComparand` — and picking the winner - // is a semantic ruling, not this change's call. Classifying it as `'clause'` - // rather than `'true'` is precisely what keeps the identity reduction from - // silently ruling on it: the shape compiles exactly as it did before. + // [#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 + // does NOT short-circuit, so the refusal cannot be skipped by an identity that + // resolves the enclosing node first. Gating it in the compile branch instead + // would let `{ $or: [ { a: {} }, {} ] }` slip through untouched — the `{}` + // disjunct reduces the whole `$or` to TRUE, the emitter returns before it ever + // reaches `{ a: {} }`, and the shape would be refused or ignored depending on + // its SIBLINGS. That is the "gate conditional on evaluation order" this + // function's own doc comment warns against. + // + // Note what is deliberately NOT changed: the VERDICT. A field key still + // contributes `'clause'`, exactly as #5134 classified it. This adds a refusal, + // it does not reclassify a surviving shape, so every filter that compiled + // before compiles byte-identically now. + if (isEmptyFieldConstraint(value)) throw emptyFieldConstraintError(key, here); + + // A field key always contributes a predicate. return 'clause'; } @@ -829,10 +873,13 @@ function nullGuardForFieldSpec(spec: unknown): NullGuard { // A scalar / Date / array comparand is an implicit `=`; a NULL column fails it. if (typeof spec !== 'object' || spec instanceof Date || Array.isArray(spec)) return 'requireValue'; const entries = Object.entries(spec as Record); - // `{ field: {} }` compiles to no SQL at all. Guarding it would turn a shape - // that emits nothing into a live `IS NULL` predicate — i.e. would RULE on - // #5240 from here. Left exactly as it compiles today. - if (entries.length === 0) return 'none'; + // [#5240, was #5146] The `entries.length === 0` escape that used to sit here — + // "`{ field: {} }` compiles to no SQL, so guarding it would turn a shape that + // emits nothing into a live `IS NULL` predicate, i.e. would RULE on #5240 from + // here" — is GONE, together with the ambiguity it was protecting. #5240 ruled + // the shape REFUSED, and `reduceFilterKey` raises that refusal while validating + // the tree, which `applyFilterCondition` does BEFORE its `$not` branch calls + // this rewrite. So an empty spec can no longer reach this function at all. let total = true; let nullSatisfies = true; for (const [op, value] of entries) { @@ -5967,6 +6014,13 @@ export class SqlDriver implements IDataDriver { for (const [key, value] of Object.entries(filters)) { if (['limit', 'offset', 'fields', 'orderBy'].includes(key)) continue; const column = this.remoteColumn(table, key, key); + // #5240 — `{ field: {} }` reaches this loop when NO key of the filter + // carries an operator; the combinator path refuses it on the reduction + // walk. Refused here with the SAME message so one condition has one + // wording wherever the author wrote it — this position used to answer + // with #5041's generic "cannot be bound as a SQL parameter", which + // describes a comparand and not a constraint with no operator at all. + if (isEmptyFieldConstraint(value)) throw emptyFieldConstraintError(key, `filter.${key}`); // #5041 — the plain `{ field: value }` map compiles to an implicit `=`, // so it is a comparison emitter too and gets the same gate. assertCompilableComparand(column, '=', value); @@ -6259,6 +6313,14 @@ export class SqlDriver implements IDataDriver { * that reduces to `'false'` never reaches the loop at all. So Knex is never * again in a position to silently discard a group. * + * # Zero-operator field constraints (#5240) + * + * `{ field: {} }` is REFUSED by that same reduction walk, in every position. + * It used to be this method's last remaining way to emit nothing for a key + * that looked like a predicate — so it read as TRUE here while the top-level + * `applyFilters` path refused it and the two JS backends answered FALSE. One + * declared shape, three answers; see {@link emptyFieldConstraintError}. + * * # NULL-safe negation (#5146) * * `$not` negates a predicate that {@link nullSafeNegationOperand} has first diff --git a/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-empty-field-constraint.test.ts b/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-empty-field-constraint.test.ts new file mode 100644 index 0000000000..b4fddd0ddd --- /dev/null +++ b/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-empty-field-constraint.test.ts @@ -0,0 +1,83 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5240] The wasm driver refuses `{ field: {} }` too — the fourth backend. + * + * `SqliteWasmDriver extends SqlDriver`, so the refusal is inherited and nothing + * here re-implements it. What this pins is that the inheritance actually + * DELIVERS it end to end: this driver swaps knex's transport for a custom sql.js + * dialect, and the 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), + * so the fourth backend is verified rather than assumed — which is also what the + * ruling on #5240 requires: the SAME error code from all four. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { SqliteWasmDriver } from './index.js'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +describe('[#5240] driver-sqlite-wasm inherits the zero-operator field-constraint refusal', () => { + let driver: SqliteWasmDriver; + + beforeAll(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.initObjects([ + { name: 'deal', fields: { stage: { type: 'string' }, owner: { type: 'string' } } }, + ]); + await driver.create('deal', { id: '1', stage: 'won', owner: 'u1' }, { bypassTenantAudit: true } as any); + await driver.create('deal', { id: '2', stage: 'lost', owner: 'u2' }, { 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'); + }; + + const positions: Array<[string, unknown, string]> = [ + ['top level', { stage: {} }, 'filter.stage'], + ['inside $or', { $or: [{ stage: {} }, { owner: 'u2' }] }, 'filter.$or[0].stage'], + ['inside $and', { $and: [{ stage: 'won' }, { owner: {} }] }, 'filter.$and[1].owner'], + ['inside $not', { $not: { stage: {} } }, 'filter.$not.stage'], + ]; + + for (const [name, where, position] of positions) { + it(`${name} → 400 INVALID_FILTER naming ${position}`, async () => { + const err = await refusalOf(where); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain(position); + expect(err.message).toContain('zero operators'); + }); + } + + it('ordinary filters still run through the wasm dialect unchanged', async () => { + expect(await ids({ stage: 'won' })).toEqual(['1']); + expect(await ids({ $or: [{ stage: 'won' }, { owner: 'u2' }] })).toEqual(['1', '2']); + expect(await ids({})).toEqual(['1', '2']); + }); +});