From 85ef43d028cc11b67223f6c6f6c3756b2672869a Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Thu, 6 Aug 2026 11:45:53 +0000 Subject: [PATCH 1/2] fix(driver-sql,service-analytics)!: refuse the two comparand shapes that compiled to a silent nonsense predicate (#5234) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An `$in`/`$nin` list member that cannot be bound, and a LIKE-family comparand that `String()` cannot render, both compiled to valid SQL that answered with a predicate the caller never wrote: - `{status: {$in: ['a', {foo: 1}]}}` answered as if the second member had never been written; `{status: {$nin: [{foo: 1}]}}` excluded NOTHING, so the exclusion the caller wrote silently did not happen. - `{name: {$contains: {}}}` bound `LIKE '%[object Object]%'` and MATCHED a row whose text really was `[object Object]`; `$notContains` excluded it. #5041 (PR #5223) recorded both as "Deliberately NOT extended" on the grounds that they were fail-closed. Measured, neither premise held: the `$nin` / `$notContains` direction is wider, not narrower, and the answers were wrong rather than empty. The guard lands at each package's own chokepoint rather than at the three `String()` emitters, so one `$contains` still means one thing on every face: `assertCompilableComparand` in driver-sql, `fieldLeaves` for the analytics `where` door (the only leaf producer, so it covers all three consumers of the tree), and `compileOperator` for the read-scope lowering. The fence is an allow-list, copied from driver-turso's RemoteTransport, which has refused these same two shapes since cloud#1004 / #1058 — local and remote SQLite answered the same query differently until now. Primitives are deliberately untouched: `{$contains: 5}` and `{$contains: null}` agree across every backend today and #5526 pinned the latter. Arrays are refused because they already forked inside service-analytics (`%al,be%` at the read scope, `%al%` at the `where` door). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WyvqvKMG6asi9aXjKE6xtx --- .../silent-empty-predicate-comparands.md | 66 ++++ ...river-out-of-contract-filter-input.test.ts | 41 ++- .../sql-driver-silent-empty-predicate.test.ts | 287 ++++++++++++++++++ packages/drivers/driver-sql/src/sql-driver.ts | 180 ++++++++++- .../__tests__/comparand-shape-refusal.test.ts | 255 ++++++++++++++++ .../like-metacharacter-escape.test.ts | 93 ++++++ .../service-analytics/src/comparand-shape.ts | 155 ++++++++++ .../service-analytics/src/like-pattern.ts | 19 ++ .../service-analytics/src/read-scope-sql.ts | 51 +++- .../src/strategies/filter-normalizer.ts | 50 +++ .../src/strategies/objectql-strategy.ts | 8 + 11 files changed, 1183 insertions(+), 22 deletions(-) create mode 100644 .changeset/silent-empty-predicate-comparands.md create mode 100644 packages/drivers/driver-sql/src/sql-driver-silent-empty-predicate.test.ts create mode 100644 packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts create mode 100644 packages/services/service-analytics/src/comparand-shape.ts diff --git a/.changeset/silent-empty-predicate-comparands.md b/.changeset/silent-empty-predicate-comparands.md new file mode 100644 index 0000000000..599651e928 --- /dev/null +++ b/.changeset/silent-empty-predicate-comparands.md @@ -0,0 +1,66 @@ +--- +"@objectstack/driver-sql": minor +"@objectstack/service-analytics": minor +--- + +fix(driver-sql,service-analytics)!: 两类无意义比较对象不再编译成「静默空谓词」——`$in`/`$nin` 的对象成员与 LIKE 族的对象比较值一律拒收 (#5234) + +两个形状此前都**编译通过、执行、并给出一个作者没写过的答案**,而且没有任何东西记录这件事: + +| filter | 改前 | 改后 | +|---|---|---| +| `{status: {$in: ['a', {foo: 1}]}}` | 该成员绑不上任何行,查询答得**就像第二个成员从没被写过** | `INVALID_FILTER` / 400,点名 `index 1` | +| `{status: {$nin: [{foo: 1}]}}` | `NOT IN ('[object Object]')` —— **一行都没排除**,作者写下的排除悄悄没发生 | 同上 | +| `{name: {$contains: {}}}` | `LIKE '%[object Object]%'` —— 对一行文本恰好是 `[object Object]` 的记录,**真的命中了** | `INVALID_FILTER` / 400,点名 `StringOperatorSchema` | +| `{name: {$notContains: {}}}` | 反过来:为一个没人记录的理由**排除了一条真实记录** | 同上 | + +#5041(PR #5223)在 `assertCompilableComparand` 的头注释里把这两个形状写为 "Deliberately NOT +extended",理由是它们 fail-closed(只收窄结果集)、比 #5041 实测的裸 `TypeError` 低一级。**实测下来这 +两条理由都不成立**:`$nin` / `$notContains` 方向是**放宽**(该排除的没排除,在 read-scope 下即 #5347 / +#5324 判过的 over-reach);而 `$contains: {}` 给的从来不是「零行」,是**错行**。 + +## 三份实现一起动,否则修完仍是方言 + +同一个 `String()` 宽容在本仓有多份;只收紧 `driver-sql` 会变成「哪个面接的就是哪个答案」—— +#5146 / #5332 / #5567 各花一轮消掉的那类分叉。守卫因此落在**每个包自己的收口点**,而不是三个发射器: + +- **`driver-sql`** —— `assertCompilableComparand`,#5041 已有的那一个门。 +- **`service-analytics` 的 `where` 门** —— `filter-normalizer.ts` 的 `fieldLeaves`。它是本包**唯一**的 + leaf 生产者,所以一处拒收同时覆盖三个消费方:`NativeSQLStrategy`(真正执行的语句)、 + `ObjectQLStrategy.generateSql`(`/analytics/sql` 回显)与 `ObjectQLStrategy.convertFilter`(引擎路径)。 + 这个顺序是关键而非顺手:`convertFilter` 是**生产者**,在那里 `String()` 会把对象洗成一个类型完全正确 + 的 `'[object Object]'` 字符串交给驱动,下游再严格的驱动也永远看不到它该严格的那个形状。 +- **`service-analytics` 的 read-scope 门** —— `read-scope-sql.ts` 的 `compileOperator`,它编译的 + `FilterCondition` 不经过上面那个门。 + +`like-pattern.ts` 与 `applyLike` 里的 `String(value)` **原样保留**:它们不再是缺陷所在,因为门前已经没有 +渲染不出来的值能到达。两包的谓词由 `like-metacharacter-escape.test.ts` 逐值互锁——正是该文件已经用来锁 +转义表达式的同一套办法。 + +## 围栏是 allow-list,而且每一条都是实测后决定的 + +抄 `driver-turso` `RemoteTransport` 的形状(cloud#1004 / #1058):deny-list 会把下一个被发明出来的值形状 +悄悄放进来,这正是那个 bug 熬过第一次修复的原因。顺带说明,**turso 自 #1058 起就已经拒收这两个形状**, +所以本地 SQLite 与远程 SQLite 此前对同一条查询给的是不同答案;本次改动把它们收敛到一起。 + +留在围栏内的(逐条实测,不是假设): + +- **数字 / 布尔 / `null`**:`{$contains: 5}` → `%5%`、`{$contains: null}` → `%null%` 在 `driver-sql`、 + `driver-memory` 与 analytics 两个面上**今天答案一致**,#5526 还专门把 `null` 这条钉住了。拒收它们是在 + **破坏**一致,不是建立一致——所以只拒**对象**。 +- **`Date`**:turso 的 allow-list 把它作为唯一的对象转换保留,拒收会重新叉开本地与远程。 +- **binary**:`$in` 成员照收(`isBindableComparand` 与写路径 `formatInput` 同一套分类),LIKE 拒收——它 + 绑得上但渲染不出作者想要的东西。这就是两个谓词而不是一个带 flag 的原因。 +- **`undefined`**:不可授权(JSON 没有 `undefined`),analytics 门按 #5526 / #5332 归一为 `null` 而非拒收; + 在 `driver-sql` 拒收它会**造出**一个分歧而不是消除一个,故照旧。 + +被拒的**数组**是本次唯一一个「拒收即消分叉」的形状:`{name: {$contains: ['al','be']}}` 在 `read-scope-sql` +(与 `driver-sql`)绑 `%al,be%`,在 analytics 的 `where` 门却绑 `%al%`(它读 `values[0]`,后面的成员被 +静默丢弃)。同一个包对同一条 filter 有两个答案,两个门现在都拒。 + +## 作者需要知道的迁移 + +这两个形状本来就没有能用的读法——`filter.zod.ts` 的 `StringOperatorSchema` 早就把 LIKE 族比较数声明为 +`z.string()`,本次只是让声明变成强制(Prime Directive #12,declared = enforced)。改后它们答 400 而不是 +一个错答案;把比较数换成字面值即可。`{$eq: {…}}` **不在本次范围**,仍按 `toSqlBindValue` 绑 JSON(#5526 +钉住的行为)。 diff --git a/packages/drivers/driver-sql/src/sql-driver-out-of-contract-filter-input.test.ts b/packages/drivers/driver-sql/src/sql-driver-out-of-contract-filter-input.test.ts index 01d4f4a812..42d21ece0d 100644 --- a/packages/drivers/driver-sql/src/sql-driver-out-of-contract-filter-input.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-out-of-contract-filter-input.test.ts @@ -248,13 +248,42 @@ describe('[#5347/#5348] SqlDriver refuses out-of-contract filter input', () => { expect(crossField.message).toContain('Cross-field comparison'); }); - it('the regex family keeps its non-string comparands (explicitly out of scope)', async () => { - // #5347 measured this family as AGREEING across backends and fail-closed, - // and #5041 left it out of the comparand guard on the same evidence. It - // is not tightened here, and this pins that it was not tightened by - // accident. + it('the LIKE family keeps its non-string PRIMITIVE comparands', async () => { + // This assertion used to read "the regex family keeps its non-string + // comparands (explicitly out of scope)", carrying: + // + // > #5347 measured this family as AGREEING across backends and + // > fail-closed, and #5041 left it out of the comparand guard on the + // > same evidence. It is not tightened here, and this pins that it was + // > not tightened by accident. + // + // **#5234 supersedes the OBJECT half of that** (Prime Directive #13 — the + // reversal is quoted rather than deleted so the next reader can find it + // from the sentence they remember). Both premises held only for + // primitives: + // + // - "agreeing across backends" — `{$startsWith: {}}` did agree, on the + // WRONG answer. `String({})` is `'[object Object]'`, and against a row + // storing that literal text the pattern MATCHED. This fixture has no + // such row, so the old `toEqual([])` passed because nothing was there + // to match, not because the compiled predicate was right. + // - "fail-closed" — `$notContains` and `$nin` invert it: the exclusion + // the caller wrote silently did not happen. + // + // The primitive half is UNCHANGED and still pinned here, deliberately: + // `{$contains: 1}` → `%1%` is what this driver, `driver-memory` and both + // `service-analytics` faces all give, and #5526 kept it on purpose. expect(await ids({ stage: { $contains: 1 } })).toEqual([]); - expect(await ids({ stage: { $startsWith: {} } })).toEqual([]); + expect(await ids({ stage: { $contains: null } })).toEqual([]); + expect(await ids({ stage: { $startsWith: true } })).toEqual([]); + + // The object half now refuses, in this driver's own envelope. Replaced + // rather than re-spelled: an assertion that keeps passing because nothing + // is produced pins nothing at all. + const err = await refusalOf({ stage: { $startsWith: {} } }); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('$startsWith'); }); }); }); diff --git a/packages/drivers/driver-sql/src/sql-driver-silent-empty-predicate.test.ts b/packages/drivers/driver-sql/src/sql-driver-silent-empty-predicate.test.ts new file mode 100644 index 0000000000..983845a1b0 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-silent-empty-predicate.test.ts @@ -0,0 +1,287 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5234] The two comparand shapes #5041 deliberately left open are refused, in + * the same ADR-0112 envelope every other filter refusal here speaks. + * + * `assertCompilableComparand` landed with #5041 (PR #5223) carrying an explicit + * "Deliberately NOT extended" note about: + * + * 1. a non-`$field` OBJECT member of an `$in` / `$nin` list, and + * 2. an object comparand on the `LIKE` family, which `String()` renders as the + * literal `'[object Object]'`. + * + * The reason recorded there was that both are fail-closed — they narrow the + * result set — and therefore a lesser class than the bare `TypeError` #5041 + * measured. **Both halves of that reading were measured wrong on `main` before + * this change, and the measurements are the fixtures below.** + * + * # What the pre-fix driver actually answered + * + * `ROWS` deliberately contains a row whose `name` is the literal text + * `[object Object]` — `r_literal`. It is not a curiosity: it is what turns + * "returns zero rows" into "returns the WRONG rows", and every LIKE case here is + * an exact-set assertion against it. + * + * | filter | pre-fix answer | why that is not "fail-closed" | + * |---|---|---| + * | `{name: {$contains: {}}}` | `['r_literal']` | MATCHED a row. A pattern nobody wrote selected a real record. | + * | `{name: {$notContains: {}}}` | everything EXCEPT `r_literal` | EXCLUDED a real record for a reason nothing records. | + * | `{status: {$nin: [{…}]}}` | every row | the exclusion the caller wrote silently did not happen — over-reach, not narrowing. | + * | `{status: {$in: ['a', {…}]}}` | `['r_a']` | answered exactly as if the second member had never been written. | + * | `{name: {$contains: ['al','be']}}` | `LIKE '%al,be%'` | while `service-analytics`'s `where` door binds `%al%` for the same filter — a live split, closed by refusing both. | + * + * # Reverse verification — direction predicted BEFORE running it + * + * Plain **before-green / after-red on the guard's removal**: every `refusalOf` + * assertion below fails if the two new arms of `assertCompilableComparand` are + * deleted, because the driver resolves instead of throwing. There is no + * inversion here (the guard adds arms rather than reordering a `??` chain) and + * no count-shaped gate downstream, so the two exotic directions #5046 / #5018 + * describe do not apply. + * + * Measured by disabling each arm in turn and re-running this file together with + * `sql-driver-out-of-contract-filter-input.test.ts`: + * + * - LIKE arm disabled → **8 failed / 32 passed**. The eighth is in the other + * file: the superseded `{ $startsWith: {} }` pin, which is the point of + * having replaced it rather than deleted it. + * - member arm disabled → **4 failed / 36 passed** — the `$in`, `$nin`, + * nested-array and `$between` cases. The `$field` member case is NOT among + * them, and that is the arm ordering being verified rather than a gap: + * #5041's cross-field refusal still answers first for that shape. + * + * The counts are here because a bare "reverting turns it red" claims nothing + * checkable — if a later change makes one of these arms unreachable, the count + * moves and the next reader can see that it did. + * + * The row-set assertions are the other half, and they are what makes the + * refusals meaningful rather than self-referential: `keeps` pins that every + * comparand shape which WORKS today still works, so the guard is proven narrow + * and not merely present. + * + * # Why primitives are NOT refused + * + * `filter.zod.ts` declares the LIKE comparand `z.string()`, so a strict reading + * would refuse `{$contains: 5}` too. Measured, that is the wrong move: `%5%` is + * the answer this driver, `driver-memory` and both `service-analytics` faces all + * give today, and #5526 pinned `{$contains: null}` → `%null%` on purpose. Only + * OBJECTS — the values for which `String()` has no faithful answer — are + * refused. `Date` stays accepted for the same reason (`driver-turso`'s + * allow-list keeps it as its one declared object conversion). + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { SqlDriver } from '../src/index.js'; +import type { FilterCondition } from '@objectstack/spec/data'; + +/** The shape `mapDataError` / `sendError` read off a thrown driver error. */ +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +async function refusalOf(run: () => Promise): Promise { + try { + await run(); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the driver to refuse this filter, but it resolved'); +} + +/** + * `r_literal` is the decoy that turns every "silently zero rows" claim into a + * checkable one: it is the row a `'[object Object]'` pattern selects. + * `r_five` does the same job for the number comparand that must KEEP working. + */ +const ROWS = [ + { id: 'r_a', name: 'alpha', status: 'a' }, + { id: 'r_b', name: 'beta', status: 'b' }, + { id: 'r_literal', name: '[object Object]', status: 'c' }, + { id: 'r_five', name: '5', status: 'd' }, +]; + +describe('[#5234] SqlDriver refuses the two comparand shapes that compiled to a silent nonsense predicate', () => { + let driver: SqlDriver; + + beforeEach(async () => { + driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + await driver.initObjects([ + { + name: 'probe', + fields: { + id: { type: 'text', name: 'id' }, + name: { type: 'text', name: 'name' }, + status: { type: 'text', name: 'status' }, + }, + } as any, + ]); + for (const row of ROWS) await driver.create('probe', row); + }); + + const find = (where: unknown) => + driver.find('probe', { object: 'probe', fields: ['id'], where: where as FilterCondition }); + + const ids = async (where: unknown): Promise => + ((await find(where)) as Array<{ id: string }>).map((r) => r.id).sort(); + + // ── Shape 1: a non-`$field` object member of an `$in` / `$nin` list ───────── + + describe('shape 1 — an `$in` / `$nin` list member that cannot be bound', () => { + it("the issue's repro — `{ status: { $in: ['a', { foo: 1 }] } }` — carries the full envelope", async () => { + const err = await refusalOf(() => find({ status: { $in: ['a', { foo: 1 }] } })); + + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + // The index is the only thing telling the bad entry from its legitimate + // neighbour, so it is part of the contract, not decoration. + expect(err.message).toContain('index 1'); + expect(err.message).toContain('$in'); + expect(err.message).toContain('status'); + expect(err.message).toContain('{"foo":1}'); + // Pre-fix this filter RESOLVED with ['r_a'] — the member vanished. + expect(err).not.toBeInstanceOf(TypeError); + }); + + it('`$nin` is refused too — its pre-fix direction was WIDER, not narrower', async () => { + // `NOT IN ('[object Object]')` excluded nothing: pre-fix this returned all + // four rows while claiming to exclude one. On a read-scope lowering that is + // over-reach (#5347 / #5324), which is why the issue's "fail-closed, lower + // risk" framing does not survive this case. + const err = await refusalOf(() => find({ status: { $nin: [{ foo: 1 }] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('index 0'); + expect(err.message).toContain('$nin loses the exclusion'); + }); + + it('a nested-ARRAY member is refused with the same message, not a bare bind error', async () => { + // Pre-fix this one DID throw — but as a raw knex/SQLite `TypeError` with + // no `code` and no `status`, i.e. exactly the #5041 defect at a different + // spelling. Same shape, same class, one envelope. + const err = await refusalOf(() => find({ status: { $in: ['a', [1, 2]] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('index 1'); + }); + + it('the `$field` member refusal from #5041 still answers first, unchanged', async () => { + // A `$field` member is also unbindable, so the two arms overlap. The + // cross-field message is the more actionable one and must keep winning. + const err = await refusalOf(() => find({ status: { $in: ['a', { $field: 'name' }] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.message).toContain('Cross-field comparison'); + expect(err.message).toContain('at index 1'); + }); + + it('a `$between` bound is a comparand in its own right and gets the same envelope', async () => { + // Already inside the member scan's radius since #5041 (it scans any array + // for `$field`); before this change a non-`$field` object there produced a + // bare bind error instead of a catalogued one. + const err = await refusalOf(() => find({ status: { $between: [{ a: 1 }, 'z'] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('index 0'); + }); + + it('a scalar operator handed an array keeps its OWN message', async () => { + // The member scan is scoped to $in/$nin/$between precisely so this case + // does not start reporting "index 0 of its list" for a list `$eq` never + // takes. This is the pre-#5234 message, unchanged. + const err = await refusalOf(() => find({ status: { $eq: ['a', { foo: 1 }] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.message).toContain('requires a single comparable value'); + expect(err.message).not.toContain('index'); + }); + }); + + // ── Shape 2: an object comparand on the LIKE family ──────────────────────── + + describe('shape 2 — a LIKE-family comparand with no faithful text rendering', () => { + it("the issue's repro — `{ name: { $contains: {} } }` — MATCHED a real row pre-fix", async () => { + // The pre-fix answer was `['r_literal']`, not `[]`: the fixture stores the + // exact text `String({})` produces. "Silently zero rows" understated it. + const err = await refusalOf(() => find({ name: { $contains: {} } })); + + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('$contains'); + expect(err.message).toContain('name'); + expect(err.message).toContain('[object Object]'); + // Name the declaration being enforced — this refusal is `z.string()` made + // real, not a new rule (Prime Directive #12). + expect(err.message).toContain('StringOperatorSchema'); + }); + + for (const op of ['$contains', '$notContains', '$startsWith', '$endsWith', '$regex'] as const) { + it(`\`${op}\` refuses an object comparand`, async () => { + const err = await refusalOf(() => find({ name: { [op]: { foo: 1 } } })); + expect(err.code, op).toBe('INVALID_FILTER'); + expect(err.status, op).toBe(400); + expect(err.message, op).toContain(op); + }); + } + + it('an ARRAY comparand is refused — it answered two ways inside `service-analytics`', async () => { + // This driver (and `read-scope-sql`) bind `%al,be%` via `String(array)`; + // the analytics `where` door reads `values[0]` and binds `%al%`. Refusing + // it closes a live split rather than opening one. + const err = await refusalOf(() => find({ name: { $contains: ['al', 'be'] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.message).toContain('an array'); + // NOT reported as a bad list member: `$contains` takes no list. + expect(err.message).not.toContain('index'); + }); + + it('the `$field` refusal from #5041 still answers first on this family too', async () => { + const err = await refusalOf(() => find({ name: { $contains: { $field: 'status' } } })); + expect(err.message).toContain('Cross-field comparison'); + }); + }); + + // ── The narrowness proof: everything that worked still works ─────────────── + + describe('the guard is narrow — every comparand shape that compiled before still does', () => { + it('`$in` keeps binding every legitimate member type', async () => { + expect(await ids({ status: { $in: ['a', 'b'] } })).toEqual(['r_a', 'r_b']); + // null in an IN list is standard SQL and matches nothing — not an error. + expect(await ids({ status: { $in: ['a', null] } })).toEqual(['r_a']); + expect(await ids({ status: { $in: ['a', 5, true] } })).toEqual(['r_a']); + expect(await ids({ status: { $in: ['a', new Date('2020-01-01')] } })).toEqual(['r_a']); + expect(await ids({ status: { $nin: ['a', 'b', 'c'] } })).toEqual(['r_five']); + }); + + it('a binary member binds — bindable is the test, not "primitive"', async () => { + // `isBindableComparand` admits `ArrayBuffer.isView`, matching what the + // write path (`formatInput`) accepts. Refusing it would have been a + // narrower fence than the driver's own classification. + expect(await ids({ status: { $in: ['a', new Uint8Array([1, 2])] } })).toEqual(['r_a']); + }); + + it('the LIKE family keeps every primitive comparand, including the two #5526 pinned', async () => { + expect(await ids({ name: { $contains: 'alp' } })).toEqual(['r_a']); + // `%5%` — agrees with driver-memory and both analytics faces. #5526 kept + // this deliberately; refusing it would have created a split. + expect(await ids({ name: { $contains: 5 } })).toEqual(['r_five']); + // `%null%`, not `%%`. #5526 converged analytics onto this driver's reading. + expect(await ids({ name: { $contains: null } })).toEqual([]); + expect(await ids({ name: { $contains: true } })).toEqual([]); + expect(await ids({ name: { $startsWith: 'be' } })).toEqual(['r_b']); + expect(await ids({ name: { $endsWith: 'ta' } })).toEqual(['r_b']); + // `[object Object]` carries no `a`, so `r_literal` is legitimately kept + // here — the decoy earns its place on both sides of the partition. + expect(await ids({ name: { $notContains: 'a' } })).toEqual(['r_five', 'r_literal']); + }); + + it('a `Date` comparand still renders — the one object form the allow-list keeps', async () => { + // `driver-turso`'s allow-list keeps `Date` as its single declared object + // conversion, so refusing it here would have forked local from remote. + await expect(find({ name: { $contains: new Date('2020-01-01') } })).resolves.toEqual([]); + }); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 0eae7377f1..d4810c79ff 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -575,9 +575,10 @@ function crossFieldComparisonError(field: string, op: string, ref: string, index * * The list-shaped operators (`$in` / `$nin` / `$between`) are deliberately * ABSENT: an array is their legitimate comparand, and they compile through - * their own `whereIn` / `whereBetween` arms. Only their MEMBERS are inspected - * (for `$field`), never their arity — the existing descriptive `$between` - * refusal stays the one that answers a malformed range. + * their own `whereIn` / `whereBetween` arms. Only their MEMBERS are inspected — + * for `$field` (#5041) and, since #5234, for bindability — never their arity: + * the existing descriptive `$between` refusal stays the one that answers a + * malformed range. See {@link LIST_COMPARAND_OPERATORS}. */ const SCALAR_COMPARAND_OPERATORS: ReadonlySet = new Set([ '$eq', '$ne', '$gt', '$gte', '$lt', '$lte', @@ -599,6 +600,54 @@ function isBindableComparand(value: unknown): boolean { return value instanceof Date || ArrayBuffer.isView(value); } +/** + * [#5234] Operators whose comparand becomes the TEXT of a `LIKE` pattern, i.e. + * the ones {@link SqlDriver.applyLike} serves. Kept separate from + * {@link SCALAR_COMPARAND_OPERATORS} because the two ask different questions of + * the same value: a scalar operator needs a value a driver can BIND, a pattern + * operator needs one that has a faithful TEXT rendering. Every value in the + * first set except a binary buffer is also in the second, but the reason is not + * the same reason, and the messages a caller needs differ. + * + * `like` / `ilike` are absent on purpose: they arrive already carrying a + * pattern and compile through the scalar bind arm, which already refuses an + * object. + */ +const TEXT_PATTERN_OPERATORS: ReadonlySet = new Set([ + '$contains', '$notContains', '$startsWith', '$endsWith', '$regex', +]); + +/** + * [#5234] Operators for which an ARRAY is the legitimate comparand, so it is + * each MEMBER that must be individually compilable. + * + * Scoping the member scan to these three is what keeps a scalar operator that + * received an array answering with its own message ("requires a single + * comparable value") instead of reporting the first bad member of a list it + * should never have been given. + */ +const LIST_COMPARAND_OPERATORS: ReadonlySet = new Set(['$in', '$nin', '$between']); + +/** + * [#5234] Does this value have a faithful rendering as the text of a LIKE + * pattern? + * + * An ALLOW-list, deliberately, and the same one `driver-turso`'s + * `RemoteTransport.serializeComparand` settled on for the identical question in + * remote mode (cloud#1004 / #1058): a deny-list silently re-admits whatever + * value form is invented next, which is exactly how that bug survived its first + * fix. `undefined` is inside the fence only because it is not authorable (JSON + * has no `undefined`) and the analytics door normalises it to `null` rather than + * refusing it (#5526) — refusing it HERE would invent a disagreement rather than + * close one. See {@link unrenderableTextComparandError} for the rest. + */ +function isRenderableTextComparand(value: unknown): boolean { + if (value === null || value === undefined) return true; + const kind = typeof value; + if (kind === 'string' || kind === 'number' || kind === 'bigint' || kind === 'boolean') return true; + return value instanceof Date; +} + /** * [#5041] The one gate every comparison comparand passes before it becomes a * bind parameter, covering both halves of the gap the issue measured: @@ -617,24 +666,67 @@ function isBindableComparand(value: unknown): boolean { * {@link SCALAR_COMPARAND_OPERATORS} so the legitimate array binds keep * working untouched. * - * Deliberately NOT extended to two neighbouring shapes, both of which return - * zero rows today rather than failing to bind: a non-`$field` object MEMBER of - * an `$in`/`$nin` list, and the `LIKE` family (`$contains`/`$startsWith`/…), - * which stringifies its comparand to `[object Object]`. Those are a different - * defect class — a filter applied nonsensically, not one that cannot be applied - * — and their direction is fail-closed (they narrow the result set, so they are - * not a filter bypass). Widening this guard to cover them would change the - * behaviour of paths that do not throw today, beyond what #5041 measured; see - * the #5041 PR discussion for the measurement. + * 3. **[#5234] The two shapes #5041 deliberately left out** — a non-`$field` + * OBJECT member of an `$in`/`$nin` list, and an object comparand on the + * `LIKE` family. #5041 read them as a lesser class ("a filter applied + * nonsensically, not one that cannot be applied") whose direction was + * fail-closed, and stopped. Both halves of that reading were measured wrong + * on `main` before this change: + * + * - The direction is not uniformly fail-closed. `{status: {$nin: [{…}]}}` + * compiles to `NOT IN ('[object Object]')`, which excludes NOTHING — an + * exclusion the caller wrote that silently does not happen. On a + * read-scope lowering that is over-reach, the same direction #5347 / #5324 + * ruled on. `$notContains` does it too: it EXCLUDED the one fixture row + * whose text happened to be `[object Object]`. + * - The answers were never merely "zero rows". Against a row literally + * named `[object Object]`, `{name: {$contains: {}}}` MATCHED it. That is + * not a narrowed result set, it is a wrong one. + * + * Two more measurements decided the exact fence: + * + * - An ARRAY comparand on the LIKE family already forked INSIDE + * `service-analytics`: `{name: {$contains: ['al','be']}}` binds `%al,be%` + * through `read-scope-sql` (and here, via `String(array)`) but `%al%` + * through the analytics `where` door, which reads `values[0]`. Refusing + * the array closes a live split rather than opening one. + * - `driver-turso`'s `RemoteTransport` has refused BOTH of these shapes + * since cloud#1004 / #1058, with the reasoning written out: refusing an + * uncompilable comparand "in one family while tolerating it in the other + * would leave the failure mode alive at a different spelling." So local + * SQLite and remote SQLite answered the same query differently. This + * change is what converges them, and it copies that fix's ALLOW-list + * shape (see {@link isRenderableTextComparand}) rather than inventing a + * second policy. + * + * What stays accepted is measured, not assumed: `{$contains: 5}` → `%5%` + * and `{$contains: null}` → `%null%` agree across this driver, + * `driver-memory` and both analytics faces today, and #5526 pinned the + * `null` reading deliberately. Primitives are therefore untouched; only + * objects — for which `String()` has no faithful answer — are refused. */ function assertCompilableComparand(field: string, op: string, value: unknown): void { const ref = fieldReferenceOf(value); if (ref !== null) throw crossFieldComparisonError(field, op, ref); + // [#5234] The pattern family answers first: an array IS an object here, so + // the member scan below would otherwise report `{$contains: ['a', {}]}` as a + // bad LIST member — a message about a list the operator never takes. + if (TEXT_PATTERN_OPERATORS.has(op) && !isRenderableTextComparand(value)) { + throw unrenderableTextComparandError(field, op, value); + } + if (Array.isArray(value)) { for (const [index, member] of value.entries()) { const memberRef = fieldReferenceOf(member); if (memberRef !== null) throw crossFieldComparisonError(field, op, memberRef, index); + // [#5234] Every member of a list operator's array is a comparand in its + // own right and gets the same bind test the whole comparand gets. Scoped + // to the operators for which an array is legitimate, so a scalar operator + // handed an array keeps answering with its own message below. + if (LIST_COMPARAND_OPERATORS.has(op) && !isBindableComparand(member)) { + throw unbindableListMemberError(field, op, member, index); + } } // An array IS the comparand for the list operators; only a scalar operator // is wrong to receive one, and that falls through to the check below. @@ -650,6 +742,60 @@ function assertCompilableComparand(field: string, op: string, value: unknown): v ); } +/** + * [#5234] A member of an `$in` / `$nin` / `$between` list that cannot become a + * bind parameter. + * + * The list case is the one that hides. A scalar operator handed an object fails + * to bind and at least says so; a LIST simply loses the member — Knex binds it, + * the statement is valid, and the offending entry can never equal any stored + * value. So `{status: {$in: ['a', {…}]}}` answers exactly as if the author had + * written `{$in: ['a']}`, and `{status: {$nin: [{…}]}}` excludes nothing at all + * while claiming to exclude something. Neither is reported anywhere. + * + * The message names the INDEX because that is the only thing distinguishing the + * bad entry from its legitimate neighbours — the same reason + * {@link crossFieldComparisonError} takes one. + */ +function unbindableListMemberError(field: string, op: string, value: unknown, index: number): Error { + return unsupportedFilterError( + `Operator "${op}" on field "${field}" has a value at index ${index} of its list that cannot be ` + + `bound as a SQL parameter: ${safeShapePreview(value)}. Every member of an $in/$nin/$between ` + + `list is a comparand in its own right — use a string, number, boolean, null, Date or binary ` + + `value. Refusing rather than binding it: the member can equal no stored value, so the list ` + + `silently loses that entry (and a $nin loses the exclusion the caller wrote).`, + ); +} + +/** + * [#5234] An object where the `LIKE` family expects the text of a pattern. + * + * `applyLike` reaches the comparand through `String(value)`, and `String({})` is + * the literal `'[object Object]'`. The result is a syntactically perfect, + * parameterised `LIKE` against a string the author never wrote — and it is not + * merely always-false: a stored value that happens to READ `[object Object]` + * matches it, which is how this was measured. `$notContains` inverts that into + * excluding a real row for a reason nothing records. + * + * `filter.zod.ts`'s `StringOperatorSchema` declares every one of these + * comparands `z.string()`, so this refusal enforces a declaration that already + * exists rather than adding a rule (Prime Directive #12 — declared = enforced). + * It stops at OBJECTS on purpose: a number, boolean or `null` renders to text + * the same way on this driver, on `driver-memory` and on both `service-analytics` + * faces, and #5526 pinned `{$contains: null}` → `%null%` deliberately. Refusing + * those would break agreement instead of creating it. + */ +function unrenderableTextComparandError(field: string, op: string, value: unknown): Error { + return unsupportedFilterError( + `Operator "${op}" on field "${field}" matches against the TEXT of a pattern, but received ` + + `${Array.isArray(value) ? 'an array' : 'an object'} (${safeShapePreview(value)}). The spec ` + + `declares this comparand a string (filter.zod.ts StringOperatorSchema); a string, number, ` + + `boolean, null or Date is accepted. Refusing rather than stringifying it: String({}) is ` + + `"[object Object]", so the pattern that ran was one the caller never wrote — valid SQL, ` + + `and a row storing that literal text would have matched it.`, + ); +} + /** A short, non-throwing rendering of an offending comparand for the message. */ function safeShapePreview(value: unknown): string { try { @@ -6288,6 +6434,16 @@ export class SqlDriver implements IDataDriver { * for character, by `service-analytics`'s `like-metacharacter-escape.test.ts`. * A third hand-copy is the thing to refuse: import from one of the two, or add * a consumer to that test. + * + * **[#5234] `String(value)` is safe here because nothing unrenderable reaches + * it.** {@link assertCompilableComparand} refuses an object comparand on this + * family before any emitter runs, so the only values arriving are the ones + * {@link isRenderableTextComparand} admits — a string, number, bigint, + * boolean, `null`, `undefined` or `Date`, each of which `String()` renders + * faithfully. Do NOT add a second, tolerant reading of an object here: the + * `[object Object]` pattern this used to build was valid SQL matching a + * literal nobody wrote, and `service-analytics` refuses the same shape at its + * own two doors so one `$contains` still means one thing on every face. */ private applyLike( builder: any, diff --git a/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts b/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts new file mode 100644 index 0000000000..b7c48f73ce --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts @@ -0,0 +1,255 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5234] Both of this package's filter doors refuse a comparand shape they + * cannot express, instead of compiling it into a nonsense predicate. + * + * The defect this pins is `driver-sql`'s (issue #5234 is filed against it), but + * the same `String()` tolerance had THREE implementations in this repo, and + * tightening one of them alone would have produced "whichever face took the + * query is the answer you get" — the split #5146 / #5332 / #5567 each spent a + * round removing. So the guard lands at this package's two doors as well: + * + * - **`filter-normalizer.ts`'s `fieldLeaves`** — the analytics `where` door, + * and the ONLY producer of leaf nodes here, so one refusal covers all three + * consumers of the tree: `NativeSQLStrategy` (the statement that executes), + * `ObjectQLStrategy.generateSql` (the `/analytics/sql` echo) and + * `ObjectQLStrategy.convertFilter` (the engine path). + * - **`read-scope-sql.ts`'s `compileOperator`** — the ADR-0021 D-C read-scope + * (tenant + RLS) lowering, which compiles a `FilterCondition` that never + * passes through the normalizer. + * + * # The measured splits this closes + * + * Two, both real on `main` before this change: + * + * | filter | analytics `where` door | `read-scope-sql` | `driver-sql` | + * |---|---|---|---| + * | `{name: {$contains: ['al','be']}}` | `%al%` (reads `values[0]`) | `%al,be%` | `%al,be%` | + * | `{name: {$contains: {$field: 'x'}}}` | `%[object Object]%` | `%[object Object]%` | REFUSED (#5041) | + * + * The first row is one package answering its own question two ways — an array + * comparand loses every member after the first on one door and is joined with + * commas on the other. The second is this package binding a pattern for a shape + * `driver-sql` has refused since #5041. + * + * # Reverse verification — direction predicted BEFORE running it + * + * Plain before-green / after-red on the guard's removal: each `refusalOf` here + * resolves rather than throwing if `assertCompilableComparand` (normalizer) or + * `assertRenderableText` / `assertCompilableMembers` (read scope) are deleted. + * No inversion and no count-shaped gate is involved. Measured by disabling all + * three call sites and re-running this file: **17 failed / 8 passed**. The 8 are + * the `keeps` blocks and the predicate table at the bottom — they are the other + * half of the proof, pinning that every shape which compiled before still + * compiles, so the guard is shown to be narrow and not merely present. + * + * # Envelopes differ ON PURPOSE, diagnosis does not + * + * The analytics door answers `INVALID_FILTER` / 400 — a caller authored that + * filter. The read scope answers `READ_SCOPE_COMPILE_FAILED` / 500 fail-closed — + * a policy produced it, and #5367 ruled that route withholds its message. One + * sentence, two envelopes; `comparand-shape.ts` owns the sentence. + */ + +import { describe, it, expect } from 'vitest'; +import type { FilterCondition } from '@objectstack/spec/data'; + +import { normalizeAnalyticsFilterTree } from '../strategies/filter-normalizer.js'; +import { compileScopedFilterToSql } from '../read-scope-sql.js'; +import { + isBindableComparand, + isRenderableTextComparand, +} from '../comparand-shape.js'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +function refusalOf(run: () => unknown): WireBearingError { + try { + run(); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the compiler to refuse this filter, but it returned'); +} + +const tree = (where: unknown) => normalizeAnalyticsFilterTree({ where } as any); +const scope = (where: unknown) => compileScopedFilterToSql(where as FilterCondition, 'person'); + +// ── The analytics `where` door ─────────────────────────────────────────────── + +describe('[#5234] the analytics `where` door refuses an uncompilable comparand', () => { + describe('shape 1 — an `$in` / `$nin` member that cannot be bound', () => { + it("the issue's repro — `{ status: { $in: ['a', { foo: 1 }] } }`", () => { + const err = refusalOf(() => tree({ status: { $in: ['a', { foo: 1 }] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('index 1'); + expect(err.message).toContain('$in'); + expect(err.message).toContain('{"foo":1}'); + }); + + it('`$nin` too — the direction there is WIDER, not narrower', () => { + const err = refusalOf(() => tree({ status: { $nin: [{ foo: 1 }] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.message).toContain('index 0'); + expect(err.message).toContain('$nin loses the exclusion'); + }); + + it('a nested-array member is refused as well', () => { + const err = refusalOf(() => tree({ status: { $in: ['a', [1, 2]] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.message).toContain('index 1'); + }); + }); + + describe('shape 2 — a LIKE-family comparand with no faithful text rendering', () => { + for (const op of ['$contains', '$notContains', '$startsWith', '$endsWith'] as const) { + it(`\`${op}\` refuses an object comparand`, () => { + const err = refusalOf(() => tree({ name: { [op]: { foo: 1 } } })); + expect(err.code, op).toBe('INVALID_FILTER'); + expect(err.status, op).toBe(400); + expect(err.message, op).toContain(op); + expect(err.message, op).toContain('StringOperatorSchema'); + }); + } + + it('an ARRAY comparand is refused — this door used to silently drop every member but the first', () => { + const err = refusalOf(() => tree({ name: { $contains: ['al', 'be'] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.message).toContain('an array'); + }); + + it('`{$field: …}` is refused here, converging with `driver-sql`', () => { + // Not a special case: a field reference is an object, and this door had no + // opinion about objects at all. `driver-sql` has refused it since #5041. + const err = refusalOf(() => tree({ name: { $contains: { $field: 'status' } } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.message).toContain('$field'); + }); + }); + + describe('the guard is narrow — every shape that normalised before still does', () => { + it('keeps the legitimate `$in` member types', () => { + expect(tree({ status: { $in: ['a', 'b'] } })).toEqual({ + kind: 'leaf', member: 'status', operator: 'in', values: ['a', 'b'], + }); + expect(tree({ status: { $in: ['a', null, 5, true] } })).toEqual({ + kind: 'leaf', member: 'status', operator: 'in', values: ['a', null, 5, true], + }); + }); + + it('keeps every primitive LIKE comparand, including the two #5526 pinned', () => { + // `{$contains: 5}` → `%5%` and `{$contains: null}` → `%null%` agree across + // driver-sql, driver-memory and both faces here. #5526 kept them on + // purpose; refusing them would have created a split, not closed one. + expect(tree({ name: { $contains: 5 } })).toEqual({ + kind: 'leaf', member: 'name', operator: 'contains', values: [5], + }); + expect(tree({ name: { $contains: null } })).toEqual({ + kind: 'leaf', member: 'name', operator: 'contains', values: [null], + }); + expect(tree({ name: { $startsWith: 'x_' } })).toEqual({ + kind: 'leaf', member: 'name', operator: 'startsWith', values: ['x_'], + }); + }); + + it('`{$eq: {…}}` is deliberately UNTOUCHED — a separate account', () => { + // #5526 pinned `toSqlBindValue({a:1})` → `'{"a":1}'`. Refusing it is the + // analytics-side half of #5041, which this change does not open. + expect(tree({ name: { $eq: { a: 1 } } })).toEqual({ + kind: 'leaf', member: 'name', operator: 'equals', values: [{ a: 1 }], + }); + }); + }); +}); + +// ── The read-scope door ────────────────────────────────────────────────────── + +describe('[#5234] the read-scope lowering refuses the same two shapes, fail-closed', () => { + it('an `$in` object member is refused rather than bound', () => { + const err = refusalOf(() => scope({ status: { $in: ['a', { foo: 1 }] } })); + expect(err.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err.status).toBe(500); + expect(err.message).toContain('read-scope-sql'); + expect(err.message).toContain('index 1'); + }); + + it('a `$nin` object member is refused — pre-fix that exclusion silently did not happen', () => { + // `NOT IN ('[object Object]')` excludes nothing. On a tenant/RLS predicate + // an exclusion that does not happen is over-reach, which is the same reading + // #5347 / #5324 made on this file. + const err = refusalOf(() => scope({ status: { $nin: [{ foo: 1 }] } })); + expect(err.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err.message).toContain('index 0'); + }); + + it('a `$between` bound is a comparand in its own right', () => { + const err = refusalOf(() => scope({ score: { $between: [{ a: 1 }, 10] } })); + expect(err.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err.message).toContain('index 0'); + }); + + for (const op of ['$contains', '$notContains', '$startsWith', '$endsWith'] as const) { + it(`\`${op}\` refuses an object comparand instead of binding '%[object Object]%'`, () => { + const err = refusalOf(() => scope({ name: { [op]: { foo: 1 } } })); + expect(err.code, op).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err.status, op).toBe(500); + expect(err.message, op).toContain(op); + }); + } + + it('an ARRAY LIKE comparand is refused — the other half of the measured split', () => { + const err = refusalOf(() => scope({ name: { $contains: ['al', 'be'] } })); + expect(err.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err.message).toContain('an array'); + }); + + describe('the guard is narrow here too', () => { + it('keeps binding every legitimate `$in` member', () => { + expect(scope({ status: { $in: ['a', null, 7, true] } }).params).toEqual(['a', null, 7, true]); + }); + + it('keeps every primitive LIKE comparand', () => { + expect(scope({ name: { $contains: 5 } }).params).toEqual(['%5%', '\\']); + expect(scope({ name: { $contains: null } }).params).toEqual(['%null%', '\\']); + expect(scope({ name: { $startsWith: '_admin' } }).params).toEqual(['\\_admin%', '\\']); + }); + + it('an empty `$in` / `$nin` still lowers to its boolean constant, not a refusal', () => { + // The member scan runs AFTER the arity identities (#5134), so the empty + // list keeps compiling to `1 = 0` / `1 = 1` rather than becoming an error. + expect(scope({ status: { $in: [] } }).sql).toContain('1 = 0'); + expect(scope({ status: { $nin: [] } }).sql).toContain('1 = 1'); + }); + }); +}); + +// ── The rule itself ────────────────────────────────────────────────────────── + +describe('[#5234] the two predicates, stated once', () => { + it('`isBindableComparand` admits exactly what a driver can bind', () => { + for (const v of [null, undefined, 'x', 0, 1.5, 9n, true, false, new Date(), new Uint8Array([1])]) { + expect(isBindableComparand(v), String(v)).toBe(true); + } + for (const v of [{}, { a: 1 }, [1, 2], { $field: 'x' }, () => 1]) { + expect(isBindableComparand(v), JSON.stringify(v) ?? 'fn').toBe(false); + } + }); + + it('`isRenderableTextComparand` is that set minus binary', () => { + // A buffer BINDS but renders to nothing a caller meant, which is why the two + // questions are two predicates rather than one with a flag. + expect(isBindableComparand(new Uint8Array([1]))).toBe(true); + expect(isRenderableTextComparand(new Uint8Array([1]))).toBe(false); + for (const v of [null, undefined, 'x', 0, 9n, true, new Date()]) { + expect(isRenderableTextComparand(v), String(v)).toBe(true); + } + for (const v of [{}, { a: 1 }, [1, 2]]) { + expect(isRenderableTextComparand(v), JSON.stringify(v)).toBe(false); + } + }); +}); diff --git a/packages/services/service-analytics/src/__tests__/like-metacharacter-escape.test.ts b/packages/services/service-analytics/src/__tests__/like-metacharacter-escape.test.ts index 9d8e9f5edb..e171ed23d8 100644 --- a/packages/services/service-analytics/src/__tests__/like-metacharacter-escape.test.ts +++ b/packages/services/service-analytics/src/__tests__/like-metacharacter-escape.test.ts @@ -73,6 +73,16 @@ import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js'; import { ObjectQLStrategy } from '../strategies/objectql-strategy.js'; import { compileScopedFilterToSql } from '../read-scope-sql.js'; import { escapeLikePattern, likePattern, LIKE_ESCAPE_CHAR } from '../like-pattern.js'; +import { isBindableComparand, isRenderableTextComparand } from '../comparand-shape.js'; + +/** A label for a comparand that survives `undefined`, a `Date` and a buffer. */ +function describe1(v: unknown): string { + if (v === undefined) return 'undefined'; + if (v instanceof Date) return `Date(${v.toISOString()})`; + if (ArrayBuffer.isView(v)) return 'Uint8Array'; + if (typeof v === 'bigint') return `${v}n`; + return JSON.stringify(v) ?? String(v); +} /** * The issue's four measured rows, plus a pair for the escape character itself. @@ -208,6 +218,89 @@ describe('[#5567] analytics LIKE compilers escape their comparand', () => { }); }); + // ── [#5234] Which comparands may reach that transform at all ──────────────── + + /** + * The escape expression above answers "how is a comparand rendered"; this + * block answers "which comparands are rendered at all", and the two must move + * together. `escapeLikePattern` still takes `unknown` and still calls + * `String()` unconditionally — that is only safe while both packages agree on + * the fence in front of it, so the fence is mirrored here exactly the way + * `driverSqlEscape` mirrors the transform. + * + * Why it is a fence and not a tolerant `String()`: `String({})` is the literal + * `'[object Object]'`, which builds a valid, parameterised `LIKE` pattern + * nobody wrote — and against a row storing that text it MATCHES. The + * `$notContains` direction then excludes a real row. Measured before the fix + * on `driver-sql`, `driver-memory` and both faces here. + */ + describe('[#5234] the comparand fence in front of that transform', () => { + /** + * `driver-sql`'s `isRenderableTextComparand`, mirrored — same reason the + * escape expression is mirrored above. If either package widens or narrows + * its allow-list, this assertion is what goes red. + */ + const driverSqlRenderable = (v: unknown): boolean => { + if (v === null || v === undefined) return true; + const kind = typeof v; + if (kind === 'string' || kind === 'number' || kind === 'bigint' || kind === 'boolean') return true; + return v instanceof Date; + }; + + /** `driver-sql`'s `isBindableComparand`, mirrored — the `$in`-member half. */ + const driverSqlBindable = (v: unknown): boolean => { + if (v === null || v === undefined) return true; + const kind = typeof v; + if (kind === 'string' || kind === 'number' || kind === 'bigint' || kind === 'boolean') return true; + return v instanceof Date || ArrayBuffer.isView(v); + }; + + /** + * One table, both predicates, spanning every branch of each: the primitives + * #5526 deliberately kept, the `Date` `driver-turso`'s allow-list keeps as + * its one object conversion, the binary that binds but does not render, and + * the object shapes #5234 refuses. + */ + const VALUES: unknown[] = [ + '_admin', '', 'plain', 0, 5, -1.5, 9n, true, false, null, undefined, + new Date('2026-01-01T00:00:00.000Z'), new Uint8Array([1, 2]), + {}, { foo: 1 }, { $field: 'other' }, ['al', 'be'], [], + ]; + + it('`isRenderableTextComparand` matches `driver-sql`, value for value', () => { + for (const v of VALUES) { + expect(isRenderableTextComparand(v), `renderable(${describe1(v)})`).toBe(driverSqlRenderable(v)); + } + }); + + it('`isBindableComparand` matches `driver-sql`, value for value', () => { + for (const v of VALUES) { + expect(isBindableComparand(v), `bindable(${describe1(v)})`).toBe(driverSqlBindable(v)); + } + }); + + it('every value the fence admits, `String()` renders faithfully', () => { + // The property that makes the unconditional `String()` in + // `escapeLikePattern` correct rather than merely unexercised. + for (const v of VALUES.filter(isRenderableTextComparand)) { + expect(String(v), describe1(v)).not.toBe('[object Object]'); + } + // …and the refused ones are exactly the values that would have produced a + // pattern nobody wrote. + expect(String({})).toBe('[object Object]'); + expect(String(['al', 'be'])).toBe('al,be'); + }); + + it('the array case is a SPLIT this fence closes, not a behaviour it invents', () => { + // `read-scope-sql` bound `%al,be%` (String of the whole array) while the + // analytics `where` door bound `%al%` (it reads `values[0]`), for one + // filter. Both doors now refuse it, so there is no third answer to pick. + expect(isRenderableTextComparand(['al', 'be'])).toBe(false); + expect(likePattern('contains', 'al')).toBe('%al%'); + expect(likePattern('contains', ['al', 'be'] as unknown as string)).toBe('%al,be%'); + }); + }); + // ── Binding layer: the issue's measured table, all three compilers ────────── describe("the issue's binding table — every compiler binds the ESCAPED pattern", () => { diff --git a/packages/services/service-analytics/src/comparand-shape.ts b/packages/services/service-analytics/src/comparand-shape.ts new file mode 100644 index 0000000000..8f53544012 --- /dev/null +++ b/packages/services/service-analytics/src/comparand-shape.ts @@ -0,0 +1,155 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Which comparand SHAPES this package's filter compilers can express (#5234). + * + * Two questions, asked of every value that reaches a predicate: + * + * 1. can it become a bound parameter at all ({@link isBindableComparand})? + * 2. does it have a faithful rendering as the TEXT of a `LIKE` pattern + * ({@link isRenderableTextComparand})? + * + * They are different questions about the same value — a binary buffer binds + * fine and renders to nothing meaningful — and they are asked at different + * operators, so they are two predicates rather than one with a flag. + * + * ## Why this file exists at all + * + * `driver-sql`'s `applyLike` and this package's {@link likePattern} both reached + * their comparand through `String(value)`, and `String({})` is the literal + * `'[object Object]'`. The result was never an error: it was a parameterised, + * syntactically perfect `LIKE '%[object Object]%'` — a pattern the author never + * wrote. Measured against a row whose text really is `[object Object]`, that + * pattern MATCHED it, and `$notContains` EXCLUDED it. The `$in` / `$nin` half is + * quieter still: an object member binds, compares equal to nothing, and the list + * silently loses an entry — so `{status: {$nin: [{…}]}}` excludes nothing while + * claiming to exclude something. + * + * ## The fence is an ALLOW-list, and it is measured + * + * An allow-list because a deny-list silently re-admits whatever value form is + * invented next — the lesson `driver-turso`'s `RemoteTransport` wrote down when + * it refused these same two shapes in remote mode (cloud#1004 / #1058), which is + * also the precedent this rule follows rather than inventing a second policy. + * + * What stays IN the fence was measured across every face before being kept, not + * assumed: + * + * | comparand | `driver-sql` | `driver-memory` | analytics (both doors) | + * |---|---|---|---| + * | `{$contains: 5}` | `%5%` | `%5%` | `%5%` | + * | `{$contains: null}` | `%null%` | no match | `%null%` (#5526 pinned) | + * | `{$contains: {}}` | matched a row reading `[object Object]` | same | same | + * | `{$contains: ['al','be']}` | `%al,be%` | — | `%al,be%` (read scope) / `%al%` (`where` door) | + * + * The primitives agree, so refusing them would BREAK agreement — #5526 kept + * `{$contains: 5}` deliberately for exactly that reason. The last row is the + * opposite case: an array comparand already answered two different ways inside + * this one package, so refusing it closes a live split. + * + * ## Mirrored, not imported — and held by a test + * + * These predicates restate `driver-sql`'s `isBindableComparand` / + * `isRenderableTextComparand` (`packages/drivers/driver-sql/src/sql-driver.ts`) + * for the same reason `like-pattern.ts` restates its escape expression: + * `service-analytics` depends on no driver (see its `package.json` — only + * `@objectstack/core` and `@objectstack/spec`), and those are module-private + * functions with no export to reach for. What stops the two from drifting is not + * these comments but `__tests__/like-metacharacter-escape.test.ts`, which asserts + * both predicates against a mirrored copy of the driver's expressions over a + * shared value table. A THIRD hand-copy is the thing to refuse: import from one + * of the two, or add a consumer to that test. + */ + +/** + * Can this value be handed to a driver as a bound parameter at all? + * + * Character for character the classification `driver-sql`'s + * `isBindableComparand` applies. (`ArrayBuffer.isView` covers `Buffer`, which is + * a `Uint8Array`.) + */ +export function isBindableComparand(value: unknown): boolean { + if (value === null || value === undefined) return true; + const kind = typeof value; + if (kind === 'string' || kind === 'number' || kind === 'bigint' || kind === 'boolean') return true; + return value instanceof Date || ArrayBuffer.isView(value); +} + +/** + * Does this value have a faithful rendering as the text of a `LIKE` pattern? + * + * The bindable set minus binary, which binds but renders to nothing a caller + * meant. `undefined` is inside the fence because it is not authorable (JSON has + * no `undefined`) and {@link comparand} already normalises it to `null` rather + * than refusing it (#5526, #5332) — refusing it here would invent a + * disagreement instead of closing one. + */ +export function isRenderableTextComparand(value: unknown): boolean { + if (value === null || value === undefined) return true; + const kind = typeof value; + if (kind === 'string' || kind === 'number' || kind === 'bigint' || kind === 'boolean') return true; + return value instanceof Date; +} + +/** + * The Filter Protocol operators whose comparand becomes the text of a `LIKE` + * pattern — the ones every compiler in this package routes through + * {@link likePattern}. + * + * `$regex` is absent because this package's operator vocabulary does not carry + * it: `MONGO_TO_CUBE_OP` has no entry and `read-scope-sql` refuses it by name. + * (`driver-sql` DOES list it, because the better-auth adapter emits it there for + * a substring search.) + */ +export const TEXT_PATTERN_OPERATORS: ReadonlySet = new Set([ + '$contains', '$notContains', '$startsWith', '$endsWith', +]); + +/** A short, non-throwing rendering of an offending comparand for a message. */ +export function shapePreview(value: unknown): string { + try { + const json = JSON.stringify(value); + if (typeof json !== 'string') return typeof value; + return json.length > 80 ? `${json.slice(0, 77)}...` : json; + } catch { + return typeof value; + } +} + +/** + * The sentence both doors say about an object where a `LIKE` pattern's text + * belongs, so the analytics `where` door and the read-scope lowering do not + * describe one rule two ways. Each door wraps it in its OWN envelope — a 400 + * `INVALID_FILTER` for a caller-authored filter, a fail-closed compile refusal + * for a read scope — because the envelope is what differs between them, not the + * diagnosis. + */ +export function unrenderableTextComparandMessage(op: string, field: string, value: unknown): string { + return ( + `"${op}" on "${field}" matches against the TEXT of a pattern, but its comparand is ` + + `${Array.isArray(value) ? 'an array' : 'an object'} (${shapePreview(value)}). filter.zod.ts ` + + `declares it a string (StringOperatorSchema); a string, number, boolean, null or Date is ` + + `accepted. Refusing rather than stringifying it: String({}) is "[object Object]", so the ` + + `pattern that ran would be one nobody wrote — and a row storing that literal text matches it.` + ); +} + +/** + * The sentence both doors say about a list member that cannot be bound. See + * {@link unrenderableTextComparandMessage} for why the message is shared and the + * envelope is not. + */ +export function unbindableListMemberMessage( + op: string, + field: string, + value: unknown, + index: number, +): string { + return ( + `"${op}" on "${field}" has a value at index ${index} of its list that cannot be bound as a SQL ` + + `parameter: ${shapePreview(value)}. Every member of an $in/$nin/$between list is a comparand ` + + `in its own right — use a string, number, boolean, null, Date or binary value. Refusing rather ` + + `than binding it: the member can equal no stored value, so the list silently loses that entry ` + + `(and a $nin loses the exclusion the caller wrote).` + ); +} diff --git a/packages/services/service-analytics/src/like-pattern.ts b/packages/services/service-analytics/src/like-pattern.ts index 91b12d418d..9e850caa76 100644 --- a/packages/services/service-analytics/src/like-pattern.ts +++ b/packages/services/service-analytics/src/like-pattern.ts @@ -79,6 +79,25 @@ * {@link escapeLikePattern} against `applyLike`'s expression character for * character. A third hand-copy of this logic anywhere is the thing to refuse — * import from here, or add a consumer to that test. + * + * ## `String(value)` is safe here because nothing unrenderable reaches it (#5234) + * + * The `String()` below used to be the whole defect on the other side: `String({})` + * is the literal `'[object Object]'`, so an object comparand built a parameterised + * `LIKE '%[object Object]%'` — valid SQL, a pattern nobody wrote, and one that + * MATCHED a row whose text really was `[object Object]`. This function is NOT the + * place that was fixed. Both of this package's doors refuse an object comparand + * before a pattern is built — `filter-normalizer.ts`'s `fieldLeaves` for the + * analytics `where` path and `read-scope-sql.ts`'s `compileOperator` for the RLS + * lowering — using the one rule in `comparand-shape.ts`, and `driver-sql`'s + * `assertCompilableComparand` does the same for `applyLike`. + * + * So `escapeLikePattern` keeps its `unknown` parameter and its unconditional + * `String()` on purpose: what arrives is a string, number, bigint, boolean, + * `null`, `undefined` or `Date`, each of which `String()` renders faithfully, and + * a number comparand (`{$contains: 5}` → `%5%`) is deliberately still accepted — + * it agrees across every face and #5526 kept it. Do NOT add a tolerant reading of + * an object here; add it to neither door either. */ /** diff --git a/packages/services/service-analytics/src/read-scope-sql.ts b/packages/services/service-analytics/src/read-scope-sql.ts index 32868989ca..b58c380a77 100644 --- a/packages/services/service-analytics/src/read-scope-sql.ts +++ b/packages/services/service-analytics/src/read-scope-sql.ts @@ -3,6 +3,12 @@ import type { FilterCondition } from '@objectstack/spec/data'; import type { RegisteredErrorCode } from '@objectstack/spec/api'; import { likePattern, LIKE_ESCAPE_CHAR } from './like-pattern.js'; +import { + isBindableComparand, + isRenderableTextComparand, + unbindableListMemberMessage, + unrenderableTextComparandMessage, +} from './comparand-shape.js'; /** * Compile an RLS / tenant read-scope `FilterCondition` into a parameterized, @@ -330,6 +336,38 @@ function bindLike(params: unknown[], pattern: string): string { return `${bind(params, pattern)} ESCAPE ${bind(params, LIKE_ESCAPE_CHAR)}`; } +/** + * [#5234] The comparand-SHAPE gate for this door. + * + * `compileScopedFilterToSql` takes a `FilterCondition` that never passes through + * `filter-normalizer`'s `fieldLeaves`, so this module needs the two checks in + * its own right — same rule, stated once in `comparand-shape.ts`, wrapped in + * THIS module's envelope. The envelope difference is the point: a read scope is + * compiled from a policy, not authored by the caller, so an uncompilable + * comparand here is a 500 fail-closed refusal (see the header) rather than a 400. + * + * The direction matters more here than anywhere else this rule lands. A + * read-scope `{$nin: [{…}]}` compiled to `NOT IN ('[object Object]')`, which + * excludes NOTHING — the scope's exclusion silently did not happen, which is + * over-reach on a tenant/RLS predicate rather than a loose filter. That is the + * same reading #5347 / #5324 made on this very file, and the reason the #5234 + * issue's "fail-closed, so lower risk" framing does not survive contact with the + * `$nin` / `$notContains` half. + */ +function assertCompilableMembers(op: string, field: string, members: unknown[]): void { + members.forEach((member, index) => { + if (!isBindableComparand(member)) { + throw readScopeCompileError(`[read-scope-sql] ${unbindableListMemberMessage(op, field, member, index)}`); + } + }); +} + +/** [#5234] See {@link assertCompilableMembers}; this is the LIKE-family half. */ +function assertRenderableText(op: string, field: string, val: unknown): void { + if (isRenderableTextComparand(val)) return; + throw readScopeCompileError(`[read-scope-sql] ${unrenderableTextComparandMessage(op, field, val)}`); +} + function compileOperator(col: string, op: string, val: unknown, field: string, params: unknown[]): string { switch (op) { case '$eq': return val === null ? `${col} IS NULL` : `${col} = ${bind(params, val)}`; @@ -341,23 +379,28 @@ function compileOperator(col: string, op: string, val: unknown, field: string, p case '$in': { if (!Array.isArray(val)) throw readScopeCompileError(`[read-scope-sql] $in for "${field}" needs an array (fail-closed).`); if (val.length === 0) return FALSE_CLAUSE; // IN () matches nothing — safe + assertCompilableMembers(op, field, val); return `${col} IN (${val.map((v) => bind(params, v)).join(', ')})`; } case '$nin': { if (!Array.isArray(val)) throw readScopeCompileError(`[read-scope-sql] $nin for "${field}" needs an array (fail-closed).`); if (val.length === 0) return '1 = 1'; // NOT IN () excludes nothing + assertCompilableMembers(op, field, val); return `${col} NOT IN (${val.map((v) => bind(params, v)).join(', ')})`; } case '$between': { if (!Array.isArray(val) || val.length !== 2) throw readScopeCompileError(`[read-scope-sql] $between for "${field}" needs [min,max] (fail-closed).`); + assertCompilableMembers(op, field, val); return `${col} BETWEEN ${bind(params, val[0])} AND ${bind(params, val[1])}`; } // [#5567] The comparand is a LITERAL, so it is escaped and the escape // character is bound with it. See {@link bindLike}. - case '$contains': return `${col} LIKE ${bindLike(params, likePattern('contains', val))}`; - case '$notContains': return `${col} NOT LIKE ${bindLike(params, likePattern('contains', val))}`; - case '$startsWith': return `${col} LIKE ${bindLike(params, likePattern('starts', val))}`; - case '$endsWith': return `${col} LIKE ${bindLike(params, likePattern('ends', val))}`; + // [#5234] …and it must be a value `String()` can render, which is asserted + // BEFORE `likePattern` sees it — see {@link assertRenderableText}. + case '$contains': assertRenderableText(op, field, val); return `${col} LIKE ${bindLike(params, likePattern('contains', val))}`; + case '$notContains': assertRenderableText(op, field, val); return `${col} NOT LIKE ${bindLike(params, likePattern('contains', val))}`; + case '$startsWith': assertRenderableText(op, field, val); return `${col} LIKE ${bindLike(params, likePattern('starts', val))}`; + case '$endsWith': assertRenderableText(op, field, val); return `${col} LIKE ${bindLike(params, likePattern('ends', val))}`; case '$null': return val ? `${col} IS NULL` : `${col} IS NOT NULL`; case '$exists': return val ? `${col} IS NOT NULL` : `${col} IS NULL`; default: diff --git a/packages/services/service-analytics/src/strategies/filter-normalizer.ts b/packages/services/service-analytics/src/strategies/filter-normalizer.ts index 8251a576d7..6b640ee466 100644 --- a/packages/services/service-analytics/src/strategies/filter-normalizer.ts +++ b/packages/services/service-analytics/src/strategies/filter-normalizer.ts @@ -217,6 +217,13 @@ import { isFilterAST, parseFilterAST, VALID_AST_OPERATORS } from '@objectstack/spec/data'; import { StandardErrorCode } from '@objectstack/spec/api'; +import { + isBindableComparand, + isRenderableTextComparand, + TEXT_PATTERN_OPERATORS, + unbindableListMemberMessage, + unrenderableTextComparandMessage, +} from '../comparand-shape.js'; export interface NormalizedAnalyticsFilter { member: string; @@ -381,6 +388,48 @@ function andOf(children: NormalizedFilterNode[]): NormalizedFilterNode | null { return { kind: 'and', children }; } +/** + * [#5234] The comparand-SHAPE gate for the analytics `where` door. + * + * This runs before a leaf exists, which is the whole reason it is here rather + * than at the three emitters. {@link fieldLeaves} is the ONLY producer of leaf + * nodes in this package, so one refusal here covers all three consumers of the + * tree at once — `NativeSQLStrategy.buildFilterClause` (the statement that + * executes), `ObjectQLStrategy.buildFilterClauseSql` (the `/analytics/sql` echo) + * and `ObjectQLStrategy.convertFilter` (the engine path). Guarding the emitters + * instead would have been three guards, three envelopes, and one of them — + * `convertFilter`'s `String(v0)` — would still have LAUNDERED the object into + * `'[object Object]'` before any driver could refuse it, so a strict driver + * downstream could never see the shape it was strict about. + * + * Prime Directive #12, applied literally: refuse at the door, do not tolerate at + * the consumer. `read-scope-sql.ts` is this package's OTHER door — it compiles a + * `FilterCondition` that never passes through here — and carries the same two + * checks in its own fail-closed envelope. + * + * Only the two shapes #5234 measured are refused; `$eq` and friends keep binding + * an object as JSON (`toSqlBindValue`), which is a separate account. + */ +function assertCompilableComparand(opKey: string, field: string, value: unknown): void { + if (TEXT_PATTERN_OPERATORS.has(opKey)) { + // An array reaches this door as `values[0]` — i.e. every member after the + // first is silently DROPPED — while `read-scope-sql` and `driver-sql` + // stringify the whole array. That split is why an array is refused and not + // merely stringified consistently. + if (!isRenderableTextComparand(value)) { + throw invalidFilterError(`[analytics] ${unrenderableTextComparandMessage(opKey, field, value)}`); + } + return; + } + if ((opKey === '$in' || opKey === '$nin') && Array.isArray(value)) { + value.forEach((member, index) => { + if (!isBindableComparand(member)) { + throw invalidFilterError(`[analytics] ${unbindableListMemberMessage(opKey, field, member, index)}`); + } + }); + } +} + /** * Compile one `field: value | { $op: … }` entry into its leaves. * @@ -524,6 +573,7 @@ function fieldLeaves(key: string, raw: unknown): NormalizedFilterNode[] { ); } const v = wrapper[opKey]; + assertCompilableComparand(opKey, key, v); leaf(cubeOp, Array.isArray(v) ? v.map(comparand) : [comparand(v)]); } return out; diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index 22af5b80e3..0298b23a31 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -1040,6 +1040,14 @@ export class ObjectQLStrategy implements AnalyticsStrategy { * string — `String(…)`, the same normalisation `like-pattern.ts` applies at the * two SQL emitters and `driver-sql`'s `applyLike` applies at the driver, so one * `$contains` means one thing on every face (#5567's invariant). + * + * [#5234] Those four `String(…)` calls now only ever see a value that renders + * faithfully: `fieldLeaves` refuses an object comparand on this family before a + * leaf exists. That ordering is load-bearing rather than incidental — this arm + * is a PRODUCER for the engine, so stringifying an object here would have + * laundered it into `'[object Object]'` and handed a driver a perfectly + * well-typed string. A strict driver downstream could never have seen the shape + * it was strict about, which is why the guard sits at the door and not here. */ private convertFilter(operator: string, values?: unknown[]): unknown { if (operator === 'set') return { $ne: null }; From 0ffc0659bba8f6335b4aff157a5e4ab16ef80542 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 13:07:47 +0000 Subject: [PATCH 2/2] =?UTF-8?q?test(driver-sql):=20=E8=B7=9F=E8=BF=9B=20#6?= =?UTF-8?q?210=20=E7=9A=84=20DriverQuery=20=E6=94=B6=E7=AA=84,=E5=B9=B6?= =?UTF-8?q?=E6=8A=8A=E5=8F=8D=E5=90=91=E9=AA=8C=E8=AF=81=E8=AE=A1=E6=95=B0?= =?UTF-8?q?=E9=87=8D=E6=B5=8B=E5=88=B0=20d367f03?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `find()` 调用去掉 `object` 键:#6210 把驱动查询参数收窄为 `DriverQuery`, 对象名只由第一个实参给出。没有用 `as any` 绕过——拒收类测试一旦用越契约的 输入构造查询,就不再是在验证调用方真能发出的形状。 - 反向验证计数重测:LIKE 臂停用 → 8 failed / 40 passed;成员臂停用 → 4 failed / 44 passed(此前记的 32 / 36 是首写时对 40 个测试测的)。失败数 两次都不变,是承重的那一半;通过数随邻测增删而漂移,已在注释里说明。 - 补记 analytics 侧同法实测:两个门回退到 main 版本 → 17 failed / 41 passed, 而 `like-metacharacter-escape.test.ts` 在该回退下保持绿——它锁的是两包谓词 之间的一致,拒收文件锁的是门确实调用了谓词,两者都需要。 Refs #5234 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WyvqvKMG6asi9aXjKE6xtx --- .../sql-driver-silent-empty-predicate.test.ts | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/drivers/driver-sql/src/sql-driver-silent-empty-predicate.test.ts b/packages/drivers/driver-sql/src/sql-driver-silent-empty-predicate.test.ts index 983845a1b0..a5a73f61df 100644 --- a/packages/drivers/driver-sql/src/sql-driver-silent-empty-predicate.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-silent-empty-predicate.test.ts @@ -41,19 +41,30 @@ * describe do not apply. * * Measured by disabling each arm in turn and re-running this file together with - * `sql-driver-out-of-contract-filter-input.test.ts`: + * `sql-driver-out-of-contract-filter-input.test.ts` (48 tests across the two, + * re-measured on `main` at d367f03 — i.e. after #6210's `DriverQuery` narrowing): * - * - LIKE arm disabled → **8 failed / 32 passed**. The eighth is in the other + * - LIKE arm disabled → **8 failed / 40 passed**. The eighth is in the other * file: the superseded `{ $startsWith: {} }` pin, which is the point of * having replaced it rather than deleted it. - * - member arm disabled → **4 failed / 36 passed** — the `$in`, `$nin`, + * - member arm disabled → **4 failed / 44 passed** — the `$in`, `$nin`, * nested-array and `$between` cases. The `$field` member case is NOT among * them, and that is the arm ordering being verified rather than a gap: * #5041's cross-field refusal still answers first for that shape. * * The counts are here because a bare "reverting turns it red" claims nothing * checkable — if a later change makes one of these arms unreachable, the count - * moves and the next reader can see that it did. + * moves and the next reader can see that it did. The FAILURE counts are the + * load-bearing half; the pass counts drift as neighbouring tests are added (they + * read 32 / 36 when this file was first written, against 40 tests). + * + * The analytics side answers the same way, measured the same way: reverting + * `read-scope-sql.ts` and `filter-normalizer.ts` to their pre-#5234 `main` + * versions turns `comparand-shape-refusal.test.ts` **17 failed / 41 passed**. + * `like-metacharacter-escape.test.ts` stays GREEN under that revert, and that is + * correct rather than a hole: it pins the two packages' PREDICATES against each + * other, while this refusal file pins that the doors actually call them. Both + * are needed — parity with nothing calling it is #4984's dead-rule shape. * * The row-set assertions are the other half, and they are what makes the * refusals meaningful rather than self-referential: `keeps` pins that every @@ -124,8 +135,12 @@ describe('[#5234] SqlDriver refuses the two comparand shapes that compiled to a for (const row of ROWS) await driver.create('probe', row); }); + // No `object` key: #6210 narrowed the driver query parameter to `DriverQuery`, + // whose object is the first argument and nothing else. Kept in step here rather + // than cast away — a rejection test that builds its query off-contract stops + // exercising the shape a caller can actually send. const find = (where: unknown) => - driver.find('probe', { object: 'probe', fields: ['id'], where: where as FilterCondition }); + driver.find('probe', { fields: ['id'], where: where as FilterCondition }); const ids = async (where: unknown): Promise => ((await find(where)) as Array<{ id: string }>).map((r) => r.id).sort();