fix(cli): enforce content_path project-root containment for every consumer (CLI-2339)

apps/cli/AGENTS.md

@@ -251,7 +251,7 @@ This rule is consistent with the repo-wide **Refactoring Policy** ("delete obsol
 
 ### Config validation has one home
 
-Config validation is implemented exactly once: `src/command-internal/legacy-config-validate.ts` (`legacyValidateResolvedConfig`). Both the db/migration loader (`legacy-db-config.toml-read.ts`) and the status/stop resolver (`legacy-local-config-values.ts`) build a `LegacyConfigValidationInput` from their own pipelines and call it — do not add per-command reimplementations of these checks. When a validation branch or message changes, change it there. `legacy-config-validate.parity.unit.test.ts` feeds the same broken configs through both real pipelines and asserts identical error strings; extend it when adding a branch both callers share.
+Config validation is implemented exactly once: `src/command-internal/legacy-config-validate.ts` (`legacyValidateResolvedConfig`). Both the db/migration loader (`legacy-db-config.toml-read.ts`) and the status/stop resolver (`legacy-local-config-values.ts`) build a `LegacyConfigValidationInput` from their own pipelines and call it — do not add per-command reimplementations of these checks. When a validation branch or message changes, change it there. `legacy-config-validate.parity.unit.test.ts` feeds the same broken configs through both real pipelines and asserts identical error strings; extend it when adding a branch both callers share. `content_path` project-root containment (absolute paths, `..` escapes, and in-root symlinks pointing outside all rejected) is part of this same home, enforced inside `legacyResolveEmailTemplateContentPath` — any new consumer of `content_path` resolution gets containment for free and must not re-derive it locally.
 
 ---
 

apps/cli/src/command-internal/legacy-config-validate.deep-missing-path.unit.test.ts

@@ -0,0 +1,56 @@
+import { join } from "node:path";
+import { describe, expect, it, vi } from "vitest";
+
+/**
+ * Regression coverage for the iterative (not recursive) ancestor walk-up rewrite in
+ * `canonicalPathForContainment` (CLI-2339's symlink-containment hardening pass) — a security
+ * review found the earlier, fully-recursive walk-up blew the JS call stack around ~20,000 missing
+ * path components.
+ *
+ * A REAL filesystem cannot actually construct a path this deep: every real syscall
+ * (`realpathSync`/`lstatSync`) enforces the OS's own `PATH_MAX` (~1024 bytes on macOS, ~4096 on
+ * Linux), which caps a real missing-component chain at a few hundred to low thousands of
+ * components — nowhere near the 5,000 needed to meaningfully exercise (and rule out a stack-depth
+ * regression in) the walk-up loop itself. This file mocks `node:fs` at the filesystem boundary
+ * instead — the sanctioned seam for this kind of test per this workspace's testing conventions —
+ * so the loop's OWN iteration count is what's under test, not the host OS's path-length limit.
+ * Isolated into its own file (rather than folded into `legacy-config-validate.unit.test.ts`)
+ * because `vi.mock` is file-scoped: every other test in that file relies on a REAL filesystem
+ * (real symlinks, real dangling/looping targets), which a module-wide `node:fs` mock would break.
+ */
+vi.mock("node:fs", () => ({
+  realpathSync: vi.fn((path: string) => {
+    if (path === FAKE_EXISTING_BASE) return FAKE_EXISTING_BASE;
+    throw Object.assign(new Error(`ENOENT: no such file or directory, realpath '${path}'`), {
+      code: "ENOENT",
+    });
+  }),
+  lstatSync: vi.fn(() => undefined),
+  readlinkSync: vi.fn(() => {
+    throw new Error("readlinkSync should never be reached — no path in this fixture is a symlink");
+  }),
+  statSync: vi.fn(() => {
+    throw new Error("statSync should never be reached by the template content_path branch");
+  }),
+}));
+
+const FAKE_EXISTING_BASE = "/fake/project-root";
+
+describe("canonicalPathForContainment (via legacyResolveEmailTemplateContentPath)", () => {
+  it("resolves a 5,000-component-deep missing content_path without blowing the call stack", async () => {
+    const { legacyResolveEmailTemplateContentPath } = await import("./legacy-config-validate.ts");
+
+    const missingSegments = Array.from({ length: 5000 }, (_, i) => `missing-${i}`);
+    const contentPath = `${missingSegments.join("/")}/invite.html`;
+
+    const resolved = legacyResolveEmailTemplateContentPath({
+      section: "template",
+      name: "invite",
+      contentPath,
+      contentPresent: false,
+      base: FAKE_EXISTING_BASE,
+    });
+
+    expect(resolved).toBe(join(FAKE_EXISTING_BASE, contentPath));
+  });
+});

apps/cli/src/command-internal/legacy-config-validate.parity.unit.test.ts

@@ -102,6 +102,24 @@ const scenarios: ReadonlyArray<ParityScenario> = [
     },
     message: "Invalid config for auth.email.notification.password_changed.content_path",
   },
