fix: coord review remediations (path traversal, JSON parse, race condition) (#81)

Co-authored-by: Jason Woltje <jason@diversecanvas.com>
Co-committed-by: Jason Woltje <jason@diversecanvas.com>

apps/gateway/src/coord/coord.controller.ts

@@ -1,15 +1,39 @@
-import { Controller, Get, NotFoundException, Param, Query, UseGuards } from '@nestjs/common';
+import {
+  BadRequestException,
+  Controller,
+  Get,
+  Inject,
+  NotFoundException,
+  Param,
+  Query,
+  UseGuards,
+} from '@nestjs/common';
+import path from 'node:path';
 import { AuthGuard } from '../auth/auth.guard.js';
 import { CoordService } from './coord.service.js';
 
+/** Only paths under these roots are allowed for coord queries. */
+const ALLOWED_ROOTS = [process.cwd()];
+
+function resolveAndValidatePath(raw: string | undefined): string {
+  const resolved = path.resolve(raw ?? process.cwd());
+  const isAllowed = ALLOWED_ROOTS.some(
+    (root) => resolved === root || resolved.startsWith(`${root}/`),
+  );
+  if (!isAllowed) {
+    throw new BadRequestException('projectPath is outside the allowed workspace');
+  }
+  return resolved;
+}
+
 @Controller('api/coord')
 @UseGuards(AuthGuard)
 export class CoordController {
-  constructor(private readonly coordService: CoordService) {}
+  constructor(@Inject(CoordService) private readonly coordService: CoordService) {}
 
   @Get('status')
   async missionStatus(@Query('projectPath') projectPath?: string) {
-    const resolvedPath = projectPath ?? process.cwd();
+    const resolvedPath = resolveAndValidatePath(projectPath);
     const status = await this.coordService.getMissionStatus(resolvedPath);
     if (!status) throw new NotFoundException('No active coord mission found');
     return status;
@@ -17,13 +41,13 @@ export class CoordController {
 
   @Get('tasks')
   async listTasks(@Query('projectPath') projectPath?: string) {
-    const resolvedPath = projectPath ?? process.cwd();
+    const resolvedPath = resolveAndValidatePath(projectPath);
     return this.coordService.listTasks(resolvedPath);
   }
 
   @Get('tasks/:taskId')
   async taskStatus(@Param('taskId') taskId: string, @Query('projectPath') projectPath?: string) {
-    const resolvedPath = projectPath ?? process.cwd();
+    const resolvedPath = resolveAndValidatePath(projectPath);
     const detail = await this.coordService.getTaskStatus(resolvedPath, taskId);
     if (!detail) throw new NotFoundException(`Task ${taskId} not found in coord mission`);
     return detail;

packages/coord/src/mission.ts

@@ -379,7 +379,14 @@ export async function loadMission(projectPath: string): Promise<Mission> {
     throw error;
   }
 
-  const mission = normalizeMission(JSON.parse(raw), resolvedProjectPath);
+  let parsed: unknown;
+  try {
+    parsed = JSON.parse(raw);
+  } catch {
+    throw new Error(`Invalid JSON in mission file: ${filePath}`);
+  }
+
+  const mission = normalizeMission(parsed, resolvedProjectPath);
   if (mission.status === 'inactive') {
     throw new Error('Mission exists but is inactive. Re-initialize with mosaic coord init.');
   }

packages/coord/src/runner.ts

@@ -195,11 +195,12 @@ function resolveLaunchCommand(
     return [runtime, '-p', prompt];
   }
 
+  const hasPromptPlaceholder = configuredCommand.some((value) => value === '{prompt}');
   const withInterpolation = configuredCommand.map((value) =>
     value === '{prompt}' ? prompt : value,
   );
 
-  if (withInterpolation.includes(prompt)) {
+  if (hasPromptPlaceholder) {
     return withInterpolation;
   }
 
@@ -224,28 +225,9 @@ async function writeAtomicJson(filePath: string, payload: unknown): Promise<void
 async function readSessionLock(mission: Mission): Promise<SessionLockState | undefined> {
   const filePath = sessionLockPath(mission);
 
+  let raw: string;
   try {
-    const raw = await fs.readFile(filePath, 'utf8');
-    const data = JSON.parse(raw) as Partial<SessionLockState>;
-
-    if (
-      typeof data.session_id !== 'string' ||
-      (data.runtime !== 'claude' && data.runtime !== 'codex') ||
-      typeof data.pid !== 'number' ||
-      typeof data.started_at !== 'string' ||
-      typeof data.project_path !== 'string'
-    ) {
-      return undefined;
-    }
-
-    return {
-      session_id: data.session_id,
-      runtime: data.runtime,
-      pid: data.pid,
-      started_at: data.started_at,
-      project_path: data.project_path,
-      milestone_id: data.milestone_id,
-    };
+    raw = await fs.readFile(filePath, 'utf8');
   } catch (error) {
     if (
       typeof error === 'object' &&
@@ -257,6 +239,32 @@ async function readSessionLock(mission: Mission): Promise<SessionLockState | und
     }
     throw error;
   }
+
+  let data: Partial<SessionLockState>;
+  try {
+    data = JSON.parse(raw) as Partial<SessionLockState>;
+  } catch {
+    return undefined;
+  }
+
+  if (
+    typeof data.session_id !== 'string' ||
+    (data.runtime !== 'claude' && data.runtime !== 'codex') ||
+    typeof data.pid !== 'number' ||
+    typeof data.started_at !== 'string' ||
+    typeof data.project_path !== 'string'
+  ) {
+    return undefined;
+  }
+
+  return {
+    session_id: data.session_id,
+    runtime: data.runtime,
+    pid: data.pid,
+    started_at: data.started_at,
+    project_path: data.project_path,
+    milestone_id: data.milestone_id,
+  };
 }
 
 async function writeSessionLock(mission: Mission, lock: SessionLockState): Promise<void> {

packages/coord/src/status.ts

@@ -78,7 +78,12 @@ async function readActiveSession(mission: Mission): Promise<MissionSession | und
     throw error;
   }
 
-  const lock = JSON.parse(lockRaw) as SessionLockState;
+  let lock: SessionLockState;
+  try {
+    lock = JSON.parse(lockRaw) as SessionLockState;
+  } catch {
+    return undefined;
+  }
   if (
     typeof lock.session_id !== 'string' ||
     (lock.runtime !== 'claude' && lock.runtime !== 'codex') ||
@@ -106,10 +111,10 @@ async function readActiveSession(mission: Mission): Promise<MissionSession | und
   };
 }
 
-export async function getMissionStatus(mission: Mission): Promise<MissionStatusSummary> {
-  const freshMission = await loadMission(mission.projectPath);
-  const tasks = await readTasks(freshMission);
-
+async function buildStatusSummary(
+  freshMission: Mission,
+  tasks: MissionTask[],
+): Promise<MissionStatusSummary> {
   const done = tasks.filter((task) => task.status === 'done').length;
   const inProgress = tasks.filter((task) => task.status === 'in-progress').length;
   const pending = tasks.filter((task) => task.status === 'not-started').length;
@@ -151,6 +156,12 @@ export async function getMissionStatus(mission: Mission): Promise<MissionStatusS
   };
 }
 
+export async function getMissionStatus(mission: Mission): Promise<MissionStatusSummary> {
+  const freshMission = await loadMission(mission.projectPath);
+  const tasks = await readTasks(freshMission);
+  return buildStatusSummary(freshMission, tasks);
+}
+
 export async function getTaskStatus(mission: Mission, taskId: string): Promise<TaskDetail> {
   const freshMission = await loadMission(mission.projectPath);
   const tasks = await readTasks(freshMission);
@@ -164,7 +175,7 @@ export async function getTaskStatus(mission: Mission, taskId: string): Promise<T
     throw new Error(`Duplicate task IDs found: ${taskId}`);
   }
 
-  const summary = await getMissionStatus(freshMission);
+  const summary = await buildStatusSummary(freshMission, tasks);
 
   return {
     missionId: freshMission.id,

packages/coord/src/tasks-file.ts

@@ -219,6 +219,7 @@ async function acquireLock(lockPath: string): Promise<void> {
         const stats = await fs.stat(lockPath);
         if (Date.now() - stats.mtimeMs > TASKS_LOCK_STALE_MS) {
           await fs.rm(lockPath, { force: true });
+          await delay(TASKS_LOCK_RETRY_MS);
           continue;
         }
       } catch (statError) {
@@ -240,6 +241,7 @@ async function releaseLock(lockPath: string): Promise<void> {
 
 async function writeAtomic(filePath: string, content: string): Promise<void> {
   const directory = path.dirname(filePath);
+  await fs.mkdir(directory, { recursive: true });
   const tempPath = path.join(
     directory,
     `.TASKS.md.tmp-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}`,