From b6ea956b7938e868269687df54172feeb56f5b2d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 20:25:53 +0000 Subject: [PATCH] fix(service-analytics): fence `$icontains` comparands on the analytics `where` door (#7693) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `$icontains` was the one text-pattern operator the #5234 comparand fence never covered on the analytics `where` door. It arrived after the fence: #6520 added it to `MONGO_TO_CUBE_OP` and gave `read-scope-sql.ts`'s arm its `assertRenderableText` call, but not the entry in `comparand-shape.ts`'s `TEXT_PATTERN_OPERATORS` — the set the `where` door's shape gate reads. One operator, two answers inside one package. Measured on origin/main @ b54aaab: `{name: {$icontains: {foo: 1}}}` compiled to the leaf `{operator: 'icontains', values: [{foo: 1}]}` and `NativeSQLStrategy.generateSql` bound `'%[object Object]%'` into a parameterised, syntactically perfect `LIKE` nobody wrote — while the sibling read-scope door refused the same comparand. `$contains` refused it on both. The fix is the entry in `TEXT_PATTERN_OPERATORS`; the gate in `filter-normalizer.ts` is generic over the set. Consumer measurement, quoted exact name across the repo: within this package the set has exactly one reader (`filter-normalizer.ts` :358 import, :577 gate) plus its definition — every other hit is a comment or `driver-sql`'s module-private set of the same name, which has listed `$icontains` since #6520. Tests: #7598's RECORDED GAP pin in `cross-field-reference-refusal.test.ts` is FLIPPED to assert the refusal (`INVALID_FILTER` / 400, LIKE-family wording, byte-identical to what `$contains` says), the shared #5222 corpus is now driven whole there instead of filtering `$icontains` out, and the LIKE-family loops in `comparand-shape-refusal.test.ts` gain their fifth member on both doors. Reverse-verified over the whole package: deleting the entry gives 5 failed / 1553 passed, exactly the `where`-door cells; every read-scope `$icontains` assertion and every narrowness control stays green, so the entry is shown to close a hole rather than retire the operator #6520 added. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01M7h5X9FCDSAkT531cJZ9D2 --- ...ontains-comparand-fence-analytics-where.md | 40 ++++++ .../__tests__/comparand-shape-refusal.test.ts | 19 ++- .../cross-field-reference-refusal.test.ts | 121 ++++++++++++++---- .../service-analytics/src/comparand-shape.ts | 22 +++- 4 files changed, 171 insertions(+), 31 deletions(-) create mode 100644 .changeset/icontains-comparand-fence-analytics-where.md diff --git a/.changeset/icontains-comparand-fence-analytics-where.md b/.changeset/icontains-comparand-fence-analytics-where.md new file mode 100644 index 0000000000..a59594bc28 --- /dev/null +++ b/.changeset/icontains-comparand-fence-analytics-where.md @@ -0,0 +1,40 @@ +--- +"@objectstack/service-analytics": patch +--- + +fix(service-analytics): fence `$icontains` comparands on the analytics `where` door (#7693) + +`$icontains` was the one text-pattern operator the #5234 comparand fence never +covered on the analytics `where` door. It arrived after the fence: #6520 added +it to `filter-normalizer.ts`'s `MONGO_TO_CUBE_OP` and gave `read-scope-sql.ts`'s +arm its `assertRenderableText` call, but not the entry in `comparand-shape.ts`'s +`TEXT_PATTERN_OPERATORS` — the set the `where` door's shape gate reads. So one +operator had **two answers inside one package**. Measured on `origin/main` @ +`b54aaab`: + +| filter | analytics `where` door | `read-scope-sql` | +|---|---|---| +| `{name: {$contains: {foo: 1}}}` | REFUSED (`INVALID_FILTER` / 400) | REFUSED (`READ_SCOPE_COMPILE_FAILED` / 500) | +| `{name: {$icontains: {foo: 1}}}` | **compiled** — `NativeSQLStrategy` bound `'%[object Object]%'` | REFUSED | + +The compiled statement was the #5234 defect verbatim: a parameterised, +syntactically perfect `LIKE` pattern nobody wrote, which a row whose text really +is `[object Object]` matches. `driver-sql`'s own `TEXT_PATTERN_OPERATORS` has +listed the operator since #6520, and #7158 closed the same gap at objectql +`having`; this closes the third and last face. + +**What changes for a caller.** A malformed `$icontains` comparand — an object, a +`{$field}` reference, or an array — on the `/analytics` `where` door is now +refused with `INVALID_FILTER` / 400 and the same sentence `$contains` gets, +instead of compiling into a pattern that matches the wrong rows. A well-formed +comparand is untouched: strings, numbers, `null`, booleans and `Date`s compile +exactly as before, ASCII fold and metacharacter escaping included. The +read-scope door is unchanged — it already refused these shapes. + +Held by `__tests__/cross-field-reference-refusal.test.ts`, where #7598's +RECORDED GAP pin is flipped to assert the refusal and the shared `#5222` corpus +is now driven whole (its `$icontains` case no longer has to be filtered out), +and by the fifth member added to the LIKE-family loops in +`__tests__/comparand-shape-refusal.test.ts`. Reverse-verified against the whole +package: deleting the entry turns exactly those five `where`-door cells red and +leaves every read-scope and narrowness control green. 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 index 299b71f5f4..b8abed50cd 100644 --- a/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts +++ b/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts @@ -50,6 +50,18 @@ * 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. + * + * # [#7693] The family's fifth member + * + * The LIKE-family loops below drive `TEXT_PATTERN_OPERATORS` by name, and they + * used to name FOUR operators. `$icontains` arrived after this fence (#6520) + * and got the read-scope door's `assertRenderableText` call but not the entry + * in that set, so `{name: {$icontains: {foo: 1}}}` reproduced row 2 of the + * table above exactly — `%[object Object]%` on the `where` door, REFUSED on the + * read-scope one — one operator with two answers inside one package. #7693 + * added the entry; the loops now name all five, and the `where`-door arm of the + * `$icontains` row is the case that only passes because of it (the read-scope + * arm was already green and is here as the no-regression control). */ import { describe, it, expect } from 'vitest'; @@ -107,7 +119,8 @@ describe('[#5234] the analytics `where` door refuses an uncompilable comparand', }); describe('shape 2 — a LIKE-family comparand with no faithful text rendering', () => { - for (const op of ['$contains', '$notContains', '$startsWith', '$endsWith'] as const) { + // [#7693] `$icontains` is the fifth member — see this file's header. + for (const op of ['$contains', '$notContains', '$startsWith', '$endsWith', '$icontains'] as const) { it(`\`${op}\` refuses an object comparand`, () => { const err = refusalOf(() => tree({ name: { [op]: { foo: 1 } } })); expect(err.code, op).toBe('INVALID_FILTER'); @@ -225,7 +238,9 @@ describe('[#5234] the read-scope lowering refuses the same two shapes, fail-clos expect(err.message).toContain('index 0'); }); - for (const op of ['$contains', '$notContains', '$startsWith', '$endsWith'] as const) { + // [#7693] `$icontains` was ALREADY refused on this door — it is in the loop as + // the no-regression control for the entry added on the other one. + for (const op of ['$contains', '$notContains', '$startsWith', '$endsWith', '$icontains'] 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'); diff --git a/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts b/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts index 1cfa544333..0286fa026d 100644 --- a/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts +++ b/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts @@ -80,15 +80,28 @@ * - `comparand-shape-refusal.test.ts` stayed green in full (1 file failed, 1 * passed), confirming nothing in the #5234 pins depends on this change. * - * ## The one cell that does NOT refuse on the `where` door, recorded not hidden + * ## The one cell that did NOT refuse on the `where` door — CLOSED by #7693 * - * `$icontains` is absent from `comparand-shape.ts`'s `TEXT_PATTERN_OPERATORS`, - * so the analytics `where` door applies NO comparand-shape gate to it at all — - * a #5234-class hole that arrived with the operator itself (#6520 added - * `$icontains` to `MONGO_TO_CUBE_OP` and to `read-scope-sql`'s - * `assertRenderableText`, but not to that set). It is out of this card's scope - * and is filed separately; the block at the bottom pins the CURRENT behaviour so - * the gap is visible in test output rather than discovered again from scratch. + * This file used to end in a RECORDED GAP block: `$icontains` was absent from + * `comparand-shape.ts`'s `TEXT_PATTERN_OPERATORS`, so the analytics `where` + * door applied NO comparand-shape gate to it at all — a #5234-class hole that + * arrived with the operator itself (#6520 added `$icontains` to + * `MONGO_TO_CUBE_OP` and to `read-scope-sql`'s `assertRenderableText`, but not + * to that set). #7598 left it alone deliberately and filed it as #7693. + * + * #7693 added the entry, so the pin FLIPPED: the block at the bottom now + * asserts the refusal it used to record the absence of, and the corpus loop + * below no longer has to filter the operator out of `CROSS_FIELD_REFUSALS`. + * + * Reverse-verified by deleting the entry again and re-running the WHOLE + * package: **5 failed / 1553 passed** (green: 1558 / 0). The five are exactly + * the `where`-door cells — the three new ones here, this file's now-unfiltered + * corpus case, and `comparand-shape-refusal.test.ts`'s fifth loop member. What + * stayed GREEN is the other half of the proof: every read-scope `$icontains` + * assertion (that door's refusal comes from its own `assertRenderableText`, not + * from this entry), and every narrowness control below — a well-formed + * `$icontains` comparand compiles identically in both states, so the entry is + * shown to close a hole rather than retire the operator #6520 added. */ import { describe, it, expect } from 'vitest'; @@ -171,11 +184,14 @@ describe("[#7598] the #5222 corpus's SUPPORTED arm is refused by both analytics describe("[#7598] the #5222 corpus's REFUSAL arm stays refused on both doors", () => { // These are refused on the drivers too, so this block asserts CONVERGENCE - // rather than asymmetry. `$icontains` is the single exception on the `where` - // door — see the recorded-gap block at the bottom. - const cases = CROSS_FIELD_REFUSALS.filter((c) => !JSON.stringify(c.filter).includes('$icontains')); - - for (const testCase of cases) { + // rather than asymmetry. + // + // [#7693] The corpus is driven WHOLE. It used to be filtered — `$icontains` + // was the single exception on the `where` door, because it was missing from + // `TEXT_PATTERN_OPERATORS` — and dropping that filter is half of this card's + // proof: `$icontains against a field reference is refused` is a corpus case + // that only passes here once the entry exists. + for (const testCase of CROSS_FIELD_REFUSALS) { it(`the \`where\` door refuses: ${testCase.name}`, () => { const err = refusalOf(() => tree(testCase.filter)); expect(err.code, testCase.name).toBe('INVALID_FILTER'); @@ -335,21 +351,70 @@ describe('[#7598] a read scope keeps working on the ObjectQL engine path', () => }); }); -// ── A measured gap this card does not close, recorded so it is not re-found ── - -describe('[#7598] RECORDED GAP: `$icontains` has no comparand-shape gate on the `where` door', () => { - it('an object comparand still reaches the pattern builder there', () => { - // NOT an endorsement — a pin on current behaviour. `$icontains` is missing - // from `TEXT_PATTERN_OPERATORS`, so the #5234 fence has never covered it on - // this door, while `read-scope-sql` DOES refuse it (its `$icontains` arm - // calls `assertRenderableText`). One operator, two answers inside one - // package — the split #5234 closed for its four siblings. Filed separately; - // fixing it here would be a second defect riding this card. - expect(tree({ stage: { $icontains: { $field: 'owner' } } })).toEqual({ - kind: 'leaf', member: 'stage', operator: 'icontains', values: [{ $field: 'owner' }], +// ── The gap #7598 recorded here, CLOSED by #7693 ───────────────────────────── + +describe('[#7693] `$icontains` is fenced on the `where` door, like its four siblings', () => { + // This block is the FLIPPED #7598 pin. It used to assert that + // `tree({stage: {$icontains: {$field: 'owner'}}})` RETURNED a leaf, with the + // read-scope refusal beside it for contrast; the contrast is now a + // convergence, and the block only goes green when + // `TEXT_PATTERN_OPERATORS` actually carries the operator. + + it('a `{$field}` comparand is refused on BOTH doors now, not just one', () => { + const where = refusalOf(() => tree({ stage: { $icontains: { $field: 'owner' } } })); + expect(where.code).toBe('INVALID_FILTER'); + expect(where.status).toBe(400); + // The LIKE-family wording, not the #7598 cross-field one: `$icontains` is a + // pattern operator, so the diagnosis a caller gets is "this position holds + // the TEXT of a pattern" — the same sentence `$contains` gets, which is what + // makes the two operators one answer rather than two. + expect(where.message).toContain('StringOperatorSchema'); + expect(where.message).toContain('$icontains'); + + const scoped = refusalOf(() => scope({ stage: { $icontains: { $field: 'owner' } } })); + expect(scoped.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(scoped.status).toBe(500); + }); + + it('the plain malformed comparand `{foo: 1}` is refused rather than bound as `%[object Object]%`', () => { + // #5234's own shape, at the operator its fence never reached. Measured + // pre-fix on `origin/main` @ `b54aaab`: this filter compiled to the leaf + // `{operator: 'icontains', values: [{foo: 1}]}`, and + // `NativeSQLStrategy.generateSql` bound `'%[object Object]%'` into a + // syntactically perfect, parameterised `LIKE` nobody wrote. + const where = refusalOf(() => tree({ name: { $icontains: { foo: 1 } } })); + expect(where.code).toBe('INVALID_FILTER'); + expect(where.status).toBe(400); + expect(where.message).toContain('[object Object]'); + + // The envelope `$contains` gets, said about the same shape — the point of + // the card is that these two rows are now identical apart from the operator. + const sibling = refusalOf(() => tree({ name: { $contains: { foo: 1 } } })); + expect(where.code).toBe(sibling.code); + expect(where.status).toBe(sibling.status); + expect(where.message.replace('$icontains', '$contains')).toBe(sibling.message); + }); + + it('an ARRAY comparand is refused too — the other half of the #5234 shape', () => { + const err = refusalOf(() => tree({ name: { $icontains: ['al', 'be'] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.message).toContain('an array'); + }); + + it('the fence is NARROW — every legitimate `$icontains` comparand still compiles', () => { + // The control group. `$icontains` reached this door ungated, so a gate that + // over-reached would silently retire a working operator (#6520's whole + // subject) rather than close a hole. + expect(tree({ name: { $icontains: 'admin' } })).toEqual({ + kind: 'leaf', member: 'name', operator: 'icontains', values: ['admin'], + }); + expect(tree({ name: { $icontains: 5 } })).toEqual({ + kind: 'leaf', member: 'name', operator: 'icontains', values: [5], + }); + expect(tree({ name: { $icontains: null } })).toEqual({ + kind: 'leaf', member: 'name', operator: 'icontains', values: [null], }); - // The sibling door, for contrast — this is what the `where` door should say. - const err = refusalOf(() => scope({ stage: { $icontains: { $field: 'owner' } } })); - expect(err.code).toBe('READ_SCOPE_COMPILE_FAILED'); + // …and the sibling door is unmoved, which is the no-regression half. + expect(scope({ name: { $icontains: 'admin' } }).params).toEqual(['%admin%', '\\']); }); }); diff --git a/packages/services/service-analytics/src/comparand-shape.ts b/packages/services/service-analytics/src/comparand-shape.ts index 37c1212a89..5df7c1f6db 100644 --- a/packages/services/service-analytics/src/comparand-shape.ts +++ b/packages/services/service-analytics/src/comparand-shape.ts @@ -189,9 +189,29 @@ export const CROSS_FIELD_COMPARISON_OPERATORS: ReadonlySet = new Set([ * 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.) + * + * [#7693] `$icontains` belongs here for the same reason its four siblings do, + * and was missing for the ordinary reason a set goes stale: the operator + * arrived AFTER the fence. #6520 added it to `MONGO_TO_CUBE_OP` and gave + * `read-scope-sql`'s arm its `assertRenderableText` call, but not this entry — + * so the analytics `where` door, this set's only reader, applied NO + * comparand-shape gate to it at all. Measured on `origin/main` @ `b54aaab`: + * + * | door | `{name: {$icontains: {foo: 1}}}` | + * |---|---| + * | analytics `where` | compiled — `NativeSQLStrategy` bound `'%[object Object]%'` into its `LIKE` | + * | `read-scope-sql` | REFUSED (`READ_SCOPE_COMPILE_FAILED` / 500) | + * + * One operator, two answers inside one package — #5234's defect verbatim, at + * the operator its fence was never extended to. The ASCII fold `$icontains` + * adds rides ON TOP of the pattern text (`likeShape` maps it to `'contains'` + * on both executing compilers), so the question this set asks of a comparand + * is the same question and the answer had no business differing. `driver-sql`'s + * own `TEXT_PATTERN_OPERATORS` has listed it since #6520; this entry closes the + * third and last face, after #7158 closed the objectql `having` one. */ export const TEXT_PATTERN_OPERATORS: ReadonlySet = new Set([ - '$contains', '$notContains', '$startsWith', '$endsWith', + '$contains', '$notContains', '$startsWith', '$endsWith', '$icontains', ]); /** A short, non-throwing rendering of an offending comparand for a message. */