fix(security): address round-2 review — token scoping, per-task done tokens, atomic EXDEV, hydration gating

P1 security:
- Require X-Coordinator-Id for coordinator-class task routes (no unscoped fallback)
- Per-task X-Done-Token header for signal_done; shared subtaskToken no longer sufficient
- atomicWriteFile: use dirname(filePath) not os.tmpdir() to avoid cross-filesystem EXDEV
- TerminalView: install MCP ready-state watcher regardless of initial status (fix retry spawn)
- hydrateTask: throw if coordinator not registered; gate child hydration on coordinator ready

P2 correctness:
- Forward dockerImage in both createTask and retryTaskMcpStartup StartMCPServer calls
- StartRemoteServer: rebind loopback-only MCP server to 0.0.0.0 on explicit remote start
- Preserve pre-existing .mcp.json parallel-code entry; restore on deregistration

Minor: semgrep/gitleaks scripts check for installation, print instructions if missing

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: brooksc

electron/ipc/register-mcp.test.ts

@@ -462,10 +462,11 @@ describe('Layer 5 — Failure modes', () => {
     }
   });
 
-  it('remote server returns 200 with correct Bearer token', async () => {
+  it('remote server returns 200 with correct Bearer token and X-Coordinator-Id', async () => {
     const port = await findFreePort();
     const mockCoordinator = {
       listTasks: () => [],
+      isRegisteredCoordinator: () => true,
     };
     const srv = await startRemoteServer({
       port,
@@ -476,7 +477,7 @@ describe('Layer 5 — Failure modes', () => {
     });
     try {
       const res = await fetch(`http://127.0.0.1:${srv.port}/api/tasks`, {
-        headers: { Authorization: `Bearer ${srv.token}` },
+        headers: { Authorization: `Bearer ${srv.token}`, 'X-Coordinator-Id': 'test-coord' },
       });
       expect(res.status).toBe(200);
     } finally {
@@ -512,7 +513,7 @@ describe('Layer 5 — Failure modes', () => {
 describe('Layer 7 — Remote server bind address', () => {
   it('server bound to 127.0.0.1 is reachable via that address', async () => {
     const port = await findFreePort();
-    const mockCoordinator = { listTasks: () => [] };
+    const mockCoordinator = { listTasks: () => [], isRegisteredCoordinator: () => true };
     const srv = await startRemoteServer({
       port,
       staticDir: os.tmpdir(),
@@ -522,7 +523,7 @@ describe('Layer 7 — Remote server bind address', () => {
     });
     try {
       const res = await fetch(`http://127.0.0.1:${srv.port}/api/tasks`, {
-        headers: { Authorization: `Bearer ${srv.token}` },
+        headers: { Authorization: `Bearer ${srv.token}`, 'X-Coordinator-Id': 'test-coord' },
       });
       expect(res.status).toBe(200);
     } finally {
@@ -671,7 +672,10 @@ describe('Layer 6 — Log assertions (production startup messages)', () => {
 describe('Layer 8 — Coordinator routes available after late coordinator attach', () => {
   it('coordinator routes return 503 before coordinator is set, then 200 after', async () => {
     const port = await findFreePort();
-    let currentCoordinator: { listTasks: () => unknown[] } | null = null;
+    let currentCoordinator: {
+      listTasks: () => unknown[];
+      isRegisteredCoordinator?: () => boolean;
+    } | null = null;
 
     const srv = await startRemoteServer({
       port,
@@ -689,11 +693,11 @@ describe('Layer 8 — Coordinator routes available after late coordinator attach
       expect(res1.status).toBe(503);
 
       // Simulate coordinator being attached later (e.g. StartMCPServer called after StartRemoteServer)
-      currentCoordinator = { listTasks: () => [] };
+      currentCoordinator = { listTasks: () => [], isRegisteredCoordinator: () => true };
 
-      // After coordinator is set: coordinator routes should work
+      // After coordinator is set: coordinator routes should work (coordinator token requires X-Coordinator-Id)
       const res2 = await fetch(`http://127.0.0.1:${srv.port}/api/tasks`, {
-        headers: { Authorization: `Bearer ${srv.token}` },
+        headers: { Authorization: `Bearer ${srv.token}`, 'X-Coordinator-Id': 'test-coord' },
       });
       expect(res2.status).toBe(200);
       const body = (await res2.json()) as unknown[];

electron/ipc/register.ts

@@ -1024,21 +1024,11 @@ export function registerAllHandlers(win: BrowserWindow): void {
   ipcMain.handle(IPC.StartRemoteServer, async (_e, args: { port?: number }) => {
     remoteServerRequestedManually = true;
     remoteServerPendingStop = false;
-    if (remoteServer)
-      return {
-        url: remoteServer.url,
-        wifiUrl: remoteServer.wifiUrl,
-        tailscaleUrl: remoteServer.tailscaleUrl,
-        port: remoteServer.port,
-      };
 
     const thisDir = path.dirname(fileURLToPath(import.meta.url));
     const distRemote = path.join(thisDir, '..', '..', 'dist-remote');
-    // Remote access is an explicit user action — bind to all interfaces so WiFi/Tailscale clients
-    // can reach the SPA. Coordinator MCP-only mode uses 127.0.0.1 by default.
-    remoteServer = await startRemoteServer({
-      port: args.port ?? 7777,
-      host: '0.0.0.0',
+    const remoteServerOpts = {
+      host: '0.0.0.0' as const,
       staticDir: distRemote,
       getTaskName: (taskId: string) => taskNames.get(taskId) ?? taskId,
       getAgentStatus: (agentId: string) => {
@@ -1050,7 +1040,33 @@ export function registerAllHandlers(win: BrowserWindow): void {
         };
       },
       getCoordinator: () => coordinator,
-    });
+    };
+
+    if (remoteServer) {
+      // If server was started for MCP-only (loopback), rebind to 0.0.0.0 so WiFi/Tailscale
+      // clients can reach it. Skip rebind while a coordinator is active — restarting the
+      // server would break ongoing MCP connections.
+      if (remoteServerStartedForMcp && !coordinator?.hasActiveCoordinator()) {
+        const prevPort = remoteServer.port;
+        await remoteServer.stop();
+        remoteServer = null;
+        remoteServerStartedForMcp = false;
+        remoteServer = await startRemoteServer({
+          port: args.port ?? prevPort,
+          ...remoteServerOpts,
+        });
+      }
+      return {
+        url: remoteServer.url,
+        wifiUrl: remoteServer.wifiUrl,
+        tailscaleUrl: remoteServer.tailscaleUrl,
+        port: remoteServer.port,
+      };
+    }
+
+    // Remote access is an explicit user action — bind to all interfaces so WiFi/Tailscale clients
+    // can reach the SPA. Coordinator MCP-only mode uses 127.0.0.1 by default.
+    remoteServer = await startRemoteServer({ port: args.port ?? 7777, ...remoteServerOpts });
     return {
       url: remoteServer.url,
       wifiUrl: remoteServer.wifiUrl,
@@ -1226,7 +1242,8 @@ export function registerAllHandlers(win: BrowserWindow): void {
         if (args.baseBranch !== undefined) sharedValidateBranchName(args.baseBranch, 'baseBranch');
         validatePath(args.worktreePath, 'worktreePath');
         assertString(args.coordinatorTaskId, 'coordinatorTaskId');
-        coordinator?.hydrateTask({
+        if (!coordinator) throw new Error('coordinator mode not initialized');
+        coordinator.hydrateTask({
           id: args.id,
           name: args.name,
           projectId: args.projectId,
@@ -1406,10 +1423,13 @@ export function registerAllHandlers(win: BrowserWindow): void {
 
       // Merge mcpConfig into the pre-validated existingMcpContent (parsed above,
       // before any coordinator state was mutated).
+      // Capture the previous parallel-code entry so deregistration can restore it instead of
+      // unconditionally deleting it (which would remove a user-owned entry).
+      const existingServers =
+        (existingMcpContent.mcpServers as Record<string, unknown> | undefined) ?? {};
+      const previousMcpParallelCode: unknown = existingServers['parallel-code'];
       let mergedMcpJson: string | undefined;
       if (mcpJsonDir && worktreeMcpPath) {
-        const existingServers =
-          (existingMcpContent.mcpServers as Record<string, unknown> | undefined) ?? {};
         existingMcpContent.mcpServers = { ...existingServers, ...mcpConfig.mcpServers };
         mergedMcpJson = JSON.stringify(existingMcpContent, null, 2);
       }
@@ -1483,7 +1503,12 @@ export function registerAllHandlers(win: BrowserWindow): void {
       // we created the file so deregisterCoordinator can clean up correctly.
       if (mcpJsonDir && worktreeMcpPath && mergedMcpJson !== undefined) {
         atomicWriteFileSync(worktreeMcpPath, mergedMcpJson, { mode: 0o600 });
-        coordinator.setMcpJsonInfo(args.coordinatorTaskId, worktreeMcpPath, !mcpFileExistedBefore);
+        coordinator.setMcpJsonInfo(
+          args.coordinatorTaskId,
+          worktreeMcpPath,
+          !mcpFileExistedBefore,
+          previousMcpParallelCode,
+        );
 
         // Append to .git/info/exclude (local-only gitignore, not committed)
         try {

electron/mcp/atomic.ts

@@ -1,7 +1,6 @@
 import { writeFileSync, renameSync, unlinkSync } from 'fs';
 import { writeFile, rename, unlink } from 'fs/promises';
-import { join } from 'path';
-import os from 'os';
+import { join, dirname } from 'path';
 import { randomUUID } from 'crypto';
 
 /** Write `data` to `filePath` atomically: write to a temp file then rename.
@@ -11,7 +10,7 @@ export function atomicWriteFileSync(
   data: string,
   options?: { mode?: number },
 ): void {
-  const tmp = join(os.tmpdir(), `parallel-code-atomic-${randomUUID()}.tmp`);
+  const tmp = join(dirname(filePath), `.parallel-code-atomic-${randomUUID()}.tmp`);
   try {
     writeFileSync(tmp, data, options);
     renameSync(tmp, filePath);
@@ -31,7 +30,7 @@ export async function atomicWriteFile(
   data: string,
   options?: { mode?: number },
 ): Promise<void> {
-  const tmp = join(os.tmpdir(), `parallel-code-atomic-${randomUUID()}.tmp`);
+  const tmp = join(dirname(filePath), `.parallel-code-atomic-${randomUUID()}.tmp`);
   try {
     await writeFile(tmp, data, options);
     await rename(tmp, filePath);

electron/mcp/client.ts

@@ -16,6 +16,7 @@ export class MCPClient {
     private baseUrl: string,
     private token: string,
     private coordinatorId?: string,
+    private doneToken?: string,
   ) {}
 
   private async request<T>(method: string, path: string, body?: unknown): Promise<T> {
@@ -105,7 +106,19 @@ export class MCPClient {
   }
 
   async signalDone(taskId: string): Promise<void> {
-    await this.request<unknown>('POST', `/api/tasks/${encodeURIComponent(taskId)}/done`, {});
+    const url = `${this.baseUrl}/api/tasks/${encodeURIComponent(taskId)}/done`;
+    const headers: Record<string, string> = {
+      Authorization: `Bearer ${this.token}`,
+      'Content-Type': 'application/json',
+    };
+    // Per-task done token is sent as X-Done-Token so the server can verify task ownership
+    // without needing per-task bearer token classification.
+    if (this.doneToken) headers['X-Done-Token'] = this.doneToken;
+    const res = await fetch(url, { method: 'POST', headers, body: '{}' });
+    if (!res.ok) {
+      const text = await res.text().catch(() => '');
+      throw new Error(`API POST /api/tasks/.../done failed (${res.status}): ${text}`);
+    }
   }
 
   async waitForSignalDone(

electron/mcp/coordinator.ts

@@ -2,7 +2,7 @@
 // Manages task lifecycle independently of the SolidJS renderer,
 // using existing backend primitives (pty, git, tasks).
 
-import { randomUUID } from 'crypto';
+import { randomUUID, randomBytes } from 'crypto';
 import { execFile } from 'child_process';
 import { promisify } from 'util';
 import { unlinkSync, readFileSync, existsSync } from 'fs';
@@ -223,13 +223,18 @@ export class Coordinator {
     for (const task of this.tasks.values()) {
       if (task.coordinatorTaskId !== coordinatorTaskId) continue;
       if (!task.mcpConfigPath) continue;
+      // Preserve existing doneToken; generate a fresh one if not yet set (e.g. older persisted task).
+      if (!task.doneToken) task.doneToken = randomBytes(24).toString('base64url');
       const mcpConfig = {
         mcpServers: {
           'parallel-code': {
             type: 'stdio' as const,
             command: 'node',
             args: [serverPath, '--url', serverUrl, '--task-id', task.id],
-            env: { PARALLEL_CODE_MCP_TOKEN: subtaskToken },
+            env: {
+              PARALLEL_CODE_MCP_TOKEN: subtaskToken,
+              PARALLEL_CODE_MCP_DONE_TOKEN: task.doneToken,
+            },
           },
         },
       };
@@ -583,13 +588,18 @@ export class Coordinator {
       const mcpServerInfoForTask = coordinatorState.mcpServerInfo;
       if (mcpServerInfoForTask) {
         const { serverUrl, subtaskToken, serverPath } = mcpServerInfoForTask;
+        const doneToken = randomBytes(24).toString('base64url');
+        task.doneToken = doneToken;
         const mcpConfig = {
           mcpServers: {
             'parallel-code': {
               type: 'stdio' as const,
               command: 'node',
               args: [serverPath, '--url', serverUrl, '--task-id', task.id],
-              env: { PARALLEL_CODE_MCP_TOKEN: subtaskToken },
+              env: {
+                PARALLEL_CODE_MCP_TOKEN: subtaskToken,
+                PARALLEL_CODE_MCP_DONE_TOKEN: doneToken,
+              },
             },
           },
         };
@@ -735,6 +745,10 @@ export class Coordinator {
     };
   }
 
+  getTaskDoneToken(taskId: string): string | null {
+    return this.tasks.get(taskId)?.doneToken ?? null;
+  }
+
   async sendPrompt(taskId: string, prompt: string): Promise<void> {
     const task = this.tasks.get(taskId);
     if (!task) throw new Error(`Task not found: ${taskId}`);
@@ -1085,6 +1099,9 @@ export class Coordinator {
     mcpConfigPath?: string;
     preambleFileExistedBefore?: boolean;
   }): void {
+    if (!this.coordinators.has(opts.coordinatorTaskId)) {
+      throw new Error(`coordinator ${opts.coordinatorTaskId} is not registered`);
+    }
     if (this.tasks.has(opts.id)) return;
     const task: CoordinatedTask = {
       id: opts.id,
@@ -1130,13 +1147,17 @@ export class Coordinator {
     // agent gets fresh credentials instead of the stale pre-restart values.
     if (safeMcpConfigPath && serverInfo) {
       const { serverUrl, subtaskToken, serverPath } = serverInfo;
+      if (!task.doneToken) task.doneToken = randomBytes(24).toString('base64url');
       const mcpConfig = {
         mcpServers: {
           'parallel-code': {
             type: 'stdio' as const,
             command: 'node',
             args: [serverPath, '--url', serverUrl, '--task-id', task.id],
-            env: { PARALLEL_CODE_MCP_TOKEN: subtaskToken },
+            env: {
+              PARALLEL_CODE_MCP_TOKEN: subtaskToken,
+              PARALLEL_CODE_MCP_DONE_TOKEN: task.doneToken,
+            },
           },
         },
       };
@@ -1222,11 +1243,17 @@ export class Coordinator {
     });
   }
 
-  setMcpJsonInfo(coordinatorTaskId: string, mcpJsonPath: string, createdMcpJson: boolean): void {
+  setMcpJsonInfo(
+    coordinatorTaskId: string,
+    mcpJsonPath: string,
+    createdMcpJson: boolean,
+    previousMcpParallelCode?: unknown,
+  ): void {
     const state = this.coordinators.get(coordinatorTaskId);
     if (state) {
       state.mcpJsonPath = mcpJsonPath;
       state.createdMcpJson = createdMcpJson;
+      state.previousMcpParallelCode = previousMcpParallelCode;
     }
   }
 
@@ -1246,17 +1273,23 @@ export class Coordinator {
       });
     }
 
-    // Clean up coordinator .mcp.json — remove only the parallel-code key.
-    // Always read current contents (user may have added keys while running),
-    // then delete the whole file only if nothing else remains.
+    // Clean up coordinator .mcp.json — restore or remove only the parallel-code key.
+    // Always read current contents (user may have added keys while running).
+    // If there was a pre-existing parallel-code entry, restore it; otherwise delete the key.
     if (coordinator.mcpJsonPath) {
       try {
         const raw = existsSync(coordinator.mcpJsonPath)
           ? readFileSync(coordinator.mcpJsonPath, 'utf-8')
           : null;
         if (raw !== null) {
           const content = JSON.parse(raw) as { mcpServers?: Record<string, unknown> };
-          if (content.mcpServers) delete content.mcpServers['parallel-code'];
+          if (content.mcpServers) {
+            if (coordinator.previousMcpParallelCode !== undefined) {
+              content.mcpServers['parallel-code'] = coordinator.previousMcpParallelCode;
+            } else {
+              delete content.mcpServers['parallel-code'];
+            }
+          }
           const hasServers = Object.keys(content.mcpServers ?? {}).length > 0;
           const hasOtherKeys = Object.keys(content).filter((k) => k !== 'mcpServers').length > 0;
           if (!hasServers && !hasOtherKeys) {

electron/mcp/server.ts

@@ -25,6 +25,7 @@ for (let i = 0; i < args.length; i++) {
 }
 
 const token = process.env.PARALLEL_CODE_MCP_TOKEN ?? '';
+const doneToken = process.env.PARALLEL_CODE_MCP_DONE_TOKEN || undefined;
 
 if (!url || !token) {
   console.error(
@@ -34,7 +35,7 @@ if (!url || !token) {
   process.exit(1);
 }
 
-const client = new MCPClient(url, token, coordinatorId || undefined);
+const client = new MCPClient(url, token, coordinatorId || undefined, doneToken);
 
 const server = new Server(
   { name: 'parallel-code', version: '1.0.0' },

electron/mcp/types.ts

@@ -14,6 +14,7 @@ export interface CoordinatedTask {
   exitCode: number | null;
   pendingPrompt?: string;
   mcpConfigPath?: string; // path to per-task tmp config, deleted on cleanup
+  doneToken?: string; // per-task token; only the owning sub-task may call /done
   preambleFileExistedBefore?: boolean; // true if the preamble file existed before injection (even if empty)
   signalDoneAt?: Date; // set when sub-task explicitly calls signal_done
   signalDoneConsumed?: boolean; // true after wait_for_signal_done returns this task's signal
@@ -77,6 +78,8 @@ export interface CoordinatorState {
   mcpJsonPath: string;
   /** True if Parallel Code created .mcp.json from scratch; false if it was pre-existing. */
   createdMcpJson: boolean;
+  /** Previous value of mcpServers["parallel-code"] before this coordinator wrote its entry, if any. */
+  previousMcpParallelCode?: unknown;
 }
 
 // --- MCP tool input schemas ---