diff --git a/docs/byom-worker-admission.md b/docs/byom-worker-admission.md index c88d566..2c2b354 100644 --- a/docs/byom-worker-admission.md +++ b/docs/byom-worker-admission.md @@ -5,8 +5,12 @@ The limit is 32 admitted requests per worker, including the active request. When the limit is reached, the workspace endpoint returns HTTP 429 with `WORKER_QUEUE_FULL`. A different worker has an independent admission queue. -Waiting uses the caller's existing absolute dispatch deadline. It does not reset -or extend execution timeouts. Disconnecting or cancelling removes the waiting +The workspace HTTP endpoint allows at most 30 seconds for admission. After +admission and worker validation, a separate execution deadline starts. Commands +receive their requested timeout (30 seconds by default, up to five minutes), +capped by the operator's `JOB_TIMEOUT`, plus five seconds to settle the result. +Read/search/list operations receive up to 30 seconds, also capped by `JOB_TIMEOUT`. +Disconnecting or cancelling removes the waiting request without cancelling the active assignment. Expired entries are pruned; Redis key expiry also bounds state left by a crashed API process. @@ -15,12 +19,20 @@ binding and workspace operation. A waiting request cannot migrate to a replaceme worker. Existing execution acknowledgement, fencing, settlement and quarantine rules remain responsible for the active assignment. -This is compatible with existing workers and requires only a Code API update. +This is compatible with existing workers: assignments retain the same absolute +deadline and server-relative timing fields. Store callers that omit the new +internal `executionTimeoutMs` argument retain their existing absolute-deadline behavior. Existing workers still execute one assignment at a time. Parallel execution across workspaces requires separate lease claims and isolated native sandbox contexts; -this admission change does not advertise that capability. Queue time and execution -time currently share the HTTP request deadline. Separate budgets require a matching -LibreChat client change so that the client does not disconnect while waiting. +this admission change does not advertise that capability. + +LibreChat must allow queue time plus execution/settlement time and five seconds +for HTTP delivery: 65 seconds for reads, 70 seconds for default commands, and +340 seconds for five-minute commands. Either side can be upgraded first. Older +clients still cancel at their earlier deadline; newer clients preserve errors from +older servers without retrying mutations. Both updates are needed for the full +waiting budget. Any reverse proxy request timeout must accommodate these totals. +The worker package does not need an update for the deadline change. Focused regression coverage lives in `service/src/bridge/admission.test.ts` and `service/src/bridge/worker-admission.test.ts`. diff --git a/packages/code/README.md b/packages/code/README.md index 9dd3019..7e7c0aa 100644 --- a/packages/code/README.md +++ b/packages/code/README.md @@ -88,10 +88,45 @@ bubblewrap plus seccomp on Linux, and the SRT restricted-account helper on Windows. Startup fails before worker registration when the platform or its dependencies are unavailable. There is no unsandboxed command fallback. -The bridge worker remains outside the sandbox so it can maintain its outbound -Code API connection. Each command and its descendants run inside SRT with: +The native SRT manager owns process-global policy, proxy, and cleanup state. +Only one sandbox instance may own a manager, and that instance accepts one +command at a time. Overlapping calls fail before a second command starts; +they are not queued inside the sandbox. `close()` waits for the active command +and initialization before resetting the manager and removing scratch. A failed +reset keeps ownership fenced until a later `close()` succeeds. Independent +native workspaces need separate worker processes, not multiple instances of +the default manager in one process. This lifecycle guard does not enable +parallel assignments on a single bridge worker. + +The CLI hosts the native manager in a persistent, dedicated Node executor +process. It does not inherit the bridge credential, arbitrary host environment, +or Node loader/debugger options. Workspace policy and per-command masked +credentials travel over private parent/child IPC, never command-line arguments. +The bridge retains pairing and GitHub App identity management. Cancellation is +addressed to the active command; executor loss after dispatch is treated as an +uncertain mutation and is never automatically replayed. Restarting a worker +still requires its existing quarantine checks. Native platform limitations on +hard descendant teardown continue to apply. + +Embedding applications can use `NativeProcessWorkspaceCommandSandbox` from +`@librechat/code` for separate native managers in one host application, with +`prepare()`, `execute()`, and `close()`. Each instance is serial and must be +closed by its owner. The bridge scheduler remains serial until negotiated +execution slots and workspace-scoped quarantine are supported end to end. -- write access restricted to the one canonical registered workspace; +The bridge worker remains outside the sandbox so it can maintain its outbound +Code API connection. On macOS and Linux, each worker process creates an +owner-only scratch directory and grants SRT access to that exact directory +without opening the host temporary-directory root. Commands receive it through +`TMPDIR`, and orderly worker shutdown removes it. SRT's shared compatibility +scratch path is explicitly denied. Windows uses the restricted SRT account's +isolated profile and temporary directory instead. A workspace registration is +rejected if it sits inside SRT's shared scratch path or is broad enough to +contain worker scratch storage. Each command and its descendants run inside +SRT with: + +- write access restricted to the one canonical registered workspace and the + worker's private scratch directory; - read access denied to the worker's home directory except for that workspace; - paired identity and mutation-quarantine files explicitly denied; - `LIBRECHAT_CODE_*` and nonessential inherited environment variables removed; diff --git a/packages/code/src/cli.ts b/packages/code/src/cli.ts index 2810389..67ec396 100644 --- a/packages/code/src/cli.ts +++ b/packages/code/src/cli.ts @@ -27,7 +27,7 @@ import { EndpointRuntimeSupervisor, } from './runtime.js'; import { RuntimeWorkspaceCommandSandbox } from './workspace-runtime.js'; -import { NativeSrtWorkspaceCommandSandbox } from './native-sandbox.js'; +import { NativeProcessWorkspaceCommandSandbox } from './native-process.js'; import { GITHUB_ALLOWED_DOMAINS, GITHUB_CREDENTIAL_ENV_NAME, @@ -659,7 +659,7 @@ async function run( }); const nativeCommandSandbox = allowWorkspaceCommands && commandSandboxMode === 'native-srt' - ? new NativeSrtWorkspaceCommandSandbox({ + ? new NativeProcessWorkspaceCommandSandbox({ workspaceRoot: canonicalWorkerDirectory!, protectedPaths: [ identityPath, @@ -738,6 +738,7 @@ async function run( await github.provider?.getCredential(controller.signal); await nativeCommandSandbox?.prepare(); } catch (error) { + await nativeCommandSandbox?.close().catch(() => undefined); await fileRelaySupervisor?.stop().catch(() => undefined); throw error; } diff --git a/packages/code/src/index.ts b/packages/code/src/index.ts index 74ed37d..712e748 100644 --- a/packages/code/src/index.ts +++ b/packages/code/src/index.ts @@ -6,5 +6,6 @@ export * from './runtime.js'; export * from './workspace.js'; export * from './workspace-runtime.js'; export * from './native-sandbox.js'; +export * from './native-process.js'; export * from './github.js'; export * from './worker.js'; diff --git a/packages/code/src/native-process-child.test.ts b/packages/code/src/native-process-child.test.ts new file mode 100644 index 0000000..1d4866a --- /dev/null +++ b/packages/code/src/native-process-child.test.ts @@ -0,0 +1,39 @@ +import assert from 'node:assert/strict'; +import { fork } from 'node:child_process'; +import test from 'node:test'; +import { nativeExecutorEnvironment } from './native-process.js'; + +for (const signal of ['SIGINT', 'SIGHUP', 'SIGTERM'] as const) { + test( + `executor routes ${signal} through shutdown rather than default signal exit`, + { skip: process.platform === 'win32', timeout: 10_000 }, + async (t) => { + const child = fork( + new URL('./native-process-child.js', import.meta.url), + [], + { + execArgv: [], + env: nativeExecutorEnvironment(process.env), + stdio: ['ignore', 'ignore', 'ignore', 'ipc'], + }, + ); + t.after(() => { + child.kill('SIGKILL'); + }); + const exited = new Promise<{ + code: number | null; + signal: NodeJS.Signals | null; + }>((resolve, reject) => { + child.once('error', reject); + child.once('exit', (code, exitSignal) => + resolve({ code, signal: exitSignal }), + ); + }); + // An invalid-state reply proves module initialization and signal-handler + // installation finished without requiring platform SRT dependencies. + child.once('message', () => child.kill(signal)); + child.send({ id: 'startup-probe', type: 'probe' }); + assert.deepEqual(await exited, { code: 1, signal: null }); + }, + ); +} diff --git a/packages/code/src/native-process-child.ts b/packages/code/src/native-process-child.ts new file mode 100644 index 0000000..164147e --- /dev/null +++ b/packages/code/src/native-process-child.ts @@ -0,0 +1,111 @@ +import { NativeSrtWorkspaceCommandSandbox } from './native-sandbox.js'; +import { WorkspaceToolError } from './workspace.js'; +import type { NativeSrtWorkspaceCommandSandboxOptions } from './native-sandbox.js'; +import type { WorkspaceExecuteCommandRequest } from './protocol.js'; + +// This entrypoint is private to a forked trusted executor. No HTTP listener, +// argv credentials, bridge token, or persisted pairing material is required. +let sandbox: NativeSrtWorkspaceCommandSandbox | undefined; +let active: { id: string; controller: AbortController } | undefined; +let busy = false; +let credentials: Record = {}; +let wrappedCommand: string | undefined; + +if (!process.send) throw new Error('Native executor requires IPC'); +function reply(message: object): void { + if (!process.connected) return; + try { + process.send?.({ ...message, fatal: shuttingDown }, () => undefined); + } catch { + /* Parent was lost. */ + } +} +let shuttingDown = false; +const shutdown = () => { + if (shuttingDown) return; + shuttingDown = true; + active?.controller.abort(); + void (sandbox?.close() ?? Promise.resolve()).finally(() => process.exit(1)); + setTimeout(() => process.exit(1), 5000); +}; +process.on('disconnect', shutdown); +process.on('SIGTERM', shutdown); +process.on('SIGINT', shutdown); +process.on('SIGHUP', shutdown); +process.on('message', async (raw: unknown) => { + if (shuttingDown) return; + const message = raw as { + id: string; + type: string; + options: Omit< + NativeSrtWorkspaceCommandSandboxOptions, + 'maskedEnvironment' + > & { + variables?: NonNullable< + NativeSrtWorkspaceCommandSandboxOptions['maskedEnvironment'] + >['variables']; + }; + request: WorkspaceExecuteCommandRequest; + credentials?: Record; + wrappedCommand?: string; + }; + if (!message || typeof message.id !== 'string') return; + if (message.type === 'cancel') { + if (active?.id === message.id) active.controller.abort(); + return; + } + if (busy) return; + busy = true; + try { + let result: unknown; + if (message.type === 'prepare' && !sandbox) { + const { variables, ...options } = message.options; + sandbox = new NativeSrtWorkspaceCommandSandbox({ + ...options, + ...(variables + ? { + maskedEnvironment: { + variables, + async resolve() { + return credentials; + }, + wrapCommand(command) { + return wrappedCommand ?? command; + }, + }, + } + : {}), + }); + await sandbox.prepare(); + } else if (message.type === 'execute' && sandbox) { + active = { id: message.id, controller: new AbortController() }; + credentials = message.credentials ?? {}; + wrappedCommand = message.wrappedCommand; + result = await sandbox.execute(message.request, active.controller.signal); + } else if (message.type === 'close' && sandbox) { + await sandbox.close(); + } else throw new Error('Invalid executor state'); + reply({ id: message.id, ok: true, result }); + } catch (error) { + reply({ + id: message.id, + ok: false, + code: + error instanceof WorkspaceToolError + ? error.code + : 'COMMAND_UNAVAILABLE', + ...(error instanceof WorkspaceToolError + ? { errorMessage: error.message.slice(0, 1024) } + : {}), + mutation: + error instanceof WorkspaceToolError + ? error.mutationMayHaveCommitted + : true, + }); + } finally { + active = undefined; + credentials = {}; + wrappedCommand = undefined; + busy = false; + } +}); diff --git a/packages/code/src/native-process.test.ts b/packages/code/src/native-process.test.ts new file mode 100644 index 0000000..ffb068e --- /dev/null +++ b/packages/code/src/native-process.test.ts @@ -0,0 +1,380 @@ +import assert from 'node:assert/strict'; +import { EventEmitter } from 'node:events'; +import test from 'node:test'; +import type { ChildProcess, ForkOptions } from 'node:child_process'; +import { + NativeProcessWorkspaceCommandSandbox, + nativeExecutorEnvironment, +} from './native-process.js'; +import { WorkspaceToolError } from './workspace.js'; + +const request = { + protocolVersion: 1 as const, + operation: 'execute_command' as const, + workspaceId: 'primary', + command: 'printf ok', + timeoutMs: 1000, + maxOutputBytes: 64, +}; +const result = { + protocolVersion: 1, + operation: 'execute_command', + workspaceId: 'primary', + stdout: 'ok', + stderr: '', + exitCode: 0, + truncated: false, + timedOut: false, +}; + +function fixture( + execute?: (child: EventEmitter, message: Record) => void, + prepare?: (child: EventEmitter, message: Record) => void, +) { + const child = new EventEmitter() as ChildProcess; + let options: ForkOptions | undefined; + let killCalls = 0; + const messages: Record[] = []; + Object.assign(child, { + connected: true, + send(message: Record, callback: (error: null) => void) { + messages.push(message); + callback(null); + queueMicrotask(() => { + if (message.type === 'prepare' && prepare) + return prepare(child, message); + if (message.type === 'execute' && execute) + return execute(child, message); + if (message.type === 'cancel') return; + child.emit('message', { + id: message.id, + ok: true, + ...(message.type === 'execute' ? { result } : {}), + }); + }); + return true; + }, + kill() { + killCalls += 1; + child.emit('exit', 1); + return true; + }, + }); + return { + get killCalls() { + return killCalls; + }, + child, + messages, + get options() { + return options; + }, + fork(_path: URL, args: string[], value: ForkOptions) { + assert.deepEqual(args, []); + options = value; + return child; + }, + }; +} + +test('executor bootstrap excludes bridge credentials and Node injection variables', async () => { + assert.deepEqual( + nativeExecutorEnvironment({ + PATH: '/bin', + HOME: '/home/user', + NODE_OPTIONS: '--require bad.js', + LIBRECHAT_CODE_WORKER_TOKEN: 'secret', + GITHUB_TOKEN: 'secret', + AWS_SECRET_ACCESS_KEY: 'secret', + }), + { PATH: '/bin', HOME: '/home/user' }, + ); + const fake = fixture(); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { + workspaceRoot: '/workspace', + environment: { + PATH: '/bin', + NODE_OPTIONS: 'secret', + LIBRECHAT_CODE_WORKER_TOKEN: 'secret', + }, + }, + fake.fork, + ); + await sandbox.prepare(); + assert.deepEqual(fake.options?.execArgv, []); + assert.deepEqual(fake.options?.env, { PATH: '/bin' }); + assert.equal(JSON.stringify(fake.messages).includes('secret'), false); + await sandbox.close(); +}); + +test('executor hands credentials over IPC only for the current command', async () => { + const fake = fixture(); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { + workspaceRoot: '/workspace', + maskedEnvironment: { + variables: [{ name: 'TOKEN', injectHosts: ['github.com'] }], + async resolve() { + return { TOKEN: 'per-command-secret' }; + }, + wrapCommand(command) { + return `wrapped ${command}`; + }, + }, + }, + fake.fork, + ); + assert.deepEqual(await sandbox.execute(request), result); + assert.equal( + JSON.stringify(fake.options).includes('per-command-secret'), + false, + ); + assert.equal( + JSON.stringify(fake.messages[0]).includes('per-command-secret'), + false, + ); + assert.deepEqual(fake.messages[1].credentials, { + TOKEN: 'per-command-secret', + }); + assert.equal(fake.messages[1].wrappedCommand, 'wrapped printf ok'); + await sandbox.close(); +}); + +test('executor loss after dispatch is an uncertain mutation and is never replayed', async () => { + const fake = fixture((child) => child.emit('exit', 1)); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { workspaceRoot: '/workspace' }, + fake.fork, + ); + await assert.rejects( + sandbox.execute(request), + (error: unknown) => + error instanceof WorkspaceToolError && error.mutationMayHaveCommitted, + ); + await assert.rejects(sandbox.execute(request), /unavailable/); + assert.equal(fake.messages.filter((m) => m.type === 'execute').length, 1); + await sandbox.close(); +}); + +test('executor cancellation targets the active request and preserves mutation certainty', async () => { + let dispatched!: () => void; + const dispatch = new Promise((resolve) => { + dispatched = resolve; + }); + const fake = fixture(() => dispatched()); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { workspaceRoot: '/workspace' }, + fake.fork, + ); + const controller = new AbortController(); + const execution = sandbox.execute(request, controller.signal); + await dispatch; + await assert.rejects(sandbox.execute(request), /unavailable/); + controller.abort(); + const command = fake.messages.find((m) => m.type === 'execute')!; + assert.deepEqual(fake.messages.at(-1), { type: 'cancel', id: command.id }); + fake.child.emit('message', { + id: command.id, + ok: false, + code: 'EXECUTION_ABORTED', + mutation: true, + }); + await assert.rejects( + execution, + (error: unknown) => + error instanceof WorkspaceToolError && + error.code === 'EXECUTION_ABORTED' && + error.mutationMayHaveCommitted, + ); + await sandbox.close(); +}); + +test('executor rejects mismatched results as uncertain and fences subsequent commands', async () => { + const fake = fixture((child, message) => + child.emit('message', { + id: message.id, + ok: true, + result: { ...result, workspaceId: 'another-workspace' }, + }), + ); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { workspaceRoot: '/workspace' }, + fake.fork, + ); + await assert.rejects( + sandbox.execute(request), + (error: unknown) => + error instanceof WorkspaceToolError && error.mutationMayHaveCommitted, + ); + await assert.rejects(sandbox.execute(request), /unavailable/); + await sandbox.close(); +}); + +test('executor close drains an active command before closing IPC', 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 closing = sandbox.close(); + await new Promise((resolve) => setImmediate(resolve)); + assert.equal( + fake.messages.some((m) => m.type === 'close'), + false, + ); + const command = fake.messages.find((m) => m.type === 'execute')!; + fake.child.emit('message', { id: command.id, ok: true, result }); + assert.deepEqual(await execution, result); + await closing; + assert.equal(fake.messages.filter((m) => m.type === 'close').length, 1); + await assert.rejects(sandbox.execute(request), /unavailable/); +}); + +test('executor startup loss is not reported as an applied mutation', async () => { + const fake = fixture(); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { workspaceRoot: '/workspace' }, + (path, args, options) => { + const child = fake.fork(path, args, options); + queueMicrotask(() => child.emit('error', new Error('startup failed'))); + return child; + }, + ); + await assert.rejects( + sandbox.execute(request), + (error: unknown) => + error instanceof WorkspaceToolError && !error.mutationMayHaveCommitted, + ); + assert.equal( + fake.messages.some((m) => m.type === 'execute'), + false, + ); + await sandbox.close(); +}); + +test('executor shutdown receipt fences reuse before the OS exit event', async () => { + const fake = fixture((child, message) => + child.emit('message', { + id: message.id, + ok: false, + fatal: true, + mutation: true, + code: 'EXECUTION_ABORTED', + }), + ); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { workspaceRoot: '/workspace' }, + fake.fork, + ); + await assert.rejects(sandbox.execute(request)); + await assert.rejects(sandbox.execute(request), /unavailable/); + assert.equal(fake.messages.filter((m) => m.type === 'execute').length, 1); + await sandbox.close(); +}); + +test('executor preserves bounded startup diagnostics and conventional host settings', async () => { + assert.deepEqual( + nativeExecutorEnvironment( + { + HTTPS_PROXY: 'http://proxy:8080', + PATHEXT: '.EXE', + NODE_OPTIONS: 'unsafe', + }, + 'win32', + ), + { HTTPS_PROXY: 'http://proxy:8080', PATHEXT: '.EXE' }, + ); + const fake = fixture(undefined, (child, message) => + child.emit('message', { + id: message.id, + ok: false, + mutation: false, + code: 'COMMAND_UNAVAILABLE', + errorMessage: 'Native sandbox dependencies are unavailable: bubblewrap', + }), + ); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { workspaceRoot: '/workspace' }, + fake.fork, + ); + await assert.rejects( + sandbox.prepare(), + /dependencies are unavailable: bubblewrap/, + ); + assert.equal( + fake.killCalls, + 1, + 'failed prepare must terminate without caller cleanup', + ); + await sandbox.close(); +}); + +test('executor matches POSIX names exactly and folds names only on Windows', () => { + const env = { + PATH: '/bin', + Path: 'private', + home: 'private', + Temp: 'private', + PATHEXT: 'private', + https_proxy: 'http://proxy:8080', + custom_PROXY: 'private', + }; + assert.deepEqual(nativeExecutorEnvironment(env, 'linux'), { + PATH: '/bin', + https_proxy: 'http://proxy:8080', + }); + assert.deepEqual( + nativeExecutorEnvironment({ Path: 'C:\\bin', Temp: 'C:\\temp' }, 'win32'), + { Path: 'C:\\bin', Temp: 'C:\\temp' }, + ); +}); + +test('executor classifies every pre-dispatch setup failure as mutation-atomic', async () => { + for (const failure of ['fork', 'credential', 'wrapper', 'abort'] as const) { + const fake = fixture(); + const controller = new AbortController(); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { + workspaceRoot: '/workspace', + maskedEnvironment: { + variables: [], + async resolve() { + if (failure === 'abort') controller.abort(); + if (failure === 'credential' || failure === 'abort') + throw new Error('private provider error'); + return {}; + }, + wrapCommand(command) { + if (failure === 'wrapper') throw new Error('wrapper failed'); + return command; + }, + }, + }, + failure === 'fork' + ? () => { + throw new Error('fork failed'); + } + : fake.fork, + ); + await assert.rejects( + sandbox.execute(request, controller.signal), + (error: unknown) => + error instanceof WorkspaceToolError && + !error.mutationMayHaveCommitted && + error.code === + (failure === 'abort' ? 'EXECUTION_ABORTED' : 'COMMAND_UNAVAILABLE'), + ); + assert.equal( + fake.messages.some((m) => m.type === 'execute'), + false, + ); + await sandbox.close(); + } +}); diff --git a/packages/code/src/native-process.ts b/packages/code/src/native-process.ts new file mode 100644 index 0000000..fc44e3d --- /dev/null +++ b/packages/code/src/native-process.ts @@ -0,0 +1,357 @@ +import { fork } from 'node:child_process'; +import { randomUUID } from 'node:crypto'; +import { WorkspaceToolError } from './workspace.js'; +import { isWorkspaceToolRequest, isWorkspaceToolResult } from './protocol.js'; +import type { ChildProcess, ForkOptions } from 'node:child_process'; +import type { NativeSrtWorkspaceCommandSandboxOptions } from './native-sandbox.js'; +import type { WorkspaceCommandSandbox } from './workspace.js'; +import type { + WorkspaceExecuteCommandRequest, + WorkspaceExecuteCommandResult, +} from './protocol.js'; + +export type NativeProcessSandboxOptions = Omit< + NativeSrtWorkspaceCommandSandboxOptions, + 'manager' | 'spawnCommand' | 'platform' +>; + +/** Only OS discovery and conventional proxy settings cross into the executor. + * In particular, never inherit NODE_OPTIONS, bridge identity, or app secrets. */ +export function nativeExecutorEnvironment( + source: NodeJS.ProcessEnv, + platform: NodeJS.Platform = process.platform, +): NodeJS.ProcessEnv { + const allowed = new Set([ + 'PATH', + 'HOME', + 'TMPDIR', + 'LANG', + 'LC_ALL', + 'LC_CTYPE', + 'LOGNAME', + 'USER', + 'SHELL', + 'TERM', + 'COLORTERM', + 'NO_COLOR', + 'HTTP_PROXY', + 'HTTPS_PROXY', + 'ALL_PROXY', + 'NO_PROXY', + 'http_proxy', + 'https_proxy', + 'all_proxy', + 'no_proxy', + ]); + if (platform === 'win32') { + for (const name of [ + 'USERPROFILE', + 'SYSTEMROOT', + 'WINDIR', + 'COMSPEC', + 'TEMP', + 'TMP', + 'LOCALAPPDATA', + 'APPDATA', + 'PROGRAMDATA', + 'PROGRAMFILES', + 'PROGRAMFILES(X86)', + 'SYSTEMDRIVE', + 'PATHEXT', + 'HOMEDRIVE', + 'HOMEPATH', + ]) { + allowed.add(name); + } + } + return Object.fromEntries( + Object.entries(source).filter( + ([name, value]) => + value != null && + allowed.has(platform === 'win32' ? name.toUpperCase() : name), + ), + ); +} + +/** One persistent, process-isolated SRT manager per workspace. No automatic + * restart/replay: losing IPC after execution starts is an ambiguous mutation. */ +export class NativeProcessWorkspaceCommandSandbox + implements WorkspaceCommandSandbox +{ + readonly mutationFailuresAreAtomic = true as const; + private child?: ChildProcess; + private ready?: Promise; + private active?: Promise; + private closing?: Promise; + private failed = false; + private terminationTimer?: ReturnType; + private pending?: { + id: string; + resolve(value: unknown): void; + reject(error: Error): void; + mutation: boolean; + }; + + constructor( + private readonly options: NativeProcessSandboxOptions, + private readonly forkExecutor: ( + path: URL, + args: string[], + options: ForkOptions, + ) => ChildProcess = fork, + ) {} + + async prepare(): Promise { + if (this.failed || this.closing) throw this.unavailable(false); + if (this.ready) return this.ready; + this.ready = this.start(); + return this.ready; + } + + private unavailable(mutation: boolean): WorkspaceToolError { + return new WorkspaceToolError( + 'Native executor is unavailable', + 'COMMAND_UNAVAILABLE', + mutation, + ); + } + + private async start(): Promise { + const child = this.forkExecutor( + new URL('./native-process-child.js', import.meta.url), + [], + { + execArgv: [], + env: nativeExecutorEnvironment(this.options.environment ?? process.env), + stdio: ['ignore', 'ignore', 'ignore', 'ipc'], + serialization: 'json', + }, + ); + this.child = child; + child.on('message', (raw: unknown) => { + const message = raw as { + id?: unknown; + ok?: unknown; + result?: unknown; + mutation?: unknown; + code?: unknown; + errorMessage?: unknown; + fatal?: unknown; + }; + if ( + !message || + typeof message !== 'object' || + message.id !== this.pending?.id + ) + return; + const pending = this.pending; + if (!pending) return; + if (message.fatal === true) this.failed = true; + if (message.ok === true) pending.resolve(message.result); + else { + const code = + message.code === 'INVALID_PATH' || + message.code === 'INVALID_REQUEST' || + message.code === 'EXECUTION_ABORTED' || + message.code === 'REGISTRATION_INVALID' + ? message.code + : 'COMMAND_UNAVAILABLE'; + pending.reject( + new WorkspaceToolError( + typeof message.errorMessage === 'string' && + message.errorMessage.length <= 1024 + ? message.errorMessage + : 'Native executor request failed', + code, + pending.mutation && message.mutation !== false, + ), + ); + } + }); + const lost = () => { + this.failed = true; + this.pending?.reject(this.unavailable(this.pending.mutation)); + }; + child.on('error', lost); + child.on('exit', lost); + child.on('disconnect', lost); + const { + workspaceRoot, + protectedPaths, + allowedDomains, + homeDirectory, + shellPath, + } = this.options; + await this.rpc( + 'prepare', + { + options: { + workspaceRoot, + protectedPaths, + allowedDomains, + homeDirectory, + shellPath, + variables: this.options.maskedEnvironment?.variables, + }, + }, + 30_000, + false, + ).catch((error) => { + this.failed = true; + this.terminate(); + throw error; + }); + } + + async execute( + request: WorkspaceExecuteCommandRequest, + signal?: AbortSignal, + ): Promise { + if ( + !isWorkspaceToolRequest(request) || + request.operation !== 'execute_command' + ) { + throw new WorkspaceToolError('Invalid native command', 'INVALID_REQUEST'); + } + if (this.active || this.closing || this.failed) + throw this.unavailable(false); + const active = this.executeOnce(request, signal); + this.active = active; + try { + return await active; + } finally { + this.active = undefined; + } + } + + private async executeOnce( + request: WorkspaceExecuteCommandRequest, + signal?: AbortSignal, + ): Promise { + if (signal?.aborted) + throw new WorkspaceToolError('Command aborted', 'EXECUTION_ABORTED'); + let credentials: Record | undefined; + let wrappedCommand: string | undefined; + try { + await this.prepare(); + if (signal?.aborted) throw new Error('aborted'); + credentials = await this.options.maskedEnvironment?.resolve(signal); + if (signal?.aborted) throw new Error('aborted'); + wrappedCommand = this.options.maskedEnvironment?.wrapCommand?.( + request.command, + process.platform, + ); + if (signal?.aborted) throw new Error('aborted'); + } catch (error) { + // No execute RPC has been sent: setup, token refresh and wrapping cannot + // have mutated the workspace. Do not quarantine it for setup failures. + if (signal?.aborted) + throw new WorkspaceToolError('Command aborted', 'EXECUTION_ABORTED'); + throw error instanceof WorkspaceToolError + ? new WorkspaceToolError(error.message, error.code, false) + : new WorkspaceToolError( + 'Native executor setup failed before dispatch', + 'COMMAND_UNAVAILABLE', + ); + } + const result = await this.rpc( + 'execute', + { request, credentials, wrappedCommand }, + (request.timeoutMs ?? 30_000) + 5_000, + true, + signal, + ); + if (signal?.aborted) { + throw new WorkspaceToolError( + 'Command aborted', + 'EXECUTION_ABORTED', + true, + ); + } + if (!isWorkspaceToolResult(request, result)) { + this.failed = true; + this.terminate(); + throw this.unavailable(true); + } + return result as WorkspaceExecuteCommandResult; + } + + private async rpc( + type: string, + payload: object, + timeoutMs: number, + mutation: boolean, + signal?: AbortSignal, + ): Promise { + if (this.pending || !this.child?.connected || this.failed) + throw this.unavailable(false); + const id = randomUUID(); + const child = this.child; + let timer: ReturnType; + const abort = () => { + try { + if (child.connected) + child.send({ type: 'cancel', id }, () => undefined); + } catch { + this.failed = true; + this.terminate(); + } + }; + try { + return await new Promise((resolve, reject) => { + this.pending = { id, resolve, reject, mutation }; + timer = setTimeout(() => { + this.failed = true; + this.terminate(); + reject(this.unavailable(mutation)); + }, timeoutMs); + signal?.addEventListener('abort', abort, { once: true }); + const sendFailed = () => { + this.failed = true; + this.terminate(); + reject(this.unavailable(mutation)); + }; + try { + child.send({ type, id, ...payload }, (error) => { + if (error) sendFailed(); + }); + } catch { + sendFailed(); + } + if (signal?.aborted) abort(); + }); + } finally { + clearTimeout(timer!); + signal?.removeEventListener('abort', abort); + this.pending = undefined; + } + } + + async close(): Promise { + if (this.closing) return this.closing; + this.closing = this.stop(); + return this.closing; + } + + private async stop(): Promise { + await this.active?.catch(() => undefined); + await this.ready?.catch(() => undefined); + try { + if (this.child?.connected && !this.failed) + await this.rpc('close', {}, 10_000, false); + } finally { + this.failed = true; + this.terminate(); + } + } + + private terminate(): void { + const child = this.child; + if (!child || this.terminationTimer) return; + // Give SRT time to abort/reap its command, then bound executor shutdown. + this.terminationTimer = setTimeout(() => child.kill('SIGKILL'), 6000); + this.terminationTimer.unref(); + child.once('exit', () => clearTimeout(this.terminationTimer)); + child.kill('SIGTERM'); + } +} diff --git a/packages/code/src/native-sandbox.test.ts b/packages/code/src/native-sandbox.test.ts index 5ee411f..2d40db0 100644 --- a/packages/code/src/native-sandbox.test.ts +++ b/packages/code/src/native-sandbox.test.ts @@ -1,4 +1,5 @@ import assert from 'node:assert/strict'; +import { spawn } from 'node:child_process'; import { EventEmitter } from 'node:events'; import { access, @@ -6,6 +7,7 @@ import { mkdir, realpath, rm, + stat, writeFile, } from 'node:fs/promises'; import { tmpdir, homedir } from 'node:os'; @@ -34,6 +36,7 @@ function fakeManager( beforeWrap?: () => Promise; appendGitSafeDirectory?: boolean; inheritedGitEnvironment?: Record; + initializeError?: Error; wrappedEnvironment?: NodeJS.ProcessEnv; } = {}, ) { @@ -41,6 +44,7 @@ function fakeManager( let reset = false; let credentialSeenDuringWrap: string | undefined; let gitLfsRequiredSeenDuringWrap: string | undefined; + let scratchSelectorSeenDuringWrap: string | undefined; const manager = { isSupportedPlatform: () => true, async checkDependenciesAsync() { @@ -48,11 +52,13 @@ function fakeManager( }, async initialize(value: SandboxRuntimeConfig) { config = value; + if (options.initializeError) throw options.initializeError; }, async wrapWithSandboxArgv(command: string) { await options.beforeWrap?.(); credentialSeenDuringWrap = process.env.LIBRECHAT_CODE_TEST_CREDENTIAL; gitLfsRequiredSeenDuringWrap = process.env.GIT_CONFIG_VALUE_3; + scratchSelectorSeenDuringWrap = process.env.CLAUDE_CODE_TMPDIR; const ambientGitEnvironment = Object.fromEntries( Object.entries(process.env).filter( ([name, value]) => name.startsWith('GIT_CONFIG_') && value != null, @@ -105,9 +111,136 @@ function fakeManager( get gitLfsRequiredSeenDuringWrap() { return gitLfsRequiredSeenDuringWrap; }, + get scratchSelectorSeenDuringWrap() { + return scratchSelectorSeenDuringWrap; + }, }; } +test('exclusive lifecycle rejects a second workspace sharing an SRT manager', async (t) => { + const root = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); + t.after(() => rm(root, { recursive: true, force: true })); + const fake = fakeManager(); + const first = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + manager: fake.manager, + }); + const second = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + manager: fake.manager, + }); + t.after(() => first.close()); + t.after(() => second.close()); + await first.prepare(); + await assert.rejects( + second.prepare(), + /already belongs to another workspace/, + ); + await second.close(); + assert.equal( + fake.reset, + false, + 'a rejected owner must not reset the live manager', + ); + assert.equal((await first.execute(request)).stdout, 'hello'); + await first.close(); + await second.prepare(); + assert.equal((await second.execute(request)).stdout, 'hello'); +}); + +test('exclusive lifecycle rejects overlapping commands and waits before resetting', async (t) => { + const root = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); + t.after(() => rm(root, { recursive: true, force: true })); + let entered!: () => void; + const wrapping = new Promise((resolve) => { + entered = resolve; + }); + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + const fake = fakeManager({ + beforeWrap: async () => { + entered(); + await gate; + }, + }); + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + manager: fake.manager, + }); + t.after(() => sandbox.close()); + const execution = sandbox.execute(request); + await wrapping; + await assert.rejects(sandbox.execute(request), /active command/); + const closing = sandbox.close(); + const secondClose = sandbox.close(); + await new Promise((resolve) => setImmediate(resolve)); + assert.equal(fake.reset, false); + await assert.rejects(sandbox.prepare(), /closing/); + release(); + assert.equal((await execution).stdout, 'hello'); + await Promise.all([closing, secondClose]); + assert.equal(fake.reset, true); +}); + +test('exclusive lifecycle retains ownership after a failed reset until cleanup succeeds', async (t) => { + const root = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); + t.after(() => rm(root, { recursive: true, force: true })); + const fake = fakeManager(); + let failReset = true; + fake.manager.reset = async () => { + if (failReset) throw new Error('reset failed'); + }; + const first = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + manager: fake.manager, + }); + const second = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + manager: fake.manager, + }); + await first.prepare(); + await assert.rejects(first.close(), /reset failed/); + await assert.rejects(first.execute(request), /requires cleanup/); + await assert.rejects(second.prepare(), /already belongs/); + failReset = false; + await first.close(); + await second.prepare(); + await second.close(); +}); + +test('exclusive lifecycle waits for initialization before resetting the manager', async (t) => { + const root = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); + t.after(() => rm(root, { recursive: true, force: true })); + const fake = fakeManager(); + let entered!: () => void; + const initializing = new Promise((resolve) => { + entered = resolve; + }); + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + fake.manager.initialize = async () => { + entered(); + await gate; + }; + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + manager: fake.manager, + }); + const preparing = sandbox.prepare(); + await initializing; + const closing = sandbox.close(); + await new Promise((resolve) => setImmediate(resolve)); + assert.equal(fake.reset, false); + release(); + await preparing; + await closing; + assert.equal(fake.reset, true); +}); + test('initializes SRT with a default-deny network and scrubbed worker credentials', async (t) => { const root = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); const identity = join(tmpdir(), 'librechat-code-identity.json'); @@ -133,23 +266,187 @@ test('initializes SRT with a default-deny network and scrubbed worker credential join(await realpath(tmpdir()), 'librechat-code-identity.json'), ); const canonicalHome = await realpath(homedir()); + const scratchDirectory = fake.config?.filesystem.allowWrite[1]; + assert.equal(typeof scratchDirectory, 'string'); assert.deepEqual(fake.config?.network.allowedDomains, []); assert.equal(fake.config?.network.strictAllowlist, true); assert.equal(fake.config?.network.allowAllUnixSockets, false); - assert.deepEqual(fake.config?.filesystem.allowRead, [canonicalRoot]); - assert.deepEqual(fake.config?.filesystem.allowWrite, [canonicalRoot]); + assert.deepEqual(fake.config?.filesystem.allowRead, [ + canonicalRoot, + scratchDirectory, + ]); + assert.deepEqual(fake.config?.filesystem.allowWrite, [ + canonicalRoot, + scratchDirectory, + ]); + assert.equal((await stat(scratchDirectory!)).mode & 0o777, 0o700); assert.ok(fake.config?.filesystem.denyRead.includes(canonicalHome)); assert.ok(fake.config?.filesystem.denyWrite.includes(canonicalIdentity)); + assert.ok( + fake.config?.filesystem.denyWrite.some((path) => + path.endsWith('/tmp/claude'), + ), + ); const denied = fake.config?.credentials?.envVars?.map(({ name }) => name); assert.ok(denied?.includes('LIBRECHAT_CODE_WORKER_TOKEN')); assert.ok(denied?.includes('AWS_SECRET_ACCESS_KEY')); assert.ok(!denied?.includes('PATH')); assert.ok(denied?.includes('Path')); assert.ok(denied?.includes('lc_api_token')); + assert.ok(denied?.includes('CLAUDE_CODE_TMPDIR')); + assert.ok(denied?.includes('CLAUDE_TMPDIR')); + await sandbox.close(); + assert.equal(fake.reset, true); + await assert.rejects(access(scratchDirectory!)); +}); + +test('provides an isolated scratch directory to commands and restores the host environment', async (t) => { + const root = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); + t.after(() => rm(root, { recursive: true, force: true })); + const originalTmpdir = process.env.TMPDIR; + const originalSrtTmpdir = process.env.CLAUDE_CODE_TMPDIR; + const originalLegacySrtTmpdir = process.env.CLAUDE_TMPDIR; + const fake = fakeManager(); + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + manager: fake.manager, + }); + + const result = await sandbox.execute({ + ...request, + maxOutputBytes: 1_024, + command: 'touch "$TMPDIR/probe" && printf %s "$TMPDIR"', + }); + + assert.equal(result.exitCode, 0); + assert.match(result.stdout, /librechat-code-srt-/); + await access(join(result.stdout, 'probe')); + assert.equal(fake.scratchSelectorSeenDuringWrap, result.stdout); + assert.equal(process.env.TMPDIR, originalTmpdir); + assert.equal(process.env.CLAUDE_CODE_TMPDIR, originalSrtTmpdir); + assert.equal(process.env.CLAUDE_TMPDIR, originalLegacySrtTmpdir); await sandbox.close(); + await assert.rejects(access(result.stdout)); +}); + +test('removes scratch storage when SRT initialization fails', async (t) => { + const root = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); + t.after(() => rm(root, { recursive: true, force: true })); + const fake = fakeManager({ initializeError: new Error('init failed') }); + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + manager: fake.manager, + }); + + await assert.rejects(sandbox.prepare(), /init failed/); + const scratchDirectory = fake.config?.filesystem.allowWrite[1]; + assert.equal(typeof scratchDirectory, 'string'); + await assert.rejects(access(scratchDirectory!)); assert.equal(fake.reset, true); }); +test('rejects workspaces nested inside SRT shared scratch storage', async (t) => { + if (process.platform === 'win32') return; + const sharedRoot = '/tmp/claude'; + await mkdir(sharedRoot, { recursive: true }); + const root = await mkdtemp(join(sharedRoot, 'librechat-code-native-')); + t.after(() => rm(root, { recursive: true, force: true })); + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + manager: fakeManager().manager, + }); + + await assert.rejects( + sandbox.prepare(), + (error: WorkspaceToolError) => + error.code === 'REGISTRATION_INVALID' && + /inherited writable path/.test(error.message), + ); +}); + +test('rejects a workspace that contains worker scratch storage', async () => { + if (process.platform === 'win32') return; + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: tmpdir(), + manager: fakeManager().manager, + }); + + await assert.rejects( + sandbox.prepare(), + (error: WorkspaceToolError) => + error.code === 'REGISTRATION_INVALID' && + /contain worker scratch storage/.test(error.message), + ); + await sandbox.close(); +}); + +test('keeps concurrent sandbox scratch directories independent', async (t) => { + const firstRoot = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); + const secondRoot = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); + t.after(() => rm(firstRoot, { recursive: true, force: true })); + t.after(() => rm(secondRoot, { recursive: true, force: true })); + let releaseWrap!: () => void; + let wrapStarted!: () => void; + const wrapStartedPromise = new Promise((resolve) => { + wrapStarted = resolve; + }); + const holdWrap = new Promise((resolve) => { + releaseWrap = resolve; + }); + const firstFake = fakeManager({ + async beforeWrap() { + wrapStarted(); + await holdWrap; + }, + }); + const secondFake = fakeManager(); + const firstSandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: firstRoot, + manager: firstFake.manager, + }); + const secondSandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: secondRoot, + manager: secondFake.manager, + }); + const firstExecution = firstSandbox.execute({ + ...request, + command: 'printf first', + }); + await wrapStartedPromise; + await secondSandbox.prepare(); + const firstScratch = firstFake.config?.filesystem.allowWrite[1]; + const secondScratch = secondFake.config?.filesystem.allowWrite[1]; + assert.equal(typeof firstScratch, 'string'); + assert.equal(typeof secondScratch, 'string'); + assert.notEqual(firstScratch, secondScratch); + assert.ok(!secondScratch!.startsWith(`${firstScratch}/`)); + releaseWrap(); + await firstExecution; + await firstSandbox.close(); + await access(secondScratch!); + await secondSandbox.close(); +}); + +test('removes scratch storage after a command revokes traversal permissions', async (t) => { + if (process.platform === 'win32') return; + const root = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); + t.after(() => rm(root, { recursive: true, force: true })); + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + manager: fakeManager().manager, + }); + + const result = await sandbox.execute({ + ...request, + command: + 'printf %s "$TMPDIR"; mkdir "$TMPDIR/locked"; touch "$TMPDIR/locked/file"; chmod 000 "$TMPDIR/locked" "$TMPDIR"', + }); + + assert.equal(result.exitCode, 0); + await sandbox.close(); + await assert.rejects(access(result.stdout)); +}); + const proxyEnvironment = { HTTP_PROXY: 'http://upstream.invalid:8080', HTTPS_PROXY: 'http://upstream.invalid:8080', @@ -586,16 +883,26 @@ test('terminates detached command descendants before returning', async (t) => { test('reports cancellation after command start as a potentially committed mutation', async (t) => { const root = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); t.after(() => rm(root, { recursive: true, force: true })); + let commandStarted!: () => void; + const commandStartedPromise = new Promise((resolve) => { + commandStarted = resolve; + }); const sandbox = new NativeSrtWorkspaceCommandSandbox({ workspaceRoot: root, manager: fakeManager().manager, + spawnCommand(command, args, options) { + const child = spawn(command, [...args], options); + commandStarted(); + return child; + }, }); const controller = new AbortController(); const execution = sandbox.execute( { ...request, command: 'sleep 30' }, controller.signal, ); - setTimeout(() => controller.abort(), 25); + await commandStartedPromise; + controller.abort(); await assert.rejects( execution, diff --git a/packages/code/src/native-sandbox.ts b/packages/code/src/native-sandbox.ts index 9743ad7..bc57044 100644 --- a/packages/code/src/native-sandbox.ts +++ b/packages/code/src/native-sandbox.ts @@ -1,6 +1,6 @@ import { spawn } from 'node:child_process'; import { randomUUID } from 'node:crypto'; -import { homedir } from 'node:os'; +import { homedir, tmpdir } from 'node:os'; import { basename, dirname, @@ -11,7 +11,17 @@ import { sep, } from 'node:path'; import { constants as fsConstants } from 'node:fs'; -import { access, realpath, stat } from 'node:fs/promises'; +import { + access, + chmod, + lstat, + mkdtemp, + open, + readdir, + realpath, + rm, + stat, +} from 'node:fs/promises'; import { SandboxManager } from '@anthropic-ai/sandbox-runtime'; @@ -21,6 +31,11 @@ import { BRIDGE_WORKSPACE_COMMAND_DEFAULT_TIMEOUT_MS, isWorkspaceToolRequest, } from './protocol.js'; +import { + assertPrivateStorageAcl, + assertPrivateStorageAncestors, + removePrivateStorageAcl, +} from './private-storage.js'; import { WorkspaceToolError } from './workspace.js'; import type { @@ -96,6 +111,17 @@ const { ...TRUSTED_GIT_CONFIG_ENTRIES } = TRUSTED_GIT_ENVIRONMENT; +const NATIVE_SANDBOX_SCRATCH_PREFIX = 'librechat-code-srt-'; +// SRT grants these shared compatibility paths by default. A worker-specific +// TMPDIR must also deny them or separate worker processes can exchange files. +const SRT_SHARED_SCRATCH_PATHS = ['/tmp/claude', '/private/tmp/claude']; +const SRT_SCRATCH_SELECTOR_NAMES = [ + 'CLAUDE_CODE_TMPDIR', + 'CLAUDE_TMPDIR', +] as const; +// Capture this before any command wrapper can temporarily mutate process.env. +const HOST_TEMPORARY_ROOT = tmpdir(); + interface NativeSandboxManager { isSupportedPlatform(): boolean; checkDependenciesAsync(): Promise<{ warnings: string[]; errors: string[] }>; @@ -113,6 +139,13 @@ interface NativeSandboxManager { reset(): Promise; } +// SRT's default manager is process-global, including its policy and cleanup +// state. Distinct workspace objects must not reconfigure the same manager. +const managerOwners = new WeakMap< + NativeSandboxManager, + NativeSrtWorkspaceCommandSandbox +>(); + type SpawnCommand = ( command: string, args: readonly string[], @@ -211,6 +244,10 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox private readonly platform: NodeJS.Platform; private initialized?: Promise; private canonicalRoot?: string; + private scratchDirectory?: string; + private execution?: Promise; + private closing?: Promise; + private resetFailed = false; constructor( private readonly options: NativeSrtWorkspaceCommandSandboxOptions, @@ -227,9 +264,27 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox } private async initialize(): Promise { + if (this.closing || this.resetFailed) { + throw new WorkspaceToolError( + 'Native sandbox is closing or requires cleanup', + 'COMMAND_UNAVAILABLE', + ); + } if (this.initialized) return this.initialized; + const owner = managerOwners.get(this.manager); + if (owner && owner !== this) { + throw new WorkspaceToolError( + 'Native sandbox manager already belongs to another workspace; use a separate worker process', + 'COMMAND_UNAVAILABLE', + ); + } + managerOwners.set(this.manager, this); this.initialized = this.initializeOnce().catch(async (error) => { - await this.manager.reset().catch(() => undefined); + await this.manager.reset().catch(() => { + this.resetFailed = true; + }); + await this.removeScratchDirectory().catch(() => undefined); + if (!this.resetFailed) managerOwners.delete(this.manager); this.initialized = undefined; throw error; }); @@ -266,6 +321,26 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox 'REGISTRATION_INVALID', ); } + const sharedScratchPaths = await Promise.all( + (this.platform === 'win32' ? [] : SRT_SHARED_SCRATCH_PATHS).map( + canonicalPath, + ), + ); + const inheritedWritablePaths = [ + ...sharedScratchPaths, + ...(await Promise.all( + [join(home, '.npm', '_logs'), join(home, '.claude', 'debug')].map( + canonicalPath, + ), + )), + ]; + const deniedInheritedWritablePaths = [...new Set(inheritedWritablePaths)]; + if (deniedInheritedWritablePaths.some((path) => isWithin(path, root))) { + throw new WorkspaceToolError( + 'Native sandbox workspace cannot be inside an inherited writable path', + 'REGISTRATION_INVALID', + ); + } const dependencies = await this.manager.checkDependenciesAsync(); if (dependencies.errors.length > 0) { throw new WorkspaceToolError( @@ -283,6 +358,17 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox ); } } + const canonicalScratchDirectory = + await this.createScratchDirectory(sharedScratchPaths); + if ( + canonicalScratchDirectory && + isWithin(root, canonicalScratchDirectory) + ) { + throw new WorkspaceToolError( + 'Native sandbox workspace cannot contain worker scratch storage', + 'REGISTRATION_INVALID', + ); + } const config: SandboxRuntimeConfig = { network: { allowedDomains: [...(this.options.allowedDomains ?? [])], @@ -293,10 +379,21 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox ...(this.options.maskedEnvironment ? { tlsTerminate: {} } : {}), }, filesystem: { - denyRead: [home], - allowRead: [root], - allowWrite: [root], - denyWrite: protectedPaths, + denyRead: [ + home, + ...sharedScratchPaths.filter((path) => + deniedInheritedWritablePaths.includes(path), + ), + ], + allowRead: [ + root, + ...(canonicalScratchDirectory ? [canonicalScratchDirectory] : []), + ], + allowWrite: [ + root, + ...(canonicalScratchDirectory ? [canonicalScratchDirectory] : []), + ], + denyWrite: [...protectedPaths, ...deniedInheritedWritablePaths], allowGitConfig: false, }, credentials: { @@ -305,7 +402,14 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox mode: 'deny' as const, })), envVars: [ - ...deniedEnvironmentNames(this.environment, this.platform) + ...deniedEnvironmentNames( + { + ...this.environment, + CLAUDE_CODE_TMPDIR: '', + CLAUDE_TMPDIR: '', + }, + this.platform, + ) .filter((name) => { const normalized = normalizedEnvironmentName( name, @@ -342,6 +446,25 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox async execute( request: WorkspaceExecuteCommandRequest, signal?: AbortSignal, + ): Promise { + if (this.execution || this.closing) { + throw new WorkspaceToolError( + 'Native sandbox already has an active command or is closing', + 'COMMAND_UNAVAILABLE', + ); + } + const execution = this.executeExclusive(request, signal); + this.execution = execution; + try { + return await execution; + } finally { + this.execution = undefined; + } + } + + private async executeExclusive( + request: WorkspaceExecuteCommandRequest, + signal?: AbortSignal, ): Promise { if ( !isWorkspaceToolRequest(request) || @@ -388,6 +511,7 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox { ...TRUSTED_GIT_ENVIRONMENT, ...(credentialEnvironment ?? {}), + ...this.scratchSelectorEnvironment(), }, () => this.manager.wrapWithSandboxArgv( @@ -476,6 +600,7 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox cwd, env: { ...wrapped.env, + ...this.scratchEnvironment(), ...TRUSTED_GIT_CONFIG_ENTRIES, GIT_CONFIG_COUNT: wrapped.env.GIT_CONFIG_COUNT ?? TRUSTED_GIT_CONFIG_COUNT, @@ -618,10 +743,141 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox : 1; } + private async createScratchDirectory( + sharedScratchPaths: string[], + ): Promise { + // Windows SRT supplies the restricted account's private TEMP directory. + if (this.platform === 'win32') return undefined; + const canonicalTemporaryRoot = await canonicalPath(HOST_TEMPORARY_ROOT); + const sharedScratchRoot = sharedScratchPaths.find((path) => + isWithin(path, canonicalTemporaryRoot), + ); + const scratchDirectory = await mkdtemp( + join( + sharedScratchRoot + ? dirname(sharedScratchRoot) + : canonicalTemporaryRoot, + NATIVE_SANDBOX_SCRATCH_PREFIX, + ), + ); + try { + await assertPrivateStorageAncestors(scratchDirectory); + const scratchHandle = await open(scratchDirectory, 'r'); + try { + await removePrivateStorageAcl(scratchHandle, scratchDirectory); + await scratchHandle.chmod(0o700); + await assertPrivateStorageAcl( + scratchHandle, + scratchDirectory, + true, + ); + if (((await scratchHandle.stat()).mode & 0o777) !== 0o700) { + throw new Error('Native sandbox scratch directory is not private'); + } + } finally { + await scratchHandle.close(); + } + this.scratchDirectory = await realpath(scratchDirectory); + return this.scratchDirectory; + } catch (error) { + await rm(scratchDirectory, { recursive: true, force: true }).catch( + () => undefined, + ); + throw error; + } + } + + private scratchEnvironment(): NodeJS.ProcessEnv { + const scratchDirectory = this.scratchDirectory; + if (!scratchDirectory) return {}; + return this.platform === 'win32' + ? { + TMPDIR: scratchDirectory, + TEMP: scratchDirectory, + TMP: scratchDirectory, + } + : { TMPDIR: scratchDirectory }; + } + + private scratchSelectorEnvironment(): NodeJS.ProcessEnv { + const scratchDirectory = this.scratchDirectory; + if (!scratchDirectory) return {}; + return Object.fromEntries( + SRT_SCRATCH_SELECTOR_NAMES.map((name) => [name, scratchDirectory]), + ); + } + + private async removeScratchDirectory(): Promise { + const scratchDirectory = this.scratchDirectory; + if (!scratchDirectory) return; + try { + await rm(scratchDirectory, { recursive: true, force: true }); + } catch { + await this.restoreScratchTraversal(scratchDirectory); + await rm(scratchDirectory, { recursive: true, force: true }); + } + this.scratchDirectory = undefined; + } + + private async restoreScratchTraversal(root: string): Promise { + const pending = [root]; + for (let index = 0; index < pending.length; index += 1) { + const directory = pending[index]; + const metadata = await lstat(directory).catch( + (error: NodeJS.ErrnoException) => { + if (error.code === 'ENOENT') return undefined; + throw error; + }, + ); + if (!metadata?.isDirectory()) continue; + // Commands own their scratch contents and may remove all directory mode + // bits. Restore traversal before opening the directory with O_NOFOLLOW. + await chmod(directory, 0o700); + let handle; + try { + handle = await open( + directory, + fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW, + ); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (['ENOENT', 'ELOOP', 'ENOTDIR'].includes(code ?? '')) continue; + throw error; + } + try { + if (!(await handle.stat()).isDirectory()) continue; + } finally { + await handle.close(); + } + for (const entry of await readdir(directory, { withFileTypes: true })) { + if (entry.isDirectory()) pending.push(join(directory, entry.name)); + } + } + } + async close(): Promise { - if (!this.initialized) return; - await this.manager.reset(); + if (this.closing) return this.closing; + const closing = this.closeExclusive(); + this.closing = closing; + try { + await closing; + } finally { + this.closing = undefined; + } + } + + private async closeExclusive(): Promise { + // Never reset proxy/credential state or remove scratch beneath a live child. + await this.execution?.catch(() => undefined); + await this.initialized?.catch(() => undefined); + if (managerOwners.get(this.manager) === this) { + this.resetFailed = true; + await this.manager.reset(); + this.resetFailed = false; + managerOwners.delete(this.manager); + } this.initialized = undefined; this.canonicalRoot = undefined; + await this.removeScratchDirectory(); } } diff --git a/service/src/bridge/store.ts b/service/src/bridge/store.ts index 43634c7..2fa6733 100644 --- a/service/src/bridge/store.ts +++ b/service/src/bridge/store.ts @@ -573,6 +573,7 @@ export class RedisBridgeStore { requireTenantBinding?: boolean; request: WorkspaceToolRequest; deadlineAtMs: number; + executionTimeoutMs?: number; signal: AbortSignal; }): Promise { if (!isWorkspaceToolRequest(args.request)) { @@ -614,12 +615,20 @@ export class RedisBridgeStore { workspaceRequest?: WorkspaceToolRequest; runtimeSessionId?: string; deadlineAtMs: number; + executionTimeoutMs?: number; signal: AbortSignal; finalize?: ( settlement: CodeBridgeSettlement, registration: RegisteredBridgeWorker, ) => Promise; }): Promise { + if (args.executionTimeoutMs !== undefined && ( + args.workspaceRequest == null || + !Number.isSafeInteger(args.executionTimeoutMs) || + args.executionTimeoutMs < 1 || args.executionTimeoutMs > 305_000 + )) { + throw new BridgeStoreError('ASSIGNMENT_INVALID', 'Invalid workspace execution budget'); + } this.assertDispatchActive(args.signal, args.deadlineAtMs); const dispatchable = await this.dispatchCommand( () => this.dispatchableRegistration(args.workerId), @@ -682,7 +691,8 @@ export class RedisBridgeStore { const assignmentId = randomBytes(18).toString('base64url'); const leaseToken = randomBytes(32).toString('base64url'); - const ttlSeconds = assignmentTtlSeconds(args.deadlineAtMs); + // The lock is acquired before admission finishes; it must outlive the later execution deadline. + const ttlSeconds = assignmentTtlSeconds(args.deadlineAtMs + (args.executionTimeoutMs ?? 0)); const lockIncarnationId = registration.incarnationId; let assignment: StoredAssignment | undefined; let resultCommitted = false; @@ -748,6 +758,10 @@ export class RedisBridgeStore { throw new BridgeStoreError('WORKER_MISMATCH', 'Bridge worker capabilities changed while the request was waiting'); } } + this.assertDispatchActive(args.signal, args.deadlineAtMs); + if (args.executionTimeoutMs !== undefined) { + args = { ...args, deadlineAtMs: Date.now() + args.executionTimeoutMs }; + } const generation = await this.dispatchCommand( () => this.redis.incr(generationKey(args.workerId)), args, diff --git a/service/src/bridge/worker-admission.test.ts b/service/src/bridge/worker-admission.test.ts index 57ad3d8..db2e595 100644 --- a/service/src/bridge/worker-admission.test.ts +++ b/service/src/bridge/worker-admission.test.ts @@ -37,11 +37,13 @@ function dispatch( path: string, controller = new AbortController(), budgetMs = 5000, + executionTimeoutMs?: number, ): ReturnType { return store.dispatchWorkspaceTool({ workerId, signal: controller.signal, deadlineAtMs: Date.now() + budgetMs, + executionTimeoutMs, request: { protocolVersion: BRIDGE_PROTOCOL_VERSION, operation: 'read_file', @@ -111,7 +113,7 @@ test('an expired queued call never reaches the worker and does not strand later const first = dispatch('first'); const assignment = await store.lease(workerId, incarnationId, 1000); await expect( - dispatch('expired', new AbortController(), 25), + dispatch('expired', new AbortController(), 25, 1000), ).rejects.toMatchObject({ code: 'ASSIGNMENT_EXPIRED' }); const third = dispatch('third'); await settle(assignment); @@ -142,3 +144,29 @@ test('a queued request is rejected if the worker withdraws its capability', asyn await expect(second).rejects.toMatchObject({ code: 'WORKER_MISMATCH' }); expect(await store.lease(workerId, incarnationId, 50)).toBeUndefined(); }); + +test('execution receives a fresh budget after waiting and the lock covers long commands', async () => { + await register(); + const first = dispatch('first'); + const active = await store.lease(workerId, incarnationId, 1000); + const second = dispatch('second', new AbortController(), 1000, 305_000); + await new Promise(resolve => setTimeout(resolve, 150)); + await settle(active); + await first; + const next = await store.lease(workerId, incarnationId, 1000); + expect(next).toBeDefined(); + expect(Date.parse(next!.expiresAt) - Date.now()).toBeGreaterThan(304_000); + expect(await redis.pttl(`codeapi:bridge:v1:worker:${workerId}:lock`)).toBeGreaterThan(305_000); + await settle(next); + await second; +}); + +test('execution expires independently of an unused queue allowance', async () => { + await register(); + const completion = dispatch('short', new AbortController(), 5000, 150); + void completion.catch(() => undefined); + const assignment = await store.lease(workerId, incarnationId, 1000); + expect(assignment).toBeDefined(); + expect(Date.parse(assignment!.expiresAt) - Date.now()).toBeLessThanOrEqual(150); + await expect(completion).rejects.toMatchObject({ code: 'ASSIGNMENT_EXPIRED' }); +}); diff --git a/service/src/workspace-tools/router.test.ts b/service/src/workspace-tools/router.test.ts index b5c38ef..ae291a4 100644 --- a/service/src/workspace-tools/router.test.ts +++ b/service/src/workspace-tools/router.test.ts @@ -14,6 +14,7 @@ import { hostedAppPreviewGateway } from '../hosted-app/preview-gateway'; import { applyPrincipal } from '../auth/principal'; import { BridgeStoreError } from '../bridge/store'; import { bridgeStoreStatus, createWorkspaceToolsRouter } from './router'; +import type { WorkspaceToolRequest } from '../../../packages/code/src/protocol'; let server: Server | undefined; let logCompleted: ReturnType>; @@ -35,6 +36,46 @@ test('maps invalid worker results to an upstream failure', () => { expect(bridgeStoreStatus(new BridgeStoreError('WORKER_QUEUE_FULL', 'queue full'))).toBe(429); }); +test.each<[WorkspaceToolRequest, number, number?]>([ + [{ protocolVersion: 1, operation: 'read_file', workspaceId: 'primary', path: 'README.md' }, 30_000, undefined], + [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready' }, 35_000, undefined], + [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready', timeoutMs: 300_000 }, 305_000, undefined], + [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready' }, 6000, 1000], + [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready' }, 35_000, 600_000], +])('separates the admission deadline from execution budget for %j', async (request, expectedExecution, ceiling) => { + const app = express(); + app.use(json()); + app.use((req, _res, next) => { + applyPrincipal(req, { userId: 'user-1', tenantId: 'tenant-1', principalSource: 'librechat_jwt', codeWorkerId: 'user-worker' }); + next(); + }); + let executionBudget: number | undefined; + let queueRemaining: number | undefined; + let commandTimeout: number | undefined; + app.use(createWorkspaceToolsRouter({ + backend: 'remote-bridge', configuredWorkerId: 'user-worker', dynamicWorkers: false, + timeoutMs: ceiling, + store: { async dispatchWorkspaceTool(args) { + executionBudget = args.executionTimeoutMs; + if (args.request.operation === 'execute_command') commandTimeout = args.request.timeoutMs; + queueRemaining = args.deadlineAtMs - Date.now(); + return { protocolVersion: 1, generation: 1, leaseToken: 'lease', incarnationId: 'incarnation', status: 'rejected', error: 'fixture' }; + } }, + })); + 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('Missing listener'); + const response = await fetch(`http://127.0.0.1:${address.port}/workspace-tools/execute`, { + method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(request), + }); + await response.json(); + expect(executionBudget).toBe(expectedExecution); + if (request.operation === 'execute_command') expect(commandTimeout).toBe(expectedExecution - 5000); + expect(queueRemaining).toBeGreaterThan(29_000); + expect(queueRemaining).toBeLessThanOrEqual(30_000); +}); + test('rejects new workspace dispatches while the service is shutting down', async () => { let dispatched = false; const app = express(); @@ -167,7 +208,7 @@ test.each([ operation: 'search_text', workerId: 'user-worker', dispatchDurationMs: expect.any(Number), - deadlineBudgetMs: 30_000, + deadlineBudgetMs: 60_000, }), ); await expect(response.json()).resolves.toMatchObject({ @@ -330,7 +371,7 @@ test.each([ status: expectedStatus, errorCode, outcome: 'completed', - deadlineBudgetMs: 300_000, + deadlineBudgetMs: 60_000, dispatchDurationMs: expect.any(Number), }), ); diff --git a/service/src/workspace-tools/router.ts b/service/src/workspace-tools/router.ts index 93d1589..1708596 100644 --- a/service/src/workspace-tools/router.ts +++ b/service/src/workspace-tools/router.ts @@ -3,12 +3,16 @@ import { Router } from 'express'; import type { RequestHandler, Response } from 'express'; import type { AuthenticatedRequest } from '../types'; import type { RedisBridgeStore } from '../bridge/store'; +import type { WorkspaceToolRequest } from '../../../packages/code/src/protocol'; import { getWorkspaceToolOutcome } from './outcome'; import { getPrincipalOrReject } from '../auth/principal'; import { BridgeStoreError } from '../bridge/store'; import { checkServiceShutDown } from '../lifecycle'; -import { isWorkspaceToolRequest } from '../../../packages/code/src/protocol'; +import { + isWorkspaceToolRequest, + BRIDGE_WORKSPACE_COMMAND_DEFAULT_TIMEOUT_MS, +} from '../../../packages/code/src/protocol'; import { CODEAPI_BRIDGE_WORKER_HEADER, BridgeWorkerSelectionError, @@ -21,6 +25,7 @@ interface WorkspaceToolsRouterOptions { configuredWorkerId: string; dynamicWorkers: boolean; timeoutMs?: number; + queueTimeoutMs?: number; isShuttingDown?: () => boolean; } @@ -43,14 +48,21 @@ export function bridgeStoreStatus(error: BridgeStoreError): number { } export function createWorkspaceToolsRouter(options: WorkspaceToolsRouterOptions): Router { + const queueBudgetMs = options.queueTimeoutMs ?? 30_000; + if (!Number.isSafeInteger(queueBudgetMs) || queueBudgetMs < 1 || queueBudgetMs > 30_000) { + throw new RangeError('Workspace queue timeout must be between 1 and 30000 milliseconds'); + } + if (options.timeoutMs !== undefined && ( + !Number.isSafeInteger(options.timeoutMs) || options.timeoutMs < 1 + )) { + throw new RangeError('Workspace execution timeout must be a positive safe integer'); + } const router = Router(); router.post( '/workspace-tools/execute', asyncRoute(async (req, res) => { const outcome = getWorkspaceToolOutcome(res); - const deadlineBudgetMs = Math.max(1, options.timeoutMs ?? 30_000); - outcome.deadlineBudgetMs = deadlineBudgetMs; const principal = getPrincipalOrReject(req, res); if (!principal) { outcome.errorCode = 'UNAUTHENTICATED'; @@ -69,6 +81,16 @@ export function createWorkspaceToolsRouter(options: WorkspaceToolsRouterOptions) return; } outcome.operation = req.body.operation; + const request: WorkspaceToolRequest = req.body.operation === 'execute_command' + ? { ...req.body, timeoutMs: Math.min( + req.body.timeoutMs ?? BRIDGE_WORKSPACE_COMMAND_DEFAULT_TIMEOUT_MS, + options.timeoutMs ?? Number.MAX_SAFE_INTEGER, + ) } + : req.body; + const executionBudgetMs = request.operation === 'execute_command' + ? request.timeoutMs! + 5_000 + : Math.min(options.timeoutMs ?? 30_000, 30_000); + outcome.deadlineBudgetMs = queueBudgetMs + executionBudgetMs; let selection: { workerId: string; explicit: boolean } | undefined; try { @@ -111,8 +133,9 @@ export function createWorkspaceToolsRouter(options: WorkspaceToolsRouterOptions) tenantId: principal.tenantId, requireTenantBinding: selection.explicit && (options.dynamicWorkers || selection.workerId !== options.configuredWorkerId), - request: req.body, - deadlineAtMs: Date.now() + deadlineBudgetMs, + request, + deadlineAtMs: Date.now() + queueBudgetMs, + executionTimeoutMs: executionBudgetMs, signal: controller.signal, }).finally(() => { outcome.dispatchDurationMs = Math.round(performance.now() - dispatchStartedAt);