Skip to content

Commit a7b854f

Browse files
os-zhuangclaude
andauthored
fix(service-analytics): escape LIKE comparands in all three SQL compilers (#5567) (#5587)
The four LIKE-family operators wrap the author's comparand in wildcards. All three SQL compilers in this package concatenated it straight into that pattern position with no escaping and no ESCAPE clause, so `_` (single-character wildcard) and `%` (multi-character) stopped being literals: {name: {$contains: '_admin'}} returned ['1','2'], should be ['1'] {name: {$contains: '50%'}} returned ['3','4'], should be ['3'] Every direction is a WIDENING, and read-scope-sql.ts is the ADR-0021 D-C read-scope lowering, where that is over-reach rather than a loose filter (#5347 / #5324 on the same file). Prime Directive #3 forces machine names to snake_case, so essentially every machine-name comparand carries a `_`. Adds a package-local like-pattern.ts (escapeLikePattern / likePattern / LIKE_ESCAPE_CHAR) and routes all three compilers through it, each binding `LIKE ? ESCAPE ?`: - read-scope-sql.ts compileOperator, four arms - native-sql-strategy.ts buildFilterClause — the statement that executes - objectql-strategy.ts LIKE_SQL_OPS — the /analytics/sql echo The echo moves with the other two on purpose: its execution goes through the engine to driver-sql, which has always escaped, so leaving it raw would have re-forked echo from execution (#3601 / #3602 / #3650). The escape character is BOUND, never written as a SQL literal: MySQL applies C escape syntax inside string literals so the literal spelling is dialect-specific, and a bound value rides the existing `?` -> `$N` renumbering in both consumers, which keeps the whole concern inside the predicate layer. Second implementation of driver-sql's applyLike, deliberately — service-analytics depends on no driver and applyLike is a private method over a knex builder. The two are held to each other by an assertion in the new test, and applyLike's TSDoc now points here. Claude-Session: https://claude.ai/code/session_01BWS4heBoAitLmzCLhcYdbK Co-authored-by: Claude <noreply@anthropic.com>
1 parent 91ec1ea commit a7b854f

9 files changed

Lines changed: 715 additions & 30 deletions

File tree

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(service-analytics): the three SQL compilers compare LIKE values literally (#5567)
6+
7+
`$contains` / `$notContains` / `$startsWith` / `$endsWith` build a `LIKE` pattern
8+
around the comparand the author wrote. All three of this package's SQL compilers
9+
concatenated that comparand straight into a wildcard position — no escaping, no
10+
`ESCAPE` clause — so `_` (LIKE's single-character wildcard) and `%` (its
11+
multi-character one) stopped being literals. Measured on real SQLite, over the
12+
rows `x_admin` / `xyadmin` / `off 50% now` / `off 5012 now`:
13+
14+
| `where` | returned | correct |
15+
|----------------------------------|---------------|---------|
16+
| `{name: {$contains: '_admin'}}` | `['1','2']` | `['1']` |
17+
| `{name: {$contains: '50%'}}` | `['3','4']` | `['3']` |
18+
| `{name: {$startsWith: 'x_'}}` | `['1','2']` | `['1']` |
19+
| `{name: {$endsWith: '0% now'}}` | `['3','4']` | `['3']` |
20+
21+
Every row is a **widening** — rows the author excluded came back — and
22+
`$notContains` is the mirror image, excluding rows the author kept. One of the
23+
three call sites is the ADR-0021 D-C read-scope (tenant + RLS) lowering, where a
24+
wider predicate is over-reach rather than a loose filter (the #5347 / #5324
25+
ruling on that same file). Prime Directive #3 forces machine names to
26+
`snake_case`, so essentially every machine-name comparand carries a `_` and hit
27+
this silently.
28+
29+
All three compilers now escape the comparand and bind an explicit
30+
`ESCAPE` argument, matching what `driver-sql`'s `applyLike` has always done — so
31+
the same filter selects the same rows whichever strategy answers, and the
32+
`/analytics/sql` echo describes the statement that ran instead of a wider one.
33+
34+
**No authoring change.** A comparand with no `_`, `%` or `\` binds exactly the
35+
pattern it bound before; only its meaning when it *does* carry one changes, from
36+
wildcard to literal. If you were relying on a comparand acting as a wildcard,
37+
that was never a declared capability of these operators — the spec describes them
38+
as substring / prefix / suffix matches — and `driver-sql` already read it
39+
literally, so the reading you got depended on which strategy served the query.

packages/plugins/driver-sql/src/sql-driver.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6278,6 +6278,16 @@ export class SqlDriver implements IDataDriver {
62786278
* character (MySQL/Postgres do, but the explicit clause is correct for all
62796279
* three). `shape` positions the wildcard: `contains` → `%v%`, `starts` → `v%`,
62806280
* `ends` → `%v`.
6281+
*
6282+
* **Second implementation, deliberately** (#5567):
6283+
* `packages/services/service-analytics/src/like-pattern.ts` carries the same
6284+
* transform — same escaped character class, same three shapes, same bound
6285+
* `ESCAPE` — because `service-analytics` depends on no driver and this is a
6286+
* private method taking a knex builder, so there is nothing for it to import.
6287+
* That file's header explains the choice; it is held to THIS expression, character
6288+
* for character, by `service-analytics`'s `like-metacharacter-escape.test.ts`.
6289+
* A third hand-copy is the thing to refuse: import from one of the two, or add
6290+
* a consumer to that test.
62816291
*/
62826292
private applyLike(
62836293
builder: any,

packages/services/service-analytics/src/__tests__/like-metacharacter-escape.test.ts

Lines changed: 437 additions & 0 deletions
Large diffs are not rendered by default.

packages/services/service-analytics/src/__tests__/objectql-echo-operator-coverage.test.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -235,24 +235,33 @@ describe('[#5333] `/analytics/sql` echo — every authorable operator renders a
235235
// ── The issue's measured table, one case per row ────────────────────────────
236236

237237
describe("the issue's measured table", () => {
238+
/*
239+
* [#5567] Each pattern is now followed by a bound `ESCAPE` argument, so the
240+
* echo describes the comparand `driver-sql` actually compares (its
241+
* `applyLike` has always escaped and bound `ESCAPE`). None of these three
242+
* comparands carries a `_` or `%`, so the PATTERN is byte-identical to what
243+
* #5333 pinned — the second bind is the whole delta. The metacharacter cases,
244+
* where the pattern itself changes and the row set with it, are in
245+
* `like-metacharacter-escape.test.ts`.
246+
*/
238247
it('`$startsWith` echoes `LIKE` with the prefix pattern — was no WHERE at all', async () => {
239248
const { sql, params } = await echo({ stage: { $startsWith: 'w' } });
240249
expect(sql).toContain('WHERE');
241-
expect(sql).toContain('stage LIKE $1');
242-
expect(params).toEqual(['w%']);
250+
expect(sql).toContain('stage LIKE $1 ESCAPE $2');
251+
expect(params).toEqual(['w%', '\\']);
243252
});
244253

245254
it('`$endsWith` echoes `LIKE` with the suffix pattern — was no WHERE at all', async () => {
246255
const { sql, params } = await echo({ stage: { $endsWith: 'n' } });
247256
expect(sql).toContain('WHERE');
248-
expect(sql).toContain('stage LIKE $1');
249-
expect(params).toEqual(['%n']);
257+
expect(sql).toContain('stage LIKE $1 ESCAPE $2');
258+
expect(params).toEqual(['%n', '\\']);
250259
});
251260

252261
it('`$contains` still echoes the substring pattern — the row that already worked', async () => {
253262
const { sql, params } = await echo({ stage: { $contains: 'o' } });
254-
expect(sql).toContain('stage LIKE $1');
255-
expect(params).toEqual(['%o%']);
263+
expect(sql).toContain('stage LIKE $1 ESCAPE $2');
264+
expect(params).toEqual(['%o%', '\\']);
256265
});
257266

258267
it('the LIKE patterns are the ones the EXECUTED statement binds', async () => {

packages/services/service-analytics/src/__tests__/read-scope-sql.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,13 @@ describe('compileScopedFilterToSql', () => {
5353

5454
it('comparison + string operators', () => {
5555
expect(compileScopedFilterToSql({ amount: { $gte: 100 } }, 't').sql).toBe('"t"."amount" >= ?');
56+
// [#5567] The LIKE family binds its pattern AND the escape character, so the
57+
// comparand compares literally. `'A'` carries no metacharacter, so the
58+
// pattern itself is unchanged — the second bind is the whole delta here.
59+
// Metacharacter coverage (and the row sets) live in
60+
// `like-metacharacter-escape.test.ts`.
5661
expect(compileScopedFilterToSql({ name: { $startsWith: 'A' } }, 't')).toEqual({
57-
sql: '"t"."name" LIKE ?', params: ['A%'],
62+
sql: '"t"."name" LIKE ? ESCAPE ?', params: ['A%', '\\'],
5863
});
5964
});
6065

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* LIKE pattern construction for this package's three SQL compilers (#5567).
5+
*
6+
* A `$contains` / `$notContains` / `$startsWith` / `$endsWith` comparand is a
7+
* LITERAL the author typed. Concatenating it straight into a wildcard position
8+
* silently reinterprets it as a pattern, because `_` is LIKE's single-character
9+
* wildcard and `%` its multi-character one:
10+
*
11+
* - `{name: {$contains: '_admin'}}` matched `xyadmin` as well as `x_admin`;
12+
* - `{name: {$contains: '50%'}}` matched `off 5012 now` as well as `off 50% now`.
13+
*
14+
* Both directions are WIDENING, and one of the three call sites is
15+
* `read-scope-sql.ts` — the ADR-0021 D-C read-scope (tenant + RLS) lowering,
16+
* where a wider predicate is over-reach rather than a loose filter (#5347 /
17+
* #5324, on that same file). Prime Directive #3 forces machine names to
18+
* `snake_case`, so essentially every machine-name comparand carries a `_` and
19+
* hits this silently.
20+
*
21+
* ## The two halves are one fix
22+
*
23+
* Escaping the value and declaring the escape character are not independent
24+
* steps — either alone is a different bug:
25+
*
26+
* - Escaping alone: `%\_admin%` with no escape character in force is a search
27+
* for a literal backslash. SQLite has NO default escape character, so this
28+
* would return zero rows there.
29+
* - The clause alone: nothing in the pattern is escaped, so nothing changes.
30+
*
31+
* Hence {@link likePattern} always produces a pattern escaped for
32+
* {@link LIKE_ESCAPE_CHAR}, and every emitter pairs it with an `ESCAPE` binding.
33+
*
34+
* ## Why the escape character is BOUND, never written as a literal
35+
*
36+
* Every emitter here binds it as an ordinary placeholder (`LIKE ? ESCAPE ?`)
37+
* rather than writing `ESCAPE '\'` into the SQL text. Two reasons, both load-bearing:
38+
*
39+
* 1. **The literal spelling is not portable.** MySQL applies C escape syntax
40+
* inside string literals — "If you want a LIKE string to contain a literal
41+
* `\`, you must double it" — so the backslash escape character is spelled
42+
* `'\\'` there and `'\'` on SQLite/Postgres. These compilers do not know
43+
* which dialect will run their output. A bound value is escaped by the
44+
* driver for its own dialect, so there is exactly one spelling here.
45+
* 2. **It rides the existing placeholder plumbing.** `read-scope-sql.ts` emits
46+
* `?` and BOTH of its consumers (`NativeSQLStrategy.applyReadScope`,
47+
* `ObjectQLStrategy.generateSql`) renumber `?` → `$N` while pushing the
48+
* matching value. Because the escape character is a bound value it is
49+
* carried by that rewrite with no change at the upper layer — which answers
50+
* the "which layer does the ESCAPE clause belong to" question in the issue:
51+
* the predicate layer, entirely, because nothing above it has to know.
52+
*
53+
* Dialect support for the clause itself, confirmed against the vendors' own
54+
* reference manuals (quoted in PR for #5567): Postgres defaults to backslash and
55+
* accepts `ESCAPE`; MySQL assumes `\` unless `NO_BACKSLASH_ESCAPES` is set and
56+
* accepts `ESCAPE` with an argument that "must evaluate as a constant at
57+
* execution time" (a bound placeholder is); SQLite honours NO default escape
58+
* character at all, which is the reason the explicit clause is required rather
59+
* than merely tidy.
60+
*
61+
* ## Relationship to `driver-sql`'s `applyLike`
62+
*
63+
* This is deliberately the same transform `SqlDriver.applyLike`
64+
* (`packages/plugins/driver-sql/src/sql-driver.ts`) applies — same escaped
65+
* character class, same three wildcard shapes, same bound `ESCAPE` — and its
66+
* TSDoc points back here. It is a SECOND implementation on purpose, not an
67+
* oversight:
68+
*
69+
* - `service-analytics` depends on no driver (see its `package.json`: only
70+
* `@objectstack/core` and `@objectstack/spec`), and `applyLike` is a private
71+
* method on a knex builder — it takes a builder and a field, not a string,
72+
* so there is nothing importable even if the dependency existed.
73+
* - Promoting it to a shared package would add a new public surface to
74+
* `@objectstack/core` for three call sites inside one package. Not worth a
75+
* new export until a fourth consumer outside this package needs it.
76+
*
77+
* What keeps the two from drifting is not these comments: it is
78+
* `__tests__/like-metacharacter-escape.test.ts`, which asserts
79+
* {@link escapeLikePattern} against `applyLike`'s expression character for
80+
* character. A third hand-copy of this logic anywhere is the thing to refuse —
81+
* import from here, or add a consumer to that test.
82+
*/
83+
84+
/**
85+
* Where the wildcard sits relative to the comparand. Named exactly as
86+
* `driver-sql`'s `applyLike` names its `shape` parameter, so the two read alike:
87+
* `contains` → `%v%`, `starts` → `v%`, `ends` → `%v`.
88+
*/
89+
export type LikeShape = 'contains' | 'starts' | 'ends';
90+
91+
/**
92+
* The escape character every emitter in this package binds into its `ESCAPE`
93+
* clause. A single backslash — the value `driver-sql` binds, and the default
94+
* Postgres and MySQL already assume.
95+
*/
96+
export const LIKE_ESCAPE_CHAR = '\\';
97+
98+
/**
99+
* Escape the LIKE metacharacters (`%`, `_`) and the escape character itself
100+
* (`\`) so a comparand matches literally.
101+
*
102+
* Character for character the expression `driver-sql`'s `applyLike` uses; the
103+
* shared test holds them to each other.
104+
*/
105+
export function escapeLikePattern(value: unknown): string {
106+
return String(value).replace(/[\\%_]/g, '\\$&');
107+
}
108+
109+
/**
110+
* Build the LIKE pattern for one comparand: escaped, then wrapped in the
111+
* wildcards `shape` calls for.
112+
*
113+
* The result MUST be bound together with {@link LIKE_ESCAPE_CHAR} as the
114+
* predicate's `ESCAPE` argument — see the escaping-alone note in this file's
115+
* header for what happens on SQLite otherwise.
116+
*/
117+
export function likePattern(shape: LikeShape, value: unknown): string {
118+
const escaped = escapeLikePattern(value);
119+
return shape === 'starts' ? `${escaped}%` : shape === 'ends' ? `%${escaped}` : `%${escaped}%`;
120+
}

packages/services/service-analytics/src/read-scope-sql.ts

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import type { FilterCondition } from '@objectstack/spec/data';
4+
import { likePattern, LIKE_ESCAPE_CHAR } from './like-pattern.js';
45

56
/**
67
* Compile an RLS / tenant read-scope `FilterCondition` into a parameterized,
@@ -66,6 +67,22 @@ import type { FilterCondition } from '@objectstack/spec/data';
6667
* canonical; {@link nullSafeNegationOperand} here is the same rewrite
6768
* `sql-driver.ts` applies, so an analytics query and an ordinary `find()` scope
6869
* the same rows.
70+
*
71+
* ## The LIKE family compares LITERALS (#5567)
72+
*
73+
* `_` is LIKE's single-character wildcard and `%` its multi-character one, so a
74+
* comparand concatenated straight into a pattern position stops meaning what the
75+
* author wrote: `{owner_name: {$contains: '_admin'}}` also admitted `xyadmin`,
76+
* and `{$contains: '50%'}` also admitted `off 5012 now`. Every LIKE arm below
77+
* therefore binds an ESCAPED pattern plus its escape character — see
78+
* `like-pattern.ts` for the transform, for why the escape character is a bound
79+
* value rather than a SQL literal, and for its correspondence with `driver-sql`'s
80+
* `applyLike`.
81+
*
82+
* On THIS compiler that widening was the #5347 / #5324 shape again: a read scope
83+
* admitting rows the policy did not is over-reach, not a degraded filter. Note
84+
* the file was fail-closed everywhere else — the LIKE family was the one place an
85+
* author's literal was silently reinterpreted rather than refused.
6986
*/
7087

7188
const IDENT = /^[a-z_][a-z0-9_]*$/i;
@@ -212,6 +229,27 @@ function bind(params: unknown[], v: unknown): string {
212229
return '?';
213230
}
214231

232+
/**
233+
* [#5567] Bind a LIKE pattern together with its escape character: `? ESCAPE ?`.
234+
*
235+
* Both are ordinary bound values, so this whole concern stays inside the
236+
* predicate: `applyReadScope` (`native-sql-strategy.ts`) and `generateSql`
237+
* (`objectql-strategy.ts`) rewrite `?` → `$N` while pushing the matching value
238+
* from `params`, and they carry the escape character for free — neither consumer
239+
* needed a change. A SQL literal `ESCAPE '\'` would have pushed the problem up a
240+
* layer AND been unportable: MySQL strips one backslash inside a string literal,
241+
* so the literal spelling differs per dialect while a bound value does not.
242+
*
243+
* The clause is not optional decoration. SQLite honours no default escape
244+
* character, so the escaped pattern alone would search for a literal backslash
245+
* there and match nothing — the two halves are one fix (see `like-pattern.ts`).
246+
*/
247+
function bindLike(params: unknown[], pattern: string): string {
248+
// Left-to-right evaluation of the template puts the pattern in `params` before
249+
// the escape character, which is the order the `?` appear.
250+
return `${bind(params, pattern)} ESCAPE ${bind(params, LIKE_ESCAPE_CHAR)}`;
251+
}
252+
215253
function compileOperator(col: string, op: string, val: unknown, field: string, params: unknown[]): string {
216254
switch (op) {
217255
case '$eq': return val === null ? `${col} IS NULL` : `${col} = ${bind(params, val)}`;
@@ -234,10 +272,12 @@ function compileOperator(col: string, op: string, val: unknown, field: string, p
234272
if (!Array.isArray(val) || val.length !== 2) throw new Error(`[read-scope-sql] $between for "${field}" needs [min,max] (fail-closed).`);
235273
return `${col} BETWEEN ${bind(params, val[0])} AND ${bind(params, val[1])}`;
236274
}
237-
case '$contains': return `${col} LIKE ${bind(params, `%${String(val)}%`)}`;
238-
case '$notContains': return `${col} NOT LIKE ${bind(params, `%${String(val)}%`)}`;
239-
case '$startsWith': return `${col} LIKE ${bind(params, `${String(val)}%`)}`;
240-
case '$endsWith': return `${col} LIKE ${bind(params, `%${String(val)}`)}`;
275+
// [#5567] The comparand is a LITERAL, so it is escaped and the escape
276+
// character is bound with it. See {@link bindLike}.
277+
case '$contains': return `${col} LIKE ${bindLike(params, likePattern('contains', val))}`;
278+
case '$notContains': return `${col} NOT LIKE ${bindLike(params, likePattern('contains', val))}`;
279+
case '$startsWith': return `${col} LIKE ${bindLike(params, likePattern('starts', val))}`;
280+
case '$endsWith': return `${col} LIKE ${bindLike(params, likePattern('ends', val))}`;
241281
case '$null': return val ? `${col} IS NULL` : `${col} IS NOT NULL`;
242282
case '$exists': return val ? `${col} IS NOT NULL` : `${col} IS NULL`;
243283
default:

packages/services/service-analytics/src/strategies/native-sql-strategy.ts

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
type NormalizedFilterNode,
1212
} from './filter-normalizer.js';
1313
import { compileScopedFilterToSql } from '../read-scope-sql.js';
14+
import { likePattern, LIKE_ESCAPE_CHAR, type LikeShape } from '../like-pattern.js';
1415
import { nextUtcCalendarDay } from '@objectstack/core';
1516

1617
/**
@@ -662,12 +663,17 @@ export class NativeSQLStrategy implements AnalyticsStrategy {
662663
contains: 'LIKE', notContains: 'NOT LIKE',
663664
startsWith: 'LIKE', endsWith: 'LIKE',
664665
};
665-
/** The LIKE pattern each string operator wraps its comparand in. */
666-
const likePattern: Record<string, (v: string) => string> = {
667-
contains: (v) => `%${v}%`,
668-
notContains: (v) => `%${v}%`,
669-
startsWith: (v) => `${v}%`,
670-
endsWith: (v) => `%${v}`,
666+
/**
667+
* Where each string operator puts the wildcard. [#5567] The pattern itself is
668+
* built by the shared `likePattern`, which ESCAPES the comparand — `_` and
669+
* `%` are LIKE wildcards, so the old inline table quietly turned an author's
670+
* literal into a pattern (`$contains: '_admin'` also matched `xyadmin`).
671+
* `objectql-strategy.ts`'s `LIKE_SQL_OPS` carries the same table for the
672+
* `/analytics/sql` echo of this statement; they move together.
673+
*/
674+
const likeShape: Record<string, LikeShape> = {
675+
contains: 'contains', notContains: 'contains',
676+
startsWith: 'starts', endsWith: 'ends',
671677
};
672678

673679
// Null predicates and the LIKE family read the column as stored — the former
@@ -690,10 +696,15 @@ export class NativeSQLStrategy implements AnalyticsStrategy {
690696

691697
// The LIKE family reads the column as stored — a substring/prefix/suffix
692698
// match is on the raw text — so it keeps the un-normalised reference.
693-
const pattern = likePattern[operator];
694-
if (pattern) {
695-
params.push(pattern(values[0]));
696-
return `${rawCol} ${sqlOp} $${params.length}`;
699+
const shape = likeShape[operator];
700+
if (shape) {
701+
// [#5567] Escaped pattern AND an explicit `ESCAPE`, bound together: the
702+
// escaping alone would search for a literal backslash on SQLite (no
703+
// default escape character there), the clause alone would change nothing.
704+
params.push(likePattern(shape, values[0]));
705+
const patternRef = `$${params.length}`;
706+
params.push(LIKE_ESCAPE_CHAR);
707+
return `${rawCol} ${sqlOp} ${patternRef} ESCAPE $${params.length}`;
697708
}
698709

699710
// A bare-day `lte` bound means "through that whole day" (#3777): compile

0 commit comments

Comments
 (0)