FIX: harden Claude test isolation in desktop tests

apps/desktop/package.json

@@ -1,6 +1,6 @@
 {
   "name": "desktop",
-  "version": "0.9.9",
+  "version": "0.9.10",
   "description": "ClosedLoop Desktop",
   "author": "ClosedLoop AI <support@closedloop.ai>",
   "private": true,
@@ -9,7 +9,7 @@
   "scripts": {
     "dev": "pnpm build && ELECTRON_BIN=$(bash scripts/patch-electron-plist.sh | tail -1) && \"$ELECTRON_BIN\" dist/main/index.js",
     "start": "ELECTRON_BIN=$(bash scripts/patch-electron-plist.sh | tail -1) && \"$ELECTRON_BIN\" dist/main/index.js",
-    "prebuild": "node -e \"const{execSync:e}=require('child_process'),{writeFileSync:w}=require('fs');const h=e('git rev-parse HEAD').toString().trim();w('src/shared/build-info.ts','// AUTO-GENERATED — do not edit\\nexport const BUILD_COMMIT_HASH = \\\"'+h+'\\\";\\n');\"",
+    "prebuild": "node -e \"const{execSync:e}=require('child_process'),{writeFileSync:w}=require('fs');const h=e('git rev-parse HEAD').toString().trim();w('src/shared/build-info.ts','// AUTO-GENERATED \u2014 do not edit\\nexport const BUILD_COMMIT_HASH = \\\"'+h+'\\\";\\n');\"",
     "build": "pnpm prebuild && tsc -p tsconfig.json",
     "typecheck": "tsc -p tsconfig.json --noEmit",
     "lint": "eslint src/",

apps/desktop/test/gateway-server.test.ts

@@ -2325,6 +2325,11 @@ test("invokes plugin cache discovery when pending learnings exist", async () =>
   const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "desktop-gateway-learnings-plugin-"));
   tempPathsToClean.push(tmpDir);
 
+  // Isolate HOME so a developer's real ~/.claude/plugins/cache never spawns a real wrapper.
+  const isolatedHome = path.join(tmpDir, "isolated-home");
+  await fs.mkdir(isolatedHome, { recursive: true });
+  process.env.HOME = isolatedHome;
+
   const repoPath = path.join(tmpDir, "repo-plugin");
   const worktreeParent = path.join(tmpDir, "worktrees");
   process.env.SYMPHONY_WORKTREE_PARENT_DIR = worktreeParent;
@@ -2367,8 +2372,7 @@ test("invokes plugin cache discovery when pending learnings exist", async () =>
   assert.equal(response.status, 200);
   const body = await response.json() as Record<string, unknown>;
   assert.equal(body.status, "processing");
-  // pid is null (no plugin cache) or a number (plugin found and script spawned)
-  assert.ok(body.pid === null || typeof body.pid === "number", "pid should be null or a number");
+  assert.equal(body.pid, null, "with isolated HOME and no plugin cache, no real script should spawn");
 
   // Allow the fire-and-forget status write to complete before cleanup
   await new Promise((resolve) => setTimeout(resolve, 400));
@@ -2811,6 +2815,11 @@ test("symphony launch invokes plugin cache discovery for run-loop script", async
   const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "desktop-gateway-launch-plugin-"));
   tempPathsToClean.push(tmpDir);
 
+  // Isolate HOME so a developer's real ~/.claude/plugins/cache never spawns real run-loop.sh.
+  const isolatedHome = path.join(tmpDir, "isolated-home");
+  await fs.mkdir(isolatedHome, { recursive: true });
+  process.env.HOME = isolatedHome;
+
   const repoPath = path.join(tmpDir, "repo-launch");
   const worktreeParent = path.join(tmpDir, "worktrees");
   process.env.SYMPHONY_WORKTREE_PARENT_DIR = worktreeParent;
@@ -2849,8 +2858,7 @@ test("symphony launch invokes plugin cache discovery for run-loop script", async
   const body = await response.json() as Record<string, unknown>;
   assert.equal(body.success, true);
   assert.equal(body.ticketId, "LAUNCH-01");
-  // pid is null (no plugin cache) or a number (plugin found and script spawned)
-  assert.ok(body.pid === null || typeof body.pid === "number", "pid should be null or a number");
+  assert.equal(body.pid, null, "with isolated HOME and no plugin cache, no real run-loop should spawn");
 });
 
 test("symphony launch passes .closedloop-ai/work path (not ticket ID) as first arg to run-loop.sh", async () => {

apps/desktop/test/symphony-loop-ssrf.test.ts

@@ -6,7 +6,10 @@
  * URL from its own settings, not from the relay command payload.
  */
 import assert from "node:assert/strict";
+import fs from "node:fs/promises";
 import http from "node:http";
+import os from "node:os";
+import path from "node:path";
 import { afterEach, beforeEach, test } from "node:test";
 
 // ---------------------------------------------------------------------------
@@ -23,6 +26,10 @@ let gatewayServer: http.Server | null = null;
 let gatewayPort = 0;
 const recordedRequests: RecordedRequest[] = [];
 
+/** Per-test isolated allowlist root — avoids matching /tmp/repo or other local checkouts. */
+let ssrfAllowDir: string;
+const ssrfTempDirsToClean: string[] = [];
+
 async function startGateway(): Promise<void> {
   gatewayServer = http.createServer((req, res) => {
     void (async () => {
@@ -127,6 +134,12 @@ function buildContext(body: Record<string, unknown>): OperationRequestContext {
   } as OperationRequestContext & { _responseStatus: number; _responseBody: string };
 }
 
+/** Repo fullName whose basename does not exist under ssrfAllowDir — PLAN exits before run-loop spawn. */
+const SSRF_PLAN_REPO = {
+  fullName: "ssrf-test-org/ssrf-nonexistent-repo-7f3a1b2c",
+  branch: "main",
+} as const;
+
 // ---------------------------------------------------------------------------
 // Setup / teardown
 // ---------------------------------------------------------------------------
@@ -136,43 +149,54 @@ beforeEach(async () => {
   capturedRoutes.length = 0;
   await startGateway();
 
+  ssrfAllowDir = await fs.mkdtemp(path.join(os.tmpdir(), "symphony-ssrf-"));
+  ssrfTempDirsToClean.push(ssrfAllowDir);
+
   // Register routes with a trusted origin pointing at our test server
   const { registerSymphonyLoopRoutes } = await import(
     "../src/server/operations/symphony-loop.js"
   );
   registerSymphonyLoopRoutes(
     fakeDispatcher as never,
-    () => ["/tmp"],
+    () => [ssrfAllowDir],
     () => `http://127.0.0.1:${gatewayPort}`,
     undefined // no jobStore
   );
 });
 
 afterEach(async () => {
   await stopGateway();
+  for (const dir of ssrfTempDirsToClean.splice(0)) {
+    await fs.rm(dir, { recursive: true, force: true });
+  }
 });
 
 // ---------------------------------------------------------------------------
 // Tests
 // ---------------------------------------------------------------------------
 
 test("returns 503 when getApiOrigin is absent", async () => {
-  // Register with no getApiOrigin
-  const freshRoutes: CapturedRoute[] = [];
-  const { registerSymphonyLoopRoutes } = await import(
-    "../src/server/operations/symphony-loop.js"
-  );
-  registerSymphonyLoopRoutes(
-    { register: (m: string, p: string, h: OperationHandler) => freshRoutes.push({ method: m, path: p, handler: h }) } as never,
-    () => ["/tmp"],
-    undefined, // no getApiOrigin
-    undefined
-  );
-  const handler = freshRoutes.find((r) => r.path.includes("/loop") && !r.path.includes("kill"))!.handler;
-  const ctx = buildContext({ loopId: "test", command: "PLAN", closedLoopAuthToken: "tok" }) as OperationRequestContext & { _responseStatus: number; _responseBody: string };
-  await handler(ctx);
-  assert.equal(ctx._responseStatus, 503);
-  assert.ok(ctx._responseBody.includes("API origin not configured"));
+  const allowDir = await fs.mkdtemp(path.join(os.tmpdir(), "symphony-ssrf-no-origin-"));
+  try {
+    // Register with no getApiOrigin
+    const freshRoutes: CapturedRoute[] = [];
+    const { registerSymphonyLoopRoutes } = await import(
+      "../src/server/operations/symphony-loop.js"
+    );
+    registerSymphonyLoopRoutes(
+      { register: (m: string, p: string, h: OperationHandler) => freshRoutes.push({ method: m, path: p, handler: h }) } as never,
+      () => [allowDir],
+      undefined, // no getApiOrigin
+      undefined
+    );
+    const handler = freshRoutes.find((r) => r.path.includes("/loop") && !r.path.includes("kill"))!.handler;
+    const ctx = buildContext({ loopId: "test", command: "PLAN", closedLoopAuthToken: "tok" }) as OperationRequestContext & { _responseStatus: number; _responseBody: string };
+    await handler(ctx);
+    assert.equal(ctx._responseStatus, 503);
+    assert.ok(ctx._responseBody.includes("API origin not configured"));
+  } finally {
+    await fs.rm(allowDir, { recursive: true, force: true });
+  }
 });
 
 test("works with no apiBaseUrl field in body when getApiOrigin is configured", async () => {
@@ -183,7 +207,7 @@ test("works with no apiBaseUrl field in body when getApiOrigin is configured", a
     closedLoopAuthToken: "tok",
     // No apiBaseUrl at all
     artifacts: [],
-    repo: { fullName: "org/repo", branch: "main" },
+    repo: { ...SSRF_PLAN_REPO },
   }) as OperationRequestContext & { _responseStatus: number; _responseBody: string };
 
   await handler(ctx);
@@ -202,7 +226,7 @@ test("ignores caller-supplied apiBaseUrl -- events go to configured origin", asy
     closedLoopAuthToken: "tok",
     apiBaseUrl: "http://169.254.169.254", // attacker-controlled
     artifacts: [],
-    repo: { fullName: "org/repo", branch: "main" },
+    repo: { ...SSRF_PLAN_REPO },
   }) as OperationRequestContext & { _responseStatus: number };
 
   await handler(ctx);
@@ -223,7 +247,7 @@ test("ignores caller-supplied localhost apiBaseUrl", async () => {
     closedLoopAuthToken: "tok",
     apiBaseUrl: "http://localhost:9999", // different port than configured
     artifacts: [],
-    repo: { fullName: "org/repo", branch: "main" },
+    repo: { ...SSRF_PLAN_REPO },
   });
 
   await handler(ctx);
@@ -241,7 +265,7 @@ test("ignores caller-supplied private IP apiBaseUrl", async () => {
     closedLoopAuthToken: "tok",
     apiBaseUrl: "http://10.0.0.1:3002",
     artifacts: [],
-    repo: { fullName: "org/repo", branch: "main" },
+    repo: { ...SSRF_PLAN_REPO },
   });
 
   await handler(ctx);

apps/desktop/test/symphony-test-utils.ts

@@ -13,7 +13,10 @@ import http from "node:http";
 import os from "node:os";
 import path from "node:path";
 import { promisify } from "node:util";
-import { resetShellPathCache } from "../src/server/shell-path.js";
+import {
+  resetShellPathCache,
+  setShellPathForTest,
+} from "../src/server/shell-path.js";
 import { DesktopGatewayServer } from "../src/server/server.js";
 import { EMPTY_CAPABILITIES } from "../src/shared/contracts.js";
 
@@ -286,7 +289,9 @@ export async function setupStubClaude(tmpDir: string, scriptLines?: string[]): P
   ]).join("\n");
   await fs.writeFile(path.join(fakeBin, "claude"), stubScript, { mode: 0o755 });
   process.env.PATH = `${fakeBin}:/usr/bin:/bin`;
-  resetShellPathCache();
+  // Lock shell-path resolution to the stubbed PATH so tests never fall back to
+  // a developer's real login-shell PATH and accidentally spawn real Claude.
+  setShellPathForTest();
 }
 
 // ---------------------------------------------------------------------------