Skip to content

Commit 7a40b7a

Browse files
baozhoutaoclaude
andauthored
fix(plugin-auth): better-auth 的 contains 下译为 $contains,比较值不再当正则求值 (#5710) (#5812)
* fix(plugin-auth): translate better-auth `contains` to `$contains`, not a bare `$regex` (#5710) `convertWhere()` emitted `{ field: { $regex: value } }` for better-auth's `contains`, putting an unescaped caller-supplied comparand into a PATTERN position on the authentication path. driver-memory evaluated it as a real RegExp (`a.b` matched `axb`; an unbalanced `(` threw on the mingo path and silently matched nothing on the reference matcher), while driver-sql / -sqlite-wasm / -turso compiled it to an escaped substring LIKE — one auth query, two answers. `$contains` is in the spec's FILTER_OPERATORS and means a literal substring on every backend, which is what better-auth's `contains` means (`Where.mode` defaults to `"sensitive"`, matching the #5701 Q2=A ruling). This also retires the last live producer of `$regex`, which is the ordering constraint #5702's loud refusal waits on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwwiU9bjhwy2SWj13ho8uv * test(plugin-auth): type the memory-engine facade's query bag instead of erasing it to `any` (#5710) The new pin's read-only engine facade forwarded its query with `as any`, which the query-options-erasure ratchet counts on the test surface too (267 -> 270). Declare the driver-side `QueryAST` and forward it unchanged — nothing here is deliberately off-contract, so no erasure is warranted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwwiU9bjhwy2SWj13ho8uv --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 28f2e04 commit 7a40b7a

5 files changed

Lines changed: 235 additions & 1 deletion

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
fix(plugin-auth): better-auth 的 `contains` 下译为 `$contains`,比较值不再当正则求值 (#5710)
6+
7+
`convertWhere()` 把 better-auth 的 `contains` 译成 `{ field: { $regex: value } }`,
8+
于是一个**未转义、来自调用方**的比较值(`/admin/list-users``searchValue`
9+
SCIM 过滤值)坐进了正则的**模式位**。它的含义随后端分叉:
10+
11+
- `driver-memory``new RegExp(value)` 求值 —— `contains('a.b')` 命中 `axb`,
12+
`^x` 变成锚定,而值里一个不配对的 `(` 让模式非法(mingo 查询路径直接抛
13+
`SyntaxError`,参考匹配器则吞成静默零命中);
14+
- `driver-sql` / `driver-sqlite-wasm` / `driver-turso` 编成子串
15+
`LIKE '%value%'`(`%`/`_`/`\` 有转义、带显式 `ESCAPE`),元字符是字面量。
16+
17+
同一个认证查询,在应用测试常用的内存替身上和生产的 SQL 后端上给出**不同答案**,
18+
且分叉发生在认证路径上。
19+
20+
现在这一支发出 `$contains` —— 协议 `FILTER_OPERATORS` 里的算子,五后端都必须按
21+
**字面子串**求值,正是 better-auth `contains` 的本意(其 `Where.mode` 默认
22+
`"sensitive"`,与 #5701 Q2=A 裁定的 `$contains` 大小写敏感契约同向)。
23+
24+
**对使用方的影响**:凭 `/admin/list-users?searchValue=…` 之类接口依赖「元字符按正则
25+
生效」的调用会改变结果 —— 那是本次修复的缺陷本身,不是可依赖的行为。搜索
26+
`a.b` 从此只命中含字面 `a.b` 的行,不再命中 `axb`;含非法正则字符的搜索值不再
27+
报错或静默返回空,而是按字面子串匹配。

packages/plugins/plugin-auth/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
"jose": "^6.2.5"
3434
},
3535
"devDependencies": {
36+
"@objectstack/driver-memory": "workspace:*",
3637
"@objectstack/objectql": "workspace:*",
3738
"@types/node": "^26.1.2",
3839
"hono": "^4.12.34",
Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#5710] better-auth's `contains` is a LITERAL substring search — the adapter
5+
* must not translate it into a bare `$regex`.
6+
*
7+
* `convertWhere()` used to emit `{ field: { $regex: condition.value } }`, which
8+
* puts an unescaped, caller-supplied comparand (`/admin/list-users`'
9+
* `searchValue`, a SCIM filter value) into a PATTERN position. What that value
10+
* then means depended on the backend under the auth path:
11+
*
12+
* - driver-memory compiled it to `new RegExp(value)` — `a.b` matched `axb`,
13+
* `^x` anchored, and an unbalanced `(` was an illegal pattern;
14+
* - driver-sql / -sqlite-wasm / -turso compiled it to a substring
15+
* `LIKE '%value%'` with `%`/`_`/`\` escaped — metacharacters literal.
16+
*
17+
* So one better-auth query answered differently on the memory double an app's
18+
* tests run against and on the SQL backend production runs (#4706's shape, on
19+
* the authentication path). These pins hold the operator at `$contains` — a
20+
* member of the spec's `FILTER_OPERATORS`, i.e. one every backend is required
21+
* to evaluate, as a literal substring.
22+
*
23+
* Two faces, deliberately: the first pins WHAT the adapter emits (the contract),
24+
* the second pins what a real backend then ANSWERS (the behaviour). The first
25+
* alone cannot see a translation that is spelled right and evaluated wrong; the
26+
* second alone cannot say which operator earned the result.
27+
*/
28+
29+
import { describe, it, expect, beforeEach, vi } from 'vitest';
30+
import { InMemoryDriver } from '@objectstack/driver-memory';
31+
import { FILTER_OPERATORS } from '@objectstack/spec/data';
32+
import type { QueryAST } from '@objectstack/spec/data';
33+
import type { IDataEngine } from '@objectstack/core';
34+
import { createObjectQLAdapterFactory } from './objectql-adapter';
35+
36+
/** Keeps the driver's own lifecycle logging out of the test output. */
37+
const silentLogger = {
38+
debug: () => {},
39+
info: () => {},
40+
warn: () => {},
41+
error: () => {},
42+
} as any;
43+
44+
/**
45+
* A read-only engine facade over a REAL `InMemoryDriver`.
46+
*
47+
* Only the three read verbs the `contains` path uses are declared. The write
48+
* verbs are deliberately absent rather than stubbed: seeding goes through the
49+
* driver directly (below), so a hand-written `delete`/`update` here would be a
50+
* dispatch contract this test neither needs nor is able to honour
51+
* (`check:engine-double-contract`, #4550).
52+
*/
53+
function memoryReadEngine(driver: InMemoryDriver): IDataEngine {
54+
// The query bag is forwarded with its declared driver-side type and no `any`
55+
// erasure: `query-options/no-any-erasure` (#4674/#4918) counts a test-side
56+
// `find(obj, … as any)` too, and nothing here needs to be off-contract.
57+
return {
58+
find: (object: string, query: QueryAST) => driver.find(object, query),
59+
findOne: (object: string, query: QueryAST) => driver.findOne(object, query),
60+
count: (object: string, query?: QueryAST) => driver.count(object, query),
61+
} as unknown as IDataEngine;
62+
}
63+
64+
const NOW = new Date('2026-08-06T00:00:00.000Z').toISOString();
65+
66+
/** Rows whose `name`s differ only in how a regex would read the comparand. */
67+
const SEED = [
68+
{ id: 'u_literal', name: 'a.b', email: 'literal@example.com' },
69+
{ id: 'u_wildcard', name: 'axb', email: 'wildcard@example.com' },
70+
{ id: 'u_paren', name: 'x(y', email: 'paren@example.com' },
71+
];
72+
73+
async function seededAdapter() {
74+
const driver = new InMemoryDriver({ logger: silentLogger });
75+
await driver.connect();
76+
for (const row of SEED) {
77+
await driver.create('sys_user', { ...row, emailVerified: false, createdAt: NOW, updatedAt: NOW });
78+
}
79+
const adapter: any = (createObjectQLAdapterFactory(memoryReadEngine(driver)) as any)({} as any);
80+
return { driver, adapter };
81+
}
82+
83+
/** `findMany` with a single better-auth `contains` condition on `name`. */
84+
function containsQuery(value: string) {
85+
return {
86+
model: 'user',
87+
where: [{ field: 'name', value, operator: 'contains', connector: 'AND' }],
88+
limit: 100,
89+
} as any;
90+
}
91+
92+
describe('[#5710] convertWhere: better-auth `contains` → `$contains`', () => {
93+
let engine: IDataEngine;
94+
95+
beforeEach(() => {
96+
engine = {
97+
insert: vi.fn().mockResolvedValue({ id: '1' }),
98+
findOne: vi.fn().mockResolvedValue(null),
99+
find: vi.fn().mockResolvedValue([]),
100+
count: vi.fn().mockResolvedValue(0),
101+
} as unknown as IDataEngine;
102+
});
103+
104+
it('emits `$contains`, never a bare `$regex`, for a `contains` search', async () => {
105+
const adapter: any = (createObjectQLAdapterFactory(engine) as any)({} as any);
106+
await adapter.findMany(containsQuery('a.b'));
107+
108+
const [object, query] = (engine.find as any).mock.calls[0];
109+
expect(object).toBe('sys_user');
110+
expect(query.where).toEqual({ name: { $contains: 'a.b' } });
111+
// Spelled out separately: `toEqual` above would still pass if a second,
112+
// regex-shaped limb were added under a different field.
113+
expect(JSON.stringify(query.where)).not.toContain('$regex');
114+
});
115+
116+
it('emits an operator every backend is required to evaluate', () => {
117+
// The whole point of the flip: `$contains` is in the protocol's runtime
118+
// allowlist, `$regex` never was — it survived only because this adapter
119+
// produced it (driver-memory's `filter-refusal.ts` says so in as many
120+
// words), which is why #5702's loud refusal is ordered after this PR.
121+
expect(FILTER_OPERATORS).toContain('$contains');
122+
expect(FILTER_OPERATORS).not.toContain('$regex');
123+
});
124+
125+
it('leaves the other operators untouched', async () => {
126+
const adapter: any = (createObjectQLAdapterFactory(engine) as any)({} as any);
127+
await adapter.findMany({
128+
model: 'user',
129+
where: [
130+
{ field: 'email', value: 'a@b.com', operator: 'eq', connector: 'AND' },
131+
{ field: 'name', value: ['x', 'y'], operator: 'in', connector: 'AND' },
132+
],
133+
limit: 100,
134+
} as any);
135+
136+
const [, query] = (engine.find as any).mock.calls[0];
137+
expect(query.where).toEqual({ email: 'a@b.com', name: { $in: ['x', 'y'] } });
138+
});
139+
});
140+
141+
describe('[#5710] the comparand is a literal substring on a real backend', () => {
142+
it('does not read `.` as a wildcard — `a.b` matches `a.b`, not `axb`', async () => {
143+
const { adapter } = await seededAdapter();
144+
const rows: any[] = await adapter.findMany(containsQuery('a.b'));
145+
146+
// The pin, stated in both directions: the metacharacter row is NOT matched
147+
// (a bare `$regex` matched it through `.`), and the literal row still is.
148+
expect(rows.map((r) => r.name)).toEqual(['a.b']);
149+
});
150+
151+
it('does not read `^` as an anchor', async () => {
152+
const { adapter } = await seededAdapter();
153+
const rows: any[] = await adapter.findMany(containsQuery('^a'));
154+
155+
// As a pattern, `^a` matched `a.b` and `axb`. As a substring, nothing here
156+
// contains the two characters `^a`.
157+
expect(rows).toEqual([]);
158+
});
159+
160+
it('matches a value that is not a legal regex, instead of failing on it', async () => {
161+
const { adapter } = await seededAdapter();
162+
const rows: any[] = await adapter.findMany(containsQuery('('));
163+
164+
// `new RegExp('(')` throws — under `$regex` this comparand could not
165+
// produce an answer at all (the mingo path threw, the reference matcher
166+
// swallowed it into a silent no-match). It is just a character now.
167+
expect(rows.map((r) => r.name)).toEqual(['x(y']);
168+
});
169+
170+
it('answers an ordinary metacharacter-free search unchanged', async () => {
171+
const { adapter } = await seededAdapter();
172+
const rows: any[] = await adapter.findMany(containsQuery('xb'));
173+
174+
expect(rows.map((r) => r.name)).toEqual(['axb']);
175+
});
176+
});

packages/plugins/plugin-auth/src/objectql-adapter.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,34 @@ function convertWhere(where: CleanedWhere[]): Record<string, any> {
134134
} else if (condition.operator === 'lte') {
135135
filter[fieldName] = { $lte: condition.value };
136136
} else if (condition.operator === 'contains') {
137-
filter[fieldName] = { $regex: condition.value };
137+
// [#5710] `$contains`, NOT `$regex`. better-auth's `contains` is a
138+
// LITERAL substring search (`Where.mode` defaults to `"sensitive"`, and
139+
// the value comes straight from a caller — `/admin/list-users`'
140+
// `searchValue`), while `$regex` puts that value in a PATTERN position.
141+
//
142+
// What the bare `$regex` did, per backend, to one `contains('a.b')`:
143+
// - driver-memory: `new RegExp('a.b')` — `.` is a wildcard, so it
144+
// matched `axb`; an unbalanced `(` in the value made the pattern
145+
// illegal (a throw on the mingo path, a silent no-match on the
146+
// reference matcher's).
147+
// - driver-sql / -sqlite-wasm / -turso: compiled to a substring
148+
// `LIKE '%a.b%'` — metacharacters literal.
149+
// One operator, two answers, and the divergence only shows up when the
150+
// app's tests run on the memory double and production runs SQL (#4706).
151+
//
152+
// `$contains` is in the spec's `FILTER_OPERATORS` and every backend
153+
// evaluates it as a literal substring (SQL side escapes `%`/`_`/`\` and
154+
// emits an explicit `ESCAPE`), which is exactly better-auth's meaning.
155+
// Case semantics follow the #5701 Q2=A ruling (`$contains` is
156+
// case-SENSITIVE at the contract layer; the per-driver alignment is
157+
// #5702's budget), which matches better-auth's own `mode: 'sensitive'`
158+
// default.
159+
//
160+
// This is also the last live producer of `$regex` — it is what
161+
// `driver-memory/src/filter-refusal.ts` means by "refusing it here would
162+
// break a live producer", and the reason #5702's loud refusal is ordered
163+
// after this flip.
164+
filter[fieldName] = { $contains: condition.value };
138165
}
139166
}
140167

pnpm-lock.yaml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)