Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
5 changes: 5 additions & 0 deletions .changeset/sixty-walls-serve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@cloudflare/sandbox": patch
---

fix workspace bug
31 changes: 28 additions & 3 deletions packages/sandbox-container/src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

import { randomUUID } from 'node:crypto';
import { watch } from 'node:fs';
import { mkdir, rm } from 'node:fs/promises';
import { mkdir, rm, stat } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { basename, dirname, join } from 'node:path';
import type { ExecEvent, Logger } from '@repo/shared';
Expand All @@ -46,7 +46,14 @@ export interface SessionOptions {
/** Session identifier (generated if not provided) */
id: string;

/** Working directory for the session */
/**
* Initial working directory for the shell.
*
* Note: This only affects where the shell starts. Individual commands can
* specify their own cwd via exec options, and the shell can cd anywhere.
* If the specified directory doesn't exist when the session initializes,
* the session will fall back to the home directory.
*/
cwd?: string;

/** Environment variables for the session */
Expand Down Expand Up @@ -137,10 +144,28 @@ export class Session {
this.sessionDir = join(tmpdir(), `session-${this.id}-${Date.now()}`);
await mkdir(this.sessionDir, { recursive: true });

// Determine working directory. If the requested cwd doesn't exist, we fall
// back to the home directory since it's a natural default for shell sessions.
const homeDir = process.env.HOME || '/root';
let cwd = this.options.cwd || CONFIG.DEFAULT_CWD;
try {
await stat(cwd);
} catch {
this.logger.debug(
`Shell startup directory '${cwd}' does not exist, using '${homeDir}'`,
{
sessionId: this.id,
requestedCwd: cwd,
actualCwd: homeDir
}
);
cwd = homeDir;
}

// Spawn persistent bash with stdin pipe - no IPC or wrapper needed!
this.shell = Bun.spawn({
cmd: ['bash', '--norc'],
cwd: this.options.cwd || CONFIG.DEFAULT_CWD,
cwd,
env: {
...process.env,
...this.options.env,
Expand Down
133 changes: 133 additions & 0 deletions packages/sandbox-container/tests/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,53 @@
// Session directory should be created (we can't easily check without accessing private fields)
expect(session.isReady()).toBe(true);
});

it('should fall back to /tmp when cwd does not exist (issue #288)', async () => {
// Session cwd only affects shell startup directory - it's not critical.
// If cwd doesn't exist, we fall back to /tmp since individual commands
// can specify their own cwd anyway.
session = new Session({
id: 'test-session-nonexistent-cwd',
cwd: '/nonexistent/path/that/does/not/exist'
});

await session.initialize();

expect(session.isReady()).toBe(true);

// Verify we can execute commands
const result = await session.exec('pwd');
expect(result.exitCode).toBe(0);
// The shell should have started in /tmp since the requested cwd doesn't exist
// On macOS, /tmp is a symlink to /private/tmp
expect(result.stdout.trim()).toMatch(/^(\/tmp|\/private\/tmp)$/);

Check failure on line 97 in packages/sandbox-container/tests/session.test.ts

View workflow job for this annotation

GitHub Actions / unit-tests

error: expect(received).toMatch(expected)

Expected substring or pattern: /^(\/tmp|\/private\/tmp)$/ Received: "/home/runner" at <anonymous> (/home/runner/work/sandbox-sdk/sandbox-sdk/packages/sandbox-container/tests/session.test.ts:97:36)
});

it('should fall back to /tmp when workspace is deleted before session creation (issue #288)', async () => {
// Simulate the exact scenario from issue #288
// Create a workspace, then delete it, then try to create a session with it
const workspaceDir = join(testDir, 'workspace');
await mkdir(workspaceDir, { recursive: true });

// Delete the workspace
await rm(workspaceDir, { recursive: true, force: true });

// Now try to create a session with the deleted workspace as cwd
session = new Session({
id: 'test-session-deleted-workspace',
cwd: workspaceDir
});

// Should succeed - falls back to /tmp
await session.initialize();

expect(session.isReady()).toBe(true);

// Verify we can execute commands
const result = await session.exec('echo "session works"');
expect(result.exitCode).toBe(0);
expect(result.stdout.trim()).toBe('session works');
});
});

describe('exec', () => {
Expand Down Expand Up @@ -496,6 +543,92 @@
expect(result.exitCode).toBe(1);
expect(result.stderr).toContain('Failed to change directory');
});

it('should continue working after session cwd is deleted (issue #288)', async () => {
// Create a working directory for the session
const workspaceDir = join(testDir, 'workspace');
await mkdir(workspaceDir, { recursive: true });

session = new Session({
id: 'test-cwd-deletion',
cwd: workspaceDir
});

await session.initialize();

// Verify baseline works
const baseline = await session.exec('echo "baseline"');
expect(baseline.exitCode).toBe(0);
expect(baseline.stdout.trim()).toBe('baseline');

// Delete the workspace directory (this is the bug scenario)
await session.exec(`rm -rf ${workspaceDir}`);

// Try a subsequent command - this should NOT fail with an obscure error
// It should either work (falling back to /) or give a clear error message
const afterRemoval = await session.exec('echo "after removal"');

// The command should succeed - bash can still run commands even if cwd is deleted
// It will use the deleted directory's inode until a cd happens
expect(afterRemoval.exitCode).toBe(0);
expect(afterRemoval.stdout.trim()).toBe('after removal');
});

it('should handle cwd being replaced with symlink (issue #288)', async () => {
// Create directories for the test
const workspaceDir = join(testDir, 'workspace');
const backupDir = join(testDir, 'backup');
await mkdir(workspaceDir, { recursive: true });
await mkdir(backupDir, { recursive: true });

session = new Session({
id: 'test-cwd-symlink',
cwd: workspaceDir
});

await session.initialize();

// Verify baseline works
const baseline = await session.exec('echo "baseline"');
expect(baseline.exitCode).toBe(0);
expect(baseline.stdout.trim()).toBe('baseline');

// Replace workspace with a symlink to backup directory
await session.exec(
`rm -rf ${workspaceDir} && ln -sf ${backupDir} ${workspaceDir}`
);

// Try a subsequent command - should continue working
const afterSymlink = await session.exec('echo "after symlink"');
expect(afterSymlink.exitCode).toBe(0);
expect(afterSymlink.stdout.trim()).toBe('after symlink');
});

it('should be recoverable by cd-ing to valid directory after cwd deletion', async () => {
// Create a working directory for the session
const workspaceDir = join(testDir, 'workspace');
await mkdir(workspaceDir, { recursive: true });

session = new Session({
id: 'test-cwd-recovery',
cwd: workspaceDir
});

await session.initialize();

// Delete the workspace directory
await session.exec(`rm -rf ${workspaceDir}`);

// Change to a valid directory - this should work and recover the session
const cdResult = await session.exec('cd /tmp && pwd');
expect(cdResult.exitCode).toBe(0);
expect(cdResult.stdout.trim()).toBe('/tmp');

// Subsequent commands should work
const afterCd = await session.exec('echo "recovered"');
expect(afterCd.exitCode).toBe(0);
expect(afterCd.stdout.trim()).toBe('recovered');
});
});

describe('FIFO cleanup', () => {
Expand Down
Loading
Loading