fix(objectql): privileged driver-level reads join the ambient transaction

.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.

packages/objectql/src/engine-privileged-read-ambient-transaction.test.ts

@@ -0,0 +1,205 @@
+// 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';
+
+/**
+ * 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 = <T>(rows: T[]): T => rows[rows.length - 1];
+
+const HASH = 'sha256:9f2c';
+
+function makeRecordingDriver(name: string) {
+ const rows = new Map<string, Map<string, any>>();
+ /** 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<string, unknown>) {
+ 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<string, unknown>) {
+ 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<string, unknown>) { 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<typeof makeRecordingDriver>;
+
+ 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, '__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, '__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',
+ 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(last(primary.finds).transaction, 'outside a transaction: no handle').toBeUndefined();
+
+ let resolved: Map<string, unknown> | undefined;
+ await engine.transaction(async () => {
+ resolved = await engine.resolveInternalField('ptest_api_key', ['k1'], 'key');
+ });
+ 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.
+ expect(resolved!.get('k1')).toBe(HASH);
+ });
+
+ it('resolveSecretField', async () => {
+ await engine.resolveSecretField('ptest_api_key', 'k1', 'conn_secret');
+ expect(last(primary.finds).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(last(primary.finds).transaction, 'outside a transaction: no handle').toBeUndefined();
+
+ await engine.transaction(async () => {
+ await engine.resolveSecret('secret:s1');
+ });
+ const inside = last(primary.finds);
+ 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, '__test__');
+ 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(last(other.finds).object).toBe('ptest_foreign');
+ expect(last(other.finds).transaction, 'a foreign driver must not receive the handle').toBeUndefined();
+ });
+});

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<string, unknown>();
  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<string, unknown>).id;

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,