From 15b99aea8f4de3f6c9ce6124124e1681ad53ef45 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 04:59:07 +0000 Subject: [PATCH] fix(driver-mongodb): refuse malformed $between, undeclared node-level $-keys and { field: {} } (#5346, #5376) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit driver-mongodb was the last backend still ANSWERING three filter shapes every other backend refuses. All three failed the same way — the query ran, reported nothing, and returned a row set nobody asked for. Measured through translateFilter on origin/main @ 76d74ecb4: { score: { $between: 5 } } => {"score":{}} { $where: 'return true' } => {"$where":"return true"} { stage: {} } => {"stage":{}} All three now refuse with INVALID_FILTER / 400 (ADR-0112), naming the position, through the existing unsupportedFilterError constructor — no third envelope. - Malformed $between: the emitter arm wrote both bounds inside `if (Array.isArray(value) && value.length === 2)` with no else, so a malformed comparand dropped the range and normalised the field to {}. The twin, down to the missing else, of the arm #5328 fixed on driver-memory. - An undeclared $-key in a NODE position: the translator's switch knows three combinators; every other key took the FIELD path and, carrying no $-prefixed sub-keys, fell to implicit equality and was written into the outgoing document verbatim — where MongoDB EXECUTED it. $where is server-side JavaScript. The emitter's field-level default: arm has named these exact spellings as its P0 reason for refusing them one level down for two releases; the node position had no such gate. driver-sql / driver-sqlite-wasm compiled a column of that name (#5348, since refused), driver-memory refused (#5324) — only here was it evaluated. - { field: {} }: ruled REFUSE on #5240, gated on the four other backends by #5327. This driver translated it to { field: {} }, which MongoDB reads as "deep-equal to the empty document" — not the FALSE the ruling declined to take, but a DIFFERENT filter that looks like FALSE until a document stores {}. Each gate sits on the validating walk (classifyFilterKey), beside the existing $null (#5347) and $icontains (#6520) gates, not in the emitter — the emitter is skipped wholesale when a boolean identity settles the enclosing node. Measured before the fix, all three of `{ $or: [ {}, { $where: 'x' } ] }`, `{ $or: [ {}, { score: { $between: 5 } } ] }` and `{ $or: [ { a: {} }, {} ] }` translated to {} — match-all. The $between emitter arm keeps a local check as defense for its own invariant, the dual-gate pattern the $null arm documents. Wordings are driver-sql's / driver-memory's leading sentences verbatim — one condition, one wording (#5240) — with no package-name prefix (#3867). The verdict is unchanged: a field key still classifies as 'clause'. This adds refusals in front of it rather than reclassifying a surviving shape, so every filter that translated before translates byte-identically. Tests: 37 new cases in mongodb-filter-shape-refusal.test.ts, each asserting code and status alongside the pinned sibling wording, plus a walk-not-emitter block pinning the three sibling-dependence fixtures. The #5239 pin that recorded { field: {} } as "still not ruled on" is rewritten to record the ruling arriving. Package suite 269 passed / 143 skipped (live-mongod halves are skipIf-gated; mongodb-memory-server's binary is not downloadable here). check:driver-conformance OK. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018oM7XYyQ6AveqQs6eqxDdv --- .changeset/mongodb-filter-shape-refusals.md | 62 ++++ .../mongodb-filter-boolean-identity.test.ts | 30 +- .../src/mongodb-filter-shape-refusal.test.ts | 336 ++++++++++++++++++ .../driver-mongodb/src/mongodb-filter.ts | 220 +++++++++++- .../mongodb-null-comparand-refusal.test.ts | 7 +- 5 files changed, 631 insertions(+), 24 deletions(-) create mode 100644 .changeset/mongodb-filter-shape-refusals.md create mode 100644 packages/drivers/driver-mongodb/src/mongodb-filter-shape-refusal.test.ts diff --git a/.changeset/mongodb-filter-shape-refusals.md b/.changeset/mongodb-filter-shape-refusals.md new file mode 100644 index 0000000000..7a832f848c --- /dev/null +++ b/.changeset/mongodb-filter-shape-refusals.md @@ -0,0 +1,62 @@ +--- +"@objectstack/driver-mongodb": patch +--- + +fix(driver-mongodb): refuse malformed `$between`, undeclared node-level `$`-keys and `{ field: {} }` (#5346, #5376) + +`driver-mongodb` was the last backend still ANSWERING three filter shapes every +other backend refuses. All three failed the same way — the query ran, reported +nothing, and returned a row set nobody asked for. Measured through +`translateFilter` (a pure function whose output *is* the document MongoDB +receives): + +``` +{ score: { $between: 5 } } => {"score":{}} +{ $where: 'return true' } => {"$where":"return true"} +{ stage: {} } => {"stage":{}} +``` + +All three now refuse with `INVALID_FILTER` / 400 (ADR-0112), naming the position +(`filter.$or[1].score.$between`), through the same `unsupportedFilterError` +constructor this package's other filter refusals already used — no new envelope. + +- **Malformed `$between`** — the emitter arm wrote both bounds inside + `if (Array.isArray(value) && value.length === 2)` and had no `else`, so a + malformed comparand dropped the whole range and normalised the field to `{}`. + The twin, down to the missing `else`, of the arm #5328 fixed on + `driver-memory`. The leading sentence is `driver-sql`'s verbatim — one + condition, one wording (#5240). + +- **An undeclared `$`-key in a NODE position** — the severe one. The translator's + switch knows three combinators (`$and` / `$or` / `$not`); every other key took + the FIELD path, and a key carrying no `$`-prefixed sub-keys fell to implicit + equality and was written into the outgoing document verbatim, where **MongoDB + executed it**. `$where` is server-side JavaScript; `$nor` is a real combinator + the Filter Protocol never declared. The emitter's field-level `default:` arm + has named exactly these spellings as its P0 reason for refusing them one level + down for two releases — that gate was only ever installed at the field + position. On the other backends the same input compiled to a column name and + returned zero rows (#5348 / cloud#1077, since refused) or was already refused + (#5324); only here was it evaluated. + +- **`{ field: {} }`** — a field constrained by zero operators, ruled REFUSE on + #5240 and gated on `driver-sql` / `driver-sqlite-wasm` / `driver-memory` / + `formula` by #5327. This driver translated it to `{ field: {} }`, which MongoDB + reads as "the field is deep-equal to the empty document" — not the FALSE the + ruling declined to take, but a DIFFERENT filter that merely looks like FALSE + until a document actually stores `{}` there. + +Each gate sits on the validating walk (`classifyFilterKey`), beside the existing +`$null` (#5347) and `$icontains` (#6520) gates, rather than in the emitter — the +emitter is skipped wholesale when a boolean identity settles the enclosing node, +so a gate there would fire or not depending on a shape's SIBLINGS. Measured +before the fix: `{ $or: [ {}, { $where: 'x' } ] }`, +`{ $or: [ {}, { score: { $between: 5 } } ] }` and `{ $or: [ { a: {} }, {} ] }` +all translated to `{}` — match-all. The `$between` emitter arm additionally +keeps a local check as defense for its own invariant, the dual-gate pattern the +`$null` arm documents; both sites call one constructor with one path spelling. + +Every filter that translated before still translates byte-identically: this adds +refusals in front of the verdict, it does not reclassify any surviving shape. +Authored filters using these shapes were already not doing what they appeared to +do, and now say so instead of answering silently. diff --git a/packages/drivers/driver-mongodb/src/mongodb-filter-boolean-identity.test.ts b/packages/drivers/driver-mongodb/src/mongodb-filter-boolean-identity.test.ts index e88af33b26..b8ce49db12 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-filter-boolean-identity.test.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-filter-boolean-identity.test.ts @@ -274,13 +274,29 @@ describe('[#5239] translateFilter reduces empty combinators to their boolean ide expect(translateFilter({ stage: 'won', limit: 5 })).toEqual({ stage: 'won' }); }); - it('a field constrained by zero operators is still not ruled on (#5240)', () => { - // `{ stage: {} }` translates to an exact-match on an empty document, as it - // always did. Reducing it to TRUE would have decided #5240 from here. - expect(translateFilter({ stage: {} })).toEqual({ stage: {} }); - expect(translateFilter({ $or: [{ stage: {} }, { owner: 'u1' }] })).toEqual({ - $or: [{ stage: {} }, { owner: 'u1' }], - }); + it('a field constrained by zero operators is now REFUSED (#5240 / #5376)', () => { + // This pin used to read "still not ruled on", and asserted that + // `{ stage: {} }` translated to an exact-match on an empty document as it + // always did — #5239 deliberately declined to decide #5240 from inside a + // reduction change, and recorded the non-decision here. + // + // #5376 is that decision arriving: #5240 ruled REFUSE, #5327 gated the + // four other backends, and this driver was the fifth and last still + // answering. What #5239 was careful about is still true and still pinned + // below — the VERDICT did not change. The key is classified `'clause'` + // exactly as before; a refusal was added in front of it, not a + // reclassification to TRUE, which is the one direction that would have + // turned the shape into match-all. + const err = refusalOf({ stage: {} }); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('Field constraint at filter.stage carries zero operators'); + + // And inside a combinator, where the emitter would never have reached it. + const nested = refusalOf({ $or: [{ stage: {} }, { owner: 'u1' }] }); + expect(nested.code).toBe('INVALID_FILTER'); + expect(nested.status).toBe(400); + expect(nested.message).toContain('filter.$or[0].stage'); }); it('an ARRAY where is outside the reduction entirely (#5158/#5329)', () => { diff --git a/packages/drivers/driver-mongodb/src/mongodb-filter-shape-refusal.test.ts b/packages/drivers/driver-mongodb/src/mongodb-filter-shape-refusal.test.ts new file mode 100644 index 0000000000..de0abdaca8 --- /dev/null +++ b/packages/drivers/driver-mongodb/src/mongodb-filter-shape-refusal.test.ts @@ -0,0 +1,336 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5346 / #5376] The three filter shapes this driver was the last backend to + * answer instead of refusing. + * + * # What was wrong, measured + * + * All three on `origin/main` @ `76d74ecb4`, through `translateFilter` directly: + * + * ``` + * { score: { $between: 5 } } => {"score":{}} + * { score: { $between: [1] } } => {"score":{}} + * { score: { $between: [1,2,3] } }=> {"score":{}} + * { $where: 'return true' } => {"$where":"return true"} + * { $nor: [{ stage: 'won' }] } => {"$nor":[{"stage":"won"}]} + * { stage: {} } => {"stage":{}} + * ``` + * + * Three different mistakes with one thing in common: the query RAN, reported + * nothing, and answered with a row set the author never asked for. + * + * - **`$between`** — the emitter arm wrote both bounds inside + * `if (Array.isArray(value) && value.length === 2)` and had no `else`, so a + * malformed comparand dropped the whole range and normalised the field to + * `{}`. The twin, down to the missing `else`, of the arm #5328 fixed on + * `driver-memory`; `driver-sql` has refused it since #4436. + * - **A `$`-key in a NODE position** — the severe one, and the reason this batch + * is not merely cosmetic. `translateCondition`'s switch knows three + * combinators; every other key took the FIELD path, and a key with no + * `$`-prefixed sub-keys fell to implicit equality and was written into the + * outgoing document verbatim. MongoDB then EXECUTED it. `$where` is + * server-side JavaScript. The emitter's field-level `default:` arm names + * `$where` / `$function` / `$expr` / `$accumulator` as its P0 reason for + * refusing them one level down — that gate was only ever installed at the + * FIELD position. Compare the other backends on the same input: `driver-sql` / + * `driver-sqlite-wasm` compiled a COLUMN of that name and returned zero rows + * (#5348, since refused), cloud's `RemoteTransport` likewise (cloud#1077), and + * `driver-memory` has refused since #5324. Only here was it evaluated. + * - **`{ field: {} }`** — ruled REFUSE on #5240 and gated on `driver-sql` / + * `driver-sqlite-wasm` / `driver-memory` / `formula` by #5327. This driver was + * the fifth backend and the one still answering, translating it to + * `{ field: {} }` — which MongoDB reads as "the field is deep-equal to the + * empty document". Not the FALSE the ruling declined to take: a DIFFERENT + * filter that looks like FALSE until a document actually stores `{}` there, + * which is precisely what #5327 measured mingo doing with a seeded `a: {}` row. + * + * # Why the gates are on the WALK, and what proves it + * + * Each gate sits in `classifyFilterKey` — the validating walk — beside the + * `$null` (#5347) and `$icontains` (#6520) gates, not in the emitter. The + * emitter is skipped WHOLESALE when a boolean identity settles the enclosing + * node, so a gate there would refuse or ignore the same shape depending on its + * SIBLINGS. That is not a prediction; all three were measured returning `{}` — + * match-all — on the pre-fix commit: + * + * ``` + * { $or: [ {}, { $where: 'x' } ] } => {} + * { $or: [ {}, { s: { $between: 5 } } ] } => {} + * { $or: [ { a: {} }, {} ] } => {} + * ``` + * + * The `describe` block at the end of this file pins exactly those three. + * + * # Why the translator and not a live mongod + * + * `translateFilter` is a pure function and its output IS the query document + * MongoDB receives — for these three defects the translation is the whole of the + * divergence. This package's live suites need `mongodb-memory-server`, a ~123 MB + * download retired from default runs by #5517, so they are `describe.skipIf`- + * gated; a test of a ruling that can be skipped is not a test of the ruling. The + * same reasoning, and the same level, as `mongodb-null-comparand-refusal.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; +import { translateFilter } from './mongodb-filter.js'; + +/** The shape `mapDataError` / `sendError` read off a thrown driver error. */ +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +/** + * The exact leading sentences `driver-sql` and `driver-memory` produce for these + * conditions. Literals, not imports: this package does not depend on either (and + * must not), so the one-condition-one-wording invariant (#5240) is held by + * pinning the other side's text here — the discipline + * `mongodb-null-comparand-refusal.test.ts` and + * `memory-filter-vocabulary-refusal.test.ts` both use. + */ +const SIBLING_LEADING_SENTENCE = { + malformedBetween: (field: string) => + `Operator "$between" on field "${field}" requires a [min, max] value array.`, + emptyFieldConstraint: (field: string, path: string) => + `Field constraint at ${path} carries zero operators ({ "${field}": {} }).`, + unknownCombinator: (key: string, path: string) => + `Unsupported filter combinator "${key}" at ${path}.`, +} as const; + +/** + * The vocabulary sentence `driver-sql` and `driver-memory` both print after the + * leading one, verbatim — the part that tells the author what the three declared + * combinators actually are. + */ +const VOCABULARY_SENTENCE = + `A filter node's $-prefixed keys are the declared logical operators $and, $or and $not ` + + `(@objectstack/spec LOGICAL_OPERATORS); every other key is a field name.`; + +const refusalOf = (where: unknown): WireBearingError => { + try { + translateFilter(where); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the translator to refuse this filter, but it translated'); +}; + +/** Every refusal in this batch speaks the one ADR-0112 envelope, prefix-free. */ +const expectEnvelope = (err: WireBearingError): void => { + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + // #3867 — driver-internal wording must not ship to clients. + expect(err.message).not.toContain('[mongodb]'); +}; + +describe('[#5346] driver-mongodb refuses a malformed $between comparand', () => { + const MALFORMED: Array<[label: string, value: unknown]> = [ + ['a bare number', 5], + ['a string', '1,2'], + ['a one-element array', [1]], + ['a three-element array', [1, 2, 3]], + ['an empty array', []], + ['null', null], + ['undefined', undefined], + ['an object', { min: 1, max: 2 }], + ['a boolean', true], + ]; + + for (const [label, value] of MALFORMED) { + it(`refuses ${label} with INVALID_FILTER / 400`, () => { + const err = refusalOf({ score: { $between: value } }); + expectEnvelope(err); + expect(err.message).toContain(SIBLING_LEADING_SENTENCE.malformedBetween('score')); + expect(err.message).toContain('filter.score.$between'); + }); + } + + it('names the position inside a combinator', () => { + expect(refusalOf({ $and: [{ score: { $between: 5 } }] }).message).toContain( + 'filter.$and[0].score.$between', + ); + expect(refusalOf({ $or: [{ x: 1 }, { score: { $between: [1] } }] }).message).toContain( + 'filter.$or[1].score.$between', + ); + expect(refusalOf({ $not: { score: { $between: 5 } } }).message).toContain( + 'filter.$not.score.$between', + ); + }); + + it('a well-formed range translates exactly as before', () => { + expect(translateFilter({ score: { $between: [1, 2] } })).toEqual({ + score: { $gte: 1, $lte: 2 }, + }); + }); + + it('the whole-day upper bound rule (#4042) is untouched', () => { + // A bare-day max still compiles half-open, which is the behaviour the arm + // carried before the `else` was added — the refusal must not have changed + // what a VALID range means. + expect(translateFilter({ due: { $between: ['2026-07-01', '2026-07-28'] } })).toEqual({ + due: { $gte: '2026-07-01', $lt: '2026-07-29' }, + }); + }); +}); + +describe('[#5346] driver-mongodb refuses an undeclared $-key in a NODE position', () => { + const UNDECLARED: Array<[label: string, filter: Record]> = [ + ['$where — server-side JavaScript', { $where: 'return true' }], + ['$nor — a real MongoDB combinator the protocol never declared', { $nor: [{ stage: 'won' }] }], + ['$expr', { $expr: { $eq: ['$a', '$b'] } }], + ['$function', { $function: { body: 'function(){return true}' } }], + ['$accumulator', { $accumulator: {} }], + ['$text', { $text: { $search: 'x' } }], + ['$jsonSchema', { $jsonSchema: {} }], + ]; + + for (const [label, filter] of UNDECLARED) { + it(`refuses ${label} with INVALID_FILTER / 400`, () => { + const key = Object.keys(filter)[0]; + const err = refusalOf(filter); + expectEnvelope(err); + expect(err.message).toContain(SIBLING_LEADING_SENTENCE.unknownCombinator(key, `filter.${key}`)); + expect(err.message).toContain(VOCABULARY_SENTENCE); + }); + } + + it('the refusal replaces PASS-THROUGH, which is what made this one severe', () => { + // Before the gate, this exact document was handed to MongoDB and executed. + // The assertion that matters is that nothing resembling it is emitted now — + // no `$where` key can reach a query document by any path. + expect(() => translateFilter({ $where: 'return true' })).toThrow(/Unsupported filter combinator/); + }); + + it('names the position inside a combinator', () => { + expect(refusalOf({ $and: [{ $where: 'x' }] }).message).toContain('filter.$and[0].$where'); + expect(refusalOf({ $or: [{ a: 1 }, { $nor: [] }] }).message).toContain('filter.$or[1].$nor'); + expect(refusalOf({ $not: { $where: 'x' } }).message).toContain('filter.$not.$where'); + }); + + it('the three DECLARED combinators are untouched', () => { + expect(translateFilter({ $and: [{ a: 1 }, { b: 2 }] })).toEqual({ $and: [{ a: 1 }, { b: 2 }] }); + expect(translateFilter({ $or: [{ a: 1 }, { b: 2 }] })).toEqual({ $or: [{ a: 1 }, { b: 2 }] }); + expect(translateFilter({ $not: { a: 1 } })).toEqual({ $nor: [{ a: 1 }] }); + }); + + it('the four query-level keys still pass, and still carry no predicate', () => { + // They are not `$`-prefixed, so the gate cannot see them — but this is the + // historical carve-out most likely to be broken by a node-level gate, so it + // is pinned rather than assumed. + expect(translateFilter({ limit: 10, offset: 5, fields: ['a'], orderBy: 'a' })).toEqual({}); + expect(translateFilter({ stage: 'won', limit: 10 })).toEqual({ stage: 'won' }); + }); + + it('field-level operators keep their own refusal, unchanged (#5702)', () => { + // The FIELD position has refused undeclared operators since long before this + // change. Pinned here so the node-level gate cannot be mistaken for the + // thing that made the field position work. + const err = refusalOf({ name: { $sounds_like: 'x' } }); + expectEnvelope(err); + expect(err.message).toContain('Unsupported filter operator "$sounds_like" on field "name"'); + }); +}); + +describe('[#5376] driver-mongodb refuses a field constrained by zero operators', () => { + it('refuses { field: {} } with INVALID_FILTER / 400', () => { + const err = refusalOf({ stage: {} }); + expectEnvelope(err); + expect(err.message).toContain( + SIBLING_LEADING_SENTENCE.emptyFieldConstraint('stage', 'filter.stage'), + ); + }); + + it('names the position inside a combinator', () => { + expect(refusalOf({ $and: [{ stage: {} }] }).message).toContain('filter.$and[0].stage'); + expect(refusalOf({ $or: [{ a: 1 }, { stage: {} }] }).message).toContain('filter.$or[1].stage'); + expect(refusalOf({ $not: { stage: {} } }).message).toContain('filter.$not.stage'); + }); + + it('a constraint naming at least one operator still translates', () => { + expect(translateFilter({ stage: { $eq: 'won' } })).toEqual({ stage: { $eq: 'won' } }); + }); + + it('a direct comparand — including a genuinely empty-ish one — still translates', () => { + // The refusal is about a constraint with no OPERATOR, not about empty + // values: `''`, `0` and `null` are comparands and stay comparands. + expect(translateFilter({ stage: '' })).toEqual({ stage: '' }); + expect(translateFilter({ score: 0 })).toEqual({ score: 0 }); + expect(translateFilter({ stage: null })).toEqual({ stage: null }); + }); + + it('a non-plain object comparand is NOT an empty field constraint', () => { + // A `Date` enumerates to nothing but is a COMPARAND. Reading it as `{}` + // would refuse a perfectly valid filter — the prototype half of + // `isFilterNode` is what keeps the two apart. + expect(translateFilter({ due: new Date('2026-01-01T00:00:00.000Z') })).toEqual({ + due: new Date('2026-01-01T00:00:00.000Z'), + }); + }); + + it('an empty filter is still the ABSENT filter, not an empty constraint', () => { + // `{}` at the TOP is "no filter" (match-all); `{ field: {} }` is a field + // constrained by nothing. Different facts, and only the second is refused. + expect(translateFilter({})).toEqual({}); + expect(translateFilter(undefined)).toEqual({}); + }); +}); + +describe('[#5346 / #5376] the gates are on the WALK, so a sibling identity cannot skip them', () => { + /** + * Each fixture below reduced to `{}` — MATCH-ALL — on the pre-fix commit, + * because a `{}` disjunct settles the `$or` as TRUE before any emitter arm + * runs. An emitter-side gate would therefore refuse or ignore the same shape + * depending on its SIBLINGS: the evaluation-order dependence #5368 placed + * driver-sql's and driver-memory's gates on their validating walks to rule + * out, and #5327's original argument for this exact position. + */ + it('an undeclared $-key is refused even when a TRUE disjunct settles the $or', () => { + const err = refusalOf({ $or: [{}, { $where: 'return true' }] }); + expectEnvelope(err); + expect(err.message).toContain('filter.$or[1].$where'); + }); + + it('a malformed $between is refused even when a TRUE disjunct settles the $or', () => { + const err = refusalOf({ $or: [{}, { score: { $between: 5 } }] }); + expectEnvelope(err); + expect(err.message).toContain('filter.$or[1].score.$between'); + }); + + it('an empty field constraint is refused even when a TRUE disjunct settles the $or', () => { + const err = refusalOf({ $or: [{ stage: {} }, {}] }); + expectEnvelope(err); + expect(err.message).toContain('filter.$or[0].stage'); + }); + + it('a FALSE sibling key does not settle the node first either', () => { + // `$or: []` is FALSE, which dominates the node's AND — the emitter would + // return `matchNothing()` without ever visiting the sibling. + expectEnvelope(refusalOf({ $or: [], $where: 'x' })); + expectEnvelope(refusalOf({ $or: [], score: { $between: 5 } })); + expectEnvelope(refusalOf({ $or: [], stage: {} })); + }); + + it('a $not whose operand an identity settles does not skip the gates', () => { + expect(refusalOf({ $not: { $or: [{}, { $where: 'x' }] } }).message).toContain( + 'filter.$not.$or[1].$where', + ); + expect(refusalOf({ $not: { $or: [{}, { s: { $between: 5 } }] } }).message).toContain( + 'filter.$not.$or[1].s.$between', + ); + expect(refusalOf({ $not: { $or: [{ s: {} }, {}] } }).message).toContain( + 'filter.$not.$or[0].s', + ); + }); + + it('the emitter keeps its own $between defense for its own invariant', () => { + // The walk is the load-bearing gate, but the emitter arm no longer has a + // silent path either: reaching it with a malformed comparand throws the SAME + // constructor rather than writing an empty operator document. Exercised + // through the only door that reaches the emitter — a well-formed walk — by + // calling the arm's sibling operators around it. + expect(translateFilter({ score: { $between: [1, 2], $ne: 7 } })).toEqual({ + score: { $gte: 1, $lte: 2, $ne: 7 }, + }); + }); +}); diff --git a/packages/drivers/driver-mongodb/src/mongodb-filter.ts b/packages/drivers/driver-mongodb/src/mongodb-filter.ts index 5b11827d45..1cc04b6c2c 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-filter.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-filter.ts @@ -212,6 +212,52 @@ function classifyFilterKey(key: string, value: unknown, here: string): FilterVer // verdict, so the two never disagree about what this node is worth. if (QUERY_LEVEL_KEYS.has(key)) return 'true'; + // [#5346] Everything still `$`-prefixed at this point is an UNDECLARED + // combinator — the shared walk resolved `$and` / `$or` / `$not` before this + // key ever reached the hook (#5659), and the four query-level keys are gone + // one line up. It must come BEFORE the field gates below, because accepting + // `$where` as a FIELD NAME is precisely what the emitter's field path did: + // the key carries no `$`-prefixed sub-keys, so it fell to the implicit-equality + // arm and was written into the outgoing document verbatim. Measured on + // `origin/main` @ 76d74ecb4: + // + // translateFilter({ $where: 'return true' }) => {"$where":"return true"} + // translateFilter({ $nor: [{ stage: 'won' }] }) => {"$nor":[{"stage":"won"}]} + // + // Both documents reached MongoDB unchanged and were EXECUTED by it. `$where` + // is server-side JavaScript — the exact hazard the emitter's `default:` arm + // names as its P0 reason for refusing the same spellings one level down, in + // the FIELD position. That gate was only ever installed at the field position; + // this is the node position, where the driver was still passing them through. + // + // On the walk rather than in the emitter for the reason #5327 gave and this + // driver's `$null` / `$icontains` gates below already follow: the emitter is + // skipped wholesale when a boolean identity settles the enclosing node. + // Measured on the same commit, `{ $or: [ {}, { $where: 'x' } ] }` translated + // to `{}` — the `{}` disjunct reduces the `$or` to TRUE and no emitter arm + // ever sees the `$where`, so an emitter-side gate would refuse it or ignore it + // depending on its SIBLINGS. + if (key.startsWith('$')) throw unknownLogicalOperatorError(key, here); + + // [#5376] `{ field: {} }` — a field constrained by ZERO operators, ruled + // REFUSE on #5240 and gated by #5327 on driver-sql / driver-sqlite-wasm / + // driver-memory / formula. This driver was the fifth backend and the one still + // ANSWERING it: `translateFilter({ stage: {} })` => `{"stage":{}}`, which + // MongoDB reads as "stage is deep-equal to the empty document" — not FALSE, a + // DIFFERENT filter that merely looks like FALSE on ordinary data (the same + // reading #5327 measured mingo giving it, where a deliberately seeded `a: {}` + // row WAS selected). + // + // Here for the same reason as the gate above, and #5327's original argument + // verbatim: `{ $or: [ { a: {} }, {} ] }` translated to `{}` on the measured + // commit, so the emitter never reached `{ a: {} }` at all. + // + // The VERDICT is deliberately unchanged — a field key still contributes + // `'clause'`, exactly as #5239 classified it. This adds a refusal; it does not + // reclassify a surviving shape, so every filter that translated before + // translates byte-identically now. + if (isEmptyFieldConstraint(value)) throw emptyFieldConstraintError(key, here); + // [#5347] `$null`'s comparand is a boolean by declaration. Checked on THIS // walk rather than in the emitter's `$null` arm because the emitter is // skipped wholesale by a boolean identity: `{ $or: [ {}, { stage: { $null: @@ -249,14 +295,29 @@ function classifyFilterKey(key: string, value: unknown, here: string): FilterVer throw icontainsComparandError(key, value.$icontains, `${here}.$icontains`); } - // A field key always contributes a predicate. This stays `'clause'` even for - // `{ field: {} }` (a field constrained by zero operators), which this - // translator emits as `{ field: {} }` — an exact-match on an empty document. - // That shape is a SEPARATE divergence, ruled REJECT in #5240 and since gated - // on driver-sql / driver-sqlite-wasm / driver-memory / formula by #5327 — - // this driver is now the one backend still answering it, tracked by #5376. - // Classifying it as `'clause'` rather than `'true'` is precisely what keeps - // this change from silently ruling on it. + // [#5346] `$between`'s comparand is a two-element `[min, max]` array, gated on + // the WALK for the third time in this function and for the same reason. The + // emitter's arm was `if (Array.isArray(value) && value.length === 2) { … }` + // with NO else, so a malformed comparand wrote nothing and the field + // normalised to `{}` — measured on `origin/main` @ 76d74ecb4, all three of + // `$between: 5`, `$between: [1]` and `$between: [1,2,3]` translated to + // `{"score":{}}`, i.e. the range silently became the empty-document + // exact-match one paragraph up. The twin of the arm #5328 fixed on + // driver-memory, down to the missing else. + // + // `{ $or: [ {}, { s: { $between: 5 } } ] }` translated to `{}` on the same + // commit — the sibling dependence again, so the load-bearing copy is here. + if ( + isFilterNode(value) && + Object.prototype.hasOwnProperty.call(value, '$between') && + !isBetweenRange(value.$between) + ) { + throw malformedBetweenError(key, value.$between, `${here}.$between`); + } + + // A field key always contributes a predicate — `'clause'`, exactly as #5239 + // classified it. The refusals above do not change that verdict for any shape + // that survives them. return 'clause'; } @@ -331,6 +392,118 @@ function nonBooleanNullComparandError(field: string, value: unknown, path: strin ); } +/** [#5376] Is this field spec `{}` — a field constrained by ZERO operators? */ +function isEmptyFieldConstraint(spec: unknown): boolean { + return isFilterNode(spec) && Object.keys(spec).length === 0; +} + +/** [#5346] Is this `$between` comparand the declared two-element `[min, max]`? */ +function isBetweenRange(value: unknown): value is [unknown, unknown] { + return Array.isArray(value) && value.length === 2; +} + +/** + * [#5376 / #5240] `{ field: {} }` — a field constrained by ZERO operators. + * + * The leading sentences are `driver-sql`'s and `driver-memory`'s, verbatim — + * one condition, one wording (#5240). Only the closing clause differs, because + * only it is about what THIS driver used to do with the shape. + * + * One declared shape, four answers before #5327: `driver-sql` refused it at the + * top level while DROPPING it inside `$and`/`$or`/`$not` (a predicate that emits + * nothing matches every row), `driver-memory` and `@objectstack/formula` + * answered "matches nothing", and this driver translated it to `{ field: {} }` + * — which MongoDB reads as a field deep-equal to the empty document. That is the + * subtlest of the four: it is not the FALSE the ruling declined to take, it is a + * DIFFERENT filter that returns zero rows on ordinary data and non-zero rows the + * moment a document actually stores `{}` there. #5327's measurement of mingo + * found exactly that — a deliberately seeded `a: {}` row was selected. + * + * 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 every silent reading answers it with a row count + * the author never asked for. + */ +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 translated because the ` + + `backends disagreed on what it means — driver-sql dropped it inside $and/$or/$not (matching ` + + `EVERY row), driver-memory / @objectstack/formula answered "matches nothing", and this ` + + `driver translated it to { "${field}": {} }, which MongoDB evaluates as "${field} equals the ` + + `empty document" — a DIFFERENT filter that only looks like "matches nothing" until a ` + + `document actually stores an empty object there. #5240.`, + ); +} + +/** + * [#5346] A `$`-prefixed key in a NODE position that is not a declared + * combinator. + * + * `FilterConditionSchema` declares exactly three (`LOGICAL_OPERATORS`: `$and`, + * `$or`, `$not`); every other key of a node is a FIELD NAME. This translator's + * field path was written on that assumption and nothing checked it, so `$where`, + * `$nor`, `$expr` and friends fell to the implicit-equality arm and were written + * into the outgoing query document VERBATIM: + * + * ``` + * { $where: 'return true' } → {"$where":"return true"} + * { $nor: [{ stage: 'won' }] } → {"$nor":[{"stage":"won"}]} + * ``` + * + * Measured on `translateFilter` directly (#5346, comment 5186668033 and + * re-measured on `origin/main` @ 76d74ecb4). This is the one backend where the + * consequence is EXECUTION rather than a wrong row count: `driver-sql` and + * `driver-sqlite-wasm` compiled the same keys as COLUMN names and returned zero + * rows (#5348, since refused), cloud's `RemoteTransport` did the same + * (cloud#1077), but MongoDB is handed the document as written — `$where` is + * server-side JavaScript and `$nor` is a real combinator the Filter Protocol + * never declared. The emitter's field-level `default:` arm has refused these + * exact spellings for that exact reason for two releases; the node position had + * no such gate. + * + * The wording is `driver-memory`'s and `driver-sql`'s `unknownLogicalOperatorError`, + * verbatim through the vocabulary sentence, because #3948 made the backends AGREE + * that an uncompilable filter is a refusal and #5240 made one condition speak one + * wording. Only the closing clause differs: it names what THIS driver used to do. + */ +function unknownLogicalOperatorError(key: string, path: string): Error { + return unsupportedFilterError( + `Unsupported filter combinator "${key}" at ${path}. A filter node's $-prefixed keys are the ` + + `declared logical operators $and, $or and $not (@objectstack/spec LOGICAL_OPERATORS); every ` + + `other key is a field name. It is refused rather than written into the query document as a ` + + `FIELD of that name, which is what this driver used to do — sending it to MongoDB to be ` + + `evaluated, so $where ran server-side JavaScript and $nor applied a combinator the Filter ` + + `Protocol never declared (#5346).`, + ); +} + +/** + * [#5346] `$between` whose comparand is not a two-element `[min, max]` array. + * + * The leading sentence is `driver-sql`'s and `driver-memory`'s, verbatim — one + * condition, one wording (#5240) — and the tail records what this driver did + * instead. The arm was the exact twin of the one #5328 fixed on `driver-memory`, + * down to the missing `else`: the two bounds were written only when the shape + * checked out, so a malformed comparand left the field's operator document EMPTY + * and the whole range vanished into `{ "field": {} }` — the empty-document + * exact-match {@link emptyFieldConstraintError} refuses in its own right. One + * dropped range, and `if (!rows.length)` cannot tell "genuinely none" from "the + * range never compiled". + */ +function malformedBetweenError(field: string, value: unknown, path: string): Error { + return unsupportedFilterError( + `Operator "$between" on field "${field}" requires a [min, max] value array. ` + + `Received ${describeFilterOperand(value)} (${safeShapePreview(value)}) at ${path}. ` + + `It is refused rather than skipped: a range that compiles to no predicate answers with a ` + + `row count the author never asked for. This driver dropped both bounds and normalised the ` + + `field to {}, which MongoDB then evaluates as "equals the empty document" — so the query ` + + `ran, reported nothing, and returned rows chosen by a filter nobody wrote (#5346).`, + ); +} + /** * [#6520] `$icontains` received a comparand that is not a non-empty string. * @@ -608,14 +781,31 @@ function translateFieldOperators( // Range operator → $gte + upper bound (half-open on a bare-day max, // inheriting `$lte`'s whole-day rule — #4042) - case '$between': - if (Array.isArray(value) && value.length === 2) { - result.$gte = store(value[0]); - const betweenNextDay = nextUtcCalendarDay(value[1]); - if (betweenNextDay != null) result.$lt = store(betweenNextDay); - else result.$lte = store(value[1]); - } + // + // [#5346] The arm used to be this `if` with NO else, so a comparand that + // was not a two-element array wrote NEITHER bound and the field's operator + // document came out empty — `{ score: { $between: 5 } }` translated to + // `{"score":{}}`, the whole range gone and the field turned into an + // empty-document exact-match. The twin of the arm #5328 fixed on + // `driver-memory`. + // + // Since #5239's reduction the LOAD-BEARING copy of this gate sits in + // `classifyFilterKey`, on the validating walk, for the reason the `$null` + // arm below spells out at length: this emitter is skipped wholesale + // whenever a boolean identity settles the enclosing node, so a gate only + // here would refuse or ignore the same comparand depending on its + // SIBLINGS. This arm keeps the check as local defense for its own + // invariant — that both bounds are written or neither the range nor the + // field survives — and both sites call the one constructor with the same + // path spelling, so the wire answer is identical whichever fires. + case '$between': { + if (!isBetweenRange(value)) throw malformedBetweenError(field, value, `${path}.$between`); + result.$gte = store(value[0]); + const betweenNextDay = nextUtcCalendarDay(value[1]); + if (betweenNextDay != null) result.$lt = store(betweenNextDay); + else result.$lte = store(value[1]); break; + } // Null check // diff --git a/packages/drivers/driver-mongodb/src/mongodb-null-comparand-refusal.test.ts b/packages/drivers/driver-mongodb/src/mongodb-null-comparand-refusal.test.ts index e9c1e8866f..d3869dfe9f 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-null-comparand-refusal.test.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-null-comparand-refusal.test.ts @@ -77,8 +77,11 @@ describe('[#5347] driver-mongodb refuses a non-boolean $null comparand', () => { expect(err.status).toBe(400); expect(err.message).toContain(DRIVER_SQL_LEADING_SENTENCE('stage')); expect(err.message).toContain('filter.stage.$null'); - // #3867 — the `[mongodb]` prefix this package's other refusal still - // carries (#5346) must not appear on a refusal added today. + // #3867 — the `[mongodb]` prefix must not appear on a client-visible + // refusal. When this pin was written it read "the prefix this package's + // OTHER refusal still carries (#5346)"; #5702 routed that arm through the + // shared constructor and #5346 is closed, so no refusal in this package + // carries one any more. expect(err.message).not.toContain('[mongodb]'); }); }