fix: path traversal guards, report-server robustness, Windows-portable clean scripts

packages/cli/package.json

@@ -31,7 +31,7 @@
   ],
   "scripts": {
     "build": "npm run clean && tsc && node ./scripts/copyReportApp.mjs && node -e \"require('fs').copyFileSync('package.json','dist/package.json')\"",
-    "clean": "rm -rf dist proto tsconfig.tsbuildinfo",
+    "clean": "node -e \"const fs=require('node:fs'); for (const p of ['dist','proto','tsconfig.tsbuildinfo']) fs.rmSync(p,{recursive:true,force:true});\"",
     "start": "tsx bin/finalrun.ts",
     "dev": "tsx --tsconfig ../../tsconfig.dev.json bin/finalrun.ts",
     "test": "node ./scripts/runTests.mjs"

packages/cli/src/reportServer.test.ts

@@ -0,0 +1,71 @@
+import assert from 'node:assert/strict';
+import * as fs from 'node:fs';
+import * as fsp from 'node:fs/promises';
+import * as os from 'node:os';
+import * as path from 'node:path';
+import test from 'node:test';
+import { rebuildRunIndex } from './runIndex.js';
+import { serveReportWorkspace } from './reportServer.js';
+
+async function withWorkspace<T>(
+  body: (workspace: { workspaceRoot: string; artifactsDir: string }) => Promise<T>,
+): Promise<T> {
+  const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), 'finalrun-report-server-'));
+  const artifactsDir = path.join(rootDir, 'artifacts');
+  await fsp.mkdir(artifactsDir, { recursive: true });
+  try {
+    return await body({ workspaceRoot: rootDir, artifactsDir });
+  } finally {
+    fs.rmSync(rootDir, { recursive: true, force: true });
+  }
+}
+
+async function withServer<T>(
+  workspace: { workspaceRoot: string; artifactsDir: string },
+  body: (baseUrl: string) => Promise<T>,
+): Promise<T> {
+  await rebuildRunIndex(workspace.artifactsDir);
+  const server = await serveReportWorkspace({
+    workspaceRoot: workspace.workspaceRoot,
+    artifactsDir: workspace.artifactsDir,
+    port: 0,
+  });
+  try {
+    return await body(server.url);
+  } finally {
+    await server.close();
+  }
+}
+
+test('GET /api/report/runs/:runId returns 404 when the run is missing', async () => {
+  await withWorkspace(async (workspace) => {
+    await withServer(workspace, async (baseUrl) => {
+      const response = await fetch(`${baseUrl}/api/report/runs/missing-run`);
+      assert.equal(response.status, 404);
+      const body = (await response.json()) as { status: string };
+      assert.equal(body.status, 'error');
+    });
+  });
+});
+
+test('GET /api/report/runs/:runId returns 404 for path-traversal runIds', async () => {
+  await withWorkspace(async (workspace) => {
+    await withServer(workspace, async (baseUrl) => {
+      const encoded = encodeURIComponent('../../../etc/passwd');
+      const response = await fetch(`${baseUrl}/api/report/runs/${encoded}`);
+      assert.equal(response.status, 404);
+    });
+  });
+});
+
+test('GET /api/report/runs/:runId returns 500 for corrupt run.json', async () => {
+  await withWorkspace(async (workspace) => {
+    const runDir = path.join(workspace.artifactsDir, 'corrupt-run');
+    await fsp.mkdir(runDir, { recursive: true });
+    await fsp.writeFile(path.join(runDir, 'run.json'), 'this is not json', 'utf-8');
+    await withServer(workspace, async (baseUrl) => {
+      const response = await fetch(`${baseUrl}/api/report/runs/corrupt-run`);
+      assert.equal(response.status, 500);
+    });
+  });
+});

packages/cli/src/reportServer.ts

@@ -12,9 +12,11 @@ import * as fs from 'node:fs';
 import * as fsp from 'node:fs/promises';
 import * as path from 'node:path';
 import { createServer, type IncomingMessage, type ServerResponse } from 'node:http';
