Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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 @@ describe('Session', () => {
// Session directory should be created (we can't easily check without accessing private fields)
expect(session.isReady()).toBe(true);
});

it('should fall back to home directory 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 the home directory 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 the home directory since the requested cwd doesn't exist
const homeDir = process.env.HOME || '/root';
expect(result.stdout.trim()).toBe(homeDir);
});

it('should fall back to home directory 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 home directory
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 @@ describe('Session', () => {
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
105 changes: 105 additions & 0 deletions tests/e2e/workspace-deletion-workflow.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/**
* Workspace Deletion Workflow Tests (Issue #288)
*
* Tests that new sessions fall back to the home directory when /workspace
* doesn't exist, preventing "Unknown Error, TODO" failures.
*
* @see https://github.com/cloudflare/sandbox-sdk/issues/288
*/

import { describe, test, expect, beforeAll } from 'vitest';
import {
getSharedSandbox,
createUniqueSession
} from './helpers/global-sandbox';
import type { ExecResult } from '@repo/shared';

describe('Workspace Deletion Workflow (Issue #288)', () => {
let workerUrl: string;
let sandbox: Awaited<ReturnType<typeof getSharedSandbox>>;

beforeAll(async () => {
sandbox = await getSharedSandbox();
workerUrl = sandbox.workerUrl;
}, 120000);

/**
* Core test for issue #288: new session creation after /workspace is deleted.
*
* The bug was that creating a NEW session after /workspace was deleted would
* fail with "Unknown Error, TODO". The fix makes session initialization fall
* back to the home directory if /workspace doesn't exist.
*/
test('new session falls back to home directory when /workspace is deleted', async () => {
const setupHeaders = sandbox.createHeaders(createUniqueSession());

// Delete /workspace
const deleteResponse = await fetch(`${workerUrl}/api/execute`, {
method: 'POST',
headers: setupHeaders,
body: JSON.stringify({ command: 'rm -rf /workspace' })
});
expect(deleteResponse.status).toBe(200);

// Create a NEW session - this is where the bug occurred
const newHeaders = sandbox.createHeaders(createUniqueSession());

const execResponse = await fetch(`${workerUrl}/api/execute`, {
method: 'POST',
headers: newHeaders,
body: JSON.stringify({ command: 'pwd' })
});

expect(execResponse.status).toBe(200);
const execData = (await execResponse.json()) as ExecResult;
expect(execData.success).toBe(true);
expect(execData.stdout?.trim()).toBe('/root');

// Restore /workspace for other tests
await fetch(`${workerUrl}/api/execute`, {
method: 'POST',
headers: setupHeaders,
body: JSON.stringify({ command: 'mkdir -p /workspace' })
});
}, 90000);

/**
* Tests that new sessions work when /workspace is a symlink to a valid directory.
*/
test('new session works when /workspace is a symlink', async () => {
const setupHeaders = sandbox.createHeaders(createUniqueSession());
const backupDir = `/tmp/workspace-backup-${Date.now()}`;

// Replace /workspace with a symlink
await fetch(`${workerUrl}/api/execute`, {
method: 'POST',
headers: setupHeaders,
body: JSON.stringify({
command: `mkdir -p ${backupDir} && rm -rf /workspace && ln -sf ${backupDir} /workspace`
})
});

// Create a NEW session - should work with the symlink
const newHeaders = sandbox.createHeaders(createUniqueSession());

const execResponse = await fetch(`${workerUrl}/api/execute`, {
method: 'POST',
headers: newHeaders,
body: JSON.stringify({ command: 'pwd -P' }) // -P resolves symlinks
});

expect(execResponse.status).toBe(200);
const execData = (await execResponse.json()) as ExecResult;
expect(execData.success).toBe(true);
expect(execData.stdout?.trim()).toBe(backupDir);

// Restore /workspace as a real directory
await fetch(`${workerUrl}/api/execute`, {
method: 'POST',
headers: setupHeaders,
body: JSON.stringify({
command: `rm -f /workspace && mkdir -p /workspace && rm -rf ${backupDir}`
})
});
}, 90000);
});
Loading