fix: prevent command injection in update-check reExecWithArgs (#951)

Replace execSync with execFileSync in reExecWithArgs() to prevent shell
metacharacter injection via binary path. execFileSync bypasses the shell
entirely, executing the binary directly with an argv array.

The performAutoUpdate() call retains execSync since it legitimately needs
a shell for piping (curl | bash).

Fixes #950

Agent: security-auditor

Co-authored-by: A <6723574+louisgv@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

cli/src/__tests__/update-check.test.ts

@@ -211,33 +211,37 @@ describe("update-check", () => {
       const fetchSpy = spyOn(global, "fetch").mockImplementation(mockFetch);
 
       const { executor } = await import("../update-check.js");
-      const calls: string[] = [];
+      const execSyncCalls: string[] = [];
       const execSyncSpy = spyOn(executor, "execSync").mockImplementation((cmd: string) => {
-        calls.push(cmd);
+        execSyncCalls.push(cmd);
       });
+      const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation(() => {});
 
       const { checkForUpdates } = await import("../update-check.js");
       await checkForUpdates();
 
-      // First call: install script, second call: re-exec with original args
-      expect(calls.length).toBe(2);
-      expect(calls[0]).toContain("install.sh");
-      expect(calls[1]).toContain("spawn");
-      expect(calls[1]).toContain("claude");
-      expect(calls[1]).toContain("sprite");
+      // execSync called once for the install script
+      expect(execSyncCalls.length).toBe(1);
+      expect(execSyncCalls[0]).toContain("install.sh");
+
+      // execFileSync called once for re-exec (no shell interpretation)
+      expect(execFileSyncSpy).toHaveBeenCalledTimes(1);
+      expect(execFileSyncSpy.mock.calls[0][0]).toContain("spawn");
+      expect(execFileSyncSpy.mock.calls[0][1]).toEqual(["claude", "sprite"]);
 
       // Should show rerunning message
       const output = consoleErrorSpy.mock.calls.map((call) => call[0]).join("\n");
       expect(output).toContain("Rerunning");
 
       // Should set SPAWN_NO_UPDATE_CHECK=1 to prevent infinite loop
-      expect(execSyncSpy.mock.calls[1][1]).toHaveProperty("env");
-      expect(execSyncSpy.mock.calls[1][1].env.SPAWN_NO_UPDATE_CHECK).toBe("1");
+      expect(execFileSyncSpy.mock.calls[0][2]).toHaveProperty("env");
+      expect(execFileSyncSpy.mock.calls[0][2].env.SPAWN_NO_UPDATE_CHECK).toBe("1");
 
       expect(processExitSpy).toHaveBeenCalledWith(0);
 
       fetchSpy.mockRestore();
       execSyncSpy.mockRestore();
+      execFileSyncSpy.mockRestore();
       process.argv = originalArgv;
     });
 
@@ -254,15 +258,12 @@ describe("update-check", () => {
       const fetchSpy = spyOn(global, "fetch").mockImplementation(mockFetch);
 
       const { executor } = await import("../update-check.js");
-      let callCount = 0;
-      const execSyncSpy = spyOn(executor, "execSync").mockImplementation(() => {
-        callCount++;
-        if (callCount === 2) {
-          // Re-exec fails with exit code 1
-          const err = new Error("Command failed") as Error & { status: number };
-          err.status = 42;
-          throw err;
-        }
+      const execSyncSpy = spyOn(executor, "execSync").mockImplementation(() => {});
+      const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation(() => {
+        // Re-exec fails with exit code 42
+        const err = new Error("Command failed") as Error & { status: number };
+        err.status = 42;
+        throw err;
       });
 
       const { checkForUpdates } = await import("../update-check.js");
@@ -273,6 +274,7 @@ describe("update-check", () => {
 
       fetchSpy.mockRestore();
       execSyncSpy.mockRestore();
+      execFileSyncSpy.mockRestore();
       process.argv = originalArgv;
     });
 
@@ -293,6 +295,7 @@ describe("update-check", () => {
       const execSyncSpy = spyOn(executor, "execSync").mockImplementation((cmd: string) => {
         calls.push(cmd);
       });
+      const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation(() => {});
 
       const { checkForUpdates } = await import("../update-check.js");
       await checkForUpdates();
@@ -301,6 +304,9 @@ describe("update-check", () => {
       expect(calls.length).toBe(1);
       expect(calls[0]).toContain("install.sh");
 
+      // execFileSync should NOT be called (no re-exec without args)
+      expect(execFileSyncSpy).not.toHaveBeenCalled();
+
       // Should show "Run your spawn command again" instead
       const output = consoleErrorSpy.mock.calls.map((call) => call[0]).join("\n");
       expect(output).toContain("Run your spawn command again");
@@ -310,6 +316,7 @@ describe("update-check", () => {
 
       fetchSpy.mockRestore();
       execSyncSpy.mockRestore();
+      execFileSyncSpy.mockRestore();
       process.argv = originalArgv;
     });
   });

cli/src/update-check.ts

@@ -1,5 +1,5 @@
 import "./unicode-detect.js"; // Ensure TERM is set before using symbols
-import { execSync as nodeExecSync } from "child_process";
+import { execSync as nodeExecSync, execFileSync as nodeExecFileSync } from "child_process";
 import pc from "picocolors";
 import pkg from "../package.json" with { type: "json" };
 import { RAW_BASE } from "./manifest.js";
@@ -9,6 +9,7 @@ const VERSION = pkg.version;
 // Internal executor for testability - can be replaced in tests
 export const executor = {
   execSync: (cmd: string, options?: any) => nodeExecSync(cmd, options),
+  execFileSync: (file: string, args: string[], options?: any) => nodeExecFileSync(file, args, options),
 };
 
 // ── Constants ──────────────────────────────────────────────────────────────────
@@ -52,11 +53,6 @@ function compareVersions(current: string, latest: string): boolean {
   return false; // Versions are equal
 }
 
-/** Shell-quote a string for safe interpolation into a bash command */
-function shellQuote(s: string): string {
-  return `'${s.replace(/'/g, "'\\''")}'`;
-}
-
 /** Print boxed update banner to stderr */
 function printUpdateBanner(latestVersion: string): void {
   const line1 = `Update available: v${VERSION} -> v${latestVersion}`;
@@ -94,13 +90,11 @@ function reExecWithArgs(): void {
   }
 
   const binPath = process.argv[1] || "spawn";
-  const cmd = [shellQuote(binPath), ...args.map(shellQuote)].join(" ");
   console.error(pc.dim(`  Rerunning: spawn ${args.join(" ")}`));
   console.error();
   try {
-    executor.execSync(cmd, {
+    executor.execFileSync(binPath, args, {
       stdio: "inherit",
-      shell: "/bin/bash",
       env: { ...process.env, SPAWN_NO_UPDATE_CHECK: "1" },
     });
     process.exit(0);