From 0fa2313b41275ce8b67e091b450bae4c5cdc0638 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 16:07:41 +0000 Subject: [PATCH 1/2] fix(objectql): privileged driver-level reads join the ambient transaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The engine's three privileged read verbs — resolveSecret, resolveSecretField and resolveInternalField — read at DRIVER level on purpose: that is the only layer where a masked or `internal: true`-omitted value still exists, and bypassing hooks, field-level security and sharing is the declared trust each places in its in-process caller. What they also bypassed, not by design, was the connection the surrounding transaction is holding: buildDriverOptions threads the ambient handle (ADR-0034) onto every ordinary read, while these three passed the driver NO options, so their read went to a FRESH pooled connection. Invisible on a roomy pool; a deadlock on a single-connection one. SQLite's knex pool is max=1 (driver-sqlite-wasm and driver-sql/better-sqlite3 both), which encodes SQLite's single-writer model rather than a tuning choice. Where the two met: AuthManager.handleRequest runs SESSION_ERASURE_PATHS inside engine.transaction(...), the vendor's session re-read reaches resolveInternalField through plugin-auth's internal-field readback, and the read waited for a connection that could not be freed until the transaction waiting on the read finished. knex's acquire timeout fired and the route degraded the block into an authentication refusal. Measured on the default datasource before this change: a caller better-auth's own admin gate ADMITS answered 401 after 120,196ms with the target row still present, and a signed-in member got the same 401 after 120,025ms instead of 403 YOU_ARE_NOT_ALLOWED_TO_DELETE_USERS. After: 200 with the row deleted, 403, and an anonymous caller's 401 unchanged — all prompt. Reads only; the privileged write paths are untouched. The #5351 same-origin gate still decides whether the handle is this object's driver's to use, so a privileged read resolving to a different datasource keeps its own connection. The dogfood admin-route sweep's remove-user carve-out (which accepted UNAUTHENTICATED as an additional denial code) is deleted with the defect. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- ...ivileged-reads-join-ambient-transaction.md | 13 ++ ...rivileged-read-ambient-transaction.test.ts | 198 ++++++++++++++++++ packages/objectql/src/engine.ts | 64 +++++- ...min-route-nonadmin-refusal.dogfood.test.ts | 32 ++- .../erasure-transaction-authorization.test.ts | 178 ++++++++++++++++ 5 files changed, 461 insertions(+), 24 deletions(-) create mode 100644 .changeset/objectql-privileged-reads-join-ambient-transaction.md create mode 100644 packages/objectql/src/engine-privileged-read-ambient-transaction.test.ts create mode 100644 packages/verify/src/erasure-transaction-authorization.test.ts diff --git a/.changeset/objectql-privileged-reads-join-ambient-transaction.md b/.changeset/objectql-privileged-reads-join-ambient-transaction.md new file mode 100644 index 0000000000..c4f9c445cb --- /dev/null +++ b/.changeset/objectql-privileged-reads-join-ambient-transaction.md @@ -0,0 +1,13 @@ +--- +"@objectstack/objectql": patch +--- + +**Fix:** the engine's three privileged driver-level reads now JOIN an open ambient transaction instead of asking the connection pool for a second connection — which deadlocked `pool max=1` datasources and made `/admin/remove-user` refuse an entitled, signed-in caller with `401 UNAUTHENTICATED` (#10792). + +`resolveSecret`, `resolveSecretField` and `resolveInternalField` read at DRIVER level on purpose: that is the only layer where a masked or `internal: true`-omitted value still exists, and bypassing hooks, field-level security and sharing is the declared trust each of them places in its in-process caller. What they also bypassed — not by design — was the connection the surrounding transaction is holding. `buildDriverOptions` threads the ambient handle (ADR-0034) onto every ordinary read for exactly this reason; these three passed the driver **no options at all**, so their read went to a *fresh* pooled connection. + +On a roomy pool that is invisible: the pool simply hands out another connection. On a single-connection pool it is a deadlock. SQLite's knex pool is `max: 1` — `driver-sqlite-wasm` and `driver-sql`/better-sqlite3 both — and `pool max=1` is not a tuning choice there, it encodes SQLite's single-writer model. + +Measured on the erasure path, which is where the two met. `AuthManager.handleRequest` runs the `SESSION_ERASURE_PATHS` routes inside `engine.transaction(...)` so a refused erasure cannot leave the session and account deletes committed. Inside that transaction the vendor's session re-read reaches `resolveInternalField` through plugin-auth's internal-field readback; the read waited for a connection that could not be freed until the transaction waiting on the read finished, knex's acquire timeout fired (`Timeout acquiring a connection. The pool is probably full`), and the route degraded the block into an authentication refusal. On the default `objectstack dev` datasource, before this change: a caller better-auth's own admin gate **admits** was answered `401` after **120,196 ms** with the target row still present, and a signed-in plain member got the same `401` after **120,025 ms** instead of the `403 YOU_ARE_NOT_ALLOWED_TO_DELETE_USERS` an authorization refusal owes them. After: `200` with the row deleted, `403`, and an anonymous caller's `401` unchanged — all promptly. Postgres and MySQL (`max >= 10`) always conformed and are unaffected; the reach nonetheless mattered because SQLite is the default datasource for `objectstack dev`, the showcase/dogfood boot, and any self-host that has not configured Postgres or MySQL. + +Two properties are deliberately **not** widened. The join is reads-only — the privileged write paths are untouched. And the #5351 same-origin gate still decides whether the handle is this object's driver's to use, so a privileged read that resolves to a *different* datasource keeps its own connection rather than executing someone else's statement on the wrong one. diff --git a/packages/objectql/src/engine-privileged-read-ambient-transaction.test.ts b/packages/objectql/src/engine-privileged-read-ambient-transaction.test.ts new file mode 100644 index 0000000000..88f254fb5e --- /dev/null +++ b/packages/objectql/src/engine-privileged-read-ambient-transaction.test.ts @@ -0,0 +1,198 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The three PRIVILEGED driver-level read verbs — `resolveSecret`, +// `resolveSecretField`, `resolveInternalField` — must JOIN the open ambient +// transaction (ADR-0034) instead of asking the pool for a second connection. +// +// Why this is a security guard and not a performance one. Each of the three +// reads at DRIVER level on purpose, because that is the only layer where the +// masked/omitted value still exists; that bypasses hooks, field-level security +// and sharing BY DESIGN. What it must not also bypass is the connection the +// surrounding transaction is holding. Until this guard they passed the driver +// NO options at all, so the read went to a FRESH pooled connection — invisible +// on a roomy pool, a DEADLOCK on a single-connection one. +// +// Measured shape of that deadlock, on the erasure path +// (`runSubjectErasureAtomically` wraps better-auth's `/admin/remove-user` in +// `engine.transaction`, whose handler's session read reaches +// `resolveInternalField` through plugin-auth's internal-field readback): the +// privileged read waited for a connection that could not be freed until the +// transaction waiting on the read finished. knex's acquire timeout fired +// ("Timeout acquiring a connection. The pool is probably full"), and the vendor +// route degraded that into an AUTHENTICATION refusal — a signed-in, entitled +// caller answered `401` after ~120s on a route reachable without credentials. +// SQLite's knex pool is `max: 1` (`driver-sqlite-wasm` and `driver-sql`/ +// better-sqlite3 both) and SQLite is the default datasource for `objectstack +// dev`, the showcase boot and any unconfigured self-host; Postgres/MySQL run +// `max >= 10` and never exhibited it. +// +// Each arm carries its own REVERSE CONTROL — the same call outside a +// transaction must reach the driver with NO handle. Without it "the driver saw +// a transaction" could be satisfied by a driver that fabricates one, and the +// assertion would measure nothing. + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectQL } from './engine.js'; + +const HASH = 'sha256:9f2c'; + +function makeRecordingDriver(name: string) { + const rows = new Map>(); + /** One entry per driver-level `find`, with the transaction option it was handed. */ + const finds: Array<{ object: string; transaction: unknown }> = []; + const storeFor = (o: string) => { + let s = rows.get(o); + if (!s) { s = new Map(); rows.set(o, s); } + return s; + }; + const driver: any = { + name, + version: '0.0.0', + supports: {}, + async connect() {}, + async disconnect() {}, + async checkHealth() { return true; }, + async execute() { return null; }, + async find(object: string, ast: any, options: any) { + finds.push({ object, transaction: options?.transaction }); + const all = Array.from(storeFor(object).values()); + const id = ast?.where?.id; + if (typeof id === 'string') return all.filter((r) => r.id === id); + if (id && Array.isArray(id.$in)) return all.filter((r) => id.$in.includes(r.id)); + return all; + }, + async findOne(object: string) { + for (const r of storeFor(object).values()) return r; + return null; + }, + async create(object: string, data: Record) { + const row = { ...data, id: (data.id as string) ?? `r_${storeFor(object).size + 1}` }; + storeFor(object).set(row.id, row); + return row; + }, + async update(object: string, id: string, data: Record) { + const s = storeFor(object); + const row = { ...s.get(id), ...data, id }; + s.set(id, row); + return row; + }, + async delete(object: string, id: string) { return storeFor(object).delete(id); }, + async count() { return 0; }, + async bulkCreate() { return []; }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + async beginTransaction() { return { __trx: name, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, + async rollback() {}, + /** Seed straight into storage — no engine verb, so no find is recorded. */ + seed(object: string, row: Record) { storeFor(object).set(String(row.id), row); }, + }; + return { driver, finds }; +} + +describe('privileged driver-level reads join the ambient transaction (#10792)', () => { + let engine: ObjectQL; + let primary: ReturnType; + + beforeEach(async () => { + engine = new ObjectQL(); + primary = makeRecordingDriver('primary'); + engine.registerDriver(primary.driver, true); + await engine.init(); + engine.registry.registerObject({ + name: 'ptest_api_key', + fields: { + name: { type: 'text' }, + key: { type: 'text', internal: true }, + conn_secret: { type: 'secret' }, + }, + } as any); + engine.registry.registerObject({ + name: 'sys_secret', + fields: { + namespace: { type: 'text' }, key: { type: 'text' }, alg: { type: 'text' }, + version: { type: 'text' }, ciphertext: { type: 'text' }, kms_key_id: { type: 'text' }, + }, + } as any); + primary.driver.seed('ptest_api_key', { id: 'k1', name: 'k', key: HASH, conn_secret: 'secret:s1' }); + primary.driver.seed('sys_secret', { + id: 's1', namespace: 'ptest_api_key', key: 'conn_secret', + alg: 'aes-256-gcm', version: '1', ciphertext: 'ct', kms_key_id: 'local', + }); + engine.setCryptoProvider({ + async encrypt() { throw new Error('not used'); }, + async decrypt() { return 'PLAINTEXT'; }, + } as any); + }); + + it('resolveInternalField — the read the erasure path blocked on', async () => { + // REVERSE CONTROL first: outside a transaction there is no handle to thread, + // so a driver that fabricated one would fail here. + await engine.resolveInternalField('ptest_api_key', ['k1'], 'key'); + expect(primary.finds.at(-1)!.transaction, 'outside a transaction: no handle').toBeUndefined(); + + let resolved: Map | undefined; + await engine.transaction(async () => { + resolved = await engine.resolveInternalField('ptest_api_key', ['k1'], 'key'); + }); + const inside = primary.finds.at(-1)!; + expect(inside.object).toBe('ptest_api_key'); + expect(inside.transaction, 'inside a transaction: the ambient handle').toBeTruthy(); + // Still the right answer — joining the transaction is not a degrade. + expect(resolved!.get('k1')).toBe(HASH); + }); + + it('resolveSecretField', async () => { + await engine.resolveSecretField('ptest_api_key', 'k1', 'conn_secret'); + expect(primary.finds.at(-1)!.transaction, 'outside a transaction: no handle').toBeUndefined(); + + let plaintext: string | null = null; + await engine.transaction(async () => { + plaintext = await engine.resolveSecretField('ptest_api_key', 'k1', 'conn_secret'); + }); + // Two reads on this path — the record, then `sys_secret` via resolveSecret. + // BOTH must ride the transaction: either one alone starves a max=1 pool. + const [record, secretRow] = primary.finds.slice(-2); + expect(record.object).toBe('ptest_api_key'); + expect(record.transaction).toBeTruthy(); + expect(secretRow.object).toBe('sys_secret'); + expect(secretRow.transaction).toBeTruthy(); + expect(plaintext).toBe('PLAINTEXT'); + }); + + it('resolveSecret — the sys_secret dereference', async () => { + await engine.resolveSecret('secret:s1'); + expect(primary.finds.at(-1)!.transaction, 'outside a transaction: no handle').toBeUndefined(); + + await engine.transaction(async () => { + await engine.resolveSecret('secret:s1'); + }); + const inside = primary.finds.at(-1)!; + expect(inside.object).toBe('sys_secret'); + expect(inside.transaction).toBeTruthy(); + }); + + it('the same-origin gate still holds — a handle never reaches a FOREIGN driver', async () => { + // #5351: a transaction handle is a property of ONE driver's connection. + // Handing it to a different driver does not put that driver's statement + // inside the transaction, it executes it on the WRONG CONNECTION. The join + // above must not widen that hole: an object bound to another datasource + // keeps its own connection, which is the pre-existing (correct) behaviour. + const other = makeRecordingDriver('other_db'); + engine.registerDriver(other.driver); + engine.setDatasourceMapping([{ objectPattern: 'ptest_foreign', datasource: 'other_db' }]); + engine.registry.registerObject({ + name: 'ptest_foreign', + fields: { name: { type: 'text' }, key: { type: 'text', internal: true } }, + } as any); + other.driver.seed('ptest_foreign', { id: 'f1', name: 'f', key: HASH }); + + await engine.transaction(async () => { + // The ambient transaction belongs to `primary`; this read resolves to + // `other_db`, so it must arrive with NO handle. + await engine.resolveInternalField('ptest_foreign', ['f1'], 'key'); + }); + expect(other.finds.at(-1)!.object).toBe('ptest_foreign'); + expect(other.finds.at(-1)!.transaction, 'a foreign driver must not receive the handle').toBeUndefined(); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index bf1f6d0d07..f2f51776cb 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -5937,6 +5937,46 @@ export class ObjectQL implements IObjectQLEngine { stripSearchCompanion(rows); } + /** + * Driver options for a PRIVILEGED, driver-level read so it JOINS the open + * ambient transaction instead of asking the pool for a second connection. + * + * The three privileged read verbs — {@link resolveSecret}, + * {@link resolveSecretField}, {@link resolveInternalField} — deliberately + * read at DRIVER level, the only layer where the masked/omitted value still + * exists. That bypasses hooks, field-level security and sharing by design; + * what it must NOT bypass is the connection the surrounding transaction is + * holding. {@link buildDriverOptions} threads the ambient handle onto every + * ordinary read for that reason (ADR-0034); these three passed NO options at + * all, so their read went to a FRESH pooled connection. + * + * On a roomy pool that is invisible — the pool simply hands out a second + * connection. On a **single-connection pool it is a deadlock**: SQLite's knex + * pool is `max: 1` (`driver-sqlite-wasm` and `driver-sql`/better-sqlite3 + * both), so the open transaction holds the one connection and the privileged + * read waits for a connection that cannot be freed until the transaction + * that is waiting on the read commits. Measured on the erasure path + * (`runSubjectErasureAtomically` → better-auth `/admin/remove-user` → + * `reattachInternalFieldsOnRead` → `resolveInternalField`): the read blocked + * until knex's own acquire timeout fired ("Timeout acquiring a connection. + * The pool is probably full", from `Transaction_Sqlite.acquireConnection`), + * and the vendor route degraded that into an authentication refusal — a + * signed-in caller answered `401` after ~120 s, on a route reachable without + * credentials. Postgres/MySQL (`max >= 10`) never exhibited it. + * + * Reads only, and only the transaction: the same-origin gate (#5351) still + * decides whether the handle is this object's driver's to use, so a + * privileged read that resolves to a DIFFERENT datasource keeps its own + * connection rather than executing on the wrong one. Returns `undefined` + * when there is no ambient transaction, which is the pre-existing shape. + */ + private privilegedReadDriverOptions(object: string): { transaction: unknown } | undefined { + const tx = this.txStore.getStore()?.transaction; + if (tx === undefined) return undefined; + if (!this.transactionCoversDriverFor(object, tx)) return undefined; + return { transaction: tx }; + } + /** * Dereference a stored secret ref back to its plaintext. Intended for * privileged, server-side consumers (e.g. a datasource connection-pool @@ -5953,7 +5993,11 @@ export class ObjectQL implements IObjectQLEngine { throw new Error('Cannot resolve secret: no CryptoProvider is registered (fail-closed).'); } const secretDriver = this.getDriver('sys_secret'); - const found = await secretDriver.find('sys_secret', { where: { id } }); + const found = await secretDriver.find( + 'sys_secret', + { where: { id } }, + this.privilegedReadDriverOptions('sys_secret'), + ); const secret: any = Array.isArray(found) ? found[0] : found; if (!secret) { throw new Error(`Cannot resolve secret: sys_secret row "${id}" not found (fail-closed).`); @@ -6018,7 +6062,11 @@ export class ObjectQL implements IObjectQLEngine { ); } const driver = this.getDriver(object); - const found = await driver.find(object, { where: { id: recordId } }); + const found = await driver.find( + object, + { where: { id: recordId } }, + this.privilegedReadDriverOptions(object), + ); const row: any = Array.isArray(found) ? found[0] : found; if (!row) return null; return this.resolveSecret(row[field], opts); @@ -6100,10 +6148,14 @@ export class ObjectQL implements IObjectQLEngine { const out = new Map(); if (recordIds.length === 0) return out; const driver = this.getDriver(object); - const found = await driver.find(object, { - where: { id: { $in: [...recordIds] } }, - fields: ['id', field], - }); + const found = await driver.find( + object, + { + where: { id: { $in: [...recordIds] } }, + fields: ['id', field], + }, + this.privilegedReadDriverOptions(object), + ); for (const row of Array.isArray(found) ? found : [found]) { if (!row || typeof row !== 'object') continue; const id = (row as Record).id; diff --git a/packages/qa/dogfood/test/admin-route-nonadmin-refusal.dogfood.test.ts b/packages/qa/dogfood/test/admin-route-nonadmin-refusal.dogfood.test.ts index 90539d6f52..fde50ec010 100644 --- a/packages/qa/dogfood/test/admin-route-nonadmin-refusal.dogfood.test.ts +++ b/packages/qa/dogfood/test/admin-route-nonadmin-refusal.dogfood.test.ts @@ -585,26 +585,22 @@ describe('#9482 C9: every derived /admin/ route refuses a non-admin', () => { // vocabulary — not a validation error, which would mean the request died // before the gate and this assertion measured nothing. // - // ⚠️ #10792, found the moment #10349 made this branch executable at all. - // It was guarded by `member.code !== undefined`, and the code WAS - // undefined on every bodyless refusal — so for those routes this check - // had never once run. On the first run where it did, `remove-user` came - // back `401 UNAUTHENTICATED` for a SIGNED-IN member while its siblings + // ⚠️ #10792 CLOSED — `remove-user` used to be carved out here, accepting + // `UNAUTHENTICATED` as an additional code. It was the one erasure-wrapped + // route in this bucket, and inside that transaction the privileged read + // behind the vendor's session re-read asked a `pool max=1` datasource for + // a SECOND connection, blocked until knex's acquire timeout fired, and + // degraded into `401` for a SIGNED-IN member while its unwrapped siblings // `set-role` and `update-user` answered the same bearer - // `403 YOU_ARE_NOT_ALLOWED_*`: on that path alone the session is re-read - // inside the #7724 erasure transaction and comes back empty, so - // authentication answers a question authorization should have. + // `403 YOU_ARE_NOT_ALLOWED_*`. The privileged read now joins the ambient + // transaction, so this route answers the authorization question like + // every other member of the bucket and needs no exception. // - // Recorded as an ADDITIONAL accepted code for that one route, never as a - // pin — same reasoning as the platform-admin arm below. Pinning today's - // 401 would turn the fix red; pinning the 403 is red today; and widening - // the vocabulary for EVERY route would let the next route drift into the - // same state in silence. Delete this arm when #10792 closes. - const KNOWN_AUTHN_BEFORE_AUTHZ = 'POST /api/v1/auth/admin/remove-user'; // #10792 - const denialCodes = - route === KNOWN_AUTHN_BEFORE_AUTHZ - ? /^(YOU_ARE_NOT_ALLOWED|UNAUTHENTICATED$)/ - : /^YOU_ARE_NOT_ALLOWED/; + // ⛔ Do not re-widen the vocabulary — for this route or for all of them. + // A route that answers `UNAUTHENTICATED` to a signed-in caller is + // announcing that authentication ran where authorization should have, and + // that is precisely the state this arm exists to catch. + const denialCodes = /^YOU_ARE_NOT_ALLOWED/; if (member.code !== undefined) { expect( member.code, diff --git a/packages/verify/src/erasure-transaction-authorization.test.ts b/packages/verify/src/erasure-transaction-authorization.test.ts new file mode 100644 index 0000000000..de9bdb2241 --- /dev/null +++ b/packages/verify/src/erasure-transaction-authorization.test.ts @@ -0,0 +1,178 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #10792 — `/admin/remove-user` must answer the AUTHORIZATION question on a +// single-connection dialect, and answer it fast. +// +// `AuthManager.handleRequest` wraps the `SESSION_ERASURE_PATHS` members in +// `runSubjectErasureAtomically`, so the whole better-auth handler runs inside +// `engine.transaction(...)` — one unit of work, so a refused erasure cannot +// leave the session/account deletes committed. Inside that transaction the +// vendor's `adminMiddleware` re-reads the session, and plugin-auth's +// internal-field readback dereferences the session token through the engine's +// privileged `resolveInternalField` accessor. That accessor used to reach the +// driver with NO options, i.e. on a FRESH pooled connection. +// +// This harness boots the DEFAULT datasource, which is exactly the dialect that +// makes that fatal: sqlite-wasm's knex pool is `max: 1`. The erasure +// transaction holds the one connection; the privileged read waited for a +// connection that could not be freed until the transaction waiting on the read +// finished; knex's acquire timeout fired; and the vendor route degraded the +// failure into an AUTHENTICATION refusal. Measured before the fix, on this +// stack: `401` after ~120s for a caller the vendor's own gate ADMITS, with the +// target row still present — a dead capability AND, because the route answers +// an anonymous caller the same way, an unauthenticated-reachable way to pin a +// connection for two minutes per call. Postgres/MySQL (`max >= 10`) always +// conformed; they are the shape this file pins for SQLite too. +// +// Both halves are asserted deliberately. A fix that returns FAST but still +// answers `401` to the admitted admin would have repaired the exhaustion shape +// and left the capability dead — so `status` and `elapsed` are separate +// assertions, and the row's absence is a third. +// +// The admitted caller is a FIXTURE: better-auth's admin plugin authorizes on +// the legacy `user.role === 'admin'` scalar, which ADR-0068 D2 deliberately +// stopped synthesizing (the platform contributes `platform_admin` to +// `positions[]` instead). No caller on a stock boot passes the vendor gate, so +// the fixture places the scalar — otherwise the admitted-admin row of the +// matrix cannot be observed at all, and this file would only ever measure +// refusals. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from './harness'; + +const SYS = { isSystem: true }; + +// The route must answer well inside this. Before the fix it took ~120s (knex's +// pool-acquire timeout); the conforming dialects answered in 155ms. The bound +// is loose on purpose — this asserts "did not wait on a connection that will +// never come", not a latency budget, so ordinary CI noise cannot redden it. +const NOT_BLOCKED_MS = 30_000; + +const app = { + manifest: { + id: 'com.example.erasure-authz', + namespace: 'erasureauthz', + version: '0.0.1', + type: 'app', + name: 'Erasure Authorization Fixture', + }, + objects: [], +}; + +interface Answer { status: number; code?: string; ms: number; body: string } + +describe('#10792 — the erasure route answers authorization on a pool max=1 dialect', () => { + let stack: VerifyStack; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let ql: any; + let priorScim: string | undefined; + let admittedAdminToken = ''; + let memberToken = ''; + const targets: string[] = []; + + const fire = async ( + method: string, + path: string, + body: unknown, + token?: string, + ): Promise => { + const t0 = Date.now(); + const res = token + ? await stack.apiAs(token, method, path, body) + : await stack.api(path, { + method, + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }); + const ms = Date.now() - t0; + const text = await res.text(); + let code: string | undefined; + try { + const parsed = JSON.parse(text) as { code?: string; error?: { code?: string } }; + code = parsed?.error?.code ?? parsed?.code; + } catch { /* a bodyless or non-JSON refusal */ } + return { status: res.status, code, ms, body: text.slice(0, 160) }; + }; + + beforeAll(async () => { + priorScim = process.env.OS_SCIM_ENABLED; + // The better-auth admin plugin — which serves /admin/remove-user — is + // mounted behind this flag, the same boot the dogfood admin-route sweep uses. + process.env.OS_SCIM_ENABLED = 'true'; + stack = await bootStack(app); + ql = await stack.kernel.getServiceAsync('objectql'); + + await stack.signIn(); // seed the dev admin first, so the sign-ups below are plain members + memberToken = await stack.signUp('e10792.member@example.com', 'Member-Pass-123'); + + await stack.signUp('e10792.admitted@example.com', 'Admitted-Pass-123'); + const [admitted] = await ql.find( + 'sys_user', { where: { email: 'e10792.admitted@example.com' }, limit: 1 }, { context: SYS }, + ); + await ql.update('sys_user', { id: String(admitted.id), role: 'admin' }, { context: SYS }); + admittedAdminToken = await stack.signIn('e10792.admitted@example.com', 'Admitted-Pass-123'); + + for (let i = 0; i < 3; i++) { + const email = `e10792.target${i}@example.com`; + await stack.signUp(email, 'Target-Pass-123'); + const [t] = await ql.find('sys_user', { where: { email }, limit: 1 }, { context: SYS }); + targets.push(String(t.id)); + } + }, 300_000); + + afterAll(async () => { + await stack?.stop?.(); + if (priorScim === undefined) delete process.env.OS_SCIM_ENABLED; + else process.env.OS_SCIM_ENABLED = priorScim; + }); + + it('the fixture caller really is one the vendor gate admits — control', async () => { + // Without this control the matrix below is unreadable: a caller the vendor + // refuses everywhere would produce the same refusals for a reason that has + // nothing to do with the transaction. These neighbouring /admin/ routes are + // NOT erasure paths, so they run unwrapped and were never affected. + const listed = await fire('GET', '/auth/admin/list-users?limit=1', undefined, admittedAdminToken); + expect(listed.status, `admitted admin list-users: ${listed.body}`).toBe(200); + const updated = await fire( + 'POST', '/auth/admin/update-user', + { userId: targets[0], data: { name: 'Renamed' } }, admittedAdminToken, + ); + expect(updated.status, `admitted admin update-user: ${updated.body}`).toBe(200); + }, 120_000); + + it('an admitted admin gets 200 and the row is DELETED — not an authentication refusal', async () => { + const answer = await fire('POST', '/auth/admin/remove-user', { userId: targets[1] }, admittedAdminToken); + + // The dead-capability half. `401` here is the defect's own signature: the + // vendor route degrading a blocked read into "Sign in first" for a caller + // it had already admitted on every neighbouring route above. + expect(answer.status, `admitted admin remove-user: ${answer.status} ${answer.body}`).toBe(200); + + // The row must actually be gone — a 200 over an erasure that rolled back + // would read as a pass and leave the capability just as dead. + const survivors = await ql.find('sys_user', { where: { id: targets[1] }, limit: 1 }, { context: SYS }); + expect(survivors.length, 'the target row must be deleted').toBe(0); + + // The resource-exhaustion half, asserted separately on purpose. + expect(answer.ms, `remove-user took ${answer.ms}ms — a blocked pool acquire`).toBeLessThan(NOT_BLOCKED_MS); + }, 300_000); + + it('a signed-in plain member gets the AUTHORIZATION refusal, not 401', async () => { + const answer = await fire('POST', '/auth/admin/remove-user', { userId: targets[2] }, memberToken); + expect(answer.status, `member remove-user: ${answer.status} ${answer.body}`).toBe(403); + // The vendor's own denial vocabulary. Asserting only the status would let a + // 403 from some unrelated gate stand in for the authorization answer. + expect(answer.code).toBe('YOU_ARE_NOT_ALLOWED_TO_DELETE_USERS'); + expect(answer.ms).toBeLessThan(NOT_BLOCKED_MS); + // …and the member's target survives: a refusal must erase nothing. + const survivors = await ql.find('sys_user', { where: { id: targets[2] }, limit: 1 }, { context: SYS }); + expect(survivors.length, 'a refused erasure must not delete').toBe(1); + }, 300_000); + + it('an anonymous caller still gets the authentication refusal — unchanged', async () => { + const answer = await fire('POST', '/auth/admin/remove-user', { userId: targets[2] }, undefined); + expect(answer.status, `anon remove-user: ${answer.status} ${answer.body}`).toBe(401); + expect(answer.code).toBe('UNAUTHENTICATED'); + expect(answer.ms).toBeLessThan(NOT_BLOCKED_MS); + }, 300_000); +}); From c0aaeb3c57f36665c85b6c9a8c2f80431c8bb9cf Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 17:56:53 +0000 Subject: [PATCH 2/2] test(objectql,verify): keep the two new test files inside the TEST_DEBT ratchet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:type-check-debt --re-measure` counts raw `tsc --noEmit` errors per package with the test exclusion lifted, so a package's own green `typecheck` says nothing about the files it hides. Both new files landed inside that hidden layer and drifted the shrink-only ledger: @objectstack/objectql 354 -> 364 (+10) and @objectstack/verify 8 -> 9 (+1). Every one of the 11 was in the new files, so the drift is attributable, not inherited. Fixed rather than re-baselined — raising a shrink-only entry is maintainer-only and hands back what an earlier PR paid to press it down: - registerObject(schema, packageId, ...) requires 2 arguments; the three calls passed 1. Supplied '__test__', the packageId the sibling engine transaction tests already use. - Array.prototype.at sits above the `lib` this program targets. Replaced the seven `.at(-1)!` sites with a local indexed `last()` helper rather than widening the compiler configuration for a test convenience. - './harness' -> './harness.js'; NodeNext needs the explicit extension, and two sibling verify tests already spell it that way. Re-measured after the repair: objectql 354 = ledger 354, verify 8 = ledger 8, zero errors in either new file. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- ...rivileged-read-ambient-transaction.test.ts | 27 ++++++++++++------- .../erasure-transaction-authorization.test.ts | 2 +- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/objectql/src/engine-privileged-read-ambient-transaction.test.ts b/packages/objectql/src/engine-privileged-read-ambient-transaction.test.ts index 88f254fb5e..687c123182 100644 --- a/packages/objectql/src/engine-privileged-read-ambient-transaction.test.ts +++ b/packages/objectql/src/engine-privileged-read-ambient-transaction.test.ts @@ -34,6 +34,13 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { ObjectQL } from './engine.js'; +/** + * The last recorded find. `Array.prototype.at` sits above the `lib` this + * package's tsc program targets, so index rather than widen the compiler + * configuration for a test convenience. + */ +const last = (rows: T[]): T => rows[rows.length - 1]; + const HASH = 'sha256:9f2c'; function makeRecordingDriver(name: string) { @@ -106,14 +113,14 @@ describe('privileged driver-level reads join the ambient transaction (#10792)', key: { type: 'text', internal: true }, conn_secret: { type: 'secret' }, }, - } as any); + } as any, '__test__'); engine.registry.registerObject({ name: 'sys_secret', fields: { namespace: { type: 'text' }, key: { type: 'text' }, alg: { type: 'text' }, version: { type: 'text' }, ciphertext: { type: 'text' }, kms_key_id: { type: 'text' }, }, - } as any); + } as any, '__test__'); primary.driver.seed('ptest_api_key', { id: 'k1', name: 'k', key: HASH, conn_secret: 'secret:s1' }); primary.driver.seed('sys_secret', { id: 's1', namespace: 'ptest_api_key', key: 'conn_secret', @@ -129,13 +136,13 @@ describe('privileged driver-level reads join the ambient transaction (#10792)', // REVERSE CONTROL first: outside a transaction there is no handle to thread, // so a driver that fabricated one would fail here. await engine.resolveInternalField('ptest_api_key', ['k1'], 'key'); - expect(primary.finds.at(-1)!.transaction, 'outside a transaction: no handle').toBeUndefined(); + expect(last(primary.finds).transaction, 'outside a transaction: no handle').toBeUndefined(); let resolved: Map | undefined; await engine.transaction(async () => { resolved = await engine.resolveInternalField('ptest_api_key', ['k1'], 'key'); }); - const inside = primary.finds.at(-1)!; + const inside = last(primary.finds); expect(inside.object).toBe('ptest_api_key'); expect(inside.transaction, 'inside a transaction: the ambient handle').toBeTruthy(); // Still the right answer — joining the transaction is not a degrade. @@ -144,7 +151,7 @@ describe('privileged driver-level reads join the ambient transaction (#10792)', it('resolveSecretField', async () => { await engine.resolveSecretField('ptest_api_key', 'k1', 'conn_secret'); - expect(primary.finds.at(-1)!.transaction, 'outside a transaction: no handle').toBeUndefined(); + expect(last(primary.finds).transaction, 'outside a transaction: no handle').toBeUndefined(); let plaintext: string | null = null; await engine.transaction(async () => { @@ -162,12 +169,12 @@ describe('privileged driver-level reads join the ambient transaction (#10792)', it('resolveSecret — the sys_secret dereference', async () => { await engine.resolveSecret('secret:s1'); - expect(primary.finds.at(-1)!.transaction, 'outside a transaction: no handle').toBeUndefined(); + expect(last(primary.finds).transaction, 'outside a transaction: no handle').toBeUndefined(); await engine.transaction(async () => { await engine.resolveSecret('secret:s1'); }); - const inside = primary.finds.at(-1)!; + const inside = last(primary.finds); expect(inside.object).toBe('sys_secret'); expect(inside.transaction).toBeTruthy(); }); @@ -184,7 +191,7 @@ describe('privileged driver-level reads join the ambient transaction (#10792)', engine.registry.registerObject({ name: 'ptest_foreign', fields: { name: { type: 'text' }, key: { type: 'text', internal: true } }, - } as any); + } as any, '__test__'); other.driver.seed('ptest_foreign', { id: 'f1', name: 'f', key: HASH }); await engine.transaction(async () => { @@ -192,7 +199,7 @@ describe('privileged driver-level reads join the ambient transaction (#10792)', // `other_db`, so it must arrive with NO handle. await engine.resolveInternalField('ptest_foreign', ['f1'], 'key'); }); - expect(other.finds.at(-1)!.object).toBe('ptest_foreign'); - expect(other.finds.at(-1)!.transaction, 'a foreign driver must not receive the handle').toBeUndefined(); + expect(last(other.finds).object).toBe('ptest_foreign'); + expect(last(other.finds).transaction, 'a foreign driver must not receive the handle').toBeUndefined(); }); }); diff --git a/packages/verify/src/erasure-transaction-authorization.test.ts b/packages/verify/src/erasure-transaction-authorization.test.ts index de9bdb2241..af52f4e924 100644 --- a/packages/verify/src/erasure-transaction-authorization.test.ts +++ b/packages/verify/src/erasure-transaction-authorization.test.ts @@ -38,7 +38,7 @@ // refusals. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { bootStack, type VerifyStack } from './harness'; +import { bootStack, type VerifyStack } from './harness.js'; const SYS = { isSystem: true };