diff --git a/packages/sandbox-container/src/handlers/execute-handler.ts b/packages/sandbox-container/src/handlers/execute-handler.ts index 784b3554..541e004a 100644 --- a/packages/sandbox-container/src/handlers/execute-handler.ts +++ b/packages/sandbox-container/src/handlers/execute-handler.ts @@ -48,7 +48,9 @@ export class ExecuteHandler extends BaseHandler { body.command, { sessionId, - timeoutMs: body.timeoutMs + timeoutMs: body.timeoutMs, + env: body.env, + cwd: body.cwd } ); @@ -72,7 +74,9 @@ export class ExecuteHandler extends BaseHandler { // For non-background commands, execute and return result const result = await this.processService.executeCommand(body.command, { sessionId, - timeoutMs: body.timeoutMs + timeoutMs: body.timeoutMs, + env: body.env, + cwd: body.cwd }); if (!result.success) { @@ -105,7 +109,9 @@ export class ExecuteHandler extends BaseHandler { // Start the process for streaming const processResult = await this.processService.startProcess(body.command, { - sessionId + sessionId, + env: body.env, + cwd: body.cwd }); if (!processResult.success) { diff --git a/packages/sandbox-container/src/services/process-service.ts b/packages/sandbox-container/src/services/process-service.ts index 8f41f955..05fe435c 100644 --- a/packages/sandbox-container/src/services/process-service.ts +++ b/packages/sandbox-container/src/services/process-service.ts @@ -101,7 +101,8 @@ export class ProcessService { sessionId, command, options.cwd, - options.timeoutMs + options.timeoutMs, + options.env ); if (!result.success) { diff --git a/packages/sandbox-container/src/services/session-manager.ts b/packages/sandbox-container/src/services/session-manager.ts index 43c50a17..92f723d6 100644 --- a/packages/sandbox-container/src/services/session-manager.ts +++ b/packages/sandbox-container/src/services/session-manager.ts @@ -116,7 +116,8 @@ export class SessionManager { sessionId: string, command: string, cwd?: string, - timeoutMs?: number + timeoutMs?: number, + env?: Record ): Promise> { try { // Get or create session on demand @@ -141,7 +142,10 @@ export class SessionManager { const session = sessionResult.data; - const result = await session.exec(command, cwd ? { cwd } : undefined); + const result = await session.exec( + command, + cwd || env ? { cwd, env } : undefined + ); return { success: true, diff --git a/packages/sandbox-container/src/session.ts b/packages/sandbox-container/src/session.ts index b6217d40..d28a62fd 100644 --- a/packages/sandbox-container/src/session.ts +++ b/packages/sandbox-container/src/session.ts @@ -88,6 +88,8 @@ export interface RawExecResult { interface ExecOptions { /** Override working directory for this command only */ cwd?: string; + /** Environment variables for this command only (does not persist in session) */ + env?: Record; } /** Command handle for tracking and killing running commands */ @@ -233,7 +235,8 @@ export class Session { logFile, exitCodeFile, options?.cwd, - false + false, + options?.env ); // Write script to shell's stdin @@ -333,7 +336,8 @@ export class Session { logFile, exitCodeFile, options?.cwd, - true + true, + options?.env ); if (this.shell!.stdin && typeof this.shell!.stdin !== 'number') { @@ -624,7 +628,8 @@ export class Session { logFile: string, exitCodeFile: string, cwd?: string, - isBackground = false + isBackground = false, + env?: Record ): string { // Create unique FIFO names to prevent collisions const stdoutPipe = join(this.sessionDir!, `${cmdId}.stdout.pipe`); @@ -639,6 +644,24 @@ export class Session { const safeSessionDir = this.escapeShellPath(this.sessionDir!); const safePidFile = this.escapeShellPath(pidFile); + // Build command with environment variables if provided + // Use a subshell with export to ensure vars are available for the command + // This works with both builtins and external commands + let commandWithEnv: string; + if (env && Object.keys(env).length > 0) { + const exports = Object.entries(env) + .map(([key, value]) => { + // Escape the value for safe shell usage + const escapedValue = value.replace(/'/g, "'\\''"); + return `export ${key}='${escapedValue}'`; + }) + .join('; '); + // Wrap in subshell to isolate env vars (they don't persist in session) + commandWithEnv = `(${exports}; ${command})`; + } else { + commandWithEnv = command; + } + // Build the FIFO script // For background: monitor handles cleanup (no trap needed) // For foreground: trap handles cleanup (standard pattern) @@ -684,7 +707,7 @@ export class Session { script += ` if cd ${safeCwd}; then\n`; script += ` # Execute command in BACKGROUND (runs in subshell, enables concurrency)\n`; script += ` {\n`; - script += ` ${command}\n`; + script += ` ${commandWithEnv}\n`; script += ` CMD_EXIT=$?\n`; script += ` # Write exit code\n`; script += ` echo "$CMD_EXIT" > ${safeExitCodeFile}.tmp\n`; @@ -708,7 +731,7 @@ export class Session { } else { script += ` # Execute command in BACKGROUND (runs in subshell, enables concurrency)\n`; script += ` {\n`; - script += ` ${command}\n`; + script += ` ${commandWithEnv}\n`; script += ` CMD_EXIT=$?\n`; script += ` # Write exit code\n`; script += ` echo "$CMD_EXIT" > ${safeExitCodeFile}.tmp\n`; @@ -738,7 +761,7 @@ export class Session { script += ` PREV_DIR=$(pwd)\n`; script += ` if cd ${safeCwd}; then\n`; script += ` # Execute command, redirect to temp files\n`; - script += ` { ${command}; } < /dev/null > "$log.stdout" 2> "$log.stderr"\n`; + script += ` { ${commandWithEnv}; } < /dev/null > "$log.stdout" 2> "$log.stderr"\n`; script += ` EXIT_CODE=$?\n`; script += ` # Restore directory\n`; script += ` cd "$PREV_DIR"\n`; @@ -748,7 +771,7 @@ export class Session { script += ` fi\n`; } else { script += ` # Execute command, redirect to temp files\n`; - script += ` { ${command}; } < /dev/null > "$log.stdout" 2> "$log.stderr"\n`; + script += ` { ${commandWithEnv}; } < /dev/null > "$log.stdout" 2> "$log.stderr"\n`; script += ` EXIT_CODE=$?\n`; } diff --git a/packages/sandbox-container/src/validation/schemas.ts b/packages/sandbox-container/src/validation/schemas.ts index 5398d88a..48e52484 100644 --- a/packages/sandbox-container/src/validation/schemas.ts +++ b/packages/sandbox-container/src/validation/schemas.ts @@ -17,7 +17,9 @@ export const ExecuteRequestSchema = z.object({ command: z.string().min(1, 'Command cannot be empty'), sessionId: z.string().optional(), background: z.boolean().optional(), - timeoutMs: z.number().positive().optional() + timeoutMs: z.number().positive().optional(), + env: z.record(z.string()).optional(), + cwd: z.string().optional() }); // File operation schemas diff --git a/packages/sandbox-container/tests/services/process-service.test.ts b/packages/sandbox-container/tests/services/process-service.test.ts index 14295e11..a478cf7e 100644 --- a/packages/sandbox-container/tests/services/process-service.test.ts +++ b/packages/sandbox-container/tests/services/process-service.test.ts @@ -105,7 +105,8 @@ describe('ProcessService', () => { 'default', // sessionId 'echo "hello world"', '/tmp', // cwd - undefined // timeoutMs (not provided in options) + undefined, // timeoutMs (not provided in options) + undefined // env (not provided in options) ); }); diff --git a/packages/sandbox/src/clients/command-client.ts b/packages/sandbox/src/clients/command-client.ts index 2bc57851..252f8508 100644 --- a/packages/sandbox/src/clients/command-client.ts +++ b/packages/sandbox/src/clients/command-client.ts @@ -11,6 +11,8 @@ import type { export interface ExecuteRequest extends SessionRequest { command: string; timeoutMs?: number; + env?: Record; + cwd?: string; } /** @@ -32,17 +34,23 @@ export class CommandClient extends BaseHttpClient { * @param command - The command to execute * @param sessionId - The session ID for this command execution * @param timeoutMs - Optional timeout in milliseconds (unlimited by default) + * @param env - Optional environment variables for this command + * @param cwd - Optional working directory for this command */ async execute( command: string, sessionId: string, - timeoutMs?: number + timeoutMs?: number, + env?: Record, + cwd?: string ): Promise { try { const data: ExecuteRequest = { command, sessionId, - ...(timeoutMs !== undefined && { timeoutMs }) + ...(timeoutMs !== undefined && { timeoutMs }), + ...(env !== undefined && { env }), + ...(cwd !== undefined && { cwd }) }; const response = await this.post('/api/execute', data); diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index aeefe895..8e15bc5f 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -447,7 +447,13 @@ export class Sandbox extends Container implements ISandbox { ); } else { // Regular execution with session - const response = await this.client.commands.execute(command, sessionId); + const response = await this.client.commands.execute( + command, + sessionId, + options?.timeout, + options?.env, + options?.cwd + ); const duration = Date.now() - startTime; result = this.mapExecuteResponseToExecResult( diff --git a/packages/sandbox/tests/sandbox.test.ts b/packages/sandbox/tests/sandbox.test.ts index 35a58948..9c553aa8 100644 --- a/packages/sandbox/tests/sandbox.test.ts +++ b/packages/sandbox/tests/sandbox.test.ts @@ -142,7 +142,10 @@ describe('Sandbox - Automatic Session Management', () => { expect(sandbox.client.commands.execute).toHaveBeenCalledWith( 'echo test', - expect.stringMatching(/^sandbox-/) + expect.stringMatching(/^sandbox-/), + undefined, + undefined, + undefined ); }); @@ -284,7 +287,10 @@ describe('Sandbox - Automatic Session Management', () => { expect(sandbox.client.commands.execute).toHaveBeenCalledWith( 'echo test', - 'isolated-session' + 'isolated-session', + undefined, + undefined, + undefined ); }); @@ -387,7 +393,10 @@ describe('Sandbox - Automatic Session Management', () => { await session.exec('pwd'); expect(sandbox.client.commands.execute).toHaveBeenCalledWith( 'pwd', - 'test-session' + 'test-session', + undefined, + undefined, + undefined ); }); diff --git a/tests/e2e/exec-env-vars-repro.test.ts b/tests/e2e/exec-env-vars-repro.test.ts new file mode 100644 index 00000000..96cda840 --- /dev/null +++ b/tests/e2e/exec-env-vars-repro.test.ts @@ -0,0 +1,195 @@ +import { + describe, + test, + expect, + beforeAll, + afterAll, + afterEach, + vi +} from 'vitest'; +import { getTestWorkerUrl, WranglerDevRunner } from './helpers/wrangler-runner'; +import { + createSandboxId, + createTestHeaders, + fetchWithStartup, + cleanupSandbox +} from './helpers/test-fixtures'; + +describe('Issue #144: Exec with per-command env vars (REPRODUCTION)', () => { + describe('local', () => { + let runner: WranglerDevRunner | null; + let workerUrl: string; + let currentSandboxId: string | null = null; + + beforeAll(async () => { + const result = await getTestWorkerUrl(); + workerUrl = result.url; + runner = result.runner; + }); + + afterEach(async () => { + if (currentSandboxId) { + await cleanupSandbox(workerUrl, currentSandboxId); + currentSandboxId = null; + } + }); + + afterAll(async () => { + if (runner) { + await runner.stop(); + } + }); + + test('should pass env vars to individual exec command via options.env', async () => { + currentSandboxId = createSandboxId(); + const headers = createTestHeaders(currentSandboxId); + + // Execute command with env vars passed via options + // BUG: These env vars are currently IGNORED + const execResponse = await vi.waitFor( + async () => + fetchWithStartup(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: 'echo $EXEC_VAR', + env: { EXEC_VAR: 'from_exec_options' } + }) + }), + { timeout: 90000, interval: 2000 } + ); + + expect(execResponse.status).toBe(200); + const execData = await execResponse.json(); + expect(execData.success).toBe(true); + + // This assertion will FAIL with current implementation + // because env vars are not passed through + expect(execData.stdout.trim()).toBe('from_exec_options'); + }, 90000); + + test('should pass cwd to individual exec command via options.cwd', async () => { + currentSandboxId = createSandboxId(); + const headers = createTestHeaders(currentSandboxId); + + // Create a test directory + await vi.waitFor( + async () => + fetchWithStartup(`${workerUrl}/api/file/mkdir`, { + method: 'POST', + headers, + body: JSON.stringify({ + path: '/workspace/testdir', + recursive: true + }) + }), + { timeout: 90000, interval: 2000 } + ); + + // Execute command with cwd passed via options + // BUG: The cwd option is currently IGNORED + const execResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: 'pwd', + cwd: '/workspace/testdir' + }) + }); + + expect(execResponse.status).toBe(200); + const execData = await execResponse.json(); + expect(execData.success).toBe(true); + + // This assertion will FAIL with current implementation + // because cwd is not passed through + expect(execData.stdout.trim()).toBe('/workspace/testdir'); + }, 90000); + + test('should use per-command env vars without affecting session env', async () => { + currentSandboxId = createSandboxId(); + const headers = createTestHeaders(currentSandboxId); + + // Set a session-level env var + await vi.waitFor( + async () => + fetchWithStartup(`${workerUrl}/api/env/set`, { + method: 'POST', + headers, + body: JSON.stringify({ + envVars: { SESSION_VAR: 'session_value' } + }) + }), + { timeout: 90000, interval: 2000 } + ); + + // Execute command with per-command env var + // BUG: The per-command env var is currently IGNORED + const execResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: 'echo "SESSION=$SESSION_VAR CMD=$CMD_VAR"', + env: { CMD_VAR: 'cmd_value' } + }) + }); + + expect(execResponse.status).toBe(200); + const execData = await execResponse.json(); + expect(execData.success).toBe(true); + + // Should see both session var and command-specific var + // This assertion will FAIL because CMD_VAR is not set + expect(execData.stdout.trim()).toBe( + 'SESSION=session_value CMD=cmd_value' + ); + + // Verify session var persists but command var does not + const verifyResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: 'echo "SESSION=$SESSION_VAR CMD=$CMD_VAR"' + }) + }); + + expect(verifyResponse.status).toBe(200); + const verifyData = await verifyResponse.json(); + expect(verifyData.success).toBe(true); + + // Session var should still be set, command var should be empty + expect(verifyData.stdout.trim()).toBe('SESSION=session_value CMD='); + }, 90000); + + test('should allow multiple env vars in single exec command', async () => { + currentSandboxId = createSandboxId(); + const headers = createTestHeaders(currentSandboxId); + + // Execute command with multiple env vars + // BUG: All env vars are currently IGNORED + const execResponse = await vi.waitFor( + async () => + fetchWithStartup(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: 'echo "A=$VAR_A B=$VAR_B C=$VAR_C"', + env: { + VAR_A: 'value_a', + VAR_B: 'value_b', + VAR_C: 'value_c' + } + }) + }), + { timeout: 90000, interval: 2000 } + ); + + expect(execResponse.status).toBe(200); + const execData = await execResponse.json(); + expect(execData.success).toBe(true); + + // This assertion will FAIL because env vars are not passed + expect(execData.stdout.trim()).toBe('A=value_a B=value_b C=value_c'); + }, 90000); + }); +}); diff --git a/tests/e2e/test-worker/index.ts b/tests/e2e/test-worker/index.ts index af2ed5fe..c5d555ea 100644 --- a/tests/e2e/test-worker/index.ts +++ b/tests/e2e/test-worker/index.ts @@ -262,7 +262,11 @@ console.log('Terminal server on port ' + port); // Command execution if (url.pathname === '/api/execute' && request.method === 'POST') { - const result = await executor.exec(body.command); + const result = await executor.exec(body.command, { + env: body.env, + cwd: body.cwd, + timeout: body.timeout + }); return new Response(JSON.stringify(result), { headers: { 'Content-Type': 'application/json' } }); @@ -275,7 +279,11 @@ console.log('Terminal server on port ' + port); body.command ); const startTime = Date.now(); - const stream = await executor.execStream(body.command); + const stream = await executor.execStream(body.command, { + env: body.env, + cwd: body.cwd, + timeout: body.timeout + }); console.log( '[TestWorker] Stream received in', Date.now() - startTime, @@ -381,7 +389,13 @@ console.log('Terminal server on port ' + port); // Process start if (url.pathname === '/api/process/start' && request.method === 'POST') { - const process = await executor.startProcess(body.command); + const process = await executor.startProcess(body.command, { + processId: body.processId, + env: body.env, + cwd: body.cwd, + timeout: body.timeout, + autoCleanup: body.autoCleanup + }); return new Response(JSON.stringify(process), { headers: { 'Content-Type': 'application/json' } });