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

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

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4

* test(objectql,verify): keep the two new test files inside the TEST_DEBT ratchet

`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: claude[bot]

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