fix(platform): revoke the gateway key on every session teardown

services/platform/backend/domains/sandbox/gateway-keys.test.ts

@@ -0,0 +1,134 @@
+/**
+ * The election itself is the SQL `WHERE revoked_at_ms IS NULL` flip, so it is
+ * the integration harness that proves it against a real Postgres. What these
+ * cover is the glue around it, where the consequences are just as sharp:
+ *
+ *  - a gateway failure must NOT throw. An unreachable gateway wedging a
+ *    teardown is worse than a leaked key, so the failure is counted and
+ *    logged loudly instead.
+ *  - an exec-scoped teardown must NOT clear the session row's parked key —
+ *    that key belongs to the standing session, and a sibling turn is still
+ *    spending against it.
+ *  - the same key id arriving from both the token table and the session row
+ *    must be revoked once.
+ */
+
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+const { revokeVirtualKey } = vi.hoisted(() => ({ revokeVirtualKey: vi.fn() }));
+
+vi.mock('../../core/node_only/sandbox/llm_gateway_admin.ts', () => ({
+  revokeVirtualKey,
+}));
+
+const { revokeSessionGatewayKeys } = await import('./gateway-keys.ts');
+
+/**
+ * A tagged-template stand-in for `postgres`: answers each query with the
+ * next queued result and records how many ran, so a test can assert that
+ * the session-row statement did not.
+ */
+function fakeSql(results: unknown[][]) {
+  const queries: string[] = [];
+  const sql = (strings: TemplateStringsArray) => {
+    queries.push(strings.join('?'));
+    return Promise.resolve(results[queries.length - 1] ?? []);
+  };
+  return { sql: sql as never, queries };
+}
+
+const ARGS = { organizationId: 'org-1', sessionId: 'sess-1' };
+
+describe('revokeSessionGatewayKeys', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    revokeVirtualKey.mockResolvedValue(undefined);
+  });
+
+  it('revokes a claimed token key and the key parked on the session row', async () => {
+    const { sql } = fakeSql([[{ keyId: 'k1' }], [{ keyId: 'k2' }]]);
+
+    const out = await revokeSessionGatewayKeys(sql, ARGS);
+
+    expect(out).toEqual({ revoked: 2, failed: 0 });
+    expect(
+      revokeVirtualKey.mock.calls
+        .map(([id]) => id as string)
+        .toSorted((a, b) => a.localeCompare(b)),
+    ).toEqual(['k1', 'k2']);
+  });
+
+  it('revokes a key claimed from both places only once', async () => {
+    const { sql } = fakeSql([[{ keyId: 'k1' }], [{ keyId: 'k1' }]]);
+
+    const out = await revokeSessionGatewayKeys(sql, ARGS);
+
+    expect(out).toEqual({ revoked: 1, failed: 0 });
+    expect(revokeVirtualKey).toHaveBeenCalledTimes(1);
+  });
+
+  it('leaves the session row alone for an exec-scoped teardown', async () => {
+    const { sql, queries } = fakeSql([[{ keyId: 'k1' }]]);
+
+    const out = await revokeSessionGatewayKeys(sql, {
+      ...ARGS,
+      execId: 'exec-9',
+    });
+
+    expect(out).toEqual({ revoked: 1, failed: 0 });
+    // One statement only: the token claim. The `sandbox_sessions` clear must
+    // not run — a sibling turn on the standing session still holds that key.
+    expect(queries).toHaveLength(1);
+    expect(queries[0]).toContain('sandbox_session_tokens');
+    expect(queries.join('\n')).not.toContain('UPDATE app.sandbox_sessions');
+  });
+
+  it('skips the gateway entirely when nothing was claimed', async () => {
+    const { sql } = fakeSql([[], []]);
+
+    const out = await revokeSessionGatewayKeys(sql, ARGS);
+
+    expect(out).toEqual({ revoked: 0, failed: 0 });
+    expect(revokeVirtualKey).not.toHaveBeenCalled();
+  });
+
+  it('never throws when the gateway fails, and says the key leaked', async () => {
+    const error = vi.spyOn(console, 'error').mockImplementation(() => {});
+    revokeVirtualKey.mockRejectedValueOnce(new Error('gateway down'));
+    const { sql } = fakeSql([[{ keyId: 'k1' }, { keyId: 'k2' }], []]);
+
+    const out = await revokeSessionGatewayKeys(sql, ARGS);
+
+    // Counted, not thrown: an unreachable gateway must not wedge teardown.
+    expect(out).toEqual({ revoked: 1, failed: 1 });
+    // console.error, not warn — the key stays spendable until an operator
+    // deletes it by hand, so this has to be loud.
+    expect(error).toHaveBeenCalledTimes(1);
+    expect(String(error.mock.calls[0]?.[0])).toContain('LEAKED gateway key');
+    error.mockRestore();
+  });
+
+  it('keeps revoking the remaining keys after one fails', async () => {
+    const error = vi.spyOn(console, 'error').mockImplementation(() => {});
+    revokeVirtualKey.mockRejectedValueOnce(new Error('gateway down'));
+    const { sql } = fakeSql([
+      [{ keyId: 'k1' }, { keyId: 'k2' }, { keyId: 'k3' }],
+      [],
+    ]);
+
+    const out = await revokeSessionGatewayKeys(sql, ARGS);
+
+    expect(out).toEqual({ revoked: 2, failed: 1 });
+    expect(revokeVirtualKey).toHaveBeenCalledTimes(3);
+    error.mockRestore();
+  });
+
+  it('ignores token rows that minted no gateway key', async () => {
+    const { sql } = fakeSql([[{ keyId: null }, { keyId: 'k1' }], []]);
+
+    const out = await revokeSessionGatewayKeys(sql, ARGS);
+
+    expect(out).toEqual({ revoked: 1, failed: 0 });
+    expect(revokeVirtualKey).toHaveBeenCalledExactlyOnceWith('k1');
+  });
+});

