fix(agent): harden darwin keychain boundary

Author: shixi-li

src/agent/agent-task-executor.js

@@ -690,7 +690,10 @@ function buildFinalContext({ agent, context, desiredOutputFormat, runOutputForma
 }
 
 function buildSpawnEnv(agent, providerName, modelSpec, options = {}) {
-  const { claudeSettingsPath = null } = options;
+  const {
+    claudeSettingsPath = null,
+    applyDarwinKeychainBoundary = applyDarwinKeychainBoundaryToEnv,
+  } = options;
   const spawnEnv = { ...process.env };
   const agentCwd = agent.config?.cwd || agent.worktree?.path || process.cwd();
   const clusterId = agent.cluster?.id || agent.cluster_id || process.env.ZEROSHOT_CLUSTER_ID;
@@ -730,7 +733,7 @@ function buildSpawnEnv(agent, providerName, modelSpec, options = {}) {
   // Docker isolation never reaches buildSpawnEnv (see spawnClaudeTaskIsolated).
   // Applied before the worktree tool bins so repo-managed tool substitutes
   // stay first on PATH.
-  applyDarwinKeychainBoundaryToEnv(spawnEnv);
+  applyDarwinKeychainBoundary(spawnEnv);
 
   prependWorktreeToolBinToEnv(spawnEnv, {
     cwd: agentCwd,
@@ -830,14 +833,7 @@ function createPendingTaskLaunchHandle({
   return handle;
 }
 
-function spawnTaskProcess({
-  agent,
-  ctPath,
-  args,
-  cwd,
-  spawnEnv,
-  spawnTimeoutMs = 30000,
-}) {
+function spawnTaskProcess({ agent, ctPath, args, cwd, spawnEnv, spawnTimeoutMs = 30000 }) {
   // Timeout for spawn phase - if CLI hangs during init (e.g., opencode 429 bug), kill it.
   const SPAWN_TIMEOUT_MS = spawnTimeoutMs;
   // spawn() throws on null bytes in argv; strip them before they get there.
@@ -886,10 +882,7 @@ function spawnTaskProcess({
     const classifyCleanupOwnership = trackTaskWrapperCleanupOwnership(findPersistedTaskId);
     const rejectWithOwnership = async (error) => {
       const classifiedError = classifyCleanupOwnership(error);
-      if (
-        !callerOwnsCommandCleanup(classifiedError) &&
-        agent.currentTask === pendingLaunch
-      ) {
+      if (!callerOwnsCommandCleanup(classifiedError) && agent.currentTask === pendingLaunch) {
         let termination;
         try {
           termination = await pendingLaunch.kill(classifiedError.message);
@@ -2206,7 +2199,6 @@ function startIsolatedTail({ agent, manager, clusterId, logFilePath, state, onLi
   });
 }
 
-
 async function checkIsolatedStatus({
   agent,
   manager,

src/claude-task-runner.js

@@ -152,13 +152,16 @@ class ClaudeTaskRunner extends TaskRunner {
    * @param {boolean} [options.quiet] - Suppress console logging
    * @param {number} [options.timeout] - Task timeout in ms (default: 1 hour)
    * @param {Function} [options.onOutput] - Callback for output lines
+   * @param {Function} [options.applyDarwinKeychainBoundary] - Boundary injection seam for tests
    */
   constructor(options = {}) {
     super();
     this.messageBus = options.messageBus || null;
     this.quiet = options.quiet || false;
     this.timeout = options.timeout || 60 * 60 * 1000;
     this.onOutput = options.onOutput || null;
+    this.applyDarwinKeychainBoundary =
+      options.applyDarwinKeychainBoundary || applyDarwinKeychainBoundaryToEnv;
   }
 
   /**
@@ -392,7 +395,7 @@ class ClaudeTaskRunner extends TaskRunner {
 
     // KEYCHAIN BOUNDARY (darwin only): keep non-interactive worker descendants
     // away from the user's GUI Keychain session (issue #704).
-    applyDarwinKeychainBoundaryToEnv(spawnEnv);
+    this.applyDarwinKeychainBoundary(spawnEnv);
 
     prependWorktreeToolBinToEnv(spawnEnv, { cwd, worktreePath });
 

src/darwin-keychain-boundary.js

@@ -19,15 +19,12 @@
 const fs = require('fs');
 const os = require('os');
 const path = require('path');
+const { randomUUID } = require('node:crypto');
 
 const SHIM_DIR_RELATIVE_PATH = path.join('.zeroshot', 'keychain-shim');
 const REAL_SECURITY_PATH = '/usr/bin/security';
 const OPT_OUT_ENV_VAR = 'ZEROSHOT_ALLOW_INTERACTIVE_KEYCHAIN';
 
-function pathKeyForEnv(env) {
-  return Object.keys(env).find((key) => key.toUpperCase() === 'PATH') || 'PATH';
-}
-
 function shellQuote(value) {
   return `'${String(value).replace(/'/g, `'\\''`)}'`;
 }
@@ -96,10 +93,27 @@ function ensureDarwinKeychainShimDir(options = {}) {
     // Missing or unreadable: (re)write below.
   }
   if (existing !== script) {
-    fs.writeFileSync(shimPath, script, { mode: 0o755 });
+    const tempPath = path.join(shimDir, `.security.${process.pid}.${randomUUID()}.tmp`);
+    try {
+      fs.writeFileSync(tempPath, script, { mode: 0o755, flag: 'wx' });
+      // The creation mode is subject to umask. Set the final mode before the
+      // rename so the live path is never observable as non-executable.
+      fs.chmodSync(tempPath, 0o755);
+      fs.renameSync(tempPath, shimPath);
+    } catch (error) {
+      try {
+        // Remove any unpublished partial file without masking the publication
+        // failure that caused this cleanup path.
+        fs.rmSync(tempPath, { force: true });
+      } catch (cleanupError) {
+        error.message += ` Cleanup also failed: ${cleanupError.message}.`;
+      }
+      throw error;
+    }
+  } else {
+    // An existing matching shim may have drifted permissions.
+    fs.chmodSync(shimPath, 0o755);
   }
-  // writeFileSync's mode only applies on creation; enforce it unconditionally.
-  fs.chmodSync(shimPath, 0o755);
 
   return shimDir;
 }
@@ -139,11 +153,18 @@ function applyDarwinKeychainBoundaryToEnv(env, options = {}) {
     );
   }
 
-  const pathKey = pathKeyForEnv(env);
-  const existingEntries = (env[pathKey] || '')
-    .split(path.delimiter)
-    .filter((entry) => entry && entry !== shimDir);
-  env[pathKey] = [shimDir, ...existingEntries].join(path.delimiter);
+  // Darwin environment keys are case-sensitive: descendants consult PATH,
+  // never a differently-cased key such as Path. Preserve empty components
+  // because POSIX interprets them as the current directory. An absent PATH
+  // becomes only the shim, while an explicitly empty PATH retains its empty
+  // component after the shim (`<shim>:`).
+  const existingEntries =
+    env.PATH === undefined
+      ? []
+      : String(env.PATH)
+          .split(path.delimiter)
+          .filter((entry) => entry !== shimDir);
+  env.PATH = [shimDir, ...existingEntries].join(path.delimiter);
   return env;
 }