From e6bd55f2dc76f9a787ae724e1d62c4053f3f5ed2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 06:58:10 +0000 Subject: [PATCH 1/2] fix(codeapi): require bridge credentials only when configured Source: ClickHouse/ai@da81d707c6538b509be70e2401b649693a3f3dce --- README.md | 6 ++++-- service/src/bridge/enabled.ts | 9 +++++++++ service/src/bridge/index.ts | 2 ++ service/src/bridge/router.test.ts | 17 +++++++++++++++++ service/src/bridge/router.ts | 2 ++ service/src/secure-startup.test.ts | 18 ++++++++++++++++++ service/src/secure-startup.ts | 4 +++- 7 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 service/src/bridge/enabled.ts diff --git a/README.md b/README.md index 7bff76fb..bad5b66e 100644 --- a/README.md +++ b/README.md @@ -126,8 +126,10 @@ cut. Copy `.env.example` to `.env` and set `CODEAPI_BRIDGE_TOKEN` to a private value of at least 32 bytes (generate one with `openssl rand -hex 32`). The API exposes -bridge routes even with the default HTTP sandbox backend, so hardened mode -requires this enrollment credential. Compose defaults to +bridge routes when configured through the remote-bridge backend, paired auth, +dynamic workers, or a bridge token. Hardened deployments with none of these +configured leave bridge routes disabled and do not require a bridge token. +Enabled bridges still require this enrollment credential. Compose defaults to `CODEAPI_BRIDGE_AUTH_MODE=paired` and `CODEAPI_BRIDGE_DYNAMIC_WORKERS=true`. To restrict pairing to a fixed worker, set `CODEAPI_BRIDGE_DYNAMIC_WORKERS=false` and `CODEAPI_BRIDGE_WORKER_ID` to its ID. Keep the token outside workspaces and diff --git a/service/src/bridge/enabled.ts b/service/src/bridge/enabled.ts new file mode 100644 index 00000000..540a844a --- /dev/null +++ b/service/src/bridge/enabled.ts @@ -0,0 +1,9 @@ +import { env } from '../config'; + +/** API-only deployments can serve bridges without selecting that worker backend. */ +export function isBridgeEnabled(): boolean { + return env.SANDBOX_BACKEND === 'remote-bridge' + || env.BRIDGE_AUTH_MODE === 'paired' + || env.BRIDGE_DYNAMIC_WORKERS + || env.BRIDGE_TOKEN.length > 0; +} diff --git a/service/src/bridge/index.ts b/service/src/bridge/index.ts index fc409ad2..a1ec5d78 100644 --- a/service/src/bridge/index.ts +++ b/service/src/bridge/index.ts @@ -3,6 +3,7 @@ import { env } from '../config'; import { RedisBridgePairingStore } from './pairing'; import { createBridgeRouter } from './router'; import { RedisBridgeStore } from './store'; +import { isBridgeEnabled } from './enabled'; export const bridgeStore = new RedisBridgeStore( connection, @@ -13,6 +14,7 @@ export const bridgeStore = new RedisBridgeStore( export const bridgePairings = new RedisBridgePairingStore(connection); export default createBridgeRouter({ + enabled: isBridgeEnabled(), store: bridgeStore, pairings: bridgePairings, authMode: env.BRIDGE_AUTH_MODE, diff --git a/service/src/bridge/router.test.ts b/service/src/bridge/router.test.ts index 764613a5..af0e765c 100644 --- a/service/src/bridge/router.test.ts +++ b/service/src/bridge/router.test.ts @@ -25,6 +25,23 @@ afterEach(async () => { }); describe('paired bridge HTTP API', () => { + test('disabled bridges expose no HTTP routes', async () => { + const app = express(); + app.use('/v1/bridge', createBridgeRouter({ + enabled: false, + store: new RedisBridgeStore(redis), + pairings: new RedisBridgePairingStore(redis), + authMode: 'static', + adminToken: '', + })); + server = createServer(app); + await new Promise((resolve) => server?.listen(0, '127.0.0.1', resolve)); + const address = server.address(); + if (address == null || typeof address === 'string') throw new Error('Expected TCP listener'); + const response = await fetch(`http://127.0.0.1:${address.port}/v1/bridge/workers/test/status`); + expect(response.status).toBe(404); + }); + test('reports authenticated worker readiness without exposing identity or binding data', async () => { const store = new RedisBridgeStore(redis); const app = express(); diff --git a/service/src/bridge/router.ts b/service/src/bridge/router.ts index 72e51b4d..25b0bdaf 100644 --- a/service/src/bridge/router.ts +++ b/service/src/bridge/router.ts @@ -29,6 +29,7 @@ const PRINCIPAL_TYPES = new Set([ export type BridgeAuthMode = 'static' | 'paired'; export interface BridgeRouterOptions { + enabled?: boolean; store: RedisBridgeStore; pairings: RedisBridgePairingStore; authMode: BridgeAuthMode; @@ -130,6 +131,7 @@ function isSettlement(value: unknown): value is CodeBridgeSettlement { export function createBridgeRouter(options: BridgeRouterOptions): Router { const router = Router(); + if (options.enabled === false) return router; const configuredWorker = (workerId: string): boolean => options.allowDynamicWorkers === true || diff --git a/service/src/secure-startup.test.ts b/service/src/secure-startup.test.ts index 6820010a..79d602df 100644 --- a/service/src/secure-startup.test.ts +++ b/service/src/secure-startup.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, test } from 'bun:test'; import { env } from './config'; +import { isBridgeEnabled } from './bridge/enabled'; import { validateApiBridgePolicy, validateApiHardenedConfig, @@ -394,6 +395,23 @@ describe('sandbox backend policy', () => { expect(() => validateSandboxBackendPolicy()).not.toThrow(); }); + test('hardened HTTP and Lambda APIs start without an unused bridge credential', () => { + env.HARDENED_SANDBOX_MODE = true; + env.BRIDGE_AUTH_MODE = 'static'; + env.BRIDGE_DYNAMIC_WORKERS = false; + env.BRIDGE_TOKEN = ''; + env.BRIDGE_WORKER_ID = ''; + for (const backend of ['http', 'lambda-microvm'] as const) { + env.SANDBOX_BACKEND = backend; + expect(isBridgeEnabled()).toBe(false); + expect(() => validateApiBridgePolicy()).not.toThrow(); + } + env.BRIDGE_AUTH_MODE = 'paired'; + env.BRIDGE_DYNAMIC_WORKERS = true; + expect(isBridgeEnabled()).toBe(true); + expect(() => validateApiBridgePolicy()).toThrow('CODEAPI_BRIDGE_TOKEN'); + }); + test('API-only hardened bridge validation rejects static worker auth', () => { env.SANDBOX_BACKEND = 'http'; env.HARDENED_SANDBOX_MODE = true; diff --git a/service/src/secure-startup.ts b/service/src/secure-startup.ts index 79f69c42..26d378e3 100644 --- a/service/src/secure-startup.ts +++ b/service/src/secure-startup.ts @@ -5,6 +5,7 @@ import { } from './config'; import { INTERNAL_SERVICE_TOKEN_ENV } from './internal-service-auth'; import { isValidBridgeWorkerId } from '../../packages/code/src/protocol'; +import { isBridgeEnabled } from './bridge/enabled'; export class SecureStartupConfigError extends Error { constructor(message: string) { @@ -56,6 +57,7 @@ export function validateApiHardenedConfig(): void { /** Validate bridge credentials in every process that exposes bridge routes. */ export function validateApiBridgePolicy(): void { + if (!isBridgeEnabled()) return; if (env.BRIDGE_TOKEN !== env.BRIDGE_TOKEN.trim()) { throw new SecureStartupConfigError( 'CODEAPI_BRIDGE_TOKEN must not contain surrounding whitespace', @@ -87,7 +89,7 @@ export function validateApiBridgePolicy(): void { requireStrongSecret('CODEAPI_BRIDGE_TOKEN', env.BRIDGE_TOKEN); if (env.BRIDGE_AUTH_MODE !== 'paired') { throw new SecureStartupConfigError( - 'Hardened API deployments require CODEAPI_BRIDGE_AUTH_MODE=paired because bridge routes are always exposed', + 'Hardened API deployments with bridge routes enabled require CODEAPI_BRIDGE_AUTH_MODE=paired', ); } } From 2e3ec47fcb3f99c301ba4788a7846337737a8bdb Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 14:30:29 +0000 Subject: [PATCH 2/2] =?UTF-8?q?chore(codeapi):=20import=20=F0=9F=AB=97=20f?= =?UTF-8?q?ix:=20Drain=20Cancelled=20BYOM=20Settlements?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Source: ClickHouse/ai@2391d77aab6ff81689cf8a834d74992399f41173 Co-authored-by: danny-avila <110412045+danny-avila@users.noreply.github.com> --- packages/code/src/native-process-child.ts | 4 + packages/code/src/native-process.test.ts | 36 +++++++- packages/code/src/native-process.ts | 5 + packages/code/src/native-sandbox.test.ts | 7 +- packages/code/src/native-sandbox.ts | 4 + packages/code/src/worker.ts | 2 +- packages/code/src/workspace-worker.test.ts | 70 ++++++++++++++ packages/code/src/workspace.ts | 2 + service/src/bridge/store.ts | 50 ++++++++++ service/src/bridge/workspace-store.test.ts | 102 +++++++++++++++++++++ 10 files changed, 278 insertions(+), 4 deletions(-) diff --git a/packages/code/src/native-process-child.ts b/packages/code/src/native-process-child.ts index 164147e3..b5221731 100644 --- a/packages/code/src/native-process-child.ts +++ b/packages/code/src/native-process-child.ts @@ -101,6 +101,10 @@ process.on('message', async (raw: unknown) => { error instanceof WorkspaceToolError ? error.mutationMayHaveCommitted : true, + requiresQuarantine: + error instanceof WorkspaceToolError + ? error.requiresQuarantine + : true, }); } finally { active = undefined; diff --git a/packages/code/src/native-process.test.ts b/packages/code/src/native-process.test.ts index ffb068e4..d77ec559 100644 --- a/packages/code/src/native-process.test.ts +++ b/packages/code/src/native-process.test.ts @@ -179,13 +179,47 @@ test('executor cancellation targets the active request and preserves mutation ce ok: false, code: 'EXECUTION_ABORTED', mutation: true, + requiresQuarantine: false, }); await assert.rejects( execution, (error: unknown) => error instanceof WorkspaceToolError && error.code === 'EXECUTION_ABORTED' && - error.mutationMayHaveCommitted, + error.mutationMayHaveCommitted && + !error.requiresQuarantine, + ); + await sandbox.close(); +}); + +test('executor ignores a cleanup exemption on non-cancellation failures', async () => { + let dispatched!: () => void; + const dispatch = new Promise((resolve) => { + dispatched = resolve; + }); + const fake = fixture(() => dispatched()); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { workspaceRoot: '/workspace' }, + fake.fork, + ); + const execution = sandbox.execute(request); + await dispatch; + const command = fake.messages.find((message) => message.type === 'execute')!; + fake.child.emit('message', { + id: command.id, + ok: false, + code: 'COMMAND_UNAVAILABLE', + mutation: true, + requiresQuarantine: false, + }); + + await assert.rejects( + execution, + (error: unknown) => + error instanceof WorkspaceToolError && + error.code === 'COMMAND_UNAVAILABLE' && + error.mutationMayHaveCommitted && + error.requiresQuarantine, ); await sandbox.close(); }); diff --git a/packages/code/src/native-process.ts b/packages/code/src/native-process.ts index fc44e3d6..fe4e1aa8 100644 --- a/packages/code/src/native-process.ts +++ b/packages/code/src/native-process.ts @@ -134,6 +134,7 @@ export class NativeProcessWorkspaceCommandSandbox ok?: unknown; result?: unknown; mutation?: unknown; + requiresQuarantine?: unknown; code?: unknown; errorMessage?: unknown; fatal?: unknown; @@ -156,6 +157,9 @@ export class NativeProcessWorkspaceCommandSandbox message.code === 'REGISTRATION_INVALID' ? message.code : 'COMMAND_UNAVAILABLE'; + const processTerminationConfirmed = + code === 'EXECUTION_ABORTED' && + message.requiresQuarantine === false; pending.reject( new WorkspaceToolError( typeof message.errorMessage === 'string' && @@ -164,6 +168,7 @@ export class NativeProcessWorkspaceCommandSandbox : 'Native executor request failed', code, pending.mutation && message.mutation !== false, + pending.mutation && !processTerminationConfirmed, ), ); } diff --git a/packages/code/src/native-sandbox.test.ts b/packages/code/src/native-sandbox.test.ts index a74f3727..e2dc4bda 100644 --- a/packages/code/src/native-sandbox.test.ts +++ b/packages/code/src/native-sandbox.test.ts @@ -1030,7 +1030,8 @@ test('reports cancellation after command start as a potentially committed mutati (error: unknown) => error instanceof WorkspaceToolError && error.code === 'EXECUTION_ABORTED' && - error.mutationMayHaveCommitted === true, + error.mutationMayHaveCommitted === true && + error.requiresQuarantine === false, ); }); @@ -1157,7 +1158,9 @@ test('cleans allocated command state exactly once on every execution exit', asyn error.code === (outcome.startsWith('abort') ? 'EXECUTION_ABORTED' : 'COMMAND_UNAVAILABLE') && - error.mutationMayHaveCommitted === (outcome === 'abort-after-spawn'), + error.mutationMayHaveCommitted === (outcome === 'abort-after-spawn') && + error.requiresQuarantine === + (outcome === 'abort-after-spawn' && process.platform === 'win32'), ); } assert.equal( diff --git a/packages/code/src/native-sandbox.ts b/packages/code/src/native-sandbox.ts index 49d269c0..139a10d6 100644 --- a/packages/code/src/native-sandbox.ts +++ b/packages/code/src/native-sandbox.ts @@ -681,6 +681,10 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox 'Workspace command execution aborted', 'EXECUTION_ABORTED', true, + // POSIX commands run in a detached process group, so its + // observed close follows a group-wide SIGKILL. The Windows + // fallback cannot yet prove descendant termination. + this.platform === 'win32', ), ); return; diff --git a/packages/code/src/worker.ts b/packages/code/src/worker.ts index 545b96ca..fde84768 100644 --- a/packages/code/src/worker.ts +++ b/packages/code/src/worker.ts @@ -1562,7 +1562,7 @@ export class BridgeWorker { !( error instanceof WorkspaceToolError && this.options.workspaceTools?.mutationFailuresAreAtomic === true && - !error.mutationMayHaveCommitted + !error.requiresQuarantine )) ) { ambiguousWorkspaceMutationError = error; diff --git a/packages/code/src/workspace-worker.test.ts b/packages/code/src/workspace-worker.test.ts index 43c820fe..14bf2d61 100644 --- a/packages/code/src/workspace-worker.test.ts +++ b/packages/code/src/workspace-worker.test.ts @@ -1410,6 +1410,76 @@ test('worker clears quarantine after a composed command is cleanly rejected', as assert.deepEqual(lifecycle, ['arm', 'execute', 'settle', 'clear']); }); +test('worker clears quarantine after a command cancellation confirms process termination', async () => { + const lifecycle: string[] = []; + const baseCapabilities = { + protocolVersion: 1 as const, + operations: ['read_file' as const], + workspaces: [{ id: 'primary', operations: ['read_file' as const] }], + }; + const workspaceTools = new SandboxWorkspaceTools({ + workspaceTools: { + capabilities: baseCapabilities, + mutationFailuresAreAtomic: true, + async execute() { throw new Error('base executor must not run'); }, + }, + commandWorkspaces: ['primary'], + commandSandbox: { + mutationFailuresAreAtomic: true, + async execute() { + lifecycle.push('execute'); + throw new WorkspaceToolError( + 'Workspace command execution aborted', + 'EXECUTION_ABORTED', + true, + false, + ); + }, + }, + }); + const worker = new BridgeWorker({ + codeApiUrl: 'https://code.example/v1', + token: 'worker-secret', + workerId: 'vm-1', + incarnationId, + sandboxEndpoint: 'http://127.0.0.1:2000/api/v2', + capabilities: { + statefulWorkspace: true, + sandboxProfile: 'nsjail', + runtimes: ['bash'], + workspaceTools: workspaceTools.capabilities, + }, + workspaceTools, + workspaceMutationQuarantine: mutationQuarantine( + () => lifecycle.push('quarantine'), + () => lifecycle.push('arm'), + () => lifecycle.push('clear'), + ), + fetchImpl: async () => { + lifecycle.push('settle'); + return Response.json({ protocolVersion: 1, accepted: true }); + }, + }); + + await worker.executeAndSettle({ + protocolVersion: 1, + assignmentId: 'assignment-command-cancelled-cleanly', + workerId: 'vm-1', + incarnationId, + generation: 4, + leaseToken: 'lease-token-that-is-long-enough-for-testing', + expiresAt: new Date(Date.now() + 5_000).toISOString(), + executionKind: 'workspace_tool', + request: { + protocolVersion: 1, + operation: 'execute_command', + workspaceId: 'primary', + command: 'sleep 30', + }, + }); + assert.deepEqual(lifecycle, ['arm', 'execute', 'settle', 'clear']); +}); + test('worker retains quarantine when an atomic executor cannot confirm durability', async () => { const lifecycle: string[] = []; const workspaceCapabilities = { diff --git a/packages/code/src/workspace.ts b/packages/code/src/workspace.ts index f7770e76..5bfc07fe 100644 --- a/packages/code/src/workspace.ts +++ b/packages/code/src/workspace.ts @@ -176,6 +176,8 @@ export class WorkspaceToolError extends Error { message: string, public readonly code: WorkspaceToolErrorCode, public readonly mutationMayHaveCommitted = false, + /** Retain the durable mutation guard when process or write settlement is uncertain. */ + public readonly requiresQuarantine = mutationMayHaveCommitted, ) { super(message); this.name = 'WorkspaceToolError'; diff --git a/service/src/bridge/store.ts b/service/src/bridge/store.ts index 41e11839..32205571 100644 --- a/service/src/bridge/store.ts +++ b/service/src/bridge/store.ts @@ -23,6 +23,7 @@ import { BridgeWorkspaceSlots } from './slots'; const PREFIX = 'codeapi:bridge:v1'; const POLL_INTERVAL_MS = 100; +const CANCELLED_WORKSPACE_SETTLEMENT_GRACE_MS = 5_000; const DEFAULT_WORKER_TTL_SECONDS = 60; const DEFAULT_REDIS_COMMAND_TIMEOUT_MS = 1_000; @@ -1901,6 +1902,55 @@ export class RedisBridgeStore { // settlement before returning an error, even when the caller is gone. pollError = error; } + const workspaceRequest = + assignment.executionKind === 'workspace_tool' && + isWorkspaceToolRequest(assignment.request) + ? assignment.request + : undefined; + const cancelledMutation = + signal.aborted && + workspaceRequest != null && + (workspaceRequest.operation === 'write_file' || + workspaceRequest.operation === 'edit_file' || + workspaceRequest.operation === 'execute_command'); + if (cancelledMutation) { + try { + // Keep the acknowledged assignment available long enough for the + // worker to terminate its process tree and commit a clean rejection. + // Closing it first makes that rejection impossible to acknowledge and + // leaves the worker's durable mutation guard armed. + await this.cancel(assignment.assignmentId, assignment); + // Rejected settlements remain valid after the execution deadline. + // Give Stop its own grace so a near-timeout cancellation is not + // misclassified as an ambiguous timeout. + const cancellationDeadlineAtMs = + Date.now() + CANCELLED_WORKSPACE_SETTLEMENT_GRACE_MS; + let cancellationPollMs = POLL_INTERVAL_MS; + while (Date.now() < cancellationDeadlineAtMs) { + const raw = await boundedCommand( + this.redis.get(settlementKey(assignment.assignmentId)), + Math.max( + 1, + Math.min( + this.redisCommandTimeoutMs, + cancellationDeadlineAtMs - Date.now(), + ), + ), + 'Bridge cancelled workspace settlement poll', + ); + if (raw != null) return JSON.parse(raw) as CodeBridgeSettlement; + await delay( + Math.min( + cancellationPollMs, + Math.max(0, cancellationDeadlineAtMs - Date.now()), + ), + ); + cancellationPollMs = Math.min(cancellationPollMs * 2, 500); + } + } catch (error) { + pollError ??= error; + } + } const closeKeys = [ assignmentKey(assignment.assignmentId), settlementKey(assignment.assignmentId), diff --git a/service/src/bridge/workspace-store.test.ts b/service/src/bridge/workspace-store.test.ts index a23274d8..5b0186d0 100644 --- a/service/src/bridge/workspace-store.test.ts +++ b/service/src/bridge/workspace-store.test.ts @@ -77,6 +77,108 @@ test('dispatches a workspace tool only to a worker advertising its workspace and }); }); +test('drains an acknowledged workspace mutation cancellation before releasing it', async () => { + await store.register({ + protocolVersion: BRIDGE_PROTOCOL_VERSION, + workerId: 'workspace-worker', + incarnationId, + capabilities: { + statefulWorkspace: false, + sandboxProfile: 'native-srt', + runtimes: [], + workspaceTools: { + protocolVersion: BRIDGE_PROTOCOL_VERSION, + operations: ['execute_command'], + workspaces: [{ id: 'primary' }], + }, + }, + }); + const controller = new AbortController(); + const completion = store.dispatchWorkspaceTool({ + workerId: 'workspace-worker', + request: { + protocolVersion: BRIDGE_PROTOCOL_VERSION, + operation: 'execute_command', + workspaceId: 'primary', + command: 'sleep 30; touch delayed.txt', + }, + deadlineAtMs: Date.now() + 5_000, + executionTimeoutMs: 500, + signal: controller.signal, + }); + const assignment = (await store.lease( + 'workspace-worker', + incarnationId, + 1_000, + ))!; + await store.acknowledgeLease( + 'workspace-worker', + incarnationId, + assignment.assignmentId, + assignment.generation, + assignment.leaseToken, + ); + + await new Promise((resolve) => setTimeout(resolve, 250)); + controller.abort(); + for ( + let attempt = 0; + attempt < 100 && + !(await store.cancelled( + 'workspace-worker', + incarnationId, + assignment.assignmentId, + )); + attempt += 1 + ) { + await new Promise((resolve) => setTimeout(resolve, 5)); + } + await new Promise((resolve) => setTimeout(resolve, 350)); + expect(Date.now()).toBeGreaterThan(Date.parse(assignment.expiresAt)); + await store.settle('workspace-worker', assignment.assignmentId, { + protocolVersion: BRIDGE_PROTOCOL_VERSION, + generation: assignment.generation, + leaseToken: assignment.leaseToken, + incarnationId, + status: 'rejected', + errorCode: 'EXECUTION_ABORTED', + error: 'Workspace command execution aborted', + }); + + await expect(completion).resolves.toMatchObject({ + status: 'rejected', + errorCode: 'EXECUTION_ABORTED', + }); + + const reuse = store.dispatchWorkspaceTool({ + workerId: 'workspace-worker', + request: { + protocolVersion: BRIDGE_PROTOCOL_VERSION, + operation: 'execute_command', + workspaceId: 'primary', + command: 'printf reused', + }, + deadlineAtMs: Date.now() + 5_000, + executionTimeoutMs: 5_000, + signal: new AbortController().signal, + }); + const next = (await store.lease( + 'workspace-worker', + incarnationId, + 1_000, + ))!; + expect(next.request).toMatchObject({ command: 'printf reused' }); + await store.settle('workspace-worker', next.assignmentId, { + protocolVersion: BRIDGE_PROTOCOL_VERSION, + generation: next.generation, + leaseToken: next.leaseToken, + incarnationId, + status: 'rejected', + error: 'fixture completion', + }); + await expect(reuse).resolves.toMatchObject({ status: 'rejected' }); +}); + test('rejects a workspace tool that the selected worker did not advertise', async () => { await store.register({ protocolVersion: BRIDGE_PROTOCOL_VERSION,