Skip to content

Commit 67100d0

Browse files
authored
fix workspace bug (#289)
* fix workspace bug * fallback to tmp * fallbac to homedir instead of /tmp * Fix workspace bug in sandbox container Fixes a bug in the workspace related to sandbox functionality. * clear tests * minor nits * Delete tests/e2e/workspace-deletion-workflow.test.ts
1 parent c6349aa commit 67100d0

File tree

3 files changed

+140
-3
lines changed

3 files changed

+140
-3
lines changed

.changeset/sixty-walls-serve.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@cloudflare/sandbox": patch
3+
---
4+
5+
fix workspace bug

packages/sandbox-container/src/session.ts

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

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

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

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

147+
// Determine working directory. If the requested cwd doesn't exist, we fall
148+
// back to the home directory since it's a natural default for shell sessions.
149+
const homeDir = process.env.HOME || '/root';
150+
let cwd = this.options.cwd || CONFIG.DEFAULT_CWD;
151+
try {
152+
await stat(cwd);
153+
} catch {
154+
this.logger.debug(
155+
`Shell startup directory '${cwd}' does not exist, using '${homeDir}'`,
156+
{
157+
sessionId: this.id,
158+
requestedCwd: cwd,
159+
actualCwd: homeDir
160+
}
161+
);
162+
cwd = homeDir;
163+
}
164+
140165
// Spawn persistent bash with stdin pipe - no IPC or wrapper needed!
141166
this.shell = Bun.spawn({
142167
cmd: ['bash', '--norc'],
143-
cwd: this.options.cwd || CONFIG.DEFAULT_CWD,
168+
cwd,
144169
env: {
145170
...process.env,
146171
...this.options.env,

packages/sandbox-container/tests/session.test.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,53 @@ describe('Session', () => {
7575
// Session directory should be created (we can't easily check without accessing private fields)
7676
expect(session.isReady()).toBe(true);
7777
});
78+
79+
it('should fall back to home directory when cwd does not exist', async () => {
80+
// Session cwd only affects shell startup directory - it's not critical.
81+
// If cwd doesn't exist, we fall back to the home directory since individual
82+
// commands can specify their own cwd anyway.
83+
session = new Session({
84+
id: 'test-session-nonexistent-cwd',
85+
cwd: '/nonexistent/path/that/does/not/exist'
86+
});
87+
88+
await session.initialize();
89+
90+
expect(session.isReady()).toBe(true);
91+
92+
// Verify we can execute commands
93+
const result = await session.exec('pwd');
94+
expect(result.exitCode).toBe(0);
95+
// The shell should have started in the home directory since the requested cwd doesn't exist
96+
const homeDir = process.env.HOME || '/root';
97+
expect(result.stdout.trim()).toBe(homeDir);
98+
});
99+
100+
it('should fall back to home directory when workspace is deleted before session creation', async () => {
101+
// Simulate the scenario where workspace is deleted before session creation
102+
// Create a workspace, then delete it, then try to create a session with it
103+
const workspaceDir = join(testDir, 'workspace');
104+
await mkdir(workspaceDir, { recursive: true });
105+
106+
// Delete the workspace
107+
await rm(workspaceDir, { recursive: true, force: true });
108+
109+
// Now try to create a session with the deleted workspace as cwd
110+
session = new Session({
111+
id: 'test-session-deleted-workspace',
112+
cwd: workspaceDir
113+
});
114+
115+
// Should succeed - falls back to home directory
116+
await session.initialize();
117+
118+
expect(session.isReady()).toBe(true);
119+
120+
// Verify we can execute commands
121+
const result = await session.exec('echo "session works"');
122+
expect(result.exitCode).toBe(0);
123+
expect(result.stdout.trim()).toBe('session works');
124+
});
78125
});
79126

80127
describe('exec', () => {
@@ -496,6 +543,66 @@ describe('Session', () => {
496543
expect(result.exitCode).toBe(1);
497544
expect(result.stderr).toContain('Failed to change directory');
498545
});
546+
547+
it('should continue working after session cwd is deleted', async () => {
548+
// Create a working directory for the session
549+
const workspaceDir = join(testDir, 'workspace');
550+
await mkdir(workspaceDir, { recursive: true });
551+
552+
session = new Session({
553+
id: 'test-cwd-deletion',
554+
cwd: workspaceDir
555+
});
556+
557+
await session.initialize();
558+
559+
// Verify baseline works
560+
const baseline = await session.exec('echo "baseline"');
561+
expect(baseline.exitCode).toBe(0);
562+
expect(baseline.stdout.trim()).toBe('baseline');
563+
564+
// Delete the workspace directory (this is the bug scenario)
565+
await session.exec(`rm -rf ${workspaceDir}`);
566+
567+
// Try a subsequent command - this should NOT fail with an obscure error
568+
// It should either work (falling back to /) or give a clear error message
569+
const afterRemoval = await session.exec('echo "after removal"');
570+
571+
// The command should succeed - bash can still run commands even if cwd is deleted
572+
// It will use the deleted directory's inode until a cd happens
573+
expect(afterRemoval.exitCode).toBe(0);
574+
expect(afterRemoval.stdout.trim()).toBe('after removal');
575+
});
576+
577+
it('should handle cwd being replaced with symlink', async () => {
578+
// Create directories for the test
579+
const workspaceDir = join(testDir, 'workspace');
580+
const backupDir = join(testDir, 'backup');
581+
await mkdir(workspaceDir, { recursive: true });
582+
await mkdir(backupDir, { recursive: true });
583+
584+
session = new Session({
585+
id: 'test-cwd-symlink',
586+
cwd: workspaceDir
587+
});
588+
589+
await session.initialize();
590+
591+
// Verify baseline works
592+
const baseline = await session.exec('echo "baseline"');
593+
expect(baseline.exitCode).toBe(0);
594+
expect(baseline.stdout.trim()).toBe('baseline');
595+
596+
// Replace workspace with a symlink to backup directory
597+
await session.exec(
598+
`rm -rf ${workspaceDir} && ln -sf ${backupDir} ${workspaceDir}`
599+
);
600+
601+
// Try a subsequent command - should continue working
602+
const afterSymlink = await session.exec('echo "after symlink"');
603+
expect(afterSymlink.exitCode).toBe(0);
604+
expect(afterSymlink.stdout.trim()).toBe('after symlink');
605+
});
499606
});
500607

501608
describe('FIFO cleanup', () => {

0 commit comments

Comments
 (0)