+import { pipeline } from 'node:stream/promises';
 import {
   loadReportIndexViewModel,
   loadReportRunManifestViewModel,
+  RunManifestNotFoundError,
   type ReportWorkspaceContext,
 } from './reportViewModel.js';
 import { decodeArtifactPath, serveArtifactHttp } from './reportArtifactStream.js';
@@ -71,10 +73,14 @@ export async function serveReportWorkspace(params: {
         try {
           writeJson(response, 200, await loadReportRunManifestViewModel(runId, context));
         } catch (error) {
-          writeJson(response, 404, {
-            status: 'error',
-            message: error instanceof Error ? error.message : String(error),
-          });
+          if (error instanceof RunManifestNotFoundError) {
+            writeJson(response, 404, { status: 'error', message: error.message });
+          } else {
+            writeJson(response, 500, {
+              status: 'error',
+              message: error instanceof Error ? error.message : String(error),
+            });
+          }
         }
         return;
       }
@@ -192,7 +198,20 @@ async function tryServeFile(
     response.end();
     return true;
   }
-  fs.createReadStream(filePath).pipe(response);
+  try {
+    await pipeline(fs.createReadStream(filePath), response);
+  } catch {
+    if (!response.headersSent) {
+      response.writeHead(500, { 'Content-Type': 'text/plain; charset=utf-8' });
+      response.end('Failed to stream file.');
+    } else {
+      response.destroy();
+    }
+    // Response is already finalized (500 sent) or destroyed; signal "handled"
+    // so serveSpaAsset doesn't fall through to the SPA index.html and try to
+    // write to a closed response.
+    return true;
+  }
   return true;
 }
 

packages/cli/src/reportViewModel.test.ts

@@ -0,0 +1,125 @@
+import assert from 'node:assert/strict';
+import * as fs from 'node:fs';
+import * as fsp from 'node:fs/promises';
+import * as os from 'node:os';
+import * as path from 'node:path';
+import test from 'node:test';
+import {
+  RunManifestNotFoundError,
+  loadRunManifestRecord,
+  safeResolveWithin,
+  type ReportWorkspaceContext,
+} from './reportViewModel.js';
+
+function mkArtifactsDir(): { artifactsDir: string; cleanup: () => void } {
+  const artifactsDir = fs.mkdtempSync(path.join(os.tmpdir(), 'finalrun-report-vm-'));
+  return {
+    artifactsDir,
+    cleanup: () => {
+      fs.rmSync(artifactsDir, { recursive: true, force: true });
+    },
+  };
+}
+
+test('safeResolveWithin returns the resolved path for an in-bounds segment', () => {
+  const { artifactsDir, cleanup } = mkArtifactsDir();
+  try {
+    const resolved = safeResolveWithin(artifactsDir, 'run-1', 'run.json');
+    assert.equal(resolved, path.join(artifactsDir, 'run-1', 'run.json'));
+  } finally {
+    cleanup();
+  }
+});
+
+test('safeResolveWithin returns the base when no extra segments are passed', () => {
+  const { artifactsDir, cleanup } = mkArtifactsDir();
+  try {
+    const resolved = safeResolveWithin(artifactsDir);
+    assert.equal(resolved, path.resolve(artifactsDir));
+  } finally {
+    cleanup();
+  }
+});
+
+test('safeResolveWithin rejects parent-traversal segments', () => {
+  const { artifactsDir, cleanup } = mkArtifactsDir();
+  try {
+    assert.equal(safeResolveWithin(artifactsDir, '..', 'etc', 'passwd'), undefined);
+    assert.equal(safeResolveWithin(artifactsDir, '../../../etc/passwd'), undefined);
+  } finally {
+    cleanup();
+  }
+});
+
+test('safeResolveWithin rejects absolute segments that escape the base', () => {
+  const { artifactsDir, cleanup } = mkArtifactsDir();
+  try {
+    assert.equal(safeResolveWithin(artifactsDir, '/etc/passwd'), undefined);
+  } finally {
+    cleanup();
+  }
+});
+
+test('loadRunManifestRecord throws RunManifestNotFoundError for traversal runIds', async () => {
+  const { artifactsDir, cleanup } = mkArtifactsDir();
+  const context: ReportWorkspaceContext = { workspaceRoot: artifactsDir, artifactsDir };
+  try {
+    await assert.rejects(
+      () => loadRunManifestRecord('../../../etc/passwd', context),
+      (error: Error) => error instanceof RunManifestNotFoundError,
+    );
+  } finally {
+    cleanup();
+  }
+});
+
+test('loadRunManifestRecord throws RunManifestNotFoundError for missing runs', async () => {
+  const { artifactsDir, cleanup } = mkArtifactsDir();
+  const context: ReportWorkspaceContext = { workspaceRoot: artifactsDir, artifactsDir };
+  try {
+    await assert.rejects(
+      () => loadRunManifestRecord('does-not-exist', context),
+      (error: Error) => error instanceof RunManifestNotFoundError,
+    );
+  } finally {
+    cleanup();
+  }
+});
+
+test('loadRunManifestRecord surfaces non-ENOENT errors as generic errors', async () => {
+  const { artifactsDir, cleanup } = mkArtifactsDir();
+  try {
+    const context: ReportWorkspaceContext = { workspaceRoot: artifactsDir, artifactsDir };
+    const runDir = path.join(artifactsDir, 'corrupt-run');
+    await fsp.mkdir(runDir, { recursive: true });
+    await fsp.writeFile(path.join(runDir, 'run.json'), 'this is not json', 'utf-8');
+    await assert.rejects(
+      () => loadRunManifestRecord('corrupt-run', context),
+      (error: Error) => !(error instanceof RunManifestNotFoundError),
+    );
+  } finally {
+    cleanup();
+  }
+});
+
+test('loadRunManifestRecord rejects unsupported schema versions with a generic error', async () => {
+  const { artifactsDir, cleanup } = mkArtifactsDir();
+  try {
+    const context: ReportWorkspaceContext = { workspaceRoot: artifactsDir, artifactsDir };
+    const runDir = path.join(artifactsDir, 'old-schema');
+    await fsp.mkdir(runDir, { recursive: true });
+    await fsp.writeFile(
+      path.join(runDir, 'run.json'),
+      JSON.stringify({ schemaVersion: 1 }),
+      'utf-8',
+    );
+    await assert.rejects(
+      () => loadRunManifestRecord('old-schema', context),
+      (error: Error) =>
+        !(error instanceof RunManifestNotFoundError) &&
+        /Unsupported schema version/.test(error.message),
+    );
+  } finally {
+    cleanup();
+  }
+});

