fix(core): harden workspace path containment

.changeset/528-workspace-path-safety.md

@@ -0,0 +1,5 @@
+---
+"@gh-symphony/cli": patch
+---
+
+Harden workspace path validation against traversal and symlink escapes across platforms (hojinzs/github-symphony#528).

packages/core/src/core-conformance.test.ts

@@ -16,6 +16,9 @@ import {
   resolveIssueWorkspaceDirectory,
   scheduleRetryAt,
 } from "./index.js";
+import { mkdtempSync, rmSync, symlinkSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
 import type { RunDispatchedEvent } from "./observability/structured-events.js";
 
 describe("deriveWorkspaceKey", () => {
@@ -80,6 +83,12 @@ describe("resolveIssueWorkspaceDirectory", () => {
     expect(result).toBe("/runtime/orchestrator/abc123");
   });
 
+  it("accepts Windows-style separators in an issue workspace key", () => {
+    expect(() =>
+      resolveIssueWorkspaceDirectory("/runtime/orchestrator", "abc\\nested")
+    ).not.toThrow();
+  });
+
   it("rejects path traversal that escapes the root", () => {
     expect(() =>
       resolveIssueWorkspaceDirectory(
@@ -97,6 +106,21 @@ describe("resolveIssueWorkspaceDirectory", () => {
       resolveIssueWorkspaceDirectory("/runtime/orchestrator", ".lock")
     ).toThrow("reserved");
   });
+
+  it("rejects an issue workspace symlink that resolves outside the runtime root", () => {
+    const root = mkdtempSync(join(tmpdir(), "symphony-runtime-root-"));
+    const outside = mkdtempSync(join(tmpdir(), "symphony-runtime-outside-"));
+    symlinkSync(outside, join(root, "linked"));
+
+    try {
+      expect(() => resolveIssueWorkspaceDirectory(root, "linked")).toThrow(
+        "Issue workspace path escapes"
+      );
+    } finally {
+      rmSync(root, { force: true, recursive: true });
+      rmSync(outside, { force: true, recursive: true });
+    }
+  });
 });
 
 describe("resolveIssueRepositoryPath", () => {

packages/core/src/workspace-safety.test.ts

@@ -1,25 +1,56 @@
-import { describe, expect, it } from "vitest";
+import { mkdtempSync, rmSync, symlinkSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterEach, describe, expect, it } from "vitest";
 import { assertRepositoryAllowed, resolveWorkspaceDirectory } from "./index.js";
 
+const temporaryDirectories: string[] = [];
+
+afterEach(() => {
+  for (const directory of temporaryDirectories.splice(0)) {
+    rmSync(directory, { force: true, recursive: true });
+  }
+});
+
 describe("resolveWorkspaceDirectory", () => {
   it("keeps workspaces inside the configured root", () => {
-    expect(resolveWorkspaceDirectory("/tmp/github-symphony", "workspace-1")).toBe(
-      "/tmp/github-symphony/workspace-1"
-    );
+    expect(
+      resolveWorkspaceDirectory("/tmp/github-symphony", "workspace-1")
+    ).toBe("/tmp/github-symphony/workspace-1");
   });
 
   it("rejects path traversal", () => {
     expect(() =>
       resolveWorkspaceDirectory("/tmp/github-symphony", "../outside")
     ).toThrow("Workspace path escapes");
   });
+
+  it("rejects a symlink that resolves outside the workspace root", () => {
+    const root = mkdtempSync(join(tmpdir(), "symphony-workspace-root-"));
+    const outside = mkdtempSync(join(tmpdir(), "symphony-workspace-outside-"));
+    temporaryDirectories.push(root, outside);
+    symlinkSync(outside, join(root, "linked"));
+
+    expect(() => resolveWorkspaceDirectory(root, "linked")).toThrow(
+      "Workspace path escapes"
+    );
+    expect(() => resolveWorkspaceDirectory(root, "linked/new")).toThrow(
+      "Workspace path escapes"
+    );
+  });
+
+  it("accepts paths containing Windows-style separators as workspace names", () => {
+    expect(() =>
+      resolveWorkspaceDirectory("/tmp/github-symphony", "workspace\\nested")
+    ).not.toThrow();
+  });
 });
 
 describe("assertRepositoryAllowed", () => {
   it("rejects repositories outside the workspace allowlist", () => {
     expect(() =>
       assertRepositoryAllowed("https://github.com/acme/other.git", [
-        "https://github.com/acme/platform.git"
+        "https://github.com/acme/platform.git",
       ])
     ).toThrow("Repository is not in the workspace allowlist");
   });

packages/core/src/workspace/identity.ts

@@ -1,6 +1,7 @@
 import { resolve, join } from "node:path";
 import { createHash } from "node:crypto";
 import type { IssueSubjectIdentity } from "../domain/issue.js";
+import { isPathWithinRoot } from "./path-safety.js";
 
 const RESERVED_WORKSPACE_KEYS = new Set([
   "cache",
@@ -72,7 +73,7 @@ export function resolveIssueWorkspaceDirectory(
   const normalizedRuntimeRoot = resolve(runtimeRoot);
   const candidate = resolve(normalizedRuntimeRoot, workspaceKey);
 
-  if (!candidate.startsWith(`${normalizedRuntimeRoot}/`)) {
+  if (!isPathWithinRoot(normalizedRuntimeRoot, candidate, false)) {
     throw new Error(
       "Issue workspace path escapes the configured runtime root."
     );
@@ -87,8 +88,7 @@ export function resolveIssueWorkspaceDirectory(
 
 function isReservedWorkspaceKey(workspaceKey: string): boolean {
   return (
-    workspaceKey.startsWith(".") ||
-    RESERVED_WORKSPACE_KEYS.has(workspaceKey)
+    workspaceKey.startsWith(".") || RESERVED_WORKSPACE_KEYS.has(workspaceKey)
   );
 }
 

packages/core/src/workspace/path-safety.ts

@@ -0,0 +1,33 @@
+import { existsSync, realpathSync } from "node:fs";
+import { basename, dirname, isAbsolute, join, relative } from "node:path";
+
+function realpathWithMissingTail(path: string): string {
+  let current = path;
+  const missingTail: string[] = [];
+
+  while (!existsSync(current)) {
+    const parent = dirname(current);
+    if (parent === current) {
+      return path;
+    }
+    missingTail.unshift(basename(current));
+    current = parent;
+  }
+
+  return join(realpathSync(current), ...missingTail);
+}
+
+export function isPathWithinRoot(
+  root: string,
+  candidate: string,
+  allowRoot = true
+): boolean {
+  const realRoot = realpathWithMissingTail(root);
+  const realCandidate = realpathWithMissingTail(candidate);
+  const rel = relative(realRoot, realCandidate);
+
+  return (
+    (allowRoot && rel === "") ||
+    (rel !== "" && !rel.startsWith("..") && !isAbsolute(rel))
+  );
+}

packages/core/src/workspace/safety.ts

@@ -1,4 +1,5 @@
 import { resolve } from "node:path";
+import { isPathWithinRoot } from "./path-safety.js";
 
 export function resolveWorkspaceDirectory(
   workspaceRoot: string,
@@ -7,7 +8,7 @@ export function resolveWorkspaceDirectory(
   const normalizedRoot = resolve(workspaceRoot);
   const candidate = resolve(normalizedRoot, workspaceId);
 
-  if (candidate !== normalizedRoot && !candidate.startsWith(`${normalizedRoot}/`)) {
+  if (!isPathWithinRoot(normalizedRoot, candidate)) {
     throw new Error("Workspace path escapes the configured workspace root.");
   }
 
@@ -19,6 +20,8 @@ export function assertRepositoryAllowed(
   allowedRepositoryCloneUrls: string[]
 ): void {
   if (!allowedRepositoryCloneUrls.includes(targetRepositoryCloneUrl)) {
-    throw new Error(`Repository is not in the workspace allowlist: ${targetRepositoryCloneUrl}`);
+    throw new Error(
+      `Repository is not in the workspace allowlist: ${targetRepositoryCloneUrl}`
+    );
   }
 }