Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions packages/sandbox-container/src/handlers/execute-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ export class ExecuteHandler extends BaseHandler<Request, Response> {
body.command,
{
sessionId,
timeoutMs: body.timeoutMs
timeoutMs: body.timeoutMs,
env: body.env,
cwd: body.cwd
}
);

Expand All @@ -72,7 +74,9 @@ export class ExecuteHandler extends BaseHandler<Request, Response> {
// 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) {
Expand Down Expand Up @@ -105,7 +109,9 @@ export class ExecuteHandler extends BaseHandler<Request, Response> {

// Start the process for streaming
const processResult = await this.processService.startProcess(body.command, {
sessionId
sessionId,
env: body.env,
cwd: body.cwd
});

if (!processResult.success) {
Expand Down
3 changes: 2 additions & 1 deletion packages/sandbox-container/src/services/process-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ export class ProcessService {
sessionId,
command,
options.cwd,
options.timeoutMs
options.timeoutMs,
options.env
);

if (!result.success) {
Expand Down
8 changes: 6 additions & 2 deletions packages/sandbox-container/src/services/session-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ export class SessionManager {
sessionId: string,
command: string,
cwd?: string,
timeoutMs?: number
timeoutMs?: number,
env?: Record<string, string>
): Promise<ServiceResult<RawExecResult>> {
try {
// Get or create session on demand
Expand All @@ -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,
Expand Down
37 changes: 30 additions & 7 deletions packages/sandbox-container/src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>;
}

/** Command handle for tracking and killing running commands */
Expand Down Expand Up @@ -233,7 +235,8 @@ export class Session {
logFile,
exitCodeFile,
options?.cwd,
false
false,
options?.env
);

// Write script to shell's stdin
Expand Down Expand Up @@ -333,7 +336,8 @@ export class Session {
logFile,
exitCodeFile,
options?.cwd,
true
true,
options?.env
);

if (this.shell!.stdin && typeof this.shell!.stdin !== 'number') {
Expand Down Expand Up @@ -624,7 +628,8 @@ export class Session {
logFile: string,
exitCodeFile: string,
cwd?: string,
isBackground = false
isBackground = false,
env?: Record<string, string>
): string {
// Create unique FIFO names to prevent collisions
const stdoutPipe = join(this.sessionDir!, `${cmdId}.stdout.pipe`);
Expand All @@ -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)
Expand Down Expand Up @@ -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`;
Expand All @@ -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`;
Expand Down Expand Up @@ -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`;
Expand All @@ -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`;
}

Expand Down
4 changes: 3 additions & 1 deletion packages/sandbox-container/src/validation/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
});

Expand Down
12 changes: 10 additions & 2 deletions packages/sandbox/src/clients/command-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import type {
export interface ExecuteRequest extends SessionRequest {
command: string;
timeoutMs?: number;
env?: Record<string, string>;
cwd?: string;
}

/**
Expand All @@ -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<string, string>,
cwd?: string
): Promise<ExecuteResponse> {
try {
const data: ExecuteRequest = {
command,
sessionId,
...(timeoutMs !== undefined && { timeoutMs })
...(timeoutMs !== undefined && { timeoutMs }),
...(env !== undefined && { env }),
...(cwd !== undefined && { cwd })
};

const response = await this.post<ExecuteResponse>('/api/execute', data);
Expand Down
8 changes: 7 additions & 1 deletion packages/sandbox/src/sandbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,13 @@ export class Sandbox<Env = unknown> extends Container<Env> 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(
Expand Down
15 changes: 12 additions & 3 deletions packages/sandbox/tests/sandbox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,10 @@ describe('Sandbox - Automatic Session Management', () => {

expect(sandbox.client.commands.execute).toHaveBeenCalledWith(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TEST BUG: This test expectation doesn't match the actual implementation.

Actual signature from command-client.ts:40-48:

execute(command: string, sessionId: string, options?: {...})

This test expects 5 parameters:

.toHaveBeenCalledWith(
  'echo test',
  expect.stringMatching(/^sandbox-/),
  undefined,  // These three undefined params are wrong
  undefined,
  undefined
)

Should be:

.toHaveBeenCalledWith(
  'echo test',
  expect.stringMatching(/^sandbox-/),
  undefined  // Single options parameter
)

Or to be more robust:

.toHaveBeenCalledWith(
  'echo test',
  expect.stringMatching(/^sandbox-/),
  expect.anything()
)

Multiple tests have this same issue (lines 148-154, 290-296, 399-406). All need fixing.

'echo test',
expect.stringMatching(/^sandbox-/)
expect.stringMatching(/^sandbox-/),
undefined,
undefined,
undefined
);
});

Expand Down Expand Up @@ -284,7 +287,10 @@ describe('Sandbox - Automatic Session Management', () => {

expect(sandbox.client.commands.execute).toHaveBeenCalledWith(
'echo test',
'isolated-session'
'isolated-session',
undefined,
undefined,
undefined
);
});

Expand Down Expand Up @@ -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
);
});

Expand Down
Loading
Loading