services/platform/backend/domains/sandbox/gateway-keys.ts

@@ -0,0 +1,129 @@
+import type { Sql } from 'postgres';
+
+import { revokeVirtualKey } from '../../core/node_only/sandbox/llm_gateway_admin.ts';
+
+/**
+ * Session-teardown credential reclaim — the 0.5 twin of 0.4's
+ * `node_only/sandbox/session_teardown.ts` + the revoke half of
+ * `session_admin_actions.destroySandbox`.
+ *
+ * WHY this exists as its own seam: the gateway has NO native TTL on a
+ * virtual key, and `mintVirtualKey` deliberately gives every key a
+ * `reset_duration: '1M'` budget window because teardown is supposed to
+ * delete it long before any reset matters. A key that outlives its session
+ * is therefore a permanent bearer credential with a self-refilling monthly
+ * allowance against the org's own provider keys. Every teardown edge — TTL
+ * expiry, admin destroy, phantom heal, a deadline-failed agent run — has to
+ * come through here, and there is exactly ONE copy of the behaviour so the
+ * edges cannot drift.
+ *
+ * THE ELECTION IS THE TOKEN FLIP. `UPDATE … WHERE revoked_at_ms IS NULL
+ * RETURNING llm_gateway_key_id` hands each live key to exactly one caller,
+ * so a watchdog sweeping the same session twice, or a destroy racing the
+ * expiry sweep, revokes once and the second pass is a no-op. (The gateway
+ * itself is idempotent too — `revokeVirtualKey` treats a 404 for an unknown
+ * key as success — so even a genuine double DELETE cannot fail a teardown.)
+ *
+ * FAILURE POSTURE: best-effort per key, like 0.4. A gateway that is
+ * unreachable must never wedge a teardown, so the HTTP failure is caught —
+ * but it is caught at `console.error` with the key id, because the key stays
+ * spendable and only an operator can delete it by hand.
+ */
+
+/** Delete the given virtual keys on the gateway. Never throws: one key's
+ * failure is logged (loudly — that key stays spendable) and the rest run. */
+async function revokeGatewayKeys(
+  keyIds: string[],
+  context: string,
+): Promise<{ revoked: number; failed: number }> {
+  let revoked = 0;
+  let failed = 0;
+  for (const keyId of keyIds) {
+    try {
+      await revokeVirtualKey(keyId);
+      revoked += 1;
+    } catch (error) {
+      failed += 1;
+      // NOT a warning: the key survives with a self-refilling monthly
+      // budget against this org's provider keys, and nothing else tracks
+      // it — an operator has to delete it on the gateway by hand.
+      console.error(
+        `[sandbox] LEAKED gateway key ${keyId} (${context}): revoke failed, the key stays spendable until an operator deletes it:`,
+        error,
+      );
+    }
+  }
+  return { revoked, failed };
+}
+
+/**
+ * Claim and revoke the gateway virtual keys a torn-down session still holds.
+ *
+ * Scope:
+ *  - no `execId` — the WHOLE session (TTL expiry, destroy, phantom heal):
+ *    every unrevoked session token is marked revoked and every gateway key
+ *    among them deleted, plus the key id parked on the session row itself
+ *    (`app.sandbox_sessions.llm_gateway_key_id`, cleared here so a later
+ *    sweep can tell a revoked key from a live one — nothing reads that
+ *    column downstream, unlike the token table's copy, which the run
+ *    provenance ledger matches turns by and which therefore keeps its id
+ *    and carries `revoked_at_ms` as the mark).
+ *  - with `execId` — ONE turn of a STANDING session (a deadline-failed
+ *    task-agent run): only that exec's minted key, so a sibling turn still
+ *    running on the same `pa-<agentId>` session keeps its own credential.
+ */
+export async function revokeSessionGatewayKeys(
+  sql: Sql,
+  args: { organizationId: string; sessionId: string; execId?: string },
+): Promise<{ revoked: number; failed: number }> {
+  const execId = args.execId ?? null;
+  const now = Date.now();
+  // The flip IS the claim — see the module note. An exec-scoped teardown
+  // narrows to the key that exec's op row minted; a token row with no
+  // gateway key (the subscription lane mints none) never matches it.
+  const claimed = await sql<{ keyId: string | null }[]>`
+    UPDATE app.sandbox_session_tokens SET revoked_at_ms = ${now}
+    WHERE session_id = ${args.sessionId} AND org_id = ${args.organizationId}
+      AND revoked_at_ms IS NULL
+      AND (${execId}::text IS NULL OR llm_gateway_key_id = (
+        SELECT minted_key_id FROM app.sandbox_session_ops
+        WHERE session_id = ${args.sessionId} AND exec_id = ${execId}
+        LIMIT 1
+      ))
+    RETURNING llm_gateway_key_id AS "keyId"
+  `;
+  const keyIds = new Set(
+    claimed
+      .map((row) => row.keyId)
+      .filter((keyId): keyId is string => keyId !== null),
+  );
+  if (execId === null) {
+    // `RETURNING` answers with the NEW row, so the id has to be read before
+    // the clear — one statement, so the `FOR UPDATE` re-check makes this a
+    // claim too: a racing sweep sees the already-cleared row and skips it.
+    const parked = await sql<{ keyId: string }[]>`
+      WITH live AS (
+        SELECT id, llm_gateway_key_id AS key_id
+        FROM app.sandbox_sessions
+        WHERE session_id = ${args.sessionId}
+          AND org_id = ${args.organizationId}
+          AND llm_gateway_key_id IS NOT NULL
+        FOR UPDATE
+      ), cleared AS (
+        UPDATE app.sandbox_sessions SET llm_gateway_key_id = NULL
+        WHERE id IN (SELECT id FROM live)
+        RETURNING id
+      )
+      SELECT key_id AS "keyId" FROM live
+      WHERE id IN (SELECT id FROM cleared)
+    `;
+    for (const row of parked) keyIds.add(row.keyId);
+  }
+  if (keyIds.size === 0) return { revoked: 0, failed: 0 };
+  return revokeGatewayKeys(
+    [...keyIds],
+    execId === null
+      ? `session ${args.sessionId}`
+      : `session ${args.sessionId} exec ${execId}`,
+  );
+}

