Add timeouts to file and git operations to prevent indefinite hangs

.changeset/fix-file-operations-timeout.md

@@ -0,0 +1,36 @@
+---
+"@cloudflare/sandbox": patch
+"@repo/sandbox-container": patch
+---
+
+Fix indefinite hangs in file and git operations by adding comprehensive timeout support
+
+**Root Cause (Issue #166):**
+File operations like `listFiles()` could hang indefinitely when underlying shell commands (e.g., `find`, `git clone`) took too long or never completed. This was caused by missing timeout parameters in `executeInSession` calls throughout the codebase.
+
+**Changes:**
+
+1. **Added per-command timeout support:**
+   - Extended `Session.ExecOptions` to support `timeoutMs` parameter
+   - Updated `SessionManager.executeInSession()` to accept options object: `{ cwd?, timeoutMs? }`
+   - Per-command timeouts now override session-level defaults properly
+
+2. **Added timeouts to all file operations (13 operations in file-service.ts):**
+   - Quick operations (5s): `stat`, `exists`, `mkdir`, `delete`
+   - Medium operations (10s): `rename`
+   - I/O-heavy operations (30s): `read`, `write`, `move`, `listFiles`, `readFileStream`
+   - Documented timeout strategy in FileService class documentation
+
+3. **Added timeouts to all git operations (4 operations in git-service.ts):**
+   - Clone operations (5min): `cloneRepository` - large repos can take significant time
+   - Checkout operations (30s): `checkoutBranch` - can be slow with large repos
+   - Quick operations (10s): `getCurrentBranch`, `listBranches`
+
+4. **Added timeout to process operations:**
+   - `ProcessService.executeCommand()` now properly passes timeout through
+
+**Impact:**
+- Fixes issue #166 where `listFiles()` with options would hang indefinitely
+- Prevents similar hangs in all file, git, and process operations
+- Improves reliability for operations on large directories, slow filesystems, network mounts, or during high I/O load
+- Timeout strategy is documented and tiered based on operation complexity

packages/sandbox-container/src/services/file-service.ts

@@ -33,6 +33,26 @@ export interface FileSystemOperations {
   list(path: string, options?: ListFilesOptions, sessionId?: string): Promise<ServiceResult<FileInfo[]>>;
 }
 
+/**
+ * FileService - Handles file system operations with timeouts
+ *
+ * Timeout Strategy:
+ * All file operations use timeouts to prevent indefinite hangs. Timeouts are tiered based on
+ * operation complexity and expected execution time:
+ *
+ * - Quick operations (5s): stat, exists, mkdir, delete
+ *   These are simple filesystem metadata operations that should complete quickly
+ *
+ * - Medium operations (10s): rename
+ *   File renames are typically quick but may take longer on slow filesystems
+ *
+ * - I/O-heavy operations (30s): read, write, move, listFiles, readFileStream
+ *   These operations involve actual data transfer or directory traversal and can be slow on:
+ *   - Large files or directories
+ *   - Network filesystems (NFS, CIFS)
+ *   - High I/O load
+ *   - Symlink loops (for listFiles)
+ */
 export class FileService implements FileSystemOperations {
   private manager: FileManager;
 
@@ -98,7 +118,7 @@ export class FileService implements FileSystemOperations {
       // 3. Get file size using stat
       const escapedPath = this.escapePath(path);
       const statCommand = `stat -c '%s' ${escapedPath} 2>/dev/null`;
-      const statResult = await this.sessionManager.executeInSession(sessionId, statCommand);
+      const statResult = await this.sessionManager.executeInSession(sessionId, statCommand, { timeoutMs: 5000 });
 
       if (!statResult.success) {
         return {
@@ -134,7 +154,7 @@ export class FileService implements FileSystemOperations {
 
       // 4. Detect MIME type using file command
       const mimeCommand = `file --mime-type -b ${escapedPath}`;
-      const mimeResult = await this.sessionManager.executeInSession(sessionId, mimeCommand);
+      const mimeResult = await this.sessionManager.executeInSession(sessionId, mimeCommand, { timeoutMs: 5000 });
 
       if (!mimeResult.success) {
         return {
@@ -183,7 +203,7 @@ export class FileService implements FileSystemOperations {
       if (isBinary) {
         // Binary files: read as base64, return as-is (DO NOT decode)
         const base64Command = `base64 -w 0 < ${escapedPath}`;
-        const base64Result = await this.sessionManager.executeInSession(sessionId, base64Command);
+        const base64Result = await this.sessionManager.executeInSession(sessionId, base64Command, { timeoutMs: 30000 });
 
         if (!base64Result.success) {
           return {
@@ -220,7 +240,7 @@ export class FileService implements FileSystemOperations {
       } else {
         // Text files: read normally
         const catCommand = `cat ${escapedPath}`;
-        const catResult = await this.sessionManager.executeInSession(sessionId, catCommand);
+        const catResult = await this.sessionManager.executeInSession(sessionId, catCommand, { timeoutMs: 30000 });
 
         if (!catResult.success) {
           return {
@@ -309,7 +329,7 @@ export class FileService implements FileSystemOperations {
       const base64Content = Buffer.from(content, 'utf-8').toString('base64');
       const command = `echo '${base64Content}' | base64 -d > ${escapedPath}`;
 
-      const execResult = await this.sessionManager.executeInSession(sessionId, command);
+      const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 30000 });
 
       if (!execResult.success) {
         return execResult as ServiceResult<void>;
@@ -412,7 +432,7 @@ export class FileService implements FileSystemOperations {
       const escapedPath = this.escapePath(path);
       const command = `rm ${escapedPath}`;
 
-      const execResult = await this.sessionManager.executeInSession(sessionId, command);
+      const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 5000 });
 
       if (!execResult.success) {
         return execResult as ServiceResult<void>;
@@ -501,7 +521,7 @@ export class FileService implements FileSystemOperations {
       const escapedNewPath = this.escapePath(newPath);
       const command = `mv ${escapedOldPath} ${escapedNewPath}`;
 
-      const execResult = await this.sessionManager.executeInSession(sessionId, command);
+      const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 10000 });
 
       if (!execResult.success) {
         return execResult as ServiceResult<void>;
@@ -586,7 +606,7 @@ export class FileService implements FileSystemOperations {
       const escapedDest = this.escapePath(destinationPath);
       const command = `mv ${escapedSource} ${escapedDest}`;
 
-      const execResult = await this.sessionManager.executeInSession(sessionId, command);
+      const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 30000 });
 
       if (!execResult.success) {
         return execResult as ServiceResult<void>;
@@ -656,7 +676,7 @@ export class FileService implements FileSystemOperations {
       command += ` ${escapedPath}`;
 
       // 4. Create directory using SessionManager
-      const execResult = await this.sessionManager.executeInSession(sessionId, command);
+      const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 5000 });
 
       if (!execResult.success) {
         return execResult as ServiceResult<void>;
@@ -718,7 +738,7 @@ export class FileService implements FileSystemOperations {
       const escapedPath = this.escapePath(path);
       const command = `test -e ${escapedPath}`;
 
-      const execResult = await this.sessionManager.executeInSession(sessionId, command);
+      const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 5000 });
 
       if (!execResult.success) {
         // If execution fails, treat as non-existent
@@ -799,7 +819,7 @@ export class FileService implements FileSystemOperations {
       const command = `stat ${statCmd.args[0]} ${statCmd.args[1]} ${escapedPath}`;
 
       // 5. Get file stats using SessionManager
-      const execResult = await this.sessionManager.executeInSession(sessionId, command);
+      const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 5000 });
 
       if (!execResult.success) {
         return execResult as ServiceResult<FileStats>;
@@ -968,7 +988,7 @@ export class FileService implements FileSystemOperations {
       // Skip the base directory itself and format output
       findCommand += ` -not -path ${escapedPath} -printf '%p\\t%y\\t%s\\t%TY-%Tm-%TdT%TH:%TM:%TS\\t%m\\n'`;
 
-      const execResult = await this.sessionManager.executeInSession(sessionId, findCommand);
+      const execResult = await this.sessionManager.executeInSession(sessionId, findCommand, { timeoutMs: 30000 });
 
       if (!execResult.success) {
         return {
@@ -1165,7 +1185,7 @@ export class FileService implements FileSystemOperations {
               command = `dd if=${escapedPath} bs=${chunkSize} skip=${skip} count=${count} 2>/dev/null`;
             }
 
-            const execResult = await this.sessionManager.executeInSession(sessionId, command);
+            const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 30000 });
 
             if (!execResult.success) {
               const errorEvent = {

packages/sandbox-container/src/services/git-service.ts

@@ -87,8 +87,9 @@ export class GitService {
       const command = this.buildCommand(args);
 
       // Execute git clone (via SessionManager)
+      // Clone can take a long time for large repos or slow networks, so use 5 minute timeout
       const sessionId = options.sessionId || 'default';
-      const execResult = await this.sessionManager.executeInSession(sessionId, command);
+      const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 300000 });
 
       if (!execResult.success) {
         return execResult as ServiceResult<{ path: string; branch: string }>;
@@ -125,7 +126,8 @@ export class GitService {
       // explicitly specified or defaulted to the repository's HEAD
       const branchArgs = this.manager.buildGetCurrentBranchArgs();
       const branchCommand = this.buildCommand(branchArgs);
-      const branchExecResult = await this.sessionManager.executeInSession(sessionId, branchCommand, targetDirectory);
+      // Quick operation - 10 second timeout
+      const branchExecResult = await this.sessionManager.executeInSession(sessionId, branchCommand, { cwd: targetDirectory, timeoutMs: 10000 });
 
       if (!branchExecResult.success) {
         // If we can't get the branch, use fallback but don't fail the entire operation
@@ -221,7 +223,8 @@ export class GitService {
       const command = this.buildCommand(args);
 
       // Execute git checkout (via SessionManager)
-      const execResult = await this.sessionManager.executeInSession(sessionId, command, repoPath);
+      // Checkout can take time with large repos - 30 second timeout
+      const execResult = await this.sessionManager.executeInSession(sessionId, command, { cwd: repoPath, timeoutMs: 30000 });
 
       if (!execResult.success) {
         return execResult as ServiceResult<void>;
@@ -301,7 +304,8 @@ export class GitService {
       const command = this.buildCommand(args);
 
       // Execute command (via SessionManager)
-      const execResult = await this.sessionManager.executeInSession(sessionId, command, repoPath);
+      // Quick operation - 10 second timeout
+      const execResult = await this.sessionManager.executeInSession(sessionId, command, { cwd: repoPath, timeoutMs: 10000 });
 
       if (!execResult.success) {
         return execResult as ServiceResult<string>;
@@ -375,7 +379,8 @@ export class GitService {
       const command = this.buildCommand(args);
 
       // Execute command (via SessionManager)
-      const execResult = await this.sessionManager.executeInSession(sessionId, command, repoPath);
+      // Quick operation - 10 second timeout
+      const execResult = await this.sessionManager.executeInSession(sessionId, command, { cwd: repoPath, timeoutMs: 10000 });
 
       if (!execResult.success) {
         return execResult as ServiceResult<string[]>;

packages/sandbox-container/src/services/process-service.ts

@@ -94,8 +94,7 @@ export class ProcessService {
       const result = await this.sessionManager.executeInSession(
         sessionId,
         command,
-        options.cwd,
-        options.timeoutMs
+        { cwd: options.cwd, timeoutMs: options.timeoutMs }
       );
 
       if (!result.success) {

packages/sandbox-container/src/services/session-manager.ts

@@ -109,8 +109,7 @@ export class SessionManager {
   async executeInSession(
     sessionId: string,
     command: string,
-    cwd?: string,
-    timeoutMs?: number
+    options?: { cwd?: string; timeoutMs?: number }
   ): Promise<ServiceResult<RawExecResult>> {
     try {
       // Get or create session on demand
@@ -120,8 +119,8 @@ export class SessionManager {
       if (!sessionResult.success && (sessionResult.error!.details as InternalErrorContext)?.originalError === 'Session not found') {
         sessionResult = await this.createSession({
           id: sessionId,
-          cwd: cwd || '/workspace',
-          commandTimeoutMs: timeoutMs, // Pass timeout to session
+          cwd: options?.cwd || '/workspace',
+          commandTimeoutMs: options?.timeoutMs, // Pass timeout to session
         });
       }
 
@@ -131,7 +130,12 @@ export class SessionManager {
 
       const session = sessionResult.data;
 
-      const result = await session.exec(command, cwd ? { cwd } : undefined);
+      // Pass both cwd and timeout to session.exec()
+      const execOptions: { cwd?: string; timeoutMs?: number } = {};
+      if (options?.cwd) execOptions.cwd = options.cwd;
+      if (options?.timeoutMs !== undefined) execOptions.timeoutMs = options.timeoutMs;
+
+      const result = await session.exec(command, Object.keys(execOptions).length > 0 ? execOptions : undefined);
 
       return {
         success: true,

packages/sandbox-container/src/session.ts

@@ -88,6 +88,8 @@ export interface RawExecResult {
 interface ExecOptions {
   /** Override working directory for this command only */
   cwd?: string;
+  /** Override timeout for this command only (milliseconds) */
+  timeoutMs?: number;
 }
 
 /** Command handle for tracking and killing running commands */
@@ -230,7 +232,7 @@ export class Session {
       // 2. Shell death (shell process exits unexpectedly)
       // This allows us to detect shell termination (e.g., from 'exit' command) immediately
       const exitCode = await Promise.race([
-        this.waitForExitCode(exitCodeFile),
+        this.waitForExitCode(exitCodeFile, options?.timeoutMs),
         this.shellExitedPromise!
       ]);
 
@@ -737,8 +739,10 @@ export class Session {
    *
    * Uses fs.watch for fast detection, with polling fallback for systems where
    * fs.watch doesn't reliably detect rename() operations (common on tmpfs, overlayfs).
+   *
+   * @param timeoutMs Optional per-command timeout override (milliseconds)
    */
-  private async waitForExitCode(exitCodeFile: string): Promise<number> {
+  private async waitForExitCode(exitCodeFile: string, timeoutMs?: number): Promise<number> {
     return new Promise((resolve, reject) => {
       const dir = dirname(exitCodeFile);
       const filename = basename(exitCodeFile);
@@ -781,15 +785,17 @@ export class Session {
       }, 50); // Poll every 50ms as fallback
 
       // STEP 3: Set up timeout if configured
-      if (this.commandTimeoutMs !== undefined) {
+      // Use per-command timeout if provided, otherwise use session-level default
+      const effectiveTimeout = timeoutMs ?? this.commandTimeoutMs;
+      if (effectiveTimeout !== undefined) {
         setTimeout(() => {
           if (!resolved) {
             resolved = true;
             watcher.close();
             clearInterval(pollInterval);
-            reject(new Error(`Command timeout after ${this.commandTimeoutMs}ms`));
+            reject(new Error(`Command timeout after ${effectiveTimeout}ms`));
           }
-        }, this.commandTimeoutMs);
+        }, effectiveTimeout);
       }
 
       // STEP 4: Check if file already exists