From 10ac19b74f87ec2ff4c9ff7e81198cc1f6b5e830 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 14 Sep 2026 15:51:48 -0400 Subject: [PATCH 1/4] fix: isolate native PTC readiness and watchdog phases (#208) --- packages/code/src/native-process-child.ts | 89 ++++++-- packages/code/src/native-process.test.ts | 234 +++++++++++++++++++++- packages/code/src/native-process.ts | 140 ++++++++++--- packages/code/src/native-programmatic.ts | 15 +- packages/code/src/native-sandbox.ts | 17 +- 5 files changed, 444 insertions(+), 51 deletions(-) diff --git a/packages/code/src/native-process-child.ts b/packages/code/src/native-process-child.ts index 2e8beb38..c3b9614b 100644 --- a/packages/code/src/native-process-child.ts +++ b/packages/code/src/native-process-child.ts @@ -11,7 +11,12 @@ import type { // argv credentials, bridge token, or persisted pairing material is required. let sandbox: NativeSrtWorkspaceCommandSandbox | undefined; let programmaticExecutor: NativeWorkspaceProgrammaticExecutor | undefined; +let programmaticReady: Promise | undefined; +let programmaticFileUpstream: string | undefined; let active: { id: string; controller: AbortController } | undefined; +let commitAcknowledgement: + | { id: string; acknowledge(): void } + | undefined; let busy = false; let credentials: Record = {}; let wrappedCommand: string | undefined; @@ -25,6 +30,33 @@ function reply(message: object): void { /* Parent was lost. */ } } +async function awaitCommitAcknowledgement( + id: string, + signal: AbortSignal, +): Promise { + await new Promise((resolve, reject) => { + const abort = () => { + commitAcknowledgement = undefined; + reject( + new WorkspaceToolError( + 'Programmatic execution aborted before commit', + 'EXECUTION_ABORTED', + ), + ); + }; + commitAcknowledgement = { + id, + acknowledge() { + signal.removeEventListener('abort', abort); + commitAcknowledgement = undefined; + resolve(); + }, + }; + signal.addEventListener('abort', abort, { once: true }); + reply({ id, phase: 'commit' }); + if (signal.aborted) abort(); + }); +} let shuttingDown = false; const shutdown = () => { if (shuttingDown) return; @@ -59,19 +91,29 @@ process.on('message', async (raw: unknown) => { workspaceId?: string; credentials?: Record; wrappedCommand?: string; + programmaticShellPath?: string; + programmaticJqPath?: string; }; if (!message || typeof message.id !== 'string') return; if (message.type === 'cancel') { if (active?.id === message.id) active.controller.abort(); return; } + if (message.type === 'commit-ack') { + if (commitAcknowledgement?.id === message.id) { + commitAcknowledgement.acknowledge(); + } + return; + } if (busy) return; busy = true; + let mutationStarted = false; try { let result: unknown; if (message.type === 'prepare' && !sandbox) { - const { variables, programmaticFileUpstream, ...options } = + const { variables, programmaticFileUpstream: upstream, ...options } = message.options; + programmaticFileUpstream = upstream; sandbox = new NativeSrtWorkspaceCommandSandbox({ ...options, ...(variables @@ -89,32 +131,55 @@ process.on('message', async (raw: unknown) => { : {}), }); await sandbox.prepare(); - programmaticExecutor = programmaticFileUpstream - ? new NativeWorkspaceProgrammaticExecutor({ - sandbox, - upstreamUrl: programmaticFileUpstream, - }) - : undefined; - await programmaticExecutor?.prepare(); } else if (message.type === 'execute' && sandbox) { active = { id: message.id, controller: new AbortController() }; credentials = message.credentials ?? {}; wrappedCommand = message.wrappedCommand; + mutationStarted = true; result = await sandbox.execute(message.request, active.controller.signal); } else if ( message.type === 'programmatic' && sandbox && - programmaticExecutor && + programmaticFileUpstream && message.programmaticRequest && - typeof message.workspaceId === 'string' + typeof message.workspaceId === 'string' && + typeof message.programmaticShellPath === 'string' && + typeof message.programmaticJqPath === 'string' ) { active = { id: message.id, controller: new AbortController() }; credentials = message.credentials ?? {}; wrappedCommand = message.wrappedCommand; + if (!programmaticExecutor) { + programmaticExecutor = new NativeWorkspaceProgrammaticExecutor({ + sandbox, + upstreamUrl: programmaticFileUpstream, + shellPath: message.programmaticShellPath, + jqPath: message.programmaticJqPath, + }); + programmaticReady = programmaticExecutor.prepare( + active.controller.signal, + ); + } + try { + await programmaticReady; + } catch (error) { + programmaticExecutor = undefined; + programmaticReady = undefined; + throw error; + } result = await programmaticExecutor.execute( message.programmaticRequest, message.workspaceId, active.controller.signal, + { + async beforeCommit() { + await awaitCommitAcknowledgement( + message.id, + active!.controller.signal, + ); + mutationStarted = true; + }, + }, ); } else if (message.type === 'close' && sandbox) { await sandbox.close(); @@ -134,11 +199,11 @@ process.on('message', async (raw: unknown) => { mutation: error instanceof WorkspaceToolError ? error.mutationMayHaveCommitted - : true, + : mutationStarted, requiresQuarantine: error instanceof WorkspaceToolError ? error.requiresQuarantine - : true, + : mutationStarted, }); } finally { active = undefined; diff --git a/packages/code/src/native-process.test.ts b/packages/code/src/native-process.test.ts index 2f38d642..89651845 100644 --- a/packages/code/src/native-process.test.ts +++ b/packages/code/src/native-process.test.ts @@ -11,6 +11,7 @@ import { trustedProgrammaticExecutable, } from './native-process.js'; import { WorkspaceToolError } from './workspace.js'; +import { BRIDGE_WORKSPACE_COMMAND_MAX_TIMEOUT_MS } from './protocol.js'; test('preflight rejects relative and workspace-controlled executables including symlinks', async t => { const root = await mkdtemp(join(tmpdir(), 'native-ptc-path-')); @@ -99,6 +100,24 @@ function fixture( }; } +class ObservedWatchdogSandbox extends NativeProcessWorkspaceCommandSandbox { + readonly watchdogTimeouts: number[] = []; + readonly watchdogCallbacks: Array<() => void> = []; + + protected override scheduleRpcTimeout( + callback: () => void, + timeoutMs: number, + ): ReturnType { + this.watchdogTimeouts.push(timeoutMs); + this.watchdogCallbacks.push(callback); + return super.scheduleRpcTimeout(callback, timeoutMs); + } + + fireLatestWatchdog(): void { + this.watchdogCallbacks.at(-1)?.(); + } +} + test('executor bootstrap excludes bridge credentials and Node injection variables', async () => { assert.deepEqual( nativeExecutorEnvironment({ @@ -229,13 +248,12 @@ test('programmatic executor resolves and scopes credentials to its command', asy const message = fake.messages.find( candidate => candidate.type === 'programmatic', )!; - const prepareMessage = fake.messages.find( - candidate => candidate.type === 'prepare', - )!; - assert.equal(typeof prepareMessage.options.jqPath, 'string'); - assert.equal(prepareMessage.options.jqPath.startsWith('/'), true); + assert.equal(typeof message.programmaticShellPath, 'string'); + assert.equal(message.programmaticShellPath.startsWith('/'), true); + assert.equal(typeof message.programmaticJqPath, 'string'); + assert.equal(message.programmaticJqPath.startsWith('/'), true); assert.equal( - '/sandbox-only'.split(':').includes(dirname(prepareMessage.options.jqPath)), + '/sandbox-only'.split(':').includes(dirname(message.programmaticJqPath)), false, ); assert.deepEqual(message.credentials, { TOKEN: 'per-programmatic-secret' }); @@ -246,8 +264,109 @@ test('programmatic executor resolves and scopes credentials to its command', asy await sandbox.close(); }); +test('omitted PTC timeout gives the commit watchdog the protocol execution default', async () => { + const fake = fixture((child, message) => { + if (message.type !== 'programmatic') return; + child.emit('message', { id: message.id, phase: 'commit' }); + child.emit('message', { id: message.id, ok: true, result: {} }); + }); + const sandbox = new ObservedWatchdogSandbox( + { + workspaceRoot: tmpdir(), + programmaticFileUpstream: 'http://127.0.0.1:3190', + }, + fake.fork, + ); + + await sandbox.executeProgrammatic('primary', { + headers: {}, + body: { + language: 'bash', + version: '5.2.0', + session_id: 'session', + replay_tool_count: 0, + max_output_files: 0, + files: [{ name: 'main.sh', content: 'sleep 45' }], + }, + }); + assert.ok( + sandbox.watchdogTimeouts.at(-1)! > + BRIDGE_WORKSPACE_COMMAND_MAX_TIMEOUT_MS, + ); + await sandbox.close(); +}); + +test('PTC watchdog budgets staging separately and resets when commit begins', async () => { + const fake = fixture((child, message) => { + if (message.type !== 'programmatic') return; + child.emit('message', { id: message.id, phase: 'commit' }); + child.emit('message', { id: message.id, ok: true, result: {} }); + }); + const sandbox = new ObservedWatchdogSandbox( + { + workspaceRoot: tmpdir(), + programmaticFileUpstream: 'http://127.0.0.1:3190', + }, + fake.fork, + ); + + await sandbox.executeProgrammatic('primary', { + headers: {}, + body: { + language: 'bash', + version: '5.2.0', + session_id: 'session', + run_timeout: 1_000, + replay_tool_count: 0, + max_output_files: 0, + files: [{ name: 'main.sh', content: 'echo ready' }], + }, + }); + assert.deepEqual(sandbox.watchdogTimeouts.slice(-2), [65_000, 6_000]); + assert.ok( + fake.messages.some(message => message.type === 'commit-ack'), + 'the child must not enter the mutating phase before the parent arms it', + ); + await sandbox.close(); +}); + +test('PTC-only preflight failures do not disable ordinary native commands', async () => { + const fake = fixture(); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { + workspaceRoot: '/workspace', + shellPath: '/definitely/missing/bash', + programmaticFileUpstream: 'http://127.0.0.1:3190', + }, + fake.fork, + ); + + assert.deepEqual(await sandbox.execute(request), result); + await assert.rejects( + sandbox.executeProgrammatic('primary', { + headers: {}, + body: { + language: 'bash', + version: '5.2.0', + session_id: 'session', + files: [{ name: 'main.sh', content: 'echo ready' }], + }, + }), + (error: unknown) => + error instanceof WorkspaceToolError && + error.code === 'COMMAND_UNAVAILABLE' && + !error.mutationMayHaveCommitted, + ); + assert.deepEqual(await sandbox.execute(request), result); + await sandbox.close(); +}); + test('programmatic executor preserves a child-reported pre-dispatch failure', async () => { - const fake = fixture((child, message) => + const fake = fixture((child, message) => { + if (message.type !== 'programmatic') { + child.emit('message', { id: message.id, ok: true, result }); + return; + } child.emit('message', { id: message.id, ok: false, @@ -255,8 +374,39 @@ test('programmatic executor preserves a child-reported pre-dispatch failure', as errorMessage: 'Programmatic input download failed', mutation: false, requiresQuarantine: false, + }); + }); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { + workspaceRoot: tmpdir(), + programmaticFileUpstream: 'http://127.0.0.1:3190', + }, + fake.fork, + ); + + await assert.rejects( + sandbox.executeProgrammatic('primary', { + headers: {}, + body: { + language: 'bash', + version: '5.2.0', + session_id: 'session', + files: [{ name: 'main.sh', content: 'echo ready' }], + }, }), + (error: unknown) => + error instanceof WorkspaceToolError && + !error.mutationMayHaveCommitted && + !error.requiresQuarantine, ); + assert.deepEqual(await sandbox.execute(request), result); + await sandbox.close(); +}); + +test('executor loss during programmatic staging is not an uncertain workspace mutation', async () => { + const fake = fixture((child, message) => { + if (message.type === 'programmatic') child.emit('exit', 1); + }); const sandbox = new NativeProcessWorkspaceCommandSandbox( { workspaceRoot: tmpdir(), @@ -283,6 +433,76 @@ test('programmatic executor preserves a child-reported pre-dispatch failure', as await sandbox.close(); }); +test('programmatic staging watchdog expires without claiming a workspace mutation', async () => { + let staged!: () => void; + const staging = new Promise(resolve => { + staged = resolve; + }); + const fake = fixture((_child, message) => { + if (message.type === 'programmatic') staged(); + }); + const sandbox = new ObservedWatchdogSandbox( + { + workspaceRoot: tmpdir(), + programmaticFileUpstream: 'http://127.0.0.1:3190', + }, + fake.fork, + ); + const execution = sandbox.executeProgrammatic('primary', { + headers: {}, + body: { + language: 'bash', + version: '5.2.0', + session_id: 'session', + files: [{ name: 'main.sh', content: 'echo ready' }], + }, + }); + await staging; + sandbox.fireLatestWatchdog(); + + await assert.rejects( + execution, + (error: unknown) => + error instanceof WorkspaceToolError && + !error.mutationMayHaveCommitted && + !error.requiresQuarantine, + ); + assert.equal(fake.killCalls, 1); + await sandbox.close(); +}); + +test('executor loss after programmatic commit starts remains an uncertain mutation', async () => { + const fake = fixture((child, message) => { + if (message.type !== 'programmatic') return; + child.emit('message', { id: message.id, phase: 'commit' }); + child.emit('exit', 1); + }); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { + workspaceRoot: tmpdir(), + programmaticFileUpstream: 'http://127.0.0.1:3190', + }, + fake.fork, + ); + + await assert.rejects( + sandbox.executeProgrammatic('primary', { + headers: {}, + body: { + language: 'bash', + version: '5.2.0', + session_id: 'session', + files: [{ name: 'main.sh', content: 'echo ready' }], + }, + }), + (error: unknown) => + error instanceof WorkspaceToolError && + error.mutationMayHaveCommitted && + error.requiresQuarantine, + ); + 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( diff --git a/packages/code/src/native-process.ts b/packages/code/src/native-process.ts index 4ed3ffec..81bbd9c4 100644 --- a/packages/code/src/native-process.ts +++ b/packages/code/src/native-process.ts @@ -7,6 +7,7 @@ import { promisify } from 'node:util'; import { WorkspaceToolError } from './workspace.js'; import { NATIVE_PROGRAMMATIC_COMMAND } from './native-programmatic.js'; import { + BRIDGE_WORKSPACE_COMMAND_MAX_TIMEOUT_MS, BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_FILES, isWorkspaceToolRequest, isWorkspaceToolResult, @@ -29,6 +30,13 @@ export type NativeProcessSandboxOptions = Omit< }; const execFileAsync = promisify(execFile); +const PROGRAMMATIC_STAGING_TIMEOUT_MS = 60_000; +const PROGRAMMATIC_TRANSFER_TIMEOUT_MS = 30_000; +const RPC_SETTLEMENT_SLACK_MS = 5_000; + +type RpcTimeoutBudget = + | number + | { stagingMs: number; commitMs: number }; async function systemProgrammaticExecutable( name: string, @@ -183,11 +191,16 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan private closing?: Promise; private failed = false; private terminationTimer?: ReturnType; + private programmaticExecutables?: Promise<{ + shellPath: string; + jqPath: string; + }>; private pending?: { id: string; resolve(value: unknown): void; reject(error: Error): void; mutation: boolean; + commit?(): void; }; constructor( @@ -199,6 +212,14 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan ) => ChildProcess = fork, ) {} + /** Overridable only for deterministic watchdog tests. */ + protected scheduleRpcTimeout( + callback: () => void, + timeoutMs: number, + ): ReturnType { + return setTimeout(callback, timeoutMs); + } + async prepare(): Promise { if (this.failed || this.closing) throw this.unavailable(false); if (this.ready) return this.ready; @@ -210,10 +231,22 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan return new NativeExecutorUnavailableError(mutation); } + private async resolveProgrammaticExecutables(): Promise<{ + shellPath: string; + jqPath: string; + }> { + this.programmaticExecutables ??= resolveProgrammaticShell(this.options); + try { + return await this.programmaticExecutables; + } catch (error) { + // An operator may install or repair this optional dependency while the + // worker stays online. Keep ordinary execution live and let PTC retry. + this.programmaticExecutables = undefined; + throw error; + } + } + private async start(): Promise { - const programmaticExecutables = this.options.programmaticFileUpstream - ? await resolveProgrammaticShell(this.options) - : undefined; const child = this.forkExecutor( new URL('./native-process-child.js', import.meta.url), [], @@ -237,6 +270,7 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan code?: unknown; errorMessage?: unknown; fatal?: unknown; + phase?: unknown; }; if ( !message || @@ -246,6 +280,25 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan return; const pending = this.pending; if (!pending) return; + if (message.phase === 'commit') { + pending.mutation = true; + const commit = pending.commit; + pending.commit = undefined; + commit?.(); + try { + child.send({ type: 'commit-ack', id: pending.id }, error => { + if (!error) return; + this.failed = true; + this.terminate(); + pending.reject(this.unavailable(true)); + }); + } catch { + this.failed = true; + this.terminate(); + pending.reject(this.unavailable(true)); + } + return; + } if (message.fatal === true) this.failed = true; if (message.ok === true) pending.resolve(message.result); else { @@ -299,8 +352,7 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan protectedPaths, allowedDomains, homeDirectory, - shellPath: programmaticExecutables?.shellPath ?? shellPath, - jqPath: programmaticExecutables?.jqPath, + shellPath, programmaticFileUpstream, variables: this.options.maskedEnvironment?.variables, }, @@ -376,9 +428,12 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan ); let credentials: Record | undefined; let wrappedCommand: string | undefined; + let programmaticExecutables: { shellPath: string; jqPath: string }; try { await this.prepare(); if (signal?.aborted) throw new Error('aborted'); + programmaticExecutables = await this.resolveProgrammaticExecutables(); + 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?.( @@ -407,19 +462,11 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan workspaceId, credentials, wrappedCommand, + programmaticShellPath: programmaticExecutables.shellPath, + programmaticJqPath: programmaticExecutables.jqPath, }, - (request.body.run_timeout ?? 30_000) * - ((request.body.replay_tool_count ?? 0) > 0 ? 2 : 1) + - (Math.ceil( - request.body.files.filter(file => 'id' in file).length / 4, - ) + - Math.ceil( - (request.body.max_output_files ?? - BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_FILES) / 4, - )) * - (request.body.transfer_timeout_ms ?? 30_000) + - 5_000, - true, + this.programmaticWatchdogBudget(request), + false, signal, ); if (signal?.aborted) { @@ -437,6 +484,35 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan return result; } + private programmaticWatchdogBudget( + request: BridgeWorkspaceProgrammaticRequest, + ): Exclude { + const runTimeoutMs = Math.min( + request.body.run_timeout ?? BRIDGE_WORKSPACE_COMMAND_MAX_TIMEOUT_MS, + BRIDGE_WORKSPACE_COMMAND_MAX_TIMEOUT_MS, + ); + const transferTimeoutMs = + request.body.transfer_timeout_ms ?? PROGRAMMATIC_TRANSFER_TIMEOUT_MS; + const inputBatches = Math.ceil( + request.body.files.filter(file => 'id' in file).length / 4, + ); + const outputBatches = Math.ceil( + (request.body.max_output_files ?? + BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_FILES) / 4, + ); + return { + stagingMs: + PROGRAMMATIC_STAGING_TIMEOUT_MS + + inputBatches * transferTimeoutMs + + ((request.body.replay_tool_count ?? 0) > 0 ? runTimeoutMs : 0) + + RPC_SETTLEMENT_SLACK_MS, + commitMs: + runTimeoutMs + + outputBatches * transferTimeoutMs + + RPC_SETTLEMENT_SLACK_MS, + }; + } + private async executeOnce( request: WorkspaceExecuteCommandRequest, signal?: AbortSignal, @@ -498,7 +574,7 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan private async rpc( type: string, payload: object, - timeoutMs: number, + timeout: RpcTimeoutBudget, mutation: boolean, signal?: AbortSignal, ): Promise { @@ -506,7 +582,7 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan throw this.unavailable(false); const id = randomUUID(); const child = this.child; - let timer: ReturnType; + let timer: ReturnType | undefined; const abort = () => { try { if (child.connected) @@ -518,12 +594,24 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan }; 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); + const schedule = (timeoutMs: number): void => { + if (timer) clearTimeout(timer); + timer = this.scheduleRpcTimeout(() => { + this.failed = true; + this.terminate(); + reject(this.unavailable(this.pending?.mutation ?? mutation)); + }, timeoutMs); + }; + this.pending = { + id, + resolve, + reject, + mutation, + ...(typeof timeout === 'number' + ? {} + : { commit: () => schedule(timeout.commitMs) }), + }; + schedule(typeof timeout === 'number' ? timeout : timeout.stagingMs); signal?.addEventListener('abort', abort, { once: true }); const sendFailed = () => { this.failed = true; @@ -540,7 +628,7 @@ export class NativeProcessWorkspaceCommandSandbox implements WorkspaceCommandSan if (signal?.aborted) abort(); }); } finally { - clearTimeout(timer!); + if (timer) clearTimeout(timer); signal?.removeEventListener('abort', abort); this.pending = undefined; } diff --git a/packages/code/src/native-programmatic.ts b/packages/code/src/native-programmatic.ts index bd2700f2..9b6eea9e 100644 --- a/packages/code/src/native-programmatic.ts +++ b/packages/code/src/native-programmatic.ts @@ -216,6 +216,8 @@ export interface NativeWorkspaceProgrammaticOptions { Pick >; upstreamUrl: string; + shellPath?: string; + jqPath?: string; fetchImpl?: typeof fetch; } @@ -362,6 +364,7 @@ export class NativeWorkspaceProgrammaticExecutor { request: BridgeWorkspaceProgrammaticRequest, workspaceId: string, signal?: AbortSignal, + lifecycle?: { beforeCommit?(): Promise | void }, ): Promise { if (!isBridgeWorkspaceProgrammaticRequest(request)) { throw new WorkspaceToolError( @@ -456,7 +459,10 @@ export class NativeWorkspaceProgrammaticExecutor { errorOnExist: true, mode: constants.COPYFILE_FICLONE, }); - if (!probe) commandDispatched = true; + if (!probe) { + await lifecycle?.beforeCommit?.(); + commandDispatched = true; + } return await this.options.sandbox.executeProgrammatic( { protocolVersion: BRIDGE_PROTOCOL_VERSION, @@ -473,7 +479,12 @@ export class NativeWorkspaceProgrammaticExecutor { }, directory, signal, - { probe, workspaceRoot }, + { + probe, + workspaceRoot, + shellPath: this.options.shellPath, + jqPath: this.options.jqPath, + }, ); }; diff --git a/packages/code/src/native-sandbox.ts b/packages/code/src/native-sandbox.ts index 91e9832d..550d0d52 100644 --- a/packages/code/src/native-sandbox.ts +++ b/packages/code/src/native-sandbox.ts @@ -661,7 +661,12 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox request: WorkspaceExecuteCommandRequest, dataDirectory: string, signal?: AbortSignal, - options?: { probe?: boolean; workspaceRoot?: string }, + options?: { + probe?: boolean; + workspaceRoot?: string; + shellPath?: string; + jqPath?: string; + }, ): Promise { if (this.execution || this.closing) { throw new WorkspaceToolError( @@ -703,9 +708,13 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox canonicalDataDirectory, '_ptc_pending_result.json', ), - LIBRECHAT_CODE_BASH_PATH: this.options.shellPath ?? '/bin/bash', - ...(this.options.jqPath - ? { LIBRECHAT_CODE_JQ_PATH: this.options.jqPath } + LIBRECHAT_CODE_BASH_PATH: + options?.shellPath ?? this.options.shellPath ?? '/bin/bash', + ...((options?.jqPath ?? this.options.jqPath) + ? { + LIBRECHAT_CODE_JQ_PATH: + options?.jqPath ?? this.options.jqPath, + } : {}), PTC_HISTORY_PATH: join( canonicalDataDirectory, From 3a2c2a0a974b3b01c1c509e40cc414f75b23faab Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 14 Sep 2026 18:30:33 -0400 Subject: [PATCH 2/4] feat: Declare Named Worker Project Environments (#209) * feat: declare named worker project environments * fix: preserve environment trust and negotiated action boundaries * Harden environment loading and executor identity * Protect environment root traversal and exact config bytes * Reject self-controlled environment root aliases * Check filesystem identities at environment trust boundaries * Validate environment containment across Linux mount aliases * Handle stacked mounts conservatively without blocking unrelated paths --- packages/code/README.md | 52 +++ packages/code/package-lock.json | 18 +- packages/code/package.json | 3 +- packages/code/src/cli.ts | 249 +++++++++--- packages/code/src/environment-live.test.ts | 117 ++++++ packages/code/src/environment-mount.test.ts | 73 ++++ packages/code/src/environment-mount.ts | 123 ++++++ packages/code/src/environment.test.ts | 348 +++++++++++++++++ packages/code/src/environment.ts | 399 ++++++++++++++++++++ packages/code/src/private-storage.ts | 10 +- packages/code/src/protocol.ts | 239 ++++++++++-- packages/code/src/worker.ts | 21 +- packages/code/src/workspace-worker.test.ts | 50 +++ packages/code/src/workspace.ts | 3 + 14 files changed, 1617 insertions(+), 88 deletions(-) create mode 100644 packages/code/src/environment-live.test.ts create mode 100644 packages/code/src/environment-mount.test.ts create mode 100644 packages/code/src/environment-mount.ts create mode 100644 packages/code/src/environment.test.ts create mode 100644 packages/code/src/environment.ts diff --git a/packages/code/README.md b/packages/code/README.md index 87d9dded..c3bb16d3 100644 --- a/packages/code/README.md +++ b/packages/code/README.md @@ -652,3 +652,55 @@ To recover a quarantined native root: The workspace selector in LibreChat must preserve these registered IDs. Adding roots here does not grant a principal access or change an agent's selected root. +# Named project environments + +An operator can keep a project definition outside the coding workspace and start +the worker with `librechat-code run --environment /operator/app.yaml +--allow-workspace-commands --allow-workspace-writes`. Existing pairing settings +still identify the machine and its principal. Repeat `--environment` for independent, +non-overlapping roots (up to 32). Do not combine definitions with workspace directory, +ID, or name flags or environment variables. + +```yaml +name: app-dev +root: /projects/app +repo: example/app +ref: main +setup: + command: npm ci + timeoutMs: 300000 +actions: + - name: typecheck + command: npm run typecheck + timeoutMs: 120000 +``` + +The root must already exist; relative roots resolve from the YAML file's directory. +Repository and ref are descriptive metadata, not a clone or checkout instruction. +No Git repository is required. Definitions are loaded once at startup, hashed into +the worker's policy identity, and protected from sandbox writes. All definition +files must be outside every registered root. Unknown fields are rejected. +On Linux, startup also verifies the mount namespace so bind mounts cannot expose +definitions or their controlling paths through a workspace. The mount table is +bounded to 4 MiB, with at most 256 exposed mount boundaries; stacked and hidden +mount mappings are considered conservatively. Operators must keep mount topology stable while the +worker runs. This inspection happens at startup, not on the command hot path. + +Setup is an operator-authorized startup command under the configured native sandbox +policy. It requires commands to be enabled, runs once per worker startup before +registration, and must be idempotent for restarts. Its timeout is bounded to five +minutes and captured output to 8 KiB. Setup failure prevents registration. A crash +or uncertain termination retains the existing workspace quarantine marker; inspect +the workspace before clearing quarantine. No setup output is sent to the model. + +Named actions are fixed commands without model-supplied substitution. The bridge +advertises only their names and the definition fingerprint, never their shell source +or host root. A command request can select `environmentAction: { name, fingerprint }`; +the worker resolves the command from its loaded definition and rejects stale revisions, +unknown names, other roots, or a changed working directory. Actions use ordinary +command authorization, queueing, cancellation and quarantine. They never override +deployment approval rules or expand the pairing's principal scope. + +Rollout: update Code API and the LibreChat environment-descriptor consumer before +enabling this opt-in flag on a worker. Older validators reject the additional metadata. +Existing workers without `--environment` continue to use their existing registration. diff --git a/packages/code/package-lock.json b/packages/code/package-lock.json index ac9affc8..426b950f 100644 --- a/packages/code/package-lock.json +++ b/packages/code/package-lock.json @@ -10,7 +10,8 @@ "license": "Apache-2.0", "dependencies": { "@anthropic-ai/sandbox-runtime": "0.0.75", - "koffi": "3.2.1" + "koffi": "3.2.1", + "yaml": "2.9.1" }, "bin": { "librechat-code": "dist/cli.js" @@ -414,6 +415,21 @@ "dev": true, "license": "MIT" }, + "node_modules/yaml": { + "version": "2.9.1", + "resolved": "https://registry.npmjs.org/yaml/-/yaml-2.9.1.tgz", + "integrity": "sha512-3NxN8+78OdzbT7C/WjGsyfPAtJaN3FNDsWxv7Y7mcDsT/oOmgW8BpyQQFFBnvZE3j9Y2Sdz1ULFLezL7Eb2yFw==", + "license": "ISC", + "bin": { + "yaml": "bin.mjs" + }, + "engines": { + "node": ">= 14.6" + }, + "funding": { + "url": "https://github.com/sponsors/eemeli" + } + }, "node_modules/zod": { "version": "3.25.76", "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", diff --git a/packages/code/package.json b/packages/code/package.json index 452a8be7..b00ca807 100644 --- a/packages/code/package.json +++ b/packages/code/package.json @@ -65,6 +65,7 @@ }, "dependencies": { "@anthropic-ai/sandbox-runtime": "0.0.75", - "koffi": "3.2.1" + "koffi": "3.2.1", + "yaml": "2.9.1" } } diff --git a/packages/code/src/cli.ts b/packages/code/src/cli.ts index 289b0dc4..5498af63 100644 --- a/packages/code/src/cli.ts +++ b/packages/code/src/cli.ts @@ -5,6 +5,11 @@ import { realpath, stat } from 'node:fs/promises'; import { basename, resolve, relative, isAbsolute, sep } from 'node:path'; import { pairBridgeWorker } from './pairing.js'; +import { + loadCodeEnvironment, + assertEnvironmentDefinitionsOutsideRoots, + EnvironmentWorkspaceTools, +} from './environment.js'; import { startFileRelay } from './relay.js'; import { DockerFileRelaySupervisor } from './relay-runtime.js'; import { @@ -61,7 +66,8 @@ function workspaceSecurityIdentity( configuredToken: string | undefined, ): string { return ( - pairedPublicKey ?? required('LIBRECHAT_CODE_WORKER_TOKEN', configuredToken) + pairedPublicKey ?? + required('LIBRECHAT_CODE_WORKER_TOKEN', configuredToken) ); } @@ -70,7 +76,8 @@ function workspaceQuarantinePath(options: { workerId: string; workspaceRoot?: string; }): string { - const override = process.env.LIBRECHAT_CODE_WORKSPACE_QUARANTINE_FILE?.trim(); + const override = + process.env.LIBRECHAT_CODE_WORKSPACE_QUARANTINE_FILE?.trim(); if (override) return override; return defaultWorkspaceQuarantinePath({ ...options, @@ -88,7 +95,7 @@ function list(value: string | undefined): string[] { return ( value ?.split(',') - .map((item) => item.trim()) + .map(item => item.trim()) .filter(Boolean) ?? [] ); } @@ -127,7 +134,7 @@ function option(args: string[], name: string): string | undefined { const index = args.indexOf(name); if (index >= 0) return args[index + 1]; return args - .find((value) => value.startsWith(`${name}=`)) + .find(value => value.startsWith(`${name}=`)) ?.slice(name.length + 1); } @@ -175,7 +182,9 @@ function githubCredentials(): { try { parsedApiUrl = new URL(apiUrl); } catch { - throw new Error('LIBRECHAT_CODE_GITHUB_API_URL must be a valid URL'); + throw new Error( + 'LIBRECHAT_CODE_GITHUB_API_URL must be a valid URL', + ); } apiHost = parsedApiUrl.hostname.toLowerCase() === 'api.github.com' @@ -288,7 +297,7 @@ async function relay(): Promise { process.stdout.write( `librechat-code: file relay listening at ${handle.url}\n`, ); - await new Promise((resolve) => { + await new Promise(resolve => { process.once('SIGINT', resolve); process.once('SIGTERM', resolve); }); @@ -299,6 +308,47 @@ async function run( runtimeSessionId?: string, args: string[] = [], ): Promise { + const environmentPaths: string[] = []; + for (let i = 0; i < args.length; i++) { + if (args[i] === '--environment') { + const path = args[++i]; + if (!path || path.startsWith('--')) + throw new Error('--environment requires a YAML file'); + environmentPaths.push(path); + } else if (args[i].startsWith('--environment=')) { + const path = args[i].slice('--environment='.length); + if (!path) throw new Error('--environment requires a YAML file'); + environmentPaths.push(path); + } + } + if (environmentPaths.length > 32) + throw new Error('At most 32 environments may be registered'); + const environments = await Promise.all( + environmentPaths.map(loadCodeEnvironment), + ); + if ( + environments.length && + (runtimeSessionId != null || + args.some(arg => + [ + '--worker-dir', + '--default-workspace', + '--workspace', + '--workspace-id', + '--workspace-name', + ].some(flag => arg === flag || arg.startsWith(`${flag}=`)), + ) || + [ + process.env.LIBRECHAT_CODE_WORKER_DIR, + process.env.LIBRECHAT_CODE_WORKSPACE_ID, + process.env.LIBRECHAT_CODE_WORKSPACE_NAME, + ].some(value => value?.trim()) || + process.env.LIBRECHAT_CODE_DEFAULT_WORKSPACE?.trim().toLowerCase() === 'true') + ) { + throw new Error( + '--environment cannot be combined with workspace directory, ID, or name settings', + ); + } const configuredWorkerId = process.env.LIBRECHAT_CODE_WORKER_ID?.trim(); const configuredIdentityPath = process.env.LIBRECHAT_CODE_IDENTITY_FILE?.trim(); @@ -347,7 +397,8 @@ async function run( ); } const nsjailDockerMode = - runtimeMode === 'docker-nsjail' || runtimeMode === 'docker-macos-nsjail'; + runtimeMode === 'docker-nsjail' || + runtimeMode === 'docker-macos-nsjail'; const sandboxEndpoint = process.env.LIBRECHAT_CODE_SANDBOX_ENDPOINT ?? 'http://127.0.0.1:2000/api/v2'; @@ -374,16 +425,18 @@ async function run( runtimeSessionId == null && (fileRelayUpstream?.length ?? 0) > 0; const workspaceId = + environments[0]?.definition.name ?? option(args, '--workspace-id') ?? process.env.LIBRECHAT_CODE_WORKSPACE_ID?.trim() ?? 'primary'; const explicitWorkerDirectory = - runtimeSessionId == null + environments[0]?.definition.root ?? + (runtimeSessionId == null ? nonEmpty( option(args, '--worker-dir') ?? process.env.LIBRECHAT_CODE_WORKER_DIR?.trim(), ) - : undefined; + : undefined); const useDefaultWorkspace = runtimeSessionId == null && (args.includes('--default-workspace') || @@ -403,11 +456,25 @@ async function run( option(args, '--command-sandbox') ?? process.env.LIBRECHAT_CODE_COMMAND_SANDBOX?.trim().toLowerCase() ?? (nsjailDockerMode ? 'runtime' : 'native-srt'); - if (commandSandboxMode !== 'native-srt' && commandSandboxMode !== 'runtime') { + if ( + commandSandboxMode !== 'native-srt' && + commandSandboxMode !== 'runtime' + ) { throw new Error( 'LIBRECHAT_CODE_COMMAND_SANDBOX must be native-srt or runtime', ); } + if (environments.length && commandSandboxMode !== 'native-srt') { + throw new Error('Environment definitions require native-srt'); + } + if ( + environments.some(environment => environment.definition.setup) && + !allowWorkspaceCommands + ) { + throw new Error( + 'Environment setup requires --allow-workspace-commands', + ); + } const nativeProgrammaticEnabled = allowWorkspaceCommands && commandSandboxMode === 'native-srt' && @@ -486,7 +553,8 @@ async function run( } } const mutationQuarantinePath = - (allowWorkspaceWrites || allowWorkspaceCommands) && canonicalWorkerDirectory + (allowWorkspaceWrites || allowWorkspaceCommands) && + canonicalWorkerDirectory ? workspaceQuarantinePath({ codeApiUrl, workerId, @@ -508,14 +576,27 @@ async function run( root: canonicalWorkerDirectory, writable: allowWorkspaceWrites, name: + environments[0]?.definition.name ?? option(args, '--workspace-name') ?? process.env.LIBRECHAT_CODE_WORKSPACE_NAME?.trim() ?? (useDefaultWorkspace ? workspaceId - : defaultWorkspaceName(workerDirectory!, workspaceId)), + : defaultWorkspaceName( + workerDirectory!, + workspaceId, + )), }, ] : []; + for (const environment of environments.slice(1)) { + roots.push({ + id: environment.definition.name, + name: environment.definition.name, + root: environment.definition.root, + writable: allowWorkspaceWrites, + }); + } + await assertEnvironmentDefinitionsOutsideRoots(environments, roots); for (let i = 0; i < args.length; i++) { if ( args[i] === '--workspace' && @@ -551,16 +632,18 @@ async function run( if (roots.length > 32) throw new Error('At most 32 workspace roots may be registered'); const rootIdentities = await Promise.all( - roots.map((root) => stat(root.root)), + roots.map(root => stat(root.root)), ); - const normalized = roots.map((root) => root.root); + const normalized = roots.map(root => root.root); for (let i = 0; i < roots.length; i++) for (let j = 0; j < i; j++) { const inside = (a: string, b: string): boolean => { const path = relative(a, b); return ( path === '' || - (path !== '..' && !path.startsWith(`..${sep}`) && !isAbsolute(path)) + (path !== '..' && + !path.startsWith(`..${sep}`) && + !isAbsolute(path)) ); }; if ( @@ -578,7 +661,9 @@ async function run( workspaceLeaseSlots > 1 && (!allowWorkspaceCommands || commandSandboxMode !== 'native-srt') ) { - throw new Error('Concurrent workspace leases require native-srt commands'); + throw new Error( + 'Concurrent workspace leases require native-srt commands', + ); } if ( roots.length > 1 && @@ -589,7 +674,7 @@ async function run( ); } const rootQuarantinePaths = new Map( - roots.map((root) => [ + roots.map(root => [ root.id, workspaceQuarantinePath({ codeApiUrl, @@ -676,7 +761,10 @@ async function run( token: createHmac( 'sha256', pairedIdentity?.privateKey ?? - required('LIBRECHAT_CODE_WORKER_TOKEN', configuredToken), + required( + 'LIBRECHAT_CODE_WORKER_TOKEN', + configuredToken, + ), ) .update('librechat-code-file-relay-v1') .digest('hex'), @@ -712,8 +800,11 @@ async function run( image: runtimeImage, ...(nsjailDockerMode && runtimeSessionId == null ? (() => { - const { seccompProfile, packagesPath, profileRevision } = - nsjailLaunchProfile!; + const { + seccompProfile, + packagesPath, + profileRevision, + } = nsjailLaunchProfile!; return { capabilities: MACOS_NSJAIL_CAPABILITIES, securityOptions: [`seccomp=${seccompProfile}`], @@ -733,10 +824,12 @@ async function run( httpClient: 'bun' as const, environment: { SANDBOX_USE_CGROUPV2: 'false', - SANDBOX_REMOVE_UMOUNT_AFTER_STARTUP: 'false', + SANDBOX_REMOVE_UMOUNT_AFTER_STARTUP: + 'false', ...(workspaceMount ? { - SANDBOX_EXTERNAL_WORKSPACE_ENABLED: 'true', + SANDBOX_EXTERNAL_WORKSPACE_ENABLED: + 'true', SANDBOX_EXTERNAL_WORKSPACE_ROOT: workspaceMount.target, SANDBOX_EXTERNAL_WORKSPACE_TOKEN: @@ -745,15 +838,21 @@ async function run( : {}), ...(fileRelayProfile ? { - EGRESS_GATEWAY_URL: fileRelayProfile.url, + EGRESS_GATEWAY_URL: + fileRelayProfile.url, SANDBOX_PRIME_CONCURRENCY: String( - fileRelayLimits!.maxConcurrentRequests, + fileRelayLimits! + .maxConcurrentRequests, ), - SANDBOX_UPLOAD_CONCURRENCY: String( - fileRelayLimits!.maxConcurrentRequests, + SANDBOX_UPLOAD_CONCURRENCY: + String( + fileRelayLimits! + .maxConcurrentRequests, ), - SANDBOX_FILE_RELAY_TOKEN: fileRelayProfile.token, - SANDBOX_REQUIRE_EGRESS_MANIFEST: 'true', + SANDBOX_FILE_RELAY_TOKEN: + fileRelayProfile.token, + SANDBOX_REQUIRE_EGRESS_MANIFEST: + 'true', SANDBOX_EXECUTION_MANIFEST_PUBLIC_KEY: executionManifestPublicKey!, } @@ -766,8 +865,10 @@ async function run( bindMounts: [workspaceMount], environment: { SANDBOX_EXTERNAL_WORKSPACE_ENABLED: 'true', - SANDBOX_EXTERNAL_WORKSPACE_ROOT: workspaceMount.target, - SANDBOX_EXTERNAL_WORKSPACE_TOKEN: workspaceCommandToken!, + SANDBOX_EXTERNAL_WORKSPACE_ROOT: + workspaceMount.target, + SANDBOX_EXTERNAL_WORKSPACE_TOKEN: + workspaceCommandToken!, }, } : {}), @@ -781,6 +882,7 @@ async function run( commandPolicy, protectedPaths: [ identityPath, + ...environments.map(environment => environment.path), ...rootQuarantinePaths.values(), github.privateKeyPath, ].filter((path): path is string => path != null), @@ -814,7 +916,7 @@ async function run( ? roots.length > 1 || workspaceLeaseSlots > 1 ? new NativeWorkspaceCommandPool( new Map( - roots.map((root) => [ + roots.map(root => [ root.id, { ...nativeOptions, workspaceRoot: root.root }, ]), @@ -826,7 +928,7 @@ async function run( if (allowWorkspaceCommands && workspaceTools) { workspaceTools = new SandboxWorkspaceTools({ workspaceTools, - commandWorkspaces: roots.map((root) => root.id), + commandWorkspaces: roots.map(root => root.id), ...(nativeProgrammaticEnabled ? { programmaticLanguages: ['bash'] } : {}), @@ -839,6 +941,12 @@ async function run( }), }); } + if (workspaceTools && environments.length) { + workspaceTools = new EnvironmentWorkspaceTools( + workspaceTools, + environments, + ); + } const capabilities = { statefulWorkspace, sandboxProfile: @@ -854,6 +962,11 @@ async function run( policyDigest: createHash('sha256') .update(policy) .update( + environments.length + ? `\0environments\0${environments.map(environment => environment.fingerprint).join('\0')}` + : '', + ) + .update( allowWorkspaceCommands && commandSandboxMode === 'native-srt' ? `\0native-srt\0${serializeNativeSrtCommandPolicy(commandPolicy)}\0${commandAllowedDomains.join('\0')}\0${github.policyIdentity}` : '', @@ -863,7 +976,9 @@ async function run( ...(workspaceLeaseSlots > 1 ? { workspaceLeaseSlots, requiresReadyConfirmation: true } : {}), - ...(workspaceTools ? { workspaceTools: workspaceTools.capabilities } : {}), + ...(workspaceTools + ? { workspaceTools: workspaceTools.capabilities } + : {}), }; if (!isValidBridgeWorkerCapabilities(capabilities)) { await fileRelaySupervisor?.stop().catch(() => undefined); @@ -874,6 +989,39 @@ async function run( try { await github.provider?.getCredential(controller.signal); await nativeCommandSandbox?.prepare(); + for (const environment of option(args, '--reset-workspace-quarantine') == null ? environments : []) { + const setup = environment.definition.setup; + if (!setup || !nativeCommandSandbox) continue; + const id = environment.definition.name; + const guard = workspaceMutationGuard( + rootQuarantinePaths.get(id)!, + workerId, + id, + incarnationId, + ); + await guard.assertAvailable(); + await guard.arm('Environment setup did not settle', 'setup'); + const result = await nativeCommandSandbox.execute( + { + protocolVersion: 1, + operation: 'execute_command', + workspaceId: id, + command: setup.command, + timeoutMs: setup.timeoutMs, + maxOutputBytes: 8192, + }, + controller.signal, + ); + await guard.clear('setup'); + if (result.exitCode !== 0 || result.timedOut) { + throw new Error( + `Environment ${id} setup failed; inspect the setup command before restarting`, + ); + } + process.stdout.write( + `librechat-code: environment ${id} prepared\n`, + ); + } } catch (error) { await nativeCommandSandbox?.close().catch(() => undefined); await fileRelaySupervisor?.stop().catch(() => undefined); @@ -895,7 +1043,7 @@ async function run( ...(workspaceLeaseSlots > 1 || roots.length > 1 ? { workspaceQuarantines: new Map( - roots.map((root) => [ + roots.map(root => [ root.id, workspaceMutationGuard( rootQuarantinePaths.get(root.id)!, @@ -913,7 +1061,8 @@ async function run( roots.length === 1 ? { async assertAvailable() { - const record = await loadWorkspaceMutationQuarantine( + const record = + await loadWorkspaceMutationQuarantine( mutationQuarantinePath, ); if (record != null) { @@ -925,15 +1074,18 @@ async function run( } }, async arm(reason) { - await saveWorkspaceMutationQuarantine(mutationQuarantinePath, { + await saveWorkspaceMutationQuarantine( + mutationQuarantinePath, + { version: 1, workerId, workspaceId, ownerId: incarnationId, quarantinedAt: new Date().toISOString(), reason, - }); }, + ); + }, async clear() { await clearWorkspaceMutationQuarantine( mutationQuarantinePath, @@ -950,7 +1102,7 @@ async function run( : undefined, onIdentityChange: pairedIdentity && identityPath - ? async (identity) => { + ? async identity => { await saveBridgeIdentity(identityPath, { ...pairedIdentity, credential: identity.credential, @@ -959,10 +1111,12 @@ async function run( } : undefined, onRegistered: fileRelaySupervisor - ? async (registration) => { + ? async registration => { if ( registration.registrationGeneration == null || - !Number.isSafeInteger(registration.registrationGeneration) || + !Number.isSafeInteger( + registration.registrationGeneration, + ) || registration.registrationGeneration < 1 ) { throw new Error( @@ -975,10 +1129,14 @@ async function run( ); } : undefined, - onError: (error) => { + onError: error => { const message = - error instanceof Error ? error.message : 'unknown bridge error'; - process.stderr.write(`librechat-code: reconnecting after ${message}\n`); + error instanceof Error + ? error.message + : 'unknown bridge error'; + process.stderr.write( + `librechat-code: reconnecting after ${message}\n`, + ); }, }); if (runtimeSessionId !== undefined) { @@ -994,7 +1152,10 @@ async function run( if (resetNativeRoot != null) { await worker.refreshCredential(controller.signal); await worker.registerForMaintenance(controller.signal); - await worker.resetNativeWorkspace(resetNativeRoot, controller.signal); + await worker.resetNativeWorkspace( + resetNativeRoot, + controller.signal, + ); process.stdout.write( `librechat-code: reset acknowledged for native workspace ${resetNativeRoot}\n`, ); diff --git a/packages/code/src/environment-live.test.ts b/packages/code/src/environment-live.test.ts new file mode 100644 index 00000000..bb84c8b5 --- /dev/null +++ b/packages/code/src/environment-live.test.ts @@ -0,0 +1,117 @@ +import assert from 'node:assert/strict'; +import { spawn } from 'node:child_process'; +import { once } from 'node:events'; +import { mkdtemp, mkdir, readFile, writeFile, rm } from 'node:fs/promises'; +import { createServer } from 'node:http'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import test from 'node:test'; + +for (const { succeeds, reset } of [ + { succeeds: true, reset: false }, + { succeeds: false, reset: false }, + { succeeds: true, reset: true }, +]) { + test( + `real CLI environment setup gates registration (success=${succeeds}, reset=${reset})`, + { + skip: process.env.LIBRECHAT_CODE_LIVE_SRT_TESTS !== '1', + timeout: 20_000, + }, + async t => { + const directory = await mkdtemp(join(tmpdir(), 'code-env-live-')); + t.after(() => rm(directory, { recursive: true, force: true })); + const root = join(directory, 'project'); + await mkdir(root); + const path = join(directory, 'environment.yaml'); + await writeFile( + path, + `name: project\nroot: project\nsetup:\n command: 'printf prepared > prepared.txt; exit ${succeeds ? 0 : 2}'\n timeoutMs: 5000\n`, + ); + let registrations = 0; + let receive: (() => void) | undefined; + const registered = new Promise(resolve => { + receive = resolve; + }); + const server = createServer(async (request, response) => { + request.resume(); + if (request.url?.endsWith('/register')) { + registrations++; + if (reset) + await assert.rejects( + readFile(join(root, 'prepared.txt')), + { code: 'ENOENT' }, + ); + else + assert.equal( + await readFile(join(root, 'prepared.txt'), 'utf8'), + 'prepared', + ); + receive?.(); + } + response.writeHead(503).end(); + }); + await new Promise(resolve => + server.listen(0, '127.0.0.1', resolve), + ); + t.after(() => { + server.closeAllConnections(); + server.close(); + }); + const address = server.address(); + assert.ok(address && typeof address !== 'string'); + const child = spawn( + process.execPath, + [ + fileURLToPath(new URL('./cli.js', import.meta.url)), + 'run', + '--environment', + path, + '--allow-workspace-commands', + ...(reset + ? ['--reset-workspace-quarantine', 'project'] + : []), + ], + { + env: { + PATH: process.env.PATH, + HOME: process.env.HOME, + TMPDIR: process.env.TMPDIR, + LIBRECHAT_CODE_URL: `http://127.0.0.1:${address.port}/v1`, + LIBRECHAT_CODE_WORKER_ID: 'environment-test', + LIBRECHAT_CODE_WORKER_TOKEN: 'test-only-token', + LIBRECHAT_CODE_DEFAULT_WORKSPACE: 'false', + LIBRECHAT_CODE_WORKSPACE_QUARANTINE_FILE: join( + directory, + 'quarantine.json', + ), + }, + stdio: ['ignore', 'pipe', 'pipe'], + }, + ); + const exited = once(child, 'exit'); + t.after(() => child.kill('SIGKILL')); + let stderr = ''; + child.stderr.on('data', chunk => { + stderr += chunk.toString(); + }); + if (succeeds) { + await Promise.race([ + registered, + exited.then(() => { + throw new Error(stderr); + }), + ]); + child.kill('SIGTERM'); + await exited; + assert.ok(registrations > 0); + } else { + const [code] = await exited; + assert.notEqual(code, 0); + assert.match(stderr, /Environment project setup failed/); + assert.equal(registrations, 0); + } + }, + ); +} diff --git a/packages/code/src/environment-mount.test.ts b/packages/code/src/environment-mount.test.ts new file mode 100644 index 00000000..fdd5b581 --- /dev/null +++ b/packages/code/src/environment-mount.test.ts @@ -0,0 +1,73 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { assertEnvironmentMountIsolation } from './environment-mount.js'; + +const base = '1 0 8:1 / / rw - ext4 /dev/root rw\n'; +test('mount coordinates reject definition aliases in both directions and mounted files', () => { + for (const entry of [ + '2 1 8:1 /workspace/config /operator rw - ext4 /dev/root rw', + '2 1 8:1 /operator /workspace/config rw - ext4 /dev/root rw', + '2 1 8:1 /operator/app.yaml /workspace/app.yaml rw - ext4 /dev/root rw', + '2 1 8:1 /workspace/app.yaml /operator/app.yaml rw - ext4 /dev/root rw', + ]) + assert.throws( + () => + assertEnvironmentMountIsolation( + base + entry, + ['/operator/app.yaml'], + ['/workspace'], + ), + /mount alias/, + ); +}); +test('mount coordinates retain safe separate filesystems and escaped paths', () => { + assertEnvironmentMountIsolation( + base + '2 1 9:1 / /workspace rw - ext4 /dev/other rw', + ['/operator/app.yaml'], + ['/workspace'], + ); + assertEnvironmentMountIsolation( + base, + ['/operator/app.yaml'], + ['/workspace'], + ); + assert.throws( + () => + assertEnvironmentMountIsolation( + base + + '2 1 8:1 /workspace/my\\040config /operator rw - ext4 /dev/root rw', + ['/operator/app.yaml'], + ['/workspace'], + ), + /mount alias/, + ); + assert.throws(() => assertEnvironmentMountIsolation('invalid', [], [])); + assertEnvironmentMountIsolation( + base + '2 1 9:1 / / rw - ext4 /dev/other rw', + ['/operator/app.yaml'], + ['/workspace'], + ); + assert.throws( + () => + assertEnvironmentMountIsolation( + base + '2 1 9:1 / / rw - ext4 /dev/other rw', + ['/workspace/config/app.yaml'], + ['/workspace'], + ), + /mount alias/, + ); + const many = Array.from( + { length: 257 }, + (_, index) => + `${index + 2} 1 9:1 / /workspace/m${index} rw - ext4 /dev/other rw`, + ).join('\n'); + assert.throws( + () => + assertEnvironmentMountIsolation( + base + many, + ['/operator/app.yaml'], + ['/workspace'], + ), + /Too many/, + ); +}); diff --git a/packages/code/src/environment-mount.ts b/packages/code/src/environment-mount.ts new file mode 100644 index 00000000..bbc8483e --- /dev/null +++ b/packages/code/src/environment-mount.ts @@ -0,0 +1,123 @@ +import { open } from 'node:fs/promises'; +import { posix } from 'node:path'; + +interface Mount { + device: string; + root: string; + point: string; +} +const inside = (root: string, path: string): boolean => + path === root || path.startsWith(root === '/' ? '/' : `${root}/`); +const decode = (path: string): string => { + if (!path.startsWith('/') || /\\(?!040|011|012|134)/.test(path)) + throw new Error('Invalid environment mount table'); + return path.replace(/\\(040|011|012|134)/g, (_, octal: string) => + String.fromCharCode(parseInt(octal, 8)), + ); +}; + +/** Compare filesystem coordinates, not mount aliases. Include mounted descendants of each grant. */ +export function createEnvironmentMountIsolation( + table: string, +): (controls: readonly string[], roots: readonly string[]) => void { + if (Buffer.byteLength(table) > 4 * 1024 * 1024) + throw new Error('Environment mount table exceeds limit'); + const mounts: Mount[] = table + .trimEnd() + .split('\n') + .map(line => { + const fields = line.split(' '); + const separator = fields.indexOf('-', 6); + if ( + separator < 6 || + fields.length !== separator + 4 || + !/^\d+:\d+$/.test(fields[2] ?? '') + ) + throw new Error('Invalid environment mount table'); + return { + device: fields[2], + root: decode(fields[3] ?? ''), + point: decode(fields[4] ?? ''), + }; + }); + const cache = new Map(); + const coordinate = (path: string): { device: string; path: string }[] => { + const cached = cache.get(path); + if (cached) return cached; + // Include every possible backing mapping. Hidden/stacked mounts may cause + // conservative rejection but must never hide an accessible control path. + const result = mounts + .filter(mount => inside(mount.point, path)) + .map(mount => ({ + device: mount.device, + path: posix.join(mount.root, posix.relative(mount.point, path)), + })); + if (!result.length || result.length > 256) + throw new Error( + 'Environment path has an unsupported mount mapping', + ); + cache.set(path, result); + return result; + }; + return (controls, roots) => { + const points = new Set(roots); + for (const mount of mounts) + if (roots.some(root => inside(root, mount.point))) + points.add(mount.point); + if (points.size > 256) + throw new Error('Too many workspace mount boundaries'); + const exposed = [...points].flatMap(coordinate); + if (exposed.length > 1024) + throw new Error('Too many workspace mount mappings'); + for (const control of controls) { + const target = coordinate(control); + if ( + target.some(target => + exposed.some( + root => + root.device === target.device && + inside(root.path, target.path), + ), + ) + ) { + throw new Error( + 'Environment control path is writable through a workspace mount alias', + ); + } + } + }; +} + +export function assertEnvironmentMountIsolation( + table: string, + controls: readonly string[], + roots: readonly string[], +): void { + createEnvironmentMountIsolation(table)(controls, roots); +} + +export async function readEnvironmentMountTable(): Promise { + if (process.platform !== 'linux') return undefined; + const handle = await open('/proc/self/mountinfo', 'r'); + try { + const buffer = Buffer.alloc(4 * 1024 * 1024 + 1); + let length = 0; + while (length < buffer.length) { + const result = await handle.read( + buffer, + length, + buffer.length - length, + null, + ); + if (!result.bytesRead) break; + length += result.bytesRead; + } + if (length === buffer.length) + throw new Error('Environment mount table exceeds limit'); + return new TextDecoder('utf-8', { fatal: true }).decode( + buffer.subarray(0, length), + ); + } finally { + await handle.close(); + } +} diff --git a/packages/code/src/environment.test.ts b/packages/code/src/environment.test.ts new file mode 100644 index 00000000..9080f6a2 --- /dev/null +++ b/packages/code/src/environment.test.ts @@ -0,0 +1,348 @@ +import assert from 'node:assert/strict'; +import { + mkdtemp, + mkdir, + writeFile, + rm, + symlink, + link, + open, + realpath, +} from 'node:fs/promises'; +import { execFileSync } from 'node:child_process'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import test from 'node:test'; +import { + parseCodeEnvironment, + loadCodeEnvironment, + assertEnvironmentDefinitionsOutsideRoots, + EnvironmentWorkspaceTools, +} from './environment.js'; +import { LocalWorkspaceTools, SandboxWorkspaceTools } from './workspace.js'; +import { isValidBridgeWorkspaceToolCapabilities } from './protocol.js'; +import type { WorkspaceExecuteCommandRequest } from './protocol.js'; + +test('environment YAML validates setup and rejects unsupported policy or action fields', () => { + const definition = parseCodeEnvironment( + 'name: app\nroot: ./project\nsetup:\n command: npm ci\n', + ); + assert.equal(definition.setup?.timeoutMs, 300_000); + for (const suffix of [ + 'scope: { users: [anyone] }', + 'actions: [{}]', + 'unknown: true', + 'setup: { command: npm ci, timeoutMs: 600000 }', + 'setup: { command: npm ci, timeoutMs: -1 }', + 'setup: { command: npm ci, env: { SECRET: x } }', + 'name: duplicate', + 'repo: https://token@github.com/a/b', + ]) + assert.throws(() => + parseCodeEnvironment(`name: app\nroot: ./project\n${suffix}\n`), + ); + assert.throws(() => parseCodeEnvironment('name: &id app\nroot: *id')); + assert.throws(() => parseCodeEnvironment('x'.repeat(65_537))); + for (const field of ['setup', 'actions']) { + const command = '漢'.repeat(12_000); + const suffix = + field === 'setup' + ? `setup: { command: '${command}' }` + : `actions: [{ name: test, command: '${command}' }]`; + assert.throws(() => + parseCodeEnvironment(`name: app\nroot: project\n${suffix}`), + ); + } +}); + +test('named actions use the loaded definition, reject stale revisions and preserve command restrictions', async t => { + const directory = await mkdtemp(join(tmpdir(), 'code-env-action-')); + t.after(() => rm(directory, { recursive: true, force: true })); + const local = await LocalWorkspaceTools.create({ + workspaces: [{ id: 'app', root: directory }], + }); + const executed: WorkspaceExecuteCommandRequest[] = []; + const commands = new SandboxWorkspaceTools({ + workspaceTools: local, + commandWorkspaces: ['app'], + commandSandbox: { + mutationFailuresAreAtomic: true, + async execute(request) { + executed.push(request); + return { + protocolVersion: 1, + operation: 'execute_command', + workspaceId: 'app', + stdout: '', + stderr: '', + exitCode: 0, + timedOut: false, + truncated: false, + }; + }, + }, + }); + const environments = [ + { + path: '/operator/environment.yaml', + fingerprint: 'a'.repeat(64), + definition: { + name: 'app', + root: directory, + actions: [ + { name: 'test', command: 'npm test', timeoutMs: 2000 }, + ], + }, + }, + ]; + const tools = new EnvironmentWorkspaceTools(commands, environments); + assert.ok(isValidBridgeWorkspaceToolCapabilities(tools.capabilities)); + assert.deepEqual(tools.capabilities.workspaces[0].environment?.actions, [ + 'test', + ]); + assert.equal( + JSON.stringify(tools.capabilities).includes('npm test'), + false, + ); + const request: WorkspaceExecuteCommandRequest = { + protocolVersion: 1, + operation: 'execute_command', + workspaceId: 'app', + command: 'untrusted placeholder', + timeoutMs: 5000, + environmentAction: { name: 'test', fingerprint: 'a'.repeat(64) }, + }; + await tools.execute(request); + assert.equal(executed[0].command, 'npm test'); + assert.equal(executed[0].timeoutMs, 2000); + assert.equal(executed[0].environmentAction, undefined); + for (const altered of [ + { ...request, workspaceId: 'other' }, + { ...request, cwd: 'nested' }, + { + ...request, + environmentAction: { name: 'test', fingerprint: 'b'.repeat(64) }, + }, + { + ...request, + environmentAction: { name: 'other', fingerprint: 'a'.repeat(64) }, + }, + ]) + await assert.rejects( + tools.execute(altered), + /unavailable or its definition changed/, + ); + await assert.rejects(commands.execute(request), /not resolved/); + const readOnly = new EnvironmentWorkspaceTools(local, environments); + assert.deepEqual( + readOnly.capabilities.workspaces[0].environment?.actions, + [], + ); + await assert.rejects(readOnly.execute(request)); + assert.equal(executed.length, 1); +}); + +test('environment roots resolve relative to the definition and fingerprints cover setup', async t => { + const directory = await mkdtemp(join(tmpdir(), 'code-env-definition-')); + t.after(() => rm(directory, { recursive: true, force: true })); + await mkdir(join(directory, 'project')); + const path = join(directory, 'environment.yaml'); + await writeFile( + path, + 'name: app\nroot: project\nsetup: { command: "printf first" }\n', + ); + const first = await loadCodeEnvironment(path); + assert.ok(first.definition.root.endsWith('/project')); + await assertEnvironmentDefinitionsOutsideRoots( + [first], + [{ id: 'app', root: first.definition.root }], + ); + await writeFile( + path, + 'name: app\nroot: project\nsetup: { command: "printf second" }\n', + ); + assert.notEqual( + (await loadCodeEnvironment(path)).fingerprint, + first.fingerprint, + ); + await assert.rejects(() => + assertEnvironmentDefinitionsOutsideRoots( + [first], + [ + { + id: 'parent', + root: first.definition.root.slice(0, -'/project'.length), + }, + ], + ), + ); + await symlink(path, join(directory, 'project', 'alias.yaml')); + const alias = await loadCodeEnvironment( + join(directory, 'project', 'alias.yaml'), + ); + await assert.rejects(() => + assertEnvironmentDefinitionsOutsideRoots( + [alias], + [{ id: 'app', root: first.definition.root }], + ), + ); + assert.equal( + (await loadCodeEnvironment(join(directory, 'project', 'alias.yaml'))) + .path, + first.path, + ); +}); + +test('rejects a trusted definition with an in-workspace hard link', async t => { + const directory = await mkdtemp(join(tmpdir(), 'code-env-hardlink-')); + t.after(() => rm(directory, { recursive: true, force: true })); + await mkdir(join(directory, 'project')); + const path = join(directory, 'environment.yaml'); + await writeFile(path, 'name: app\nroot: project\n'); + await link(path, join(directory, 'project', 'alias.yaml')); + await assert.rejects(loadCodeEnvironment(path), /one link/); +}); + +test('rejects nested aliases passing through a workspace-controlled link', async t => { + const directory = await realpath( + await mkdtemp(join(tmpdir(), 'code-env-nested-')), + ); + t.after(() => rm(directory, { recursive: true, force: true })); + const root = join(directory, 'project'); + const trusted = join(directory, 'trusted'); + await mkdir(root); + await mkdir(trusted); + await writeFile( + join(trusted, 'environment.yaml'), + `name: app\nroot: ${root}\n`, + ); + await symlink(trusted, join(root, 'pivot')); + await symlink(join(root, 'pivot'), join(directory, 'alias')); + const loaded = await loadCodeEnvironment( + join(directory, 'alias', 'environment.yaml'), + ); + await assert.rejects( + () => + assertEnvironmentDefinitionsOutsideRoots( + [loaded], + [{ id: 'app', root }], + ), + /outside|mount alias/, + ); +}); + +test('reads complete definitions despite short filesystem reads', async t => { + const directory = await mkdtemp(join(tmpdir(), 'code-env-short-read-')); + t.after(() => rm(directory, { recursive: true, force: true })); + await mkdir(join(directory, 'project')); + const path = join(directory, 'environment.yaml'); + await writeFile( + path, + 'name: app\nroot: project\nsetup: { command: echo prepared }\n', + ); + const sample = await open(path); + const prototype = Object.getPrototypeOf(sample); + const read = prototype.read; + await sample.close(); + t.mock.method( + prototype, + 'read', + function ( + this: unknown, + buffer: Buffer, + offset: number, + length: number, + position: number, + ) { + return read.call( + this, + buffer, + offset, + Math.min(length, 7), + position, + ); + }, + ); + assert.equal( + (await loadCodeEnvironment(path)).definition.setup?.command, + 'echo prepared', + ); +}); + +test('rejects a root routed through another workspace and malformed UTF-8', async t => { + const directory = await realpath( + await mkdtemp(join(tmpdir(), 'code-env-root-')), + ); + t.after(() => rm(directory, { recursive: true, force: true })); + const rootA = join(directory, 'a'); + const rootB = join(directory, 'b'); + await mkdir(rootA); + await mkdir(rootB); + await symlink(rootA, join(rootB, 'pivot')); + const path = join(directory, 'environment.yaml'); + await writeFile(path, `name: a\nroot: ${join(rootB, 'pivot')}\n`); + const loaded = await loadCodeEnvironment(path); + await assert.rejects( + () => + assertEnvironmentDefinitionsOutsideRoots( + [loaded], + [ + { id: 'a', root: rootA }, + { id: 'b', root: rootB }, + ], + ), + /root traversal|mount alias/, + ); + await symlink(rootA, join(rootA, 'self-pivot')); + await writeFile(path, `name: a\nroot: ${join(rootA, 'self-pivot')}\n`); + const selfControlled = await loadCodeEnvironment(path); + await assert.rejects( + () => + assertEnvironmentDefinitionsOutsideRoots( + [selfControlled], + [{ id: 'a', root: rootA }], + ), + /root traversal|mount alias/, + ); + await writeFile( + path, + Buffer.concat([ + Buffer.from(`name: a\nroot: ${rootA}\nsetup: { command: echo `), + Buffer.from([0xff]), + Buffer.from(' }'), + ]), + ); + await assert.rejects(loadCodeEnvironment(path), /encoded data/); +}); + +test('rejects a FIFO definition without waiting for a writer', async t => { + const directory = await mkdtemp(join(tmpdir(), 'code-env-fifo-')); + t.after(() => rm(directory, { recursive: true, force: true })); + const path = join(directory, 'environment.yaml'); + execFileSync('mkfifo', ['-m', '600', path], { timeout: 2000 }); + await assert.rejects(loadCodeEnvironment(path), /Invalid environment file/); +}); + +test('rejects a filesystem-identical control directory despite a different root path', async t => { + const directory = await realpath( + await mkdtemp(join(tmpdir(), 'code-env-identity-')), + ); + t.after(() => rm(directory, { recursive: true, force: true })); + const trusted = join(directory, 'trusted'); + const alias = join(directory, 'alias'); + const project = join(directory, 'project'); + await mkdir(trusted); + await mkdir(project); + await symlink(trusted, alias); + const path = join(trusted, 'environment.yaml'); + await writeFile(path, `name: app\nroot: ${project}\n`); + const loaded = await loadCodeEnvironment(path); + // Unlike realpath-based containment, inode comparison also covers bind-mount aliases. + await assert.rejects( + assertEnvironmentDefinitionsOutsideRoots( + [loaded], + [{ id: 'alias', root: alias }], + ), + /outside|mount alias/, + ); +}); diff --git a/packages/code/src/environment.ts b/packages/code/src/environment.ts new file mode 100644 index 00000000..700fc0dc --- /dev/null +++ b/packages/code/src/environment.ts @@ -0,0 +1,399 @@ +import { createHash } from 'node:crypto'; +import { constants } from 'node:fs'; +import { open, realpath, stat } from 'node:fs/promises'; +import { dirname, isAbsolute, relative, resolve, sep } from 'node:path'; +import { parseDocument } from 'yaml'; +import { + assertPrivateStorageAcl, + assertPrivateStorageAncestors, +} from './private-storage.js'; +import { + BRIDGE_WORKSPACE_COMMAND_MAX_TIMEOUT_MS, + BRIDGE_WORKSPACE_COMMAND_MAX_BYTES, +} from './protocol.js'; +import type { LocalWorkspaceConfig } from './workspace.js'; +import { WorkspaceToolError } from './workspace.js'; +import { + createEnvironmentMountIsolation, + readEnvironmentMountTable, +} from './environment-mount.js'; +import type { WorkspaceToolExecutor } from './workspace.js'; +import type { WorkspaceToolRequest, WorkspaceToolResult } from './protocol.js'; + +export interface CodeEnvironmentDefinition { + name: string; + root: string; + repo?: string; + ref?: string; + setup?: { command: string; timeoutMs: number }; + actions?: { name: string; command: string; timeoutMs: number }[]; +} + +export interface LoadedCodeEnvironment { + path: string; + sourceParents?: string[]; + rootPaths?: string[]; + definition: CodeEnvironmentDefinition; + fingerprint: string; +} + +function record(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value); +} + +function text(value: unknown, max: number): value is string { + return ( + typeof value === 'string' && + value.trim().length > 0 && + value.length <= max && + !value.includes('\0') + ); +} + +export function parseCodeEnvironment( + source: string, +): CodeEnvironmentDefinition { + if (Buffer.byteLength(source) > 65_536) + throw new Error('Environment file exceeds 64 KiB'); + const document = parseDocument(source, { + schema: 'core', + uniqueKeys: true, + }); + if (document.errors.length || document.warnings.length) { + throw new Error('Invalid environment YAML'); + } + const value: unknown = document.toJS({ maxAliasCount: 0 }); + if ( + !record(value) || + Object.keys(value).some( + key => + !['name', 'root', 'repo', 'ref', 'setup', 'actions'].includes( + key, + ), + ) || + !text(value.name, 64) || + !/^[A-Za-z0-9][A-Za-z0-9_-]*$/.test(value.name) || + !text(value.root, 4096) || + (value.repo !== undefined && + (!text(value.repo, 256) || + !/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(value.repo))) || + (value.ref !== undefined && + (!text(value.ref, 256) || /[\r\n]/.test(value.ref))) + ) { + throw new Error( + 'Invalid environment definition: expected name, root, optional repo, ref and setup', + ); + } + let setup: CodeEnvironmentDefinition['setup']; + if (value.setup !== undefined) { + if ( + !record(value.setup) || + Object.keys(value.setup).some( + key => !['command', 'timeoutMs'].includes(key), + ) || + !text(value.setup.command, 16_384) || + Buffer.byteLength(value.setup.command) > + BRIDGE_WORKSPACE_COMMAND_MAX_BYTES + ) { + throw new Error('Invalid environment setup'); + } + const timeoutMs = + value.setup.timeoutMs ?? BRIDGE_WORKSPACE_COMMAND_MAX_TIMEOUT_MS; + if ( + typeof timeoutMs !== 'number' || + !Number.isSafeInteger(timeoutMs) || + timeoutMs < 1 || + timeoutMs > BRIDGE_WORKSPACE_COMMAND_MAX_TIMEOUT_MS + ) { + throw new Error( + `Environment setup timeout must be between 1 and ${BRIDGE_WORKSPACE_COMMAND_MAX_TIMEOUT_MS} ms`, + ); + } + setup = { command: value.setup.command, timeoutMs }; + } + let actions: CodeEnvironmentDefinition['actions']; + if (value.actions !== undefined) { + if (!Array.isArray(value.actions) || value.actions.length > 32) + throw new Error('Invalid environment actions'); + const names = new Set(); + actions = value.actions.map((action: unknown) => { + if ( + !record(action) || + Object.keys(action).some( + key => !['name', 'command', 'timeoutMs'].includes(key), + ) || + !text(action.name, 64) || + !/^[A-Za-z0-9][A-Za-z0-9_-]*$/.test(action.name) || + names.has(action.name) || + !text(action.command, 16_384) || + Buffer.byteLength(action.command) > + BRIDGE_WORKSPACE_COMMAND_MAX_BYTES + ) + throw new Error('Invalid environment action'); + const timeoutMs = action.timeoutMs ?? 30_000; + if ( + typeof timeoutMs !== 'number' || + !Number.isSafeInteger(timeoutMs) || + timeoutMs < 1 || + timeoutMs > BRIDGE_WORKSPACE_COMMAND_MAX_TIMEOUT_MS + ) + throw new Error('Invalid environment action timeout'); + names.add(action.name); + return { name: action.name, command: action.command, timeoutMs }; + }); + } + return { + name: value.name, + root: value.root, + ...(typeof value.repo === 'string' ? { repo: value.repo } : {}), + ...(typeof value.ref === 'string' ? { ref: value.ref } : {}), + ...(setup ? { setup } : {}), + ...(actions ? { actions } : {}), + }; +} + +/** Resolve actions only against the worker-owned snapshot, after normal command admission. */ +export class EnvironmentWorkspaceTools implements WorkspaceToolExecutor { + readonly mutationFailuresAreAtomic?: true; + readonly capabilities: WorkspaceToolExecutor['capabilities']; + private readonly environments: Map; + + constructor( + private readonly delegate: WorkspaceToolExecutor, + environments: LoadedCodeEnvironment[], + ) { + this.mutationFailuresAreAtomic = delegate.mutationFailuresAreAtomic; + this.environments = new Map( + environments.map(environment => [ + environment.definition.name, + environment, + ]), + ); + this.capabilities = { + ...delegate.capabilities, + workspaces: delegate.capabilities.workspaces.map(workspace => { + const environment = this.environments.get(workspace.id); + if (!environment) return workspace; + const operations = + workspace.operations ?? delegate.capabilities.operations; + return { + ...workspace, + environment: { + fingerprint: environment.fingerprint, + ...(environment.definition.repo + ? { repo: environment.definition.repo } + : {}), + ...(environment.definition.ref + ? { ref: environment.definition.ref } + : {}), + actions: operations.includes('execute_command') + ? (environment.definition.actions ?? []).map( + action => action.name, + ) + : [], + }, + }; + }), + }; + } + + async execute( + request: WorkspaceToolRequest, + signal?: AbortSignal, + ): Promise { + if ( + request.operation !== 'execute_command' || + !request.environmentAction + ) { + return this.delegate.execute(request, signal); + } + const environment = this.environments.get(request.workspaceId); + const action = environment?.definition.actions?.find( + action => action.name === request.environmentAction?.name, + ); + if ( + !environment || + environment.fingerprint !== request.environmentAction.fingerprint || + !action || + (request.cwd !== undefined && request.cwd !== '.') + ) { + throw new WorkspaceToolError( + 'Environment action is unavailable or its definition changed', + 'INVALID_REQUEST', + ); + } + const { environmentAction: _action, ...commandRequest } = request; + return this.delegate.execute( + { + ...commandRequest, + command: action.command, + timeoutMs: Math.min( + request.timeoutMs ?? action.timeoutMs, + action.timeoutMs, + ), + cwd: '.', + }, + signal, + ); + } +} + +export async function loadCodeEnvironment( + path: string, +): Promise { + const sourcePath = resolve(path); + const sourceParents = await assertPrivateStorageAncestors(sourcePath); + const canonicalPath = await realpath(sourcePath); + const handle = await open( + canonicalPath, + constants.O_RDONLY | constants.O_NONBLOCK | constants.O_NOFOLLOW, + ); + let definition: CodeEnvironmentDefinition; + try { + const metadata = await handle.stat(); + const self = process.getuid?.(); + if ( + metadata.nlink !== 1 || + (metadata.mode & 0o022) !== 0 || + (self !== undefined && metadata.uid !== self && metadata.uid !== 0) + ) { + throw new Error( + 'Environment definitions must have one link, a trusted owner and no group or other write permissions', + ); + } + await assertPrivateStorageAcl(handle, canonicalPath); + if (!metadata.isFile() || metadata.size > 65_536) + throw new Error('Invalid environment file'); + const buffer = Buffer.alloc(65_537); + let bytesRead = 0; + while (bytesRead < buffer.length) { + const result = await handle.read( + buffer, + bytesRead, + buffer.length - bytesRead, + bytesRead, + ); + if (result.bytesRead === 0) break; + bytesRead += result.bytesRead; + } + const after = await handle.stat(); + if ( + bytesRead !== metadata.size || + after.size !== metadata.size || + after.mtimeMs !== metadata.mtimeMs || + after.ctimeMs !== metadata.ctimeMs + ) { + throw new Error('Environment definition changed while reading'); + } + definition = parseCodeEnvironment( + new TextDecoder('utf-8', { fatal: true }).decode( + buffer.subarray(0, bytesRead), + ), + ); + } finally { + await handle.close(); + } + const rootPath = resolve(dirname(canonicalPath), definition.root); + const rootPaths = await assertPrivateStorageAncestors(rootPath); + const root = await realpath(rootPath); + if (!(await stat(root)).isDirectory()) + throw new Error('Environment root must be a directory'); + definition = { ...definition, root }; + return { + path: canonicalPath, + sourceParents, + rootPaths, + definition, + fingerprint: createHash('sha256') + .update(JSON.stringify(definition)) + .digest('hex'), + }; +} + +/** A workspace must never be able to rewrite a definition used on the next startup. */ +export async function assertEnvironmentDefinitionsOutsideRoots( + environments: readonly LoadedCodeEnvironment[], + roots: readonly LocalWorkspaceConfig[], +): Promise { + if (!environments.length) return; + const mountTable = await readEnvironmentMountTable(); + if (mountTable !== undefined) { + const assertMountIsolation = + createEnvironmentMountIsolation(mountTable); + assertMountIsolation( + environments.flatMap(environment => [ + environment.path, + ...(environment.sourceParents ?? []), + ]), + roots.map(root => root.root), + ); + for (const environment of environments) { + assertMountIsolation( + environment.rootPaths ?? [], + roots + .filter(root => root.id !== environment.definition.name) + .map(root => root.root), + ); + assertMountIsolation( + (environment.rootPaths ?? []).filter( + path => path !== environment.definition.root, + ), + roots + .filter(root => root.id === environment.definition.name) + .map(root => root.root), + ); + } + } + const identities = new Map>(); + const identity = (path: string): Promise => { + let result = identities.get(path); + if (!result) { + result = stat(path).then( + metadata => `${metadata.dev}:${metadata.ino}`, + ); + identities.set(path, result); + } + return result; + }; + for (const environment of environments) { + for (const root of roots) { + const rootIdentity = await identity(root.root); + // No granted workspace may control how this root resolves on restart. + { + for (const component of environment.rootPaths ?? []) { + const path = relative(root.root, component); + if (path === '' && root.id === environment.definition.name) + continue; + if ( + (await identity(component)) === rootIdentity || + path === '' || + (!isAbsolute(path) && + path !== '..' && + !path.startsWith(`..${sep}`)) + ) { + throw new Error( + 'Environment root traversal crosses a workspace-controlled component', + ); + } + } + } + for (const controlPath of [ + environment.path, + ...(environment.sourceParents ?? []), + ]) { + const path = relative(root.root, controlPath); + if ( + (await identity(controlPath)) === rootIdentity || + path === '' || + (!isAbsolute(path) && + path !== '..' && + !path.startsWith(`..${sep}`)) + ) { + throw new Error( + 'Environment definitions must be outside every registered workspace root', + ); + } + } + } + } +} diff --git a/packages/code/src/private-storage.ts b/packages/code/src/private-storage.ts index 25f23a2a..d896de33 100644 --- a/packages/code/src/private-storage.ts +++ b/packages/code/src/private-storage.ts @@ -48,12 +48,15 @@ export async function removePrivateStorageAcl( * links one component at a time so even intermediate link targets are checked. * Other local accounts cannot replace a checked entry: its parent is either * non-writable or sticky and the entry belongs to this account or root. + * Returns every traversed entry, including intermediate symlinks, so callers + * can also enforce containment restrictions without resolving those entries away. */ export async function assertPrivateStorageAncestors( path: string, allowMissing = false, -): Promise { +): Promise { assertPrivateStorageSupported(); + const visited: string[] = []; const uid = process.getuid!(); let current = '/'; const pending = (isAbsolute(path) ? path : `${process.cwd()}/${path}`).split('/'); @@ -63,7 +66,8 @@ export async function assertPrivateStorageAncestors( if (allowMissing && error.code === 'ENOENT') return undefined; throw error; }); - if (metadata === undefined) return; + if (metadata === undefined) return visited; + visited.push(current); if (metadata.uid !== uid && metadata.uid !== 0) { throw new BridgeProtocolError( `${current} is owned by another account (uid ${metadata.uid}), ` + @@ -102,7 +106,7 @@ export async function assertPrivateStorageAncestors( } let next = pending.shift(); while (next === '' || next === '.') next = pending.shift(); - if (next === undefined) return; + if (next === undefined) return visited; current = next === '..' ? dirname(current) : `${current === '/' ? '' : current}/${next}`; } } diff --git a/packages/code/src/protocol.ts b/packages/code/src/protocol.ts index b92d21ac..9199fcf0 100644 --- a/packages/code/src/protocol.ts +++ b/packages/code/src/protocol.ts @@ -21,7 +21,8 @@ export const BRIDGE_WORKSPACE_COMMAND_DEFAULT_OUTPUT_BYTES = 256 * 1024; export const BRIDGE_WORKSPACE_COMMAND_MAX_OUTPUT_BYTES = 1024 * 1024; export const BRIDGE_WORKSPACE_COMMAND_SIGNAL_MAX_LENGTH = 32; export const BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_FILES = 100; -export const BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_INPUT_FILES = BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_FILES - 2; +export const BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_INPUT_FILES = + BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_FILES - 2; export const BRIDGE_WORKSPACE_PROGRAMMATIC_TRANSFER_CONCURRENCY = 4; export const BRIDGE_WORKSPACE_PROGRAMMATIC_TRANSFER_TIMEOUT_MS = 30_000; @@ -41,26 +42,119 @@ export const BRIDGE_CANCELLED_WORKSPACE_SETTLEMENT_GRACE_MS = 5_000; * locally instead of discovering the mismatch only after mutating a workspace. */ const BRIDGE_ARTIFACT_EXTENSIONS = new Set([ - '.c', '.cs', '.cpp', '.go', '.java', '.js', '.kt', '.kts', '.lua', - '.php', '.pl', '.ps1', '.py', '.r', '.rb', '.rs', '.scala', '.sh', - '.sql', '.swift', '.ts', '.jsx', '.tsx', '.groovy', - '.css', '.htm', '.html', '.less', '.sass', '.scss', '.svg', '.svelte', '.vue', - '.adoc', '.asciidoc', '.md', '.rst', '.tex', '.txt', '.wiki', - '.csv', '.json', '.bson', '.json5', '.jsonl', '.parquet', '.tsv', - '.xml', '.yaml', '.yml', - '.ics', '.ical', '.ifb', '.icalendar', - '.conf', '.env', '.gitignore', '.ini', '.properties', '.toml', - '.doc', '.docx', '.pdf', '.ppt', '.pptx', '.xls', '.xlsx', - '.odt', '.ods', '.odp', '.rtf', - '.avif', '.bmp', '.gif', '.ico', '.jpeg', '.jpg', '.png', - '.tif', '.tiff', '.webp', - '.eot', '.ttf', '.woff', '.woff2', - '.7z', '.bz2', '.gz', '.gzip', '.rar', '.tar', '.zip', - '.tf', '.tfvars', '.tfstate', '.hcl', - '.dockerfile', '.Dockerfile', '.dockerignore', - '.helmignore', '.helmfile', '.jenkinsfile', '.vagrantfile', - '.eslintrc', '.prettierrc', '.editorconfig', '.nomad', - '.bat', '.cmd', '.deb', '.log', '.rpm', '.vbs', + '.c', + '.cs', + '.cpp', + '.go', + '.java', + '.js', + '.kt', + '.kts', + '.lua', + '.php', + '.pl', + '.ps1', + '.py', + '.r', + '.rb', + '.rs', + '.scala', + '.sh', + '.sql', + '.swift', + '.ts', + '.jsx', + '.tsx', + '.groovy', + '.css', + '.htm', + '.html', + '.less', + '.sass', + '.scss', + '.svg', + '.svelte', + '.vue', + '.adoc', + '.asciidoc', + '.md', + '.rst', + '.tex', + '.txt', + '.wiki', + '.csv', + '.json', + '.bson', + '.json5', + '.jsonl', + '.parquet', + '.tsv', + '.xml', + '.yaml', + '.yml', + '.ics', + '.ical', + '.ifb', + '.icalendar', + '.conf', + '.env', + '.gitignore', + '.ini', + '.properties', + '.toml', + '.doc', + '.docx', + '.pdf', + '.ppt', + '.pptx', + '.xls', + '.xlsx', + '.odt', + '.ods', + '.odp', + '.rtf', + '.avif', + '.bmp', + '.gif', + '.ico', + '.jpeg', + '.jpg', + '.png', + '.tif', + '.tiff', + '.webp', + '.eot', + '.ttf', + '.woff', + '.woff2', + '.7z', + '.bz2', + '.gz', + '.gzip', + '.rar', + '.tar', + '.zip', + '.tf', + '.tfvars', + '.tfstate', + '.hcl', + '.dockerfile', + '.Dockerfile', + '.dockerignore', + '.helmignore', + '.helmfile', + '.jenkinsfile', + '.vagrantfile', + '.eslintrc', + '.prettierrc', + '.editorconfig', + '.nomad', + '.bat', + '.cmd', + '.deb', + '.log', + '.rpm', + '.vbs', ]); function portableBasename(name: string): string { @@ -94,7 +188,8 @@ const BRIDGE_ARTIFACT_MEDIA_TYPES: Readonly> = { '.css': 'text/css', '.csv': 'text/csv', '.doc': 'application/msword', - '.docx': 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + '.docx': + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', '.gif': 'image/gif', '.gz': 'application/gzip', '.gzip': 'application/gzip', @@ -123,7 +218,8 @@ const BRIDGE_ARTIFACT_MEDIA_TYPES: Readonly> = { '.pdf': 'application/pdf', '.png': 'image/png', '.ppt': 'application/vnd.ms-powerpoint', - '.pptx': 'application/vnd.openxmlformats-officedocument.presentationml.presentation', + '.pptx': + 'application/vnd.openxmlformats-officedocument.presentationml.presentation', '.py': 'text/x-python', '.rst': 'text/x-rst', '.rtf': 'application/rtf', @@ -143,7 +239,8 @@ const BRIDGE_ARTIFACT_MEDIA_TYPES: Readonly> = { '.woff': 'font/woff', '.woff2': 'font/woff2', '.xls': 'application/vnd.ms-excel', - '.xlsx': 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + '.xlsx': + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', '.xml': 'application/xml', '.yaml': 'application/yaml', '.yml': 'application/yaml', @@ -180,6 +277,12 @@ export interface BridgeWorkspaceDescriptor { name?: string; /** Optional per-workspace restriction. Omitted by protocol-v1 readers. */ operations?: BridgeWorkspaceToolOperation[]; + environment?: { + fingerprint: string; + repo?: string; + ref?: string; + actions: string[]; + }; } export interface BridgeWorkspaceToolCapabilities { @@ -291,7 +394,8 @@ interface WorkspaceEditFileRequestBase { expectedBaseSha256?: string; } -export interface WorkspaceSingleEditFileRequest extends WorkspaceEditFileRequestBase { +export interface WorkspaceSingleEditFileRequest + extends WorkspaceEditFileRequestBase { /** Legacy single-edit form. */ oldText: string; /** Legacy single-edit form. */ @@ -299,7 +403,8 @@ export interface WorkspaceSingleEditFileRequest extends WorkspaceEditFileRequest edits?: never; } -export interface WorkspaceBatchEditFileRequest extends WorkspaceEditFileRequestBase { +export interface WorkspaceBatchEditFileRequest + extends WorkspaceEditFileRequestBase { /** Ordered exact replacements applied atomically as one file mutation. */ edits: WorkspaceTextEdit[]; oldText?: never; @@ -307,7 +412,8 @@ export interface WorkspaceBatchEditFileRequest extends WorkspaceEditFileRequestB } export type WorkspaceEditFileRequest = - WorkspaceSingleEditFileRequest | WorkspaceBatchEditFileRequest; + | WorkspaceSingleEditFileRequest + | WorkspaceBatchEditFileRequest; export interface WorkspaceTextEdit { oldText: string; @@ -330,20 +436,23 @@ interface WorkspacePreviewEditRequestBase { path: string; } -export interface WorkspaceSinglePreviewEditRequest extends WorkspacePreviewEditRequestBase { +export interface WorkspaceSinglePreviewEditRequest + extends WorkspacePreviewEditRequestBase { oldText: string; newText: string; edits?: never; } -export interface WorkspaceBatchPreviewEditRequest extends WorkspacePreviewEditRequestBase { +export interface WorkspaceBatchPreviewEditRequest + extends WorkspacePreviewEditRequestBase { edits: WorkspaceTextEdit[]; oldText?: never; newText?: never; } export type WorkspacePreviewEditRequest = - WorkspaceSinglePreviewEditRequest | WorkspaceBatchPreviewEditRequest; + | WorkspaceSinglePreviewEditRequest + | WorkspaceBatchPreviewEditRequest; export interface WorkspacePreviewEditResult { protocolVersion: BridgeProtocolVersion; @@ -368,6 +477,7 @@ export interface WorkspaceExecuteCommandRequest { timeoutMs?: number; /** Aggregate UTF-8 stdout and stderr budget. */ maxOutputBytes?: number; + environmentAction?: { name: string; fingerprint: string }; } export interface WorkspaceExecuteCommandResult { @@ -452,6 +562,7 @@ const WORKSPACE_PREVIEW_EDIT_REQUEST_KEYS = new Set([ ]); const WORKSPACE_TEXT_EDIT_KEYS = new Set(['oldText', 'newText']); const WORKSPACE_COMMAND_REQUEST_KEYS = new Set([ + 'environmentAction', 'protocolVersion', 'operation', 'workspaceId', @@ -720,7 +831,8 @@ export function isWorkspaceToolErrorCode( } export type BridgeSettlement = - BridgeFulfilledSettlement | BridgeRejectedSettlement; + | BridgeFulfilledSettlement + | BridgeRejectedSettlement; export interface BridgeSettlementResponse { protocolVersion: BridgeProtocolVersion; @@ -806,7 +918,8 @@ export function isBridgeWorkspaceProgrammaticRequest( (body.transfer_timeout_ms !== undefined && (!Number.isSafeInteger(body.transfer_timeout_ms) || Number(body.transfer_timeout_ms) < 1 || - Number(body.transfer_timeout_ms) > BRIDGE_WORKSPACE_PROGRAMMATIC_TRANSFER_TIMEOUT_MS)) || + Number(body.transfer_timeout_ms) > + BRIDGE_WORKSPACE_PROGRAMMATIC_TRANSFER_TIMEOUT_MS)) || (body.run_timeout !== undefined && (!Number.isSafeInteger(body.run_timeout) || Number(body.run_timeout) < 1 || @@ -826,7 +939,8 @@ export function isBridgeWorkspaceProgrammaticRequest( if ( !isSafePortableRelativePath(file.name) || file.name === '.' || - portableBasename(file.name).toLowerCase() === '_ptc_pending_result.json' || + portableBasename(file.name).toLowerCase() === + '_ptc_pending_result.json' || normalizePortableRelativePath(file.name) !== file.name || names.has(file.name) ) { @@ -875,7 +989,9 @@ export function isBridgeWorkspaceProgrammaticRequest( const segments = name.split('/'); let ancestor = ''; for (let index = 0; index < segments.length - 1; index += 1) { - ancestor = ancestor ? `${ancestor}/${segments[index]}` : segments[index]!; + ancestor = ancestor + ? `${ancestor}/${segments[index]}` + : segments[index]!; if (names.has(ancestor)) return false; } } @@ -1105,6 +1221,22 @@ export function isWorkspaceToolRequest( } if (request.operation === 'execute_command') { return ( + (request.environmentAction === undefined || + (typeof request.environmentAction === 'object' && + request.environmentAction !== null && + Object.keys(request.environmentAction).length === 2 && + typeof (request.environmentAction as { name?: unknown }) + .name === 'string' && + /^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$/.test( + (request.environmentAction as { name: string }).name, + ) && + typeof ( + request.environmentAction as { fingerprint?: unknown } + ).fingerprint === 'string' && + /^[a-f0-9]{64}$/.test( + (request.environmentAction as { fingerprint: string }) + .fingerprint, + ))) && hasOnlyKeys(request, WORKSPACE_COMMAND_REQUEST_KEYS) && typeof request.command === 'string' && request.command.trim().length > 0 && @@ -1438,11 +1570,17 @@ export function isValidBridgeWorkspaceToolCapabilities( const descriptor = workspace as Record; if ( Object.keys(descriptor).some( - key => key !== 'id' && key !== 'name' && key !== 'operations', + key => + key !== 'id' && + key !== 'name' && + key !== 'operations' && + key !== 'environment', ) || typeof descriptor.id !== 'string' || !isValidBridgeWorkerId(descriptor.id) || workspaceIds.has(descriptor.id) || + (descriptor.environment !== undefined && + !isValidCodeEnvironmentDescriptor(descriptor.environment)) || (descriptor.name !== undefined && (typeof descriptor.name !== 'string' || descriptor.name.trim().length === 0 || @@ -1469,6 +1607,37 @@ export function isValidBridgeWorkspaceToolCapabilities( }); } +export function isValidCodeEnvironmentDescriptor( + value: unknown, +): value is NonNullable { + if (typeof value !== 'object' || value === null) return false; + const environment = value as Record; + return ( + Object.keys(environment).every(key => + ['fingerprint', 'repo', 'ref', 'actions'].includes(key), + ) && + typeof environment.fingerprint === 'string' && + /^[a-f0-9]{64}$/.test(environment.fingerprint) && + (environment.repo === undefined || + (typeof environment.repo === 'string' && + environment.repo.length <= 256 && + /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(environment.repo))) && + (environment.ref === undefined || + (typeof environment.ref === 'string' && + environment.ref.trim().length > 0 && + environment.ref.length <= 256 && + !/[\0\r\n]/.test(environment.ref))) && + Array.isArray(environment.actions) && + environment.actions.length <= 32 && + environment.actions.every( + name => + typeof name === 'string' && + /^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$/.test(name), + ) && + new Set(environment.actions).size === environment.actions.length + ); +} + export function isValidBridgeWorkerCapabilities( value: unknown, ): value is BridgeWorkerCapabilities { diff --git a/packages/code/src/worker.ts b/packages/code/src/worker.ts index ffe1b350..86e85872 100644 --- a/packages/code/src/worker.ts +++ b/packages/code/src/worker.ts @@ -192,6 +192,13 @@ function workspaceCapabilitiesMatch( (workspace, index) => workspace.id === executor.workspaces[index]?.id && workspace.name === executor.workspaces[index]?.name && + workspace.environment?.fingerprint === executor.workspaces[index]?.environment?.fingerprint && + workspace.environment?.repo === executor.workspaces[index]?.environment?.repo && + workspace.environment?.ref === executor.workspaces[index]?.environment?.ref && + workspace.environment?.actions.length === executor.workspaces[index]?.environment?.actions.length && + (workspace.environment?.actions.every( + (action, actionIndex) => action === executor.workspaces[index]?.environment?.actions[actionIndex], + ) ?? executor.workspaces[index]?.environment == null) && workspace.operations?.length === executor.workspaces[index]?.operations?.length && (workspace.operations?.every( @@ -236,7 +243,9 @@ function registrationCompatibleCapabilities( return []; } const { operations: _operations, ...compatibleWorkspace } = workspace; - return [compatibleWorkspace]; + return [{ ...compatibleWorkspace, ...(workspace.environment ? { + environment: { ...workspace.environment, actions: [] }, + } : {}) }]; }); if (workspaces.length === 0) { const { workspaceTools: _workspaceTools, ...compatible } = capabilities; @@ -312,13 +321,17 @@ function supportedWorkspaceCapabilities( editOperations.has(operation), ); const workspaces = desired.workspaces.flatMap((workspace) => { - if (workspace.operations == null) return [workspace]; - const workspaceOperations = workspace.operations.filter((operation) => + const workspaceOperations = (workspace.operations ?? operations).filter((operation) => operations.includes(operation), ); return workspaceOperations.length === 0 ? [] - : [{ ...workspace, operations: workspaceOperations }]; + : [{ ...workspace, + ...(workspace.operations ? { operations: workspaceOperations } : {}), + ...(workspace.environment && !workspaceOperations.includes('execute_command') ? { + environment: { ...workspace.environment, actions: [] }, + } : {}), + }]; }); if (workspaces.length === 0) return undefined; const editFileFeatures = desired.editFileFeatures?.filter((feature) => diff --git a/packages/code/src/workspace-worker.test.ts b/packages/code/src/workspace-worker.test.ts index f6205cd4..d97735dc 100644 --- a/packages/code/src/workspace-worker.test.ts +++ b/packages/code/src/workspace-worker.test.ts @@ -7,6 +7,30 @@ import { SandboxWorkspaceTools, WorkspaceToolError } from './workspace.js'; const incarnationId = 'incarnation-00000001'; +test('worker clears named actions when command execution is not negotiated', async () => { + const workspaceTools = { + protocolVersion: 1 as const, + operations: ['read_file' as const, 'execute_command' as const], + workspaces: [{ id: 'primary', environment: { fingerprint: 'a'.repeat(64), actions: ['test'] } }], + }; + const registrations: Array = []; + 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 }, + workspaceMutationQuarantine: mutationQuarantine(), + workspaceTools: { capabilities: workspaceTools, async execute() { throw new Error('not executed'); } }, + fetchImpl: async (_input, init) => { + registrations.push(JSON.parse(String(init?.body)).capabilities.workspaceTools); + return Response.json({ protocolVersion: 1, workerId: 'vm-1', incarnationId, + registeredAt: new Date().toISOString(), leaseTtlMs: 60000, supportedWorkspaceToolOperations: ['read_file'] }); + }, + }); + await worker.register(); + assert.ok(registrations.length > 0); + for (const registration of registrations) assert.deepEqual(registration.workspaces[0].environment.actions, []); +}); + const listWorkspaceCapabilities = { protocolVersion: 1 as const, operations: [ @@ -2422,6 +2446,32 @@ test('worker refuses to advertise workspace tools without a matching executor', ); }); +test('worker refuses environment metadata that differs from its executor', () => { + const environment = { fingerprint: 'a'.repeat(64), repo: 'owner/repo', ref: 'main', actions: [] as string[] }; + const workspaceTools = { + protocolVersion: 1 as const, + operations: ['read_file' as const], + workspaces: [{ id: 'primary', environment }], + }; + for (const changed of [ + undefined, + { ...environment, fingerprint: 'b'.repeat(64) }, + { ...environment, repo: 'other/repo' }, + { ...environment, ref: 'other' }, + { ...environment, actions: ['test'] }, + ]) { + assert.throws(() => 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, workspaces: [{ id: 'primary', ...(changed ? { environment: changed } : {}) }] }, + async execute() { throw new Error('not executed'); }, + }, + }), /workspace tool capabilities require a matching executor/i); + } +}); + test('worker requires durable quarantine before advertising command execution', () => { const workspaceCapabilities = { protocolVersion: 1 as const, diff --git a/packages/code/src/workspace.ts b/packages/code/src/workspace.ts index dfda49fc..91e89f54 100644 --- a/packages/code/src/workspace.ts +++ b/packages/code/src/workspace.ts @@ -1693,6 +1693,9 @@ export class SandboxWorkspaceTools implements WorkspaceToolExecutor { if (request.operation !== 'execute_command') { return this.options.workspaceTools.execute(request, signal); } + if (request.environmentAction) { + throw new WorkspaceToolError('Environment action was not resolved by this worker', 'INVALID_REQUEST'); + } if (!this.commandWorkspaces.has(request.workspaceId)) { throw new WorkspaceToolError( 'Command execution is disabled for this workspace', From f2dcb93b78578fe0fe5eeb7cf1b6a965a5d036f1 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 14 Sep 2026 20:01:54 -0400 Subject: [PATCH 3/4] fix: Allow Trusted Own-Root Environment Symlinks (#212) * fix: Allow Trusted Own-Root Environment Symlinks * fix: Check Alias Parent Ownership by Filesystem Identity * fix: Enforce Parent Ownership Across Every Environment Path --- .github/workflows/ci.yml | 2 + packages/code/src/environment.test.ts | 62 +++++++++++++++++++++++++++ packages/code/src/environment.ts | 28 +++++++++++- 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f041e2d6..bbf9e302 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -194,6 +194,8 @@ jobs: node-version: 24.16.0 - run: npm ci - run: npm run build + - name: Native environment containment tests + run: node --test dist/environment.test.js - name: Native ACL and credential lifecycle tests run: node --test dist/macos-storage.test.js dist/private-storage.test.js dist/storage.test.js dist/github.test.js diff --git a/packages/code/src/environment.test.ts b/packages/code/src/environment.test.ts index 9080f6a2..a8b5b036 100644 --- a/packages/code/src/environment.test.ts +++ b/packages/code/src/environment.test.ts @@ -193,6 +193,68 @@ test('environment roots resolve relative to the definition and fingerprints cove ); }); +test('accepts an own root through trusted external symlinks without allowing other root identities', async t => { + const directory = await realpath(await mkdtemp(join(tmpdir(), 'code-env-own-alias-'))); + t.after(() => rm(directory, { recursive: true, force: true })); + const root = join(directory, 'project'); + await mkdir(root); + const alias = join(directory, 'alias'); + await symlink(root, alias); + await symlink(alias, join(directory, 'nested-alias')); + const path = join(directory, 'environment.yaml'); + for (const selected of [alias, join(directory, 'nested-alias')]) { + await writeFile(path, `name: app\nroot: ${selected}\n`); + const loaded = await loadCodeEnvironment(path); + assert.equal(loaded.definition.root, root); + await assertEnvironmentDefinitionsOutsideRoots([loaded], [{ id: 'app', root }]); + await assert.rejects( + assertEnvironmentDefinitionsOutsideRoots([loaded], [{ id: 'other', root }]), + /root traversal|mount alias/, + ); + } +}); + +test('rejects own-root links hidden by parent aliases or filesystem casing', async t => { + const directory = await realpath(await mkdtemp(join(tmpdir(), 'code-env-parent-alias-'))); + t.after(() => rm(directory, { recursive: true, force: true })); + const root = join(directory, 'Project'); + await mkdir(root); + await symlink(root, join(root, 'self')); + const outside = join(directory, 'outside'); + await mkdir(outside); + await symlink(root, join(outside, 'back')); + await symlink(outside, join(root, 'pivot')); + const alias = join(directory, 'parent-alias'); + await symlink(root, alias); + const path = join(directory, 'environment.yaml'); + const selectedRoots = [join(alias, 'self'), join(alias, 'pivot', 'back')]; + try { + if (await realpath(join(directory, 'project')) === root) { + selectedRoots.push(join(directory, 'project', 'self')); + selectedRoots.push(join(directory, 'project', 'pivot', 'back')); + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } + for (const selected of selectedRoots) { + await writeFile(path, `name: app\nroot: ${selected}\n`); + const loaded = await loadCodeEnvironment(path); + await assert.rejects( + assertEnvironmentDefinitionsOutsideRoots([loaded], [{ id: 'app', root }]), + /root traversal|mount alias/, + ); + } + const definition = join(outside, 'environment.yaml'); + await writeFile(definition, `name: app\nroot: ${root}\n`); + for (const selected of selectedRoots.filter(path => path.endsWith('/back'))) { + const loaded = await loadCodeEnvironment(selected.replace(/back$/, 'environment.yaml')); + await assert.rejects( + assertEnvironmentDefinitionsOutsideRoots([loaded], [{ id: 'app', root }]), + /outside|mount alias/, + ); + } +}); + test('rejects a trusted definition with an in-workspace hard link', async t => { const directory = await mkdtemp(join(tmpdir(), 'code-env-hardlink-')); t.after(() => rm(directory, { recursive: true, force: true })); diff --git a/packages/code/src/environment.ts b/packages/code/src/environment.ts index 700fc0dc..c2b2cf1f 100644 --- a/packages/code/src/environment.ts +++ b/packages/code/src/environment.ts @@ -355,6 +355,25 @@ export async function assertEnvironmentDefinitionsOutsideRoots( } return result; }; + // The entry's parent, not its symlink target, determines who can replace it. + // Compare ancestor identities so casing and directory aliases cannot make a + // workspace-controlled entry look external on case-insensitive filesystems. + const canonicalParents = new Map>(); + const controlsEntry = async (component: string, rootIdentity: string): Promise => { + const directory = dirname(component); + let canonical = canonicalParents.get(directory); + if (!canonical) { + canonical = realpath(directory); + canonicalParents.set(directory, canonical); + } + let parent = await canonical; + while (true) { + if ((await identity(parent)) === rootIdentity) return true; + const next = dirname(parent); + if (next === parent) return false; + parent = next; + } + }; for (const environment of environments) { for (const root of roots) { const rootIdentity = await identity(root.root); @@ -362,10 +381,14 @@ export async function assertEnvironmentDefinitionsOutsideRoots( { for (const component of environment.rootPaths ?? []) { const path = relative(root.root, component); - if (path === '' && root.id === environment.definition.name) + const sameRoot = (await identity(component)) === rootIdentity; + const controlled = await controlsEntry(component, rootIdentity); + // A trusted external alias may select its own root, but a + // link beneath that root is still writable by the workspace. + if (sameRoot && !controlled && root.id === environment.definition.name) continue; if ( - (await identity(component)) === rootIdentity || + controlled || sameRoot || path === '' || (!isAbsolute(path) && path !== '..' && @@ -383,6 +406,7 @@ export async function assertEnvironmentDefinitionsOutsideRoots( ]) { const path = relative(root.root, controlPath); if ( + (await controlsEntry(controlPath, rootIdentity)) || (await identity(controlPath)) === rootIdentity || path === '' || (!isAbsolute(path) && From 840537b9d95a3ad6c761729a198c4e9a0d020d90 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 14 Sep 2026 20:02:05 -0400 Subject: [PATCH 4/4] fix: Retain Quarantine After Failed Environment Setup (#213) * fix: Retain Quarantine After Failed Environment Setup * test: Run Native Environment Setup Lifecycle in CI * fix: Document and Verify Local Setup Quarantine Recovery --- .github/workflows/ci.yml | 4 ++ packages/code/README.md | 11 ++++-- packages/code/src/cli.ts | 4 +- packages/code/src/environment-live.test.ts | 43 ++++++++++++++++++---- 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bbf9e302..2bdeda21 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -198,6 +198,10 @@ jobs: run: node --test dist/environment.test.js - name: Native ACL and credential lifecycle tests run: node --test dist/macos-storage.test.js dist/private-storage.test.js dist/storage.test.js dist/github.test.js + - name: Native environment setup lifecycle tests + env: + LIBRECHAT_CODE_LIVE_SRT_TESTS: '1' + run: node --test dist/environment-live.test.js lambda-microvm-provisioning: name: Lambda MicroVM Provisioning diff --git a/packages/code/README.md b/packages/code/README.md index c3bb16d3..8b98618f 100644 --- a/packages/code/README.md +++ b/packages/code/README.md @@ -689,9 +689,14 @@ worker runs. This inspection happens at startup, not on the command hot path. Setup is an operator-authorized startup command under the configured native sandbox policy. It requires commands to be enabled, runs once per worker startup before registration, and must be idempotent for restarts. Its timeout is bounded to five -minutes and captured output to 8 KiB. Setup failure prevents registration. A crash -or uncertain termination retains the existing workspace quarantine marker; inspect -the workspace before clearing quarantine. No setup output is sent to the model. +minutes and captured output to 8 KiB. Setup failure prevents registration. A nonzero +exit, timeout, crash or uncertain termination retains the workspace quarantine marker; +inspect the workspace before running `librechat-code clear-workspace-quarantine +--worker-dir --workspace-id ` with the same +deployment and identity configuration. Only use the separate +`--reset-workspace-quarantine ` run option afterward if a server +fence also needs clearing. Only successful setup automatically clears its marker. +No setup output is sent to the model. Named actions are fixed commands without model-supplied substitution. The bridge advertises only their names and the definition fingerprint, never their shell source diff --git a/packages/code/src/cli.ts b/packages/code/src/cli.ts index 5498af63..00eca576 100644 --- a/packages/code/src/cli.ts +++ b/packages/code/src/cli.ts @@ -1012,12 +1012,12 @@ async function run( }, controller.signal, ); - await guard.clear('setup'); if (result.exitCode !== 0 || result.timedOut) { throw new Error( - `Environment ${id} setup failed; inspect the setup command before restarting`, + `Environment ${id} setup failed; inspect the workspace and use clear-workspace-quarantine with its root and workspace ID before restarting`, ); } + await guard.clear('setup'); process.stdout.write( `librechat-code: environment ${id} prepared\n`, ); diff --git a/packages/code/src/environment-live.test.ts b/packages/code/src/environment-live.test.ts index bb84c8b5..9d442982 100644 --- a/packages/code/src/environment-live.test.ts +++ b/packages/code/src/environment-live.test.ts @@ -8,13 +8,14 @@ import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; import test from 'node:test'; -for (const { succeeds, reset } of [ - { succeeds: true, reset: false }, - { succeeds: false, reset: false }, - { succeeds: true, reset: true }, +for (const { succeeds, reset, timesOut } of [ + { succeeds: true, reset: false, timesOut: false }, + { succeeds: false, reset: false, timesOut: false }, + { succeeds: false, reset: false, timesOut: true }, + { succeeds: true, reset: true, timesOut: false }, ]) { test( - `real CLI environment setup gates registration (success=${succeeds}, reset=${reset})`, + `real CLI environment setup gates registration (success=${succeeds}, reset=${reset}, timeout=${timesOut})`, { skip: process.env.LIBRECHAT_CODE_LIVE_SRT_TESTS !== '1', timeout: 20_000, @@ -27,7 +28,7 @@ for (const { succeeds, reset } of [ const path = join(directory, 'environment.yaml'); await writeFile( path, - `name: project\nroot: project\nsetup:\n command: 'printf prepared > prepared.txt; exit ${succeeds ? 0 : 2}'\n timeoutMs: 5000\n`, + `name: project\nroot: project\nsetup:\n command: 'printf prepared >> prepared.txt; ${timesOut ? 'sleep 10' : `exit ${succeeds ? 0 : 2}`}'\n timeoutMs: ${timesOut ? 1000 : 5000}\n`, ); let registrations = 0; let receive: (() => void) | undefined; @@ -38,6 +39,7 @@ for (const { succeeds, reset } of [ request.resume(); if (request.url?.endsWith('/register')) { registrations++; + await assert.rejects(readFile(join(directory, 'quarantine.json')), { code: 'ENOENT' }); if (reset) await assert.rejects( readFile(join(root, 'prepared.txt')), @@ -61,10 +63,15 @@ for (const { succeeds, reset } of [ }); const address = server.address(); assert.ok(address && typeof address !== 'string'); - const child = spawn( + const start = (clear = false) => spawn( process.execPath, [ fileURLToPath(new URL('./cli.js', import.meta.url)), + ...(clear ? [ + 'clear-workspace-quarantine', + '--worker-dir', root, + '--workspace-id', 'project', + ] : [ 'run', '--environment', path, @@ -72,6 +79,7 @@ for (const { succeeds, reset } of [ ...(reset ? ['--reset-workspace-quarantine', 'project'] : []), + ]), ], { env: { @@ -90,6 +98,7 @@ for (const { succeeds, reset } of [ stdio: ['ignore', 'pipe', 'pipe'], }, ); + const child = start(); const exited = once(child, 'exit'); t.after(() => child.kill('SIGKILL')); let stderr = ''; @@ -111,6 +120,26 @@ for (const { succeeds, reset } of [ assert.notEqual(code, 0); assert.match(stderr, /Environment project setup failed/); assert.equal(registrations, 0); + const marker = await readFile(join(directory, 'quarantine.json'), 'utf8'); + assert.equal(JSON.parse(marker).workspaceId, 'project'); + const before = await readFile(join(root, 'prepared.txt'), 'utf8'); + const retry = start(); + t.after(() => retry.kill('SIGKILL')); + let retryStderr = ''; + retry.stderr.on('data', chunk => { retryStderr += chunk.toString(); }); + const [retryCode] = await once(retry, 'exit'); + assert.notEqual(retryCode, 0); + assert.match(retryStderr, /quarantined/); + assert.equal(await readFile(join(root, 'prepared.txt'), 'utf8'), before); + assert.equal(await readFile(join(directory, 'quarantine.json'), 'utf8'), marker); + assert.equal(registrations, 0); + const recovery = start(true); + t.after(() => recovery.kill('SIGKILL')); + const [recoveryCode] = await once(recovery, 'exit'); + assert.equal(recoveryCode, 0); + await assert.rejects(readFile(join(directory, 'quarantine.json')), { code: 'ENOENT' }); + assert.equal(await readFile(join(root, 'prepared.txt'), 'utf8'), before); + assert.equal(registrations, 0); } }, );