services/platform/backend/domains/sandbox/sessions.ts

@@ -17,6 +17,7 @@ import { sessionIdForWorkflowExecution } from '../../core/sandbox/session_naming
 import { toJson } from '../../db/sql.ts';
 import { readGovernancePolicyForOrg } from '../../lib/org-config.ts';
 import { wakeParkedAgentRuns } from '../tasks/agent-runs.ts';
+import { revokeSessionGatewayKeys } from './gateway-keys.ts';
 
 /**
  * The sandbox session substrate over PG — the 0.5 twin of
@@ -455,11 +456,24 @@ export async function resumeSessionSlot(
   });
 }
 
-/** Terminal: mark destroyed + revoke tokens + drop the checkpoint + ticket. */
+/** Terminal: revoke the gateway keys + mark destroyed + revoke tokens + drop
+ * the checkpoint + ticket. The single bottom of EVERY destroy — the admin
+ * Destroy, the watchdog's phantom heal, `provisionSession`'s heal, the owner
+ * cascades — so credential reclaim cannot be missed on one of them. */
 export async function markSessionDestroyed(
   sql: Sql,
   args: { organizationId: string; sessionId: string },
 ): Promise<boolean> {
+  // Credentials FIRST: the gateway key outlives the row (no native TTL), and
+  // the token flip below is what elects a single revoker — running it after
+  // the flip would find nothing to revoke. Best-effort by construction, so a
+  // down gateway cannot wedge the destroy; see `gateway-keys.ts`.
+  await revokeSessionGatewayKeys(sql, args).catch((error: unknown) => {
+    console.error(
+      `[sandbox] gateway key reclaim for destroyed ${args.sessionId} failed:`,
+      error,
+    );
+  });
   return sql
     .begin(async (tx) => {
       const now = Date.now();

services/platform/backend/domains/sandbox/watchdogs.ts

@@ -5,6 +5,7 @@ import {
   sessionIsAlive,
 } from '../../core/node_only/sandbox/helpers/session_client.ts';
 import { wakeParkedAgentRuns } from '../tasks/agent-runs.ts';
+import { revokeSessionGatewayKeys } from './gateway-keys.ts';
 import { reconcileSession } from './service.ts';
 import { markSessionDestroyed, reapStaleAdmissionTickets } from './sessions.ts';
 
@@ -62,8 +63,8 @@ export interface SandboxWatchdogResult {
  * `recoverStuckAdmissionTickets`:
  *
  *  - EXPIRE: unpinned sessions past their TTL among the compute-holding
- *    statuses flip to `expired` (freeing their slots) and the parked-run
- *    wake fires for their orgs. The spawner's own reaper collects the
+ *    statuses flip to `expired` (freeing their slots), their gateway virtual
+ *    keys are revoked, and the parked-run wake fires for their orgs. The spawner's own reaper collects the
  *    container on its TTL — the row must not wait for it.
  *  - RECONCILE: a bounded batch of compute-holding rows is checked against
  *    the spawner; a container gone spawner-side settles the row as destroyed
@@ -89,12 +90,29 @@ export async function runSandboxWatchdog(
   options: SandboxWatchdogOptions = {},
 ): Promise<SandboxWatchdogResult> {
   const now = Date.now();
-  const expired = await sql<{ orgId: string }[]>`
+  const expired = await sql<{ orgId: string; sessionId: string }[]>`
     UPDATE app.sandbox_sessions SET status = 'expired'
     WHERE status IN ('creating', 'active', 'degraded')
       AND pinned = false AND expires_at_ms < ${now}
-    RETURNING org_id AS "orgId"
+    RETURNING org_id AS "orgId", session_id AS "sessionId"
   `;
+  // Reclaim the CREDENTIALS of every session this sweep just expired: the
+  // gateway key has no TTL of its own, so an expired row that keeps its key
+  // leaves a spendable credential live forever (0.4's
+  // `teardownExpiredSessions`). Deliberately NOT a container destroy —
+  // expiry is a lifetime cap, not a user Destroy, and the workspace is the
+  // user's state; the spawner's idle reaper collects the container.
+  for (const row of expired) {
+    await revokeSessionGatewayKeys(sql, {
+      organizationId: row.orgId,
+      sessionId: row.sessionId,
+    }).catch((error: unknown) => {
+      console.error(
+        `[watchdog] gateway key reclaim after expiring ${row.sessionId} failed:`,
+        error,
+      );
+    });
+  }
   for (const orgId of new Set(expired.map((row) => row.orgId))) {
     await wakeParkedAgentRuns(sql, orgId).catch((error: unknown) => {
       console.warn('[watchdog] capacity wake after expiry failed:', error);