From 0b042364c6be3ea0c70c18e8ad685b03c894191a Mon Sep 17 00:00:00 2001
From: Landon Cox <landon.cox@microsoft.com>
Date: Mon, 6 Jul 2026 07:51:21 -0700
Subject: [PATCH] fix: tolerate EROFS when chmod on pre-existing mcp-logs dir
 fails
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The prepareLogDirectories function only caught EPERM when chmod failed
on a pre-existing /tmp/gh-aw/mcp-logs directory, but EROFS (read-only
file system) is equally benign — it just means we cannot change
permissions on a directory managed by another process.

Also fix test mock leakage: jest.clearAllMocks() does not reset mock
implementations, so the chmodSync mock override from one test leaked
into subsequent tests. Explicitly restore the passthrough implementation
in beforeEach.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 src/workdir-setup.test.ts | 25 +++++++++++++++++++++++--
 src/workdir-setup.ts      |  5 +++--
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/src/workdir-setup.test.ts b/src/workdir-setup.test.ts
index 63e043a98..ddd2dd057 100644
--- a/src/workdir-setup.test.ts
+++ b/src/workdir-setup.test.ts
@@ -37,6 +37,10 @@ function setupWorkdirFixture({ cleanupChrootHome = true } = {}) {
     tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'workdir-setup-test-'));
     jest.clearAllMocks();
     (fs.chownSync as unknown as jest.Mock).mockImplementation(() => undefined);
+    // Restore chmodSync to its default passthrough (clearAllMocks doesn't reset implementations)
+    (fs.chmodSync as jest.Mock).mockImplementation(
+      (...args: Parameters<typeof actualFs.chmodSync>) => actualFs.chmodSync(...args),
+    );
     (getRealUserHome as jest.Mock).mockReturnValue(tempDir);
   });
 
@@ -158,7 +162,7 @@ describe('prepareWorkDirectories', () => {
       expect(() => prepareWorkDirectories(config, logPaths)).not.toThrow();
     });
 
-    it('rethrows non-EPERM errors when chmod on pre-existing mcp-logs dir fails', () => {
+    it('does not throw when chmod on pre-existing mcp-logs dir fails with EROFS', () => {
       const mcpLogsDir = '/tmp/gh-aw/mcp-logs';
       fs.mkdirSync(mcpLogsDir, { recursive: true, mode: 0o700 });
 
@@ -172,7 +176,24 @@ describe('prepareWorkDirectories', () => {
       const config = buildConfig();
       const logPaths = resolveLogPaths(config);
 
-      expect(() => prepareWorkDirectories(config, logPaths)).toThrow(erofs);
+      expect(() => prepareWorkDirectories(config, logPaths)).not.toThrow();
+    });
+
+    it('rethrows non-EPERM/EROFS errors when chmod on pre-existing mcp-logs dir fails', () => {
+      const mcpLogsDir = '/tmp/gh-aw/mcp-logs';
+      fs.mkdirSync(mcpLogsDir, { recursive: true, mode: 0o700 });
+
+      const eio = new Error("EIO: i/o error, chmod '/tmp/gh-aw/mcp-logs'") as NodeJS.ErrnoException;
+      eio.code = 'EIO';
+      (fs.chmodSync as jest.Mock).mockImplementation((target: fs.PathLike, mode: fs.Mode) => {
+        if (target === mcpLogsDir) throw eio;
+        actualFs.chmodSync(target, mode);
+      });
+
+      const config = buildConfig();
+      const logPaths = resolveLogPaths(config);
+
+      expect(() => prepareWorkDirectories(config, logPaths)).toThrow(eio);
     });
 
     it('falls back to world-writable squid logs when squid chown fails', () => {
diff --git a/src/workdir-setup.ts b/src/workdir-setup.ts
index 0574310a1..836142cd7 100644
--- a/src/workdir-setup.ts
+++ b/src/workdir-setup.ts
@@ -200,10 +200,11 @@ function prepareLogDirectories(logPaths: LogPaths): void {
       fs.chmodSync(mcpLogsDir, 0o777);
       logger.debug(`MCP logs directory permissions fixed at: ${mcpLogsDir}`);
     } catch (error) {
-      if ((error as NodeJS.ErrnoException).code !== 'EPERM') {
+      const code = (error as NodeJS.ErrnoException).code;
+      if (code !== 'EPERM' && code !== 'EROFS') {
         throw error;
       }
-      logger.debug(`MCP logs directory already exists at: ${mcpLogsDir} (chmod skipped, owned by another user)`);
+      logger.debug(`MCP logs directory already exists at: ${mcpLogsDir} (chmod skipped: ${code})`);
     }
   }
 }