fix(security): prevent command injection in sprite uploadFile (#2889)

Replace shell string interpolation with array-based exec arguments in
uploadFileSprite. Previously, remotePath and tempRemote were interpolated
into a bash -c string (`mkdir -p $(dirname '${normalizedRemote}') && mv
'${tempRemote}' '${normalizedRemote}'`), which is inherently unsafe
even with regex validation.

Now uses two separate sprite exec calls with paths passed as discrete
array arguments after `--`, and computes dirname in TypeScript using
node:path/posix instead of shell command substitution. Also fixes the
mockBunSpawn test helper to return fresh ReadableStream instances per
call, preventing "ReadableStream already used" errors.

Fixes #2880

Agent: security-auditor

Co-authored-by: B <6723574+louisgv@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

packages/cli/src/__tests__/test-helpers.ts

@@ -183,51 +183,54 @@ export function mockClackPrompts(overrides?: Partial<ClackPromptsMock>): ClackPr
  * and sprite-cov test files. Centralised here to avoid repetition.
  */
 export function mockBunSpawn(exitCode = 0, stdout = "", stderr = "") {
-  const mockProc = {
-    pid: 1234,
-    exitCode: Promise.resolve(exitCode),
-    exited: Promise.resolve(exitCode),
-    stdout: new ReadableStream({
-      start(c) {
-        c.enqueue(new TextEncoder().encode(stdout));
-        c.close();
-      },
-    }),
-    stderr: new ReadableStream({
-      start(c) {
-        c.enqueue(new TextEncoder().encode(stderr));
-        c.close();
-      },
-    }),
-    kill: mock(() => {}),
-    ref: () => {},
-    unref: () => {},
-    stdin: new WritableStream(),
-    resourceUsage: () =>
-      ({
-        cpuTime: {
-          system: 0,
-          user: 0,
-          total: 0,
+  function createMockProc() {
+    return {
+      pid: 1234,
+      exitCode: Promise.resolve(exitCode),
+      exited: Promise.resolve(exitCode),
+      stdout: new ReadableStream({
+        start(c) {
+          c.enqueue(new TextEncoder().encode(stdout));
+          c.close();
         },
-        maxRSS: 0,
-        sharedMemorySize: 0,
-        unsharedDataSize: 0,
-        unsharedStackSize: 0,
-        minorPageFaults: 0,
-        majorPageFaults: 0,
-        swapCount: 0,
-        inBlock: 0,
-        outBlock: 0,
-        ipcMessagesSent: 0,
-        ipcMessagesReceived: 0,
-        signalsReceived: 0,
-        voluntaryContextSwitches: 0,
-        involuntaryContextSwitches: 0,
-      }) satisfies ReturnType<ReturnType<typeof Bun.spawn>["resourceUsage"]>,
-  };
+      }),
+      stderr: new ReadableStream({
+        start(c) {
+          c.enqueue(new TextEncoder().encode(stderr));
+          c.close();
+        },
+      }),
+      kill: mock(() => {}),
+      ref: () => {},
+      unref: () => {},
+      stdin: new WritableStream(),
+      resourceUsage: () =>
+        ({
+          cpuTime: {
+            system: 0,
+            user: 0,
+            total: 0,
+          },
+          maxRSS: 0,
+          sharedMemorySize: 0,
+          unsharedDataSize: 0,
+          unsharedStackSize: 0,
+          minorPageFaults: 0,
+          majorPageFaults: 0,
+          swapCount: 0,
+          inBlock: 0,
+          outBlock: 0,
+          ipcMessagesSent: 0,
+          ipcMessagesReceived: 0,
+          signalsReceived: 0,
+          voluntaryContextSwitches: 0,
+          involuntaryContextSwitches: 0,
+        }) satisfies ReturnType<ReturnType<typeof Bun.spawn>["resourceUsage"]>,
+    };
+  }
+  // Return a fresh mock proc per call so ReadableStreams are not reused
   // biome-ignore lint: test mock
-  return spyOn(Bun, "spawn").mockReturnValue(mockProc as ReturnType<typeof Bun.spawn>);
+  return spyOn(Bun, "spawn").mockImplementation(() => createMockProc() as ReturnType<typeof Bun.spawn>);
 }
 
 // ── Fetch Mocks ────────────────────────────────────────────────────────────────

packages/cli/src/sprite/sprite.ts

@@ -4,6 +4,7 @@ import type { VMConnection } from "../history.js";
 
 import { existsSync } from "node:fs";
 import { join } from "node:path";
+import { dirname as posixDirname } from "node:path/posix";
 import { getErrorMessage } from "@openrouter/spawn-shared";
 import { getUserHome } from "../shared/paths.js";
 import { asyncTryCatch } from "../shared/result.js";
@@ -560,7 +561,12 @@ export async function uploadFileSprite(localPath: string, remotePath: string): P
   const basename = normalizedRemote.split("/").pop() || "file";
   const tempRemote = `/tmp/sprite_upload_${basename}_${tempRandom}`;
 
+  // Compute the parent directory in TypeScript to avoid shell interpolation
+  const parentDir = posixDirname(normalizedRemote);
+
   await spriteRetry("sprite upload", async () => {
+    // Upload the file to the temp path, then mkdir + mv using array args
+    // to avoid shell string interpolation (command injection risk).
     const proc = Bun.spawn(
       [
         spriteCmd,
@@ -571,9 +577,10 @@ export async function uploadFileSprite(localPath: string, remotePath: string): P
         "-file",
         `${localPath}:${tempRemote}`,
         "--",
-        "bash",
-        "-c",
-        `mkdir -p $(dirname '${normalizedRemote}') && mv '${tempRemote}' '${normalizedRemote}'`,
+        "mkdir",
+        "-p",
+        "--",
+        parentDir,
       ],
       {
         stdio: [
@@ -587,7 +594,35 @@ export async function uploadFileSprite(localPath: string, remotePath: string): P
     const stderrText = new Response(proc.stderr).text();
     const exitCode = await proc.exited;
     if (exitCode !== 0) {
-      throw new Error(`upload failed for ${remotePath}: ${await stderrText}`);
+      throw new Error(`upload mkdir failed for ${remotePath}: ${await stderrText}`);
+    }
+
+    // Move temp file to final destination using array args (no shell interpolation)
+    const mvProc = Bun.spawn(
+      [
+        spriteCmd,
+        ...orgFlags(),
+        "exec",
+        "-s",
+        _state.name,
+        "--",
+        "mv",
+        "--",
+        tempRemote,
+        normalizedRemote,
+      ],
+      {
+        stdio: [
+          "ignore",
+          "inherit",
+          "pipe",
+        ],
+      },
+    );
+    const mvStderrText = new Response(mvProc.stderr).text();
+    const mvExitCode = await mvProc.exited;
+    if (mvExitCode !== 0) {
+      throw new Error(`upload mv failed for ${remotePath}: ${await mvStderrText}`);
     }
   });
 }