Skip to content

Commit fe15792

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-20013-tenant-wall-post-hook
2 parents e43cda1 + 7766b62 commit fe15792

14 files changed

Lines changed: 1667 additions & 23 deletions
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
"@objectstack/driver-sqlite-wasm": patch
4+
"@objectstack/driver-turso": patch
5+
---
6+
7+
fix(driver-sql, driver-turso): on SQLite, a `$contains` / `$notContains` / `$icontains` / `$startsWith` / `$endsWith` comparand holding U+0000 is compared whole, against the whole stored value, instead of being cut at the U+0000 by `GLOB` (#19999)
8+
9+
Clause-②: no
10+
11+
On the SQLite faces these five operators compile to `GLOB`, and SQLite's `glob()` reads both the pattern and the stored value only up to their first U+0000. Nothing raised, and the filter answered a different question. Measured on `SqlDriver` over better-sqlite3 (SQLite 3.53.4), on `SqliteWasmDriver` over sql.js (3.49.1), and on `TursoDriver`'s remote transport over a local libSQL engine (3.45.1). All three answered alike. Over the values `'a'` + U+0000 + `'b'`, `'ab'` + U+0000, U+0000 + `'z'`, `'plain'` and `''`:
12+
13+
- `$contains: U+0000` and `$endsWith: U+0000` returned all five rows;
14+
- `$contains: U+0000 + 'b'` returned all five rows, where the JavaScript answer is `'a'` + U+0000 + `'b'` only;
15+
- `$startsWith: U+0000` returned `''` and U+0000 + `'z'`, where the JavaScript answer is U+0000 + `'z'` only.
16+
17+
What changes: a comparand holding U+0000 now compiles to a length-aware comparison instead. `$contains`, `$notContains`, `$icontains` and `$startsWith` use `instr()`, and `$endsWith` compares the value's trailing bytes over BLOB. Such a filter now returns the rows `driver-memory` and `@objectstack/formula` return for it. The comparand is bound as written, so `*`, `?` and `[` in it are literal, as they were before. `$icontains` still folds ASCII letters only, and `$notContains` still returns a row whose value is NULL.
18+
19+
- `@objectstack/driver-sql`: the SQLite arm of `SqlDriver`'s text-operator compiler. `SqliteWasmDriver` and `TursoDriver`'s local mode inherit it.
20+
- `@objectstack/driver-sqlite-wasm`: none of its own code changes. It inherits the fix, and its exact-text bind reaches every parameter the new comparison binds.
21+
- `@objectstack/driver-turso`: the remote transport's own emitter, changed the same way.
22+
23+
What does not change: a comparand without U+0000 compiles to the same `GLOB` with the same bound pattern as before. The Postgres and MySQL arms are untouched. `GLOB` still reads a stored value only up to its first U+0000, so for a comparand without U+0000, `$contains`, `$notContains`, `$icontains` and `$endsWith` over a stored value that holds one still compare only the part before it.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
fix(plugin-security): `security/explain` fails closed when a dependency it shares with enforcement throws, so a request that fails is no longer reported as allowed (#20002)
6+
7+
Clause-②: no
8+
9+
`POST /api/v1/security/explain` calls the same functions as the enforcement middleware. Enforcement does not catch a failure in them, so the request fails. The explain engine caught the same failure and turned it into a value that it then read as an answer. So when the sharing service's share store was unavailable, `{ object, operation: 'read', recordId }` answered `decision.record.visible: true` (`decidedBy: 'sharing'`, sharing layer `admitted`), and the caller's `find` for the same row threw. Four call sites had this problem:
10+
11+
- **The sharing read filter** (the reported case). A failure became `null`, which the record matcher reads as "no filter". So an unshared row, a shared row, and the caller's own row were all reported visible.
12+
- **The sharing service's per-record `update` / `delete` gate.** A failure became "no gate wired", so ownership, a `read` share or the OWD answered a write that the by-id `PATCH` / `DELETE` then failed on.
13+
- **The layered row-level security composition.** A failure became "no tenant wall and no business RLS". The row was reported visible, while `allowed` was `false` because of the same failure.
14+
- **An on-behalf-of delegator whose grants could not be read.** A failure became "no delegation", so the agent's own grants decided alone and `allowed` was `true`. The same `find` answered `503 SERVICE_UNAVAILABLE`.
15+
16+
Each failure is now reported as it happened. The affected layer's `record.outcome` is `not_evaluated`, with no `rowFilter` and no `matchesRecord`, and its `detail` says the layer could not be evaluated. `record.visible` is `false`, and `decidedBy` names the layer that failed: `sharing`, or `rls` for the composition. For the delegator case, the `principal` and `object_crud` layers deny and `allowed` is `false`. The response has no new keys, and `not_evaluated` is an existing outcome value.
17+
18+
Unchanged:
19+
20+
- Enforcement admits and refuses exactly what it did before.
21+
- A dependency that answers is reported exactly as before. That includes a read filter that answers "no restriction", such as an `org`-depth reader's `null`.
22+
- Failures that already failed closed are unchanged: record fetch, share listing, and permission-set resolution for the principal or the delegator.
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
'@objectstack/plugin-security': minor
3+
---
4+
5+
fix(plugin-security)!: no shipped permission set below platform admin reads a row of `sys_verification` or `sys_jwks` — the credential rows those objects declare private (#20027)
6+
7+
Clause-②: no (narrowing)
8+
9+
**BREAKING** — shipped as `minor` under the launch-window convention
10+
(`check-changeset-no-major` refuses `major` until GA; breaking-ness is carried by
11+
this banner and the ADR-0087 disposition below, never by the level).
12+
13+
`sys_verification` (one-time verification and password-reset tokens) and
14+
`sys_jwks` (JWT signing keys) declare `access: { default: 'private' }`, and the
15+
shipped permission sets documented them as denied to every principal below
16+
platform admin. The sets did not hold that: the read they grant on every
17+
better-auth-managed object is an explicit per-object entry, which the `private`
18+
posture does not govern, and neither object had a row policy behind it.
19+
20+
**What a principal can no longer read.** Every shipped set that grants that read
21+
and carries row-level security — `member_default`, `viewer_readonly`,
22+
`organization_admin` and its wall-less variant `organization_admin_no_bypass` —
23+
now declares a row policy that admits no row on each of the two objects
24+
(`sys_verification_none`, `sys_jwks_none`). That covers the rows the objects
25+
declare private, including a caller's own verification row. An MCP agent acting
26+
for a user is bounded by that user's sets and reads the same. `admin_full_access`
27+
is unchanged and still reads every row.
28+
29+
**Remedy.** None is expected to be needed: no shipped product surface reads these
30+
tables under a user context. A deployment that genuinely needs a principal below
31+
platform admin to inspect them grants that in a permission set of its own, with
32+
a row-level-security policy that names the rows it may see.
33+
34+
Unchanged: better-auth's own verification, password-reset and token-signing flows
35+
(they read and write through its adapter under system context, which no row
36+
policy reaches), the owner-scoped reads of the other `private` identity objects
37+
(`sys_device_code`, `sys_oauth_access_token`, `sys_oauth_refresh_token`), and
38+
every other managed object.
39+
40+
<!-- adr-0087: not-required (no-migration-prescription) Nothing authored moves: `packages/spec` is untouched, no object definition or column changes, and the shipped permission sets are code evaluated from the in-memory declaration, so `objectstack migrate meta` has nothing to rewrite and the ledger has no row to gain. The change is a narrowing of what the platform's own sets admit at runtime. The other categories are closed on facts: the package publishes (not `unpublished`); no ADR-0087 id covers it (not `registered` / `already-registered`); and it is runtime behaviour, not a TypeScript declaration (not `runtime-interface-only` / `type-surface-only`). -->
Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#19999] A `$contains` / `$notContains` / `$icontains` / `$startsWith` /
5+
* `$endsWith` comparand that holds U+0000 is compared WHOLE on SQLite — against
6+
* the whole stored value.
7+
*
8+
* Before the fix the SQLite arm of `textMatchPredicate` compiled every one of
9+
* them to `GLOB`, and SQLite's `glob()` reads BOTH of its arguments as C
10+
* strings: the pattern and the stored value are each cut at their first
11+
* U+0000. Measured at base `fc6ddb87a` on better-sqlite3 (SQLite 3.53.4),
12+
* sql.js (3.49.1) and a local libSQL engine (3.45.1), all three alike:
13+
* `$contains` and `$endsWith` with a comparand that STARTS with U+0000 matched
14+
* every row (the pattern was cut to `*`), and `$startsWith` of one matched the
15+
* rows that are empty before their first U+0000, `''` among them. Neither
16+
* raised.
17+
*
18+
* What the arm emits now for such a comparand is length-aware, and each piece
19+
* was measured NUL-safe on those three engines before it was used: `instr()`
20+
* (byte-exact `memcmp`, positions in characters), `lower()`, and `substr()` /
21+
* `length()` over a BLOB. `length()` and `substr()` over TEXT are NOT — they
22+
* stop at the first U+0000 (`length('a' || char(0) || 'b')` is 1) — which is
23+
* why the suffix test goes through `CAST(… AS BLOB)`.
24+
*
25+
* The expected rows are the JavaScript answer: every case is checked against
26+
* `@objectstack/formula`'s `matchesFilterCondition` (the JS baseline face) as
27+
* well as pinned literally, so the table cannot drift from what the other
28+
* faces answer. `driver-memory` answered the same 50-row probe identically.
29+
*
30+
* A comparand WITHOUT U+0000 keeps `GLOB` and binds exactly what it bound
31+
* before, so no existing plan moves; the second block pins that.
32+
*/
33+
34+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
35+
import type { Knex } from 'knex';
36+
import type { DriverOptions, FilterCondition } from '@objectstack/spec/data';
37+
import { matchesFilterCondition } from '@objectstack/formula';
38+
import { SqlDriver } from './sql-driver.js';
39+
40+
const NUL = String.fromCharCode(0x00);
41+
const BYPASS: DriverOptions = { bypassTenantAudit: true };
42+
const TABLE = 'nul_text_match';
43+
44+
/**
45+
* label → stored value. The first five are the rows the card's refinement
46+
* measured; the rest add a case-fold pair, the GLOB metacharacters behind a
47+
* U+0000, a non-ASCII letter for the ASCII-only fold, and a NULL for the
48+
* NULL-safe negation.
49+
*/
50+
const ROWS: Readonly<Record<string, string | null>> = {
51+
nul_mid: 'a' + NUL + 'b',
52+
nul_trail: 'ab' + NUL,
53+
nul_lead: NUL + 'z',
54+
plain: 'plain',
55+
empty: '',
56+
upper_nul: 'A' + NUL + 'B',
57+
glob_nul: 'x' + NUL + '*?[y',
58+
accent_nul: 'CAFÉ' + NUL,
59+
missing: null,
60+
};
61+
62+
type TextOp = '$contains' | '$notContains' | '$icontains' | '$startsWith' | '$endsWith';
63+
64+
interface NulCase {
65+
readonly op: TextOp;
66+
readonly comparand: string;
67+
readonly expected: readonly string[];
68+
}
69+
70+
const NUL_CASES: readonly NulCase[] = [
71+
// The refinement's table, each row with its JavaScript answer.
72+
{ op: '$contains', comparand: NUL, expected: ['accent_nul', 'glob_nul', 'nul_lead', 'nul_mid', 'nul_trail', 'upper_nul'] },
73+
{ op: '$endsWith', comparand: NUL, expected: ['accent_nul', 'nul_trail'] },
74+
{ op: '$contains', comparand: NUL + 'b', expected: ['nul_mid'] },
75+
{ op: '$contains', comparand: 'a' + NUL, expected: ['nul_mid'] },
76+
{ op: '$startsWith', comparand: NUL, expected: ['nul_lead'] },
77+
{ op: '$startsWith', comparand: 'a' + NUL, expected: ['nul_mid'] },
78+
// The other two shapes of a comparand that starts with U+0000.
79+
{ op: '$endsWith', comparand: NUL + 'b', expected: ['nul_mid'] },
80+
{ op: '$startsWith', comparand: NUL + 'z', expected: ['nul_lead'] },
81+
// A comparand exactly as long as the value, and longer than the values it
82+
// could end or start.
83+
{ op: '$endsWith', comparand: 'A' + NUL + 'B', expected: ['upper_nul'] },
84+
{ op: '$endsWith', comparand: 'zz' + NUL + 'b', expected: [] },
85+
{ op: '$startsWith', comparand: 'ab' + NUL + 'x', expected: [] },
86+
// The negation, NULL-safe: a missing value does not contain anything.
87+
{ op: '$notContains', comparand: NUL, expected: ['empty', 'missing', 'plain'] },
88+
{
89+
op: '$notContains',
90+
comparand: NUL + 'b',
91+
expected: ['accent_nul', 'empty', 'glob_nul', 'missing', 'nul_lead', 'nul_trail', 'plain', 'upper_nul'],
92+
},
93+
// The fold: ASCII on both sides, and nothing else.
94+
{ op: '$icontains', comparand: NUL + 'B', expected: ['nul_mid', 'upper_nul'] },
95+
{ op: '$icontains', comparand: 'a' + NUL + 'B', expected: ['nul_mid', 'upper_nul'] },
96+
{ op: '$icontains', comparand: 'cafÉ' + NUL, expected: ['accent_nul'] },
97+
{ op: '$icontains', comparand: 'café' + NUL, expected: [] },
98+
// GLOB's metacharacters stay literal behind a U+0000 (#5567's bypass, in
99+
// GLOB's spelling): read as wildcards, each of these matches far more.
100+
{ op: '$contains', comparand: NUL + '*', expected: ['glob_nul'] },
101+
{ op: '$contains', comparand: NUL + '*?[', expected: ['glob_nul'] },
102+
{ op: '$startsWith', comparand: 'x' + NUL + '*', expected: ['glob_nul'] },
103+
{ op: '$endsWith', comparand: NUL + '*?[y', expected: ['glob_nul'] },
104+
// No row holds U+0000 then `?`; read as GLOB's any-one-character, it would
105+
// exclude every row that holds a U+0000 followed by anything.
106+
{ op: '$notContains', comparand: NUL + '?', expected: Object.keys(ROWS).sort() },
107+
];
108+
109+
const show = (s: string) => JSON.stringify(s).replace(/\\u0000/g, 'U+0000');
110+
111+
/**
112+
* `$not` over `$endsWith`, which tells a NULL predicate from a false one where a
113+
* bare `$endsWith` cannot. `''` ends with no comparand holding U+0000, so each
114+
* negation below must return the `empty` row — and `substr()` over a
115+
* zero-length BLOB is NULL, not the empty blob, so the suffix construct has to
116+
* say false there itself.
117+
*/
118+
const NOT_ENDS_CASES: ReadonlyArray<{ comparand: string; expected: readonly string[] }> = [
119+
{ comparand: NUL, expected: ['empty', 'glob_nul', 'missing', 'nul_lead', 'nul_mid', 'plain', 'upper_nul'] },
120+
{
121+
comparand: NUL + 'b',
122+
expected: ['accent_nul', 'empty', 'glob_nul', 'missing', 'nul_lead', 'nul_trail', 'plain', 'upper_nul'],
123+
},
124+
{ comparand: 'a' + NUL, expected: Object.keys(ROWS).sort() },
125+
];
126+
127+
/** The JavaScript baseline face's answer over the same rows. */
128+
function jsAnswer(where: FilterCondition): string[] {
129+
return Object.entries(ROWS)
130+
.filter(([, v]) => matchesFilterCondition({ v }, where))
131+
.map(([label]) => label)
132+
.sort();
133+
}
134+
135+
describe('[#19999] the expected rows are the JavaScript answer', () => {
136+
for (const c of NUL_CASES) {
137+
it(`${c.op} ${show(c.comparand)}`, () => {
138+
expect(jsAnswer({ v: { [c.op]: c.comparand } } as FilterCondition)).toEqual([...c.expected]);
139+
});
140+
}
141+
for (const c of NOT_ENDS_CASES) {
142+
it(`$not $endsWith ${show(c.comparand)}`, () => {
143+
expect(jsAnswer({ $not: { v: { $endsWith: c.comparand } } } as FilterCondition)).toEqual([...c.expected]);
144+
});
145+
}
146+
});
147+
148+
describe('[#19999] SqlDriver on better-sqlite3 — a comparand holding U+0000 is compared whole', () => {
149+
let driver: SqlDriver;
150+
151+
const labelsWhere = async (where: FilterCondition): Promise<string[]> => {
152+
const rows = (await driver.find(TABLE, { where }, BYPASS)) as Array<{ label: string }>;
153+
return rows.map((r) => r.label).sort();
154+
};
155+
156+
beforeAll(async () => {
157+
driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
158+
await driver.initObjects([{ name: TABLE, fields: { label: { type: 'string' }, v: { type: 'text' } } }]);
159+
for (const [label, v] of Object.entries(ROWS)) await driver.create(TABLE, { label, v }, BYPASS);
160+
});
161+
162+
afterAll(async () => {
163+
await driver.disconnect();
164+
});
165+
166+
it('stored every value whole (the premise)', async () => {
167+
const rows = (await driver.execute(
168+
`select label, case when v is null then null else hex(v) end as h from ${TABLE}`,
169+
)) as Array<{
170+
label: string;
171+
h: string | null;
172+
}>;
173+
const stored = Object.fromEntries(rows.map((r) => [r.label, r.h]));
174+
for (const [label, v] of Object.entries(ROWS)) {
175+
expect(stored[label], label).toBe(v === null ? null : Buffer.from(v, 'utf8').toString('hex').toUpperCase());
176+
}
177+
});
178+
179+
for (const c of NUL_CASES) {
180+
it(`${c.op} ${show(c.comparand)} answers the JavaScript rows`, async () => {
181+
expect(await labelsWhere({ v: { [c.op]: c.comparand } } as FilterCondition)).toEqual([...c.expected]);
182+
});
183+
}
184+
185+
for (const c of NOT_ENDS_CASES) {
186+
it(`$not $endsWith ${show(c.comparand)} returns the '' row`, async () => {
187+
expect(await labelsWhere({ $not: { v: { $endsWith: c.comparand } } } as FilterCondition)).toEqual([
188+
...c.expected,
189+
]);
190+
});
191+
}
192+
});
193+
194+
describe('[#19999] the SQLite construct, compiled', () => {
195+
/** Exposes the compiled WHERE and its bindings without reaching into privates. */
196+
class CompilerProbeDriver extends SqlDriver {
197+
compile(where: FilterCondition): { sql: string; bindings: readonly unknown[] } {
198+
const builder: Knex.QueryBuilder = this.getKnex()(TABLE);
199+
this.applyFilters(builder, where);
200+
const { sql, bindings } = builder.toSQL();
201+
return { sql, bindings };
202+
}
203+
}
204+
205+
const probe = new CompilerProbeDriver({
206+
client: 'better-sqlite3',
207+
connection: { filename: ':memory:' },
208+
useNullAsDefault: true,
209+
});
210+
211+
afterAll(async () => {
212+
await probe.disconnect();
213+
});
214+
215+
/**
216+
* A comparand without U+0000 compiles exactly as it did before this change —
217+
* `GLOB`, one bound escaped pattern — so no existing plan moves (a
218+
* `$startsWith` GLOB over an indexed column can still use the index).
219+
*/
220+
it('a comparand without U+0000 keeps GLOB and its pattern', () => {
221+
expect(probe.compile({ v: { $contains: 'a*b' } })).toEqual({
222+
sql: 'select * from `nul_text_match` where `v` GLOB ?',
223+
bindings: ['*a[*]b*'],
224+
});
225+
expect(probe.compile({ v: { $startsWith: 'ab' } }).bindings).toEqual(['ab*']);
226+
expect(probe.compile({ v: { $endsWith: 'ab' } }).bindings).toEqual(['*ab']);
227+
expect(probe.compile({ v: { $icontains: 'Ab' } })).toEqual({
228+
sql: 'select * from `nul_text_match` where lower(`v`) GLOB lower(?)',
229+
bindings: ['*Ab*'],
230+
});
231+
expect(probe.compile({ v: { $notContains: 'ab' } }).sql).toMatch(/`v` NOT GLOB \?/);
232+
});
233+
234+
/**
235+
* A comparand holding U+0000 never reaches `GLOB`, and is bound RAW: the
236+
* length-aware construct has no pattern language, so there is nothing to
237+
* escape — a `[*]` class bound here would be searched for literally.
238+
*/
239+
for (const op of ['$contains', '$notContains', '$icontains', '$startsWith', '$endsWith'] as const) {
240+
it(`${op}: a comparand holding U+0000 compiles without GLOB and binds it raw`, () => {
241+
const comparand = 'a' + NUL + '*';
242+
const { sql, bindings } = probe.compile({ v: { [op]: comparand } } as FilterCondition);
243+
expect(sql).not.toMatch(/GLOB/);
244+
expect(bindings.length).toBeGreaterThan(0);
245+
for (const b of bindings) expect(b).toBe(comparand);
246+
});
247+
}
248+
});

0 commit comments

Comments
 (0)