packages/cli/src/reportViewModel.ts

@@ -21,6 +21,25 @@ import { loadRunIndex } from './runIndex.js';
 const MISSING_WORKSPACE_CONFIG_ERROR =
   'The FinalRun report server is missing workspace configuration. Start it with `finalrun start-server`.';
 
+export class RunManifestNotFoundError extends Error {
+  constructor(runId: string) {
+    super(`Run manifest not found for runId: ${runId}`);
+    this.name = 'RunManifestNotFoundError';
+  }
+}
+
+// Resolve `parts` relative to `baseDir` and refuse to escape it. Returns
+// undefined when the resolved path lies outside the base — callers turn that
+// into a not-found / rejection instead of touching the filesystem.
+export function safeResolveWithin(baseDir: string, ...parts: string[]): string | undefined {
+  const base = path.resolve(baseDir);
+  const resolved = path.resolve(base, ...parts);
+  if (resolved === base || resolved.startsWith(`${base}${path.sep}`)) {
+    return resolved;
+  }
+  return undefined;
+}
+
 export interface ReportWorkspaceContext {
   workspaceRoot: string;
   artifactsDir: string;
@@ -92,8 +111,19 @@ export async function loadRunManifestRecord(
   runId: string,
   context: ReportWorkspaceContext = resolveReportWorkspaceContext(),
 ): Promise<RunManifest> {
-  const runJsonPath = path.join(context.artifactsDir, runId, 'run.json');
-  const raw = await fsp.readFile(runJsonPath, 'utf-8');
+  const runJsonPath = safeResolveWithin(context.artifactsDir, runId, 'run.json');
+  if (!runJsonPath) {
+    throw new RunManifestNotFoundError(runId);
+  }
+  let raw: string;
+  try {
+    raw = await fsp.readFile(runJsonPath, 'utf-8');
+  } catch (error) {
+    if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
+      throw new RunManifestNotFoundError(runId);
+    }
+    throw error;
+  }
   const parsed = JSON.parse(raw) as RunManifest;
   if (parsed.schemaVersion !== 2 && parsed.schemaVersion !== 3) {
     throw new Error(`Unsupported schema version: ${parsed.schemaVersion}`);
@@ -165,8 +195,13 @@ async function readRunArtifactText(
   const normalizedPath = normalizeRunArtifactPath(runId, artifactPath);
   if (!normalizedPath) return undefined;
 
+  const runRoot = safeResolveWithin(context.artifactsDir, runId);
+  if (!runRoot) return undefined;
+  const artifactFsPath = safeResolveWithin(runRoot, normalizedPath);
+  if (!artifactFsPath) return undefined;
+
   try {
-    return await fsp.readFile(path.join(context.artifactsDir, runId, normalizedPath), 'utf-8');
+    return await fsp.readFile(artifactFsPath, 'utf-8');
   } catch {
     return undefined;
   }

packages/cloud-core/package.json

@@ -8,7 +8,7 @@
   "types": "./dist/index.d.ts",
   "scripts": {
     "build": "npm run clean && tsc",
-    "clean": "rm -rf dist tsconfig.tsbuildinfo",
+    "clean": "node -e \"const fs=require('node:fs'); for (const p of ['dist','tsconfig.tsbuildinfo']) fs.rmSync(p,{recursive:true,force:true});\"",
     "test": "node --test \"dist/**/*.test.js\""
   },
   "dependencies": {

packages/cloud-core/src/submit.test.ts

@@ -0,0 +1,46 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+import { isSafeEnvName, isSafeRelativeSegment } from './submit.js';
+
+test('isSafeRelativeSegment accepts simple relative paths', () => {
+  assert.equal(isSafeRelativeSegment('foo.yml'), true);
+  assert.equal(isSafeRelativeSegment('subdir/foo.yml'), true);
+  assert.equal(isSafeRelativeSegment('a/b/c/d.yaml'), true);
+});
+
+test('isSafeRelativeSegment rejects parent traversal', () => {
+  assert.equal(isSafeRelativeSegment('../foo.yml'), false);
+  assert.equal(isSafeRelativeSegment('../../etc/passwd'), false);
+  assert.equal(isSafeRelativeSegment('..'), false);
+  assert.equal(isSafeRelativeSegment('subdir/../../escape'), false);
+});
+
+test('isSafeRelativeSegment rejects absolute paths across platforms', () => {
+  assert.equal(isSafeRelativeSegment('/etc/passwd'), false);
+  assert.equal(isSafeRelativeSegment('C:\\Windows\\System32\\drivers\\etc\\hosts'), false);
+  assert.equal(isSafeRelativeSegment('c:\\tmp\\a'), false);
+  assert.equal(isSafeRelativeSegment('\\\\server\\share\\file.yml'), false);
+});
+
+test('isSafeRelativeSegment normalises Windows-style separators before checking', () => {
+  assert.equal(isSafeRelativeSegment('..\\foo.yml'), false);
+  assert.equal(isSafeRelativeSegment('subdir\\foo.yml'), true);
+});
+
+test('isSafeRelativeSegment rejects empty values', () => {
+  assert.equal(isSafeRelativeSegment(''), false);
+});
+
+test('isSafeEnvName accepts conservative names', () => {
+  assert.equal(isSafeEnvName('staging'), true);
+  assert.equal(isSafeEnvName('dev_1'), true);
+  assert.equal(isSafeEnvName('feature-x.2'), true);
+});
+
+test('isSafeEnvName rejects path-like and exotic values', () => {
+  assert.equal(isSafeEnvName('../etc'), false);
+  assert.equal(isSafeEnvName('/etc'), false);
+  assert.equal(isSafeEnvName('a/b'), false);
+  assert.equal(isSafeEnvName('with space'), false);
+  assert.equal(isSafeEnvName(''), false);
+});