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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .changeset/icontains-comparand-fence-analytics-where.md
Original file line numberDiff line numberDiff line change
@@ -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.
Original file line numberDiff line numberDiff line change
Expand Up@@ -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';
Expand DownExpand Up@@ -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');
Expand DownExpand Up@@ -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');
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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';
Expand DownExpand Up@@ -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');
Expand DownExpand Up@@ -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%', '\\']);
});
});
22 changes: 21 additions & 1 deletion packages/services/service-analytics/src/comparand-shape.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -189,9 +189,29 @@ export const CROSS_FIELD_COMPARISON_OPERATORS: ReadonlySet<string> = 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<string> = new Set([
'$contains', '$notContains', '$startsWith', '$endsWith',
'$contains', '$notContains', '$startsWith', '$endsWith', '$icontains',
]);

/** A short, non-throwing rendering of an offending comparand for a message. */
Expand Down
Loading