+  {
+    name: "auth.email.notification content_path resolves outside the project root",
+    toml: [
+      "[auth.email.notification.password_changed]",
+      "enabled = true",
+      'content_path = "/etc/hosts"',
+    ],
+    overrides: {
+      auth: {
+        email: {
+          notification: {
+            password_changed: { enabled: true, content_path: "/etc/hosts" },
+          },
+        },
+      },
+    },
+    message: "resolves outside the project root",
+  },
   {
     name: "db.port = 0",
     toml: ["[db]", "port = 0"],

apps/cli/src/command-internal/legacy-config-validate.ts

@@ -1,5 +1,5 @@
-import { statSync } from "node:fs";
-import { isAbsolute, join } from "node:path";
+import { lstatSync, readlinkSync, realpathSync, statSync } from "node:fs";
+import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "node:path";
 
 import {
   actionability,
@@ -757,13 +757,113 @@ export function legacySigningKeysDecodeErrorMessage(cause: unknown): string {
 
 // ── email template / notification ──
 
+/**
+ * Whether `candidatePath` resolves inside (or exactly to) `root`. Both
+ * arguments must already be canonicalized (see `canonicalPathForContainment`).
+ * Only rejects a genuine `..` traversal — a same-level sibling whose name
+ * happens to start with two dots (e.g. `..templates`) is a distinct,
+ * in-root path and must not be rejected.
+ */
+function isPathContainedInRoot(root: string, candidatePath: string): boolean {
+  const rel = relative(root, candidatePath);
+  return rel === "" || (!isAbsolute(rel) && rel !== ".." && !rel.startsWith(`..${sep}`));
+}
+
+// `readlinkSync` bypasses the OS's own `ELOOP` symlink-cycle detection when
+// manually following a dangling/unsearchable/looping symlink one hop at a
+// time (see `canonicalizeExistingPath` below), so that manual follow needs
+// its own explicit bound.
+const MAX_SYMLINK_FOLLOW_DEPTH = 40;
+
+/**
+ * Canonicalizes `path` when it exists, or returns `undefined` when it
+ * genuinely doesn't — the signal {@link canonicalPathForContainment} needs
+ * to decide whether to keep walking up towards an existing ancestor.
+ *
+ * "Exists" is decided with `lstatSync` (which doesn't itself dereference
+ * `path`), not by whether `realpathSync` succeeded — a dangling symlink, a
+ * symlink whose target directory is unsearchable (`EACCES`), or a symlink
+ * loop (`ELOOP`) all make `realpathSync` throw even though `path` genuinely
+ * exists on disk. Such a symlink is followed one hop by hand
+ * (`readlinkSync`) and its target canonicalized in turn, so the containment
+ * check always sees where the symlink actually points rather than a lexical
+ * guess that ignores it. Anything else existing-but-uncanonicalizable (or a
+ * symlink chain past {@link MAX_SYMLINK_FOLLOW_DEPTH}) is returned as-is —
+ * refusing to vouch for it lexically; the containment check still compares
+ * it honestly, and any subsequent read fails with its own real error.
+ */
+function canonicalizeExistingPath(path: string, depth: number): string | undefined {
+  try {
+    return realpathSync(path);
+  } catch {
+    const entry = lstatSync(path, { throwIfNoEntry: false });
+    if (entry === undefined) return undefined;
+    if (entry.isSymbolicLink() && depth < MAX_SYMLINK_FOLLOW_DEPTH) {
+      const target = readlinkSync(path);
+      return canonicalPathForContainment(
+        isAbsolute(target) ? target : join(dirname(path), target),
+        depth + 1,
+      );
+    }
+    return path;
+  }
+}
+
+/**
+ * Canonicalizes `path` for the containment check, tolerating a path (or an
+ * ancestor of it) that genuinely doesn't exist yet — that's the normal case
+ * for a missing template file, which should surface as a missing-file
+ * error, not a containment error. Walks up to the deepest EXISTING
+ * ancestor, resolves that with `realpathSync` (dereferencing any symlinks
+ * in it — including a symlinked project root itself, e.g. macOS's `/tmp`
+ * -> `/private/tmp`), then re-appends the missing tail lexically. The
+ * walk-up is an iterative loop, not recursion, so it stays correct against
+ * a pathologically long chain of missing ancestors (a stack-depth overflow
+ * was observed around 20,000 components with a naive recursive walk); each
+ * ancestor is still checked via {@link canonicalizeExistingPath}, so an
+ * intermediate dangling/unsearchable/looping symlink is followed rather
+ * than lexically skipped over as if it were an ordinary missing directory.
+ *
+ * A dangling, unsearchable, or looping symlink is never laundered as a
+ * missing tail component — see {@link canonicalizeExistingPath} for how
+ * "exists but can't canonicalize" is told apart from "doesn't exist" and
+ * followed to its real target. Recurses (bounded by `depth`) only to follow
+ * that kind of symlink; the ancestor walk-up itself is iterative.
+ */
+function canonicalPathForContainment(path: string, depth = 0): string {
+  const canonical = canonicalizeExistingPath(path, depth);
+  if (canonical !== undefined) return canonical;
+
+  const tail: string[] = [basename(path)];
+  let current = dirname(path);
+  for (;;) {
+    const ancestorCanonical = canonicalizeExistingPath(current, depth);
+    if (ancestorCanonical !== undefined) {
+      return tail.reduceRight((acc, name) => join(acc, name), ancestorCanonical);
+    }
+    const parent = dirname(current);
+    if (parent === current) {
+      return resolve(tail.reduceRight((acc, name) => join(acc, name), current));
+    }
+    tail.push(basename(current));
+    current = parent;
+  }
+}
+
 /**
  * Pure exclusivity decision + path to read for one template/notification entry. Throws
  * {@link LegacyConfigValidateError} with the exclusivity message when `contentPath === ""` and
- * `contentPresent`. Returns the absolute path to read, or `undefined` when there's nothing to
- * read (both `contentPath` and `content` absent — skip, not an error). `contentPath` set (even
- * when `content` is ALSO set) always wins — "both set" is not rejected, `content_path`
- * silently wins/overwrites.
+ * `contentPresent`. Returns the absolute, canonicalized (symlink-dereferenced) path to read, or
+ * `undefined` when there's nothing to read (both `contentPath` and `content` absent — skip, not
+ * an error). `contentPath` set (even when `content` is ALSO set) always wins — "both set" is not
+ * rejected, `content_path` silently wins/overwrites.
+ *
+ * The resolved candidate and `base` are both canonicalized (`canonicalPathForContainment`) and
+ * the candidate must resolve inside `base` (`isPathContainedInRoot`) — an absolute path, a `..`
+ * escape, or an in-root symlink pointing outside the project root all throw, since every caller
+ * reads or uploads the returned path's bytes. This applies unconditionally to every caller of
+ * this function (config validation, `config push` content loading, `start`'s eager pre-Docker
+ * containment pass) — there is no flag or opt-out.
  *
  * `base` is the caller-resolved project root for both templates and notifications.
  */
@@ -784,10 +884,28 @@ export function legacyResolveEmailTemplateContentPath(args: {
     }
     return undefined;
   }
-  if (args.section === "notification") {
-    return legacyResolveNotificationContentPath(args.base, args.contentPath);
+  const candidate =
+    args.section === "notification"
+      ? legacyResolveNotificationContentPath(args.base, args.contentPath)
+      : isAbsolute(args.contentPath)
+        ? args.contentPath
+        : join(args.base, args.contentPath);
+  const resolvedCanonical = canonicalPathForContainment(candidate);
+  const canonicalBase = canonicalPathForContainment(args.base);
+  if (!isPathContainedInRoot(canonicalBase, resolvedCanonical)) {
+    // Echo the DECLARED value (`args.contentPath`), not `resolvedCanonical` —
+    // the declared value is either what's literally in config.toml or an
+    // env-var override the caller already resolved, both already known to
+    // the user; the fully symlink-dereferenced canonical target is not, and
+    // echoing it back would hand a hostile config a way to probe what an
+    // in-root symlink resolves to on the runner (weak recon, but needless).
+    throw new LegacyConfigValidateError(
+      `Invalid config for auth.email.${args.section}.${args.name}.content_path: ` +
+        `"${args.contentPath}" resolves outside the project root ${args.base} — ` +
+        `move the file inside the project, or use a relative path that stays inside it.`,
+    );
   }
-  return isAbsolute(args.contentPath) ? args.contentPath : join(args.base, args.contentPath);
+  return resolvedCanonical;
 }
 
 /**