🤖 fix: block shell injection via malicious branch names

src/node/services/streamManager.ts

@@ -58,6 +58,7 @@ import { MUX_GATEWAY_SESSION_EXPIRED_MESSAGE } from "@/common/constants/muxGatew
 import { getModelStats } from "@/common/utils/tokens/modelStats";
 import { resolveModelForMetadata } from "@/common/utils/providers/modelEntries";
 import { getErrorMessage } from "@/common/utils/errors";
+import { shellQuote } from "@/common/utils/shell";
 import { classify429Capacity } from "@/common/utils/errors/classify429Capacity";
 import { normalizeLiteralRequiredToolPattern } from "@/common/utils/agentTools";
 
@@ -569,7 +570,7 @@ export class StreamManager extends EventEmitter {
     // Fire-and-forget: don't block stream completion waiting for directory deletion.
     // This is especially important for SSH where rm -rf can take 500ms-2s.
     void runtime
-      .exec(`rm -rf "${tempDirBasename}"`, {
+      .exec(`rm -rf ${shellQuote(tempDirBasename)}`, {
         cwd: tempDirParent,
         timeout: 10,
       })

src/node/utils/runtime/helpers.ts

@@ -1,6 +1,7 @@
 import type { Runtime, ExecOptions } from "@/node/runtime/Runtime";
 import { PlatformPaths } from "@/node/utils/paths.main";
 import { getLegacyPlanFilePath, getPlanFilePath } from "@/common/utils/planStorage";
+import { shellQuote } from "@/common/utils/shell";
 
 /**
  * Convenience helpers for working with streaming Runtime APIs.
@@ -162,10 +163,14 @@ export async function readPlanFile(
       // Migrate: move to new location
       try {
         const planDir = planPath.substring(0, planPath.lastIndexOf("/"));
-        await execBuffered(runtime, `mkdir -p "${planDir}" && mv "${legacyPath}" "${planPath}"`, {
-          cwd: "/tmp",
-          timeout: 5,
-        });
+        await execBuffered(
+          runtime,
+          `mkdir -p ${shellQuote(planDir)} && mv ${shellQuote(legacyPath)} ${shellQuote(planPath)}`,
+          {
+            cwd: "/tmp",
+            timeout: 5,
+          }
+        );
       } catch {
         // Migration failed, but we have the content
       }
@@ -230,10 +235,14 @@ export async function movePlanFile(
     // Resolve tildes to absolute paths - bash doesn't expand ~ inside quotes
     const resolvedOldPath = await runtime.resolvePath(oldPath);
     const resolvedNewPath = await runtime.resolvePath(newPath);
-    await execBuffered(runtime, `mv "${resolvedOldPath}" "${resolvedNewPath}"`, {
-      cwd: "/tmp",
-      timeout: 5,
-    });
+    await execBuffered(
+      runtime,
+      `mv ${shellQuote(resolvedOldPath)} ${shellQuote(resolvedNewPath)}`,
+      {
+        cwd: "/tmp",
+        timeout: 5,
+      }
+    );
   } catch {
     // No plan file to move, that's fine
   }

src/node/worktree/WorktreeManager.test.ts

@@ -1,8 +1,9 @@
-import { describe, expect, it } from "bun:test";
+import { describe, expect, it, spyOn } from "bun:test";
 import * as os from "os";
 import * as path from "path";
 import * as fsPromises from "fs/promises";
 import { execSync } from "node:child_process";
+import * as disposableExec from "@/node/utils/disposableExec";
 import type { InitLogger } from "@/node/runtime/Runtime";
 import { WorktreeManager } from "./WorktreeManager";
 
@@ -107,6 +108,78 @@ describe("WorktreeManager.deleteWorkspace", () => {
     }
   }, 20_000);
 
+  it("force-delete fallback does not execute shell payloads embedded in branch names", async () => {
+    const rootDir = await fsPromises.realpath(
+      await fsPromises.mkdtemp(path.join(os.tmpdir(), "worktree-manager-delete-"))
+    );
+    const sentinelPath = path.join(
+      os.tmpdir(),
+      `mux_injection_test_${Date.now()}_${Math.random().toString(16).slice(2)}`
+    );
+    const branchName = `feature/inject-$(touch\${IFS}${sentinelPath})`;
+
+    let execFileAsyncSpy: { mockRestore: () => void } | null = null;
+
+    try {
+      const projectPath = path.join(rootDir, "repo");
+      await fsPromises.mkdir(projectPath, { recursive: true });
+      initGitRepo(projectPath);
+
+      const srcBaseDir = path.join(rootDir, "src");
+      await fsPromises.mkdir(srcBaseDir, { recursive: true });
+
+      const manager = new WorktreeManager(srcBaseDir);
+      const initLogger = createNullInitLogger();
+
+      const createResult = await manager.createWorkspace({
+        projectPath,
+        branchName,
+        trunkBranch: "main",
+        initLogger,
+      });
+      expect(createResult.success).toBe(true);
+      if (!createResult.success) return;
+      if (!createResult.workspacePath) {
+        throw new Error("Expected workspacePath from createWorkspace");
+      }
+      const workspacePath = createResult.workspacePath;
+
+      const originalExecFileAsync = disposableExec.execFileAsync;
+      execFileAsyncSpy = spyOn(disposableExec, "execFileAsync").mockImplementation(
+        (file, args, options) => {
+          if (file === "git" && args[2] === "worktree" && args[3] === "remove") {
+            return originalExecFileAsync("git", ["definitely-invalid-command"]);
+          }
+
+          return originalExecFileAsync(file, args, options);
+        }
+      );
+
+      const deleteResult = await manager.deleteWorkspace(projectPath, branchName, true);
+      expect(deleteResult.success).toBe(true);
+
+      let workspaceExists = true;
+      try {
+        await fsPromises.access(workspacePath);
+      } catch {
+        workspaceExists = false;
+      }
+      expect(workspaceExists).toBe(false);
+
+      let sentinelExists = true;
+      try {
+        await fsPromises.access(sentinelPath);
+      } catch {
+        sentinelExists = false;
+      }
+      expect(sentinelExists).toBe(false);
+    } finally {
+      execFileAsyncSpy?.mockRestore();
+      await fsPromises.rm(sentinelPath, { force: true });
+      await fsPromises.rm(rootDir, { recursive: true, force: true });
+    }
+  }, 20_000);
+
   it("deletes merged branches when removing worktrees (safe delete)", async () => {
     const rootDir = await fsPromises.realpath(
       await fsPromises.mkdtemp(path.join(os.tmpdir(), "worktree-manager-delete-"))

src/node/worktree/WorktreeManager.ts

@@ -11,6 +11,7 @@ import { execAsync, execFileAsync } from "@/node/utils/disposableExec";
 import { getBashPath } from "@/node/utils/main/bashPath";
 import { getProjectName } from "@/node/utils/runtime/helpers";
 import { getErrorMessage } from "@/common/utils/errors";
+import { shellQuote } from "@/common/utils/shell";
 import { expandTilde } from "@/node/runtime/tildeExpansion";
 import { toPosixPath } from "@/node/utils/paths";
 import { log } from "@/node/services/log";
@@ -495,8 +496,8 @@ export class WorktreeManager {
           }
 
           // Force delete the directory (use bash shell for rm -rf on Windows)
-          // Convert to POSIX path for Git Bash compatibility on Windows
-          using rmProc = execAsync(`rm -rf "${toPosixPath(deletedPath)}"`, {
+          // shellQuote prevents command injection from malicious workspace paths
+          using rmProc = execAsync(`rm -rf ${shellQuote(toPosixPath(deletedPath))}`, {
             shell: getBashPath(),
           });
           await rmProc.result;