Skip to content

Commit a50555c

Browse files
committed
test(plugin-auth): migrate the contains witness to sqlite, now that $regex is refused (#5893)
`auth-contains-filter.test.ts` pins #5710's `contains` -> `$contains` flip. Its behavioural face ran on `InMemoryDriver` because, when #5830 measured it, driver-sql routed `$regex` through the same `applyContainsLike` as `$contains` — a SQL witness answered the defect and the fix identically, so migrating would have produced pins that pass because nothing distinguishes them. #5702 (PR #6549) deleted that fallthrough and retired the spelling: driver-sql now refuses `$regex` by name in the ADR-0112 envelope (INVALID_FILTER / 400). The expiry condition #5830 wrote for this card is met, so the file moves onto the PR #5880 harness (SqlDriver + better-sqlite3 `:memory:` + real DDL via initObjects) and witnesses its defect through the refusal. - migrate the behavioural face; measured identical answers value-for-value - add a discrimination face: the backend refuses `$regex` (code AND status, naming `$icontains`) while still answering `$contains` — the property that lets the behavioural pins fail at all, so this cannot silently rot back into an always-green pin - strengthen the `^` case with a control read, since "no rows" is the one answer a broken query and a correct one can both produce - drop plugin-auth's `@objectstack/driver-memory` devDep (now zero consumers) + lockfile ripple - revise the census comments in the runtime driver-split pin and the sibling coverage test, which still said this file must not be migrated Reverse-verified: restoring the defect turns 5 of 9 cases red — the contract face plus all four behavioural pins, every one of them via the refusal path rather than a row difference. The runtime pin's "only permanent test consumer in the repository" census had also expired independently of this change (#6468 added a second importer in the same package); it now claims a ruling about itself rather than a repo-wide count, and the second file's disposition is filed as #6664. Fixes #5893 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
1 parent e6025e9 commit a50555c

5 files changed

Lines changed: 223 additions & 58 deletions

File tree

packages/plugins/plugin-auth/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
"jose": "^6.2.5"
3939
},
4040
"devDependencies": {
41-
"@objectstack/driver-memory": "workspace:*",
4241
"@objectstack/driver-sql": "workspace:*",
4342
"@objectstack/objectql": "workspace:*",
4443
"@types/node": "^26.1.2",

packages/plugins/plugin-auth/src/auth-contains-filter.test.ts

Lines changed: 171 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* `convertWhere()` used to emit `{ field: { $regex: condition.value } }`, which
88
* puts an unescaped, caller-supplied comparand (`/admin/list-users`'
99
* `searchValue`, a SCIM filter value) into a PATTERN position. What that value
10-
* then means depended on the backend under the auth path:
10+
* then meant depended on the backend under the auth path:
1111
*
1212
* - driver-memory compiled it to `new RegExp(value)` — `a.b` matched `axb`,
1313
* `^x` anchored, and an unbalanced `(` was an illegal pattern;
@@ -20,37 +20,101 @@
2020
* member of the spec's `FILTER_OPERATORS`, i.e. one every backend is required
2121
* to evaluate, as a literal substring.
2222
*
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.
23+
* Three faces, deliberately: the first pins WHAT the adapter emits (the
24+
* contract), the second pins what a real backend then ANSWERS (the behaviour),
25+
* and the third pins the property that lets the second face FAIL at all (the
26+
* discrimination). The first alone cannot see a translation that is spelled
27+
* right and evaluated wrong; the second alone cannot say which operator earned
28+
* the result; and without the third, face 2 would be an always-green pin the
29+
* day the backend starts aliasing `$regex` again.
30+
*
31+
* ## Backend note (#5893 / #5830 / #5704)
32+
*
33+
* Face 2's backend was `InMemoryDriver` when this file landed with #5812; it is
34+
* now `@objectstack/driver-sql` + better-sqlite3 `:memory:`, the harness its
35+
* sibling `auth-where-operator-coverage.test.ts` already uses (PR #5880), built
36+
* the way the rest of the repo builds an ephemeral store (`examples/app-crm`,
37+
* `cli db clean`, PR #5715's `makeDefaultDriver()`).
38+
*
39+
* The swap was DEFERRED once, on measurement, and this is the deferral being
40+
* discharged rather than forgotten — the history matters because it names the
41+
* exact hazard this file now guards against:
42+
*
43+
* - **Then (#5830, PR #5880).** driver-sql routed `$regex` through the same
44+
* `applyContainsLike` as `$contains` (a `case '$regex':` fallthrough), so
45+
* the SQL backend answered the two operators cell-for-cell alike. Measured
46+
* there: with the defect restored, memory failed 3 of these behavioural
47+
* pins and sqlite passed all 4. Migrating then would have produced pins
48+
* that are green because nothing distinguishes them — coverage that is
49+
* blind to the very defect it names.
50+
* - **Now (#5702, PR #6549).** The fallthrough is deleted and `$regex` is
51+
* RETIRED: driver-sql refuses it by name in the ADR-0112 envelope
52+
* (`code: 'INVALID_FILTER'`, `status: 400`), prescribing `$icontains`. The
53+
* defect is therefore witnessed on the SQL arm again — not as a different
54+
* row set, but as a REFUSAL, which is a strictly better reason: a bare
55+
* `$regex` from this adapter no longer answers a subtly wrong question, it
56+
* answers nothing and says why.
57+
*
58+
* Re-measured for #5893 before this file moved (both directions, `flock`-ed
59+
* local run, and again with the defect restored):
60+
*
61+
* - fixed adapter, sqlite: `$contains 'a.b'` → `['a.b']`, `'^a'` → `[]`,
62+
* `'('` → `['x(y']`, `'xb'` → `['axb']` — identical, value for value, to
63+
* what memory answered, so the migration costs the fixture nothing;
64+
* - defect restored (adapter emits a bare `$regex` again), sqlite: all four
65+
* behavioural pins RED via the refusal, plus the contract face.
66+
*
67+
* Face 3 exists so that this stays true without anyone re-measuring by hand. It
68+
* is the load-bearing half of the migration: if driver-sql ever re-aliases
69+
* `$regex` onto the substring path, face 3 goes red and says so, instead of
70+
* face 2 quietly reverting to the always-green pin #5830 refused to create.
71+
*
72+
* Case semantics are deliberately untested here, exactly as in the sibling
73+
* file: sqlite's `LIKE` is ASCII case-INsensitive by default (so `$contains`
74+
* folds on this backend while the contract layer calls it case-SENSITIVE per
75+
* #5701 Q2=A — driver-sql pins that gap itself, in
76+
* `sql-driver-icontains-and-retired-operators.test.ts`, which asserts
77+
* `$contains` compiles WITHOUT `LOWER()` while `$icontains` compiles with it).
78+
* Every fixture below is lower-case and no assertion depends on which way that
79+
* per-driver alignment lands.
2780
*/
2881

29-
import { describe, it, expect, beforeEach, vi } from 'vitest';
30-
import { InMemoryDriver } from '@objectstack/driver-memory';
82+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
83+
import { SqlDriver } from '@objectstack/driver-sql';
3184
import { FILTER_OPERATORS } from '@objectstack/spec/data';
3285
import type { QueryAST } from '@objectstack/spec/data';
3386
import type { IDataEngine } from '@objectstack/core';
3487
import { createObjectQLAdapterFactory } from './objectql-adapter';
3588

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;
89+
/**
90+
* The columns face 2 reads, declared — a real table has to be told.
91+
*
92+
* `emailVerified` / `createdAt` / `updatedAt` used to be seeded alongside these
93+
* and are gone with the backend swap: they were camelCase keys no assertion
94+
* ever read, which only a schemaless store would have accepted (`sys_user`
95+
* spells them `email_verified` / `created_at` / `updated_at`). Declaring the
96+
* fixture down to what it actually asserts on is #5806's "resolve by declaring,
97+
* not by relaxing" — the alternative would be declaring three columns to hold
98+
* values nothing looks at.
99+
*/
100+
const SYS_USER = {
101+
name: 'sys_user',
102+
fields: {
103+
name: { type: 'text', name: 'name' },
104+
email: { type: 'text', name: 'email' },
105+
},
106+
};
43107

44108
/**
45-
* A read-only engine facade over a REAL `InMemoryDriver`.
109+
* A read-only engine facade over a REAL `SqlDriver`.
46110
*
47111
* Only the three read verbs the `contains` path uses are declared. The write
48112
* verbs are deliberately absent rather than stubbed: seeding goes through the
49113
* driver directly (below), so a hand-written `delete`/`update` here would be a
50114
* dispatch contract this test neither needs nor is able to honour
51115
* (`check:engine-double-contract`, #4550).
52116
*/
53-
function memoryReadEngine(driver: InMemoryDriver): IDataEngine {
117+
function sqlReadEngine(driver: SqlDriver): IDataEngine {
54118
// The query bag is forwarded with its declared driver-side type and no `any`
55119
// erasure: `query-options/no-any-erasure` (#4674/#4918) counts a test-side
56120
// `find(obj, … as any)` too, and nothing here needs to be off-contract.
@@ -61,23 +125,41 @@ function memoryReadEngine(driver: InMemoryDriver): IDataEngine {
61125
} as unknown as IDataEngine;
62126
}
63127

64-
const NOW = new Date('2026-08-06T00:00:00.000Z').toISOString();
65-
66128
/** Rows whose `name`s differ only in how a regex would read the comparand. */
67129
const SEED = [
68130
{ id: 'u_literal', name: 'a.b', email: 'literal@example.com' },
69131
{ id: 'u_wildcard', name: 'axb', email: 'wildcard@example.com' },
70132
{ id: 'u_paren', name: 'x(y', email: 'paren@example.com' },
71133
];
72134

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 });
135+
/**
136+
* Live `:memory:` databases, closed after each test — the database dies with
137+
* its connection, so nothing touches the host filesystem, but a file this size
138+
* would otherwise hold one open pool per behavioural case.
139+
*/
140+
const openDrivers: SqlDriver[] = [];
141+
142+
afterEach(async () => {
143+
while (openDrivers.length) {
144+
const driver = openDrivers.pop();
145+
try { await driver?.disconnect(); } catch { /* noop */ }
78146
}
79-
const adapter: any = (createObjectQLAdapterFactory(memoryReadEngine(driver)) as any)({} as any);
80-
return { driver, adapter };
147+
});
148+
149+
async function seededAdapter() {
150+
const driver = new SqlDriver({
151+
client: 'better-sqlite3',
152+
connection: { filename: ':memory:' },
153+
useNullAsDefault: true,
154+
});
155+
openDrivers.push(driver);
156+
// Real DDL through the driver's own path — the table every row below lands in
157+
// is created by the backend, not conjured by a store on first write.
158+
await driver.initObjects([SYS_USER]);
159+
for (const row of SEED) await driver.create('sys_user', row);
160+
const engine = sqlReadEngine(driver);
161+
const adapter: any = (createObjectQLAdapterFactory(engine) as any)({} as any);
162+
return { driver, engine, adapter };
81163
}
82164

83165
/** `findMany` with a single better-auth `contains` condition on `name`. */
@@ -89,6 +171,10 @@ function containsQuery(value: string) {
89171
} as any;
90172
}
91173

174+
// ---------------------------------------------------------------------------
175+
// Face 1 — the contract: which ObjectQL operator the translation emits
176+
// ---------------------------------------------------------------------------
177+
92178
describe('[#5710] convertWhere: better-auth `contains` → `$contains`', () => {
93179
let engine: IDataEngine;
94180

@@ -116,8 +202,8 @@ describe('[#5710] convertWhere: better-auth `contains` → `$contains`', () => {
116202
it('emits an operator every backend is required to evaluate', () => {
117203
// The whole point of the flip: `$contains` is in the protocol's runtime
118204
// 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.
205+
// produced it, which is why #5702's loud refusal was ordered after #5812's
206+
// flip and is now the thing face 3 reads.
121207
expect(FILTER_OPERATORS).toContain('$contains');
122208
expect(FILTER_OPERATORS).not.toContain('$regex');
123209
});
@@ -138,13 +224,18 @@ describe('[#5710] convertWhere: better-auth `contains` → `$contains`', () => {
138224
});
139225
});
140226

227+
// ---------------------------------------------------------------------------
228+
// Face 2 — the behaviour: what a real backend answers
229+
// ---------------------------------------------------------------------------
230+
141231
describe('[#5710] the comparand is a literal substring on a real backend', () => {
142232
it('does not read `.` as a wildcard — `a.b` matches `a.b`, not `axb`', async () => {
143233
const { adapter } = await seededAdapter();
144234
const rows: any[] = await adapter.findMany(containsQuery('a.b'));
145235

146236
// 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.
237+
// (a bare `$regex` matched it through `.` on the memory backend, and is
238+
// refused outright on this one), and the literal row still is.
148239
expect(rows.map((r) => r.name)).toEqual(['a.b']);
149240
});
150241

@@ -155,6 +246,14 @@ describe('[#5710] the comparand is a literal substring on a real backend', () =>
155246
// As a pattern, `^a` matched `a.b` and `axb`. As a substring, nothing here
156247
// contains the two characters `^a`.
157248
expect(rows).toEqual([]);
249+
250+
// A control read on the SAME fixture, because "no rows" is the one answer a
251+
// broken query and a correct one can both produce: an empty seed, a table
252+
// that never got its DDL, or a predicate the backend silently dropped would
253+
// all satisfy the assertion above. `a` is a substring of two of these three
254+
// rows, so this says the store is live and the predicate really selects.
255+
const control: any[] = await adapter.findMany(containsQuery('a'));
256+
expect(control.map((r) => r.name).sort()).toEqual(['a.b', 'axb']);
158257
});
159258

160259
it('matches a value that is not a legal regex, instead of failing on it', async () => {
@@ -174,3 +273,47 @@ describe('[#5710] the comparand is a literal substring on a real backend', () =>
174273
expect(rows.map((r) => r.name)).toEqual(['axb']);
175274
});
176275
});
276+
277+
// ---------------------------------------------------------------------------
278+
// Face 3 — the discrimination: this backend can tell the two operators apart
279+
// ---------------------------------------------------------------------------
280+
281+
describe('[#5893] the backend refuses the operator face 2 must never see', () => {
282+
it('refuses a bare `$regex` in the ADR-0112 envelope, naming its replacement', async () => {
283+
const { engine } = await seededAdapter();
284+
285+
// Sent through the SAME engine facade the adapter reads on, so this is the
286+
// literal path a regressed `convertWhere` would take — not a parallel one.
287+
const err: any = await engine
288+
.find('sys_user', { where: { name: { $regex: 'a.b' } } })
289+
.then(() => null, (e: unknown) => e);
290+
291+
// `code` AND `status`, never a bare `rejects.toThrow()`: the defect has two
292+
// fields and a throw-only assertion carries one bit. Before #5702 this call
293+
// did not throw at ALL — it ANSWERED, with `['a.b']` — so an assertion that
294+
// only says "the promise rejected" cannot separate "refused with the wrong
295+
// envelope" from "did not refuse at all", which are exactly the two ways
296+
// this guard can rot.
297+
expect(err).toBeInstanceOf(Error);
298+
expect(err.code).toBe('INVALID_FILTER');
299+
expect(err.status).toBe(400);
300+
// The message is the operating instruction: which operator was refused, and
301+
// what to write instead (#5702 prescribes the replacement, not a list).
302+
expect(err.message).toContain('$regex');
303+
expect(err.message).toContain('$icontains');
304+
});
305+
306+
it('answers the operator the adapter DOES emit, on the same fixture', async () => {
307+
const { engine } = await seededAdapter();
308+
309+
// The other half of the pair, and the reason this face is not just a
310+
// driver-sql test living in the wrong package: refusing everything would
311+
// satisfy the case above. `$contains` and `$regex` must be told APART by
312+
// this backend — one answers, the other is refused — because that
313+
// difference is the whole reason face 2 is allowed to live on sqlite
314+
// (#5830 measured the world where it was not, and deferred the migration).
315+
const rows = await engine.find('sys_user', { where: { name: { $contains: 'a.b' } } });
316+
317+
expect((rows as any[]).map((r) => r.name)).toEqual(['a.b']);
318+
});
319+
});

packages/plugins/plugin-auth/src/auth-where-operator-coverage.test.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,16 @@
6060
* both ways in #5830: re-dropping the `not_in` arm turns the `not_in` pin red
6161
* on sqlite exactly as it did on memory.
6262
*
63-
* Its sibling `auth-contains-filter.test.ts` is NOT interchangeable this way —
64-
* driver-sql compiles `$regex` through the same `applyContainsLike` as
65-
* `$contains` (`sql-driver.ts`, the `case '$regex':` fallthrough), so a SQL
66-
* backend cannot tell #5710's defect from its fix. That file's disposition is
67-
* #5830's open half; do not "finish the job" by copying this harness onto it.
63+
* [#5893] Its sibling `auth-contains-filter.test.ts` was NOT interchangeable
64+
* this way when the note above was written — driver-sql compiled `$regex`
65+
* through the same `applyContainsLike` as `$contains` (`sql-driver.ts`, the
66+
* `case '$regex':` fallthrough), so a SQL backend could not tell #5710's defect
67+
* from its fix, and #5830 deferred it rather than create an always-green pin.
68+
* #5702 (PR #6549) deleted the fallthrough and RETIRED the spelling: `$regex`
69+
* is now refused by name in the ADR-0112 envelope (`INVALID_FILTER` / 400), so
70+
* that file has moved onto this harness too and witnesses its defect through
71+
* the refusal. It carries its own guard for the refusal, because that property
72+
* — not a row difference — is what lets its behavioural pins fail at all.
6873
*/
6974

7075
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';

0 commit comments

Comments
 (0)