Merge pull request #4672 from open-gsd/refactor/4653-drain-containment-duplicates

fix(#4653): make path containment one decision, narrow the export, and close a symlink escape — Phase 3 of #4636
2026-09-12 19:33:20 -04:00
parent a0270a7945 2a6f74027f
commit 329b7aadb4
28 changed files with 915 additions and 270 deletions
--- a/.changeset/eager-badgers-bark.md
+++ b/.changeset/eager-badgers-bark.md
@@ -0,0 +1,5 @@
+---
+type: Changed
+pr: 4672
+---
+**The path-containment predicate is now a single exported seam** — `security.cjs` no longer exports `validatePath`. Containment is decided in exactly one place and resolved two ways: `assertWithinRoot` (throws) and `tryWithinRoot` (returns null) resolve symlinks, while `assertWithinRootLexical` and `tryWithinRootLexical` use string resolution alone and never touch the filesystem, for the few callers that must preserve a symlink rather than resolve it or that validate a destination before it exists. `requireSafePath` is preserved as an alias of the throwing form. All of them return a branded `ContainedPath` so a validated path cannot be silently swapped for an unvalidated one. The per-call-site `{ allowAbsolute: true }` flag is replaced by the named `PathAcceptance` policy, which states what it actually permits: an absolute path outside the root was always rejected and still is. The traversal rejection text `Path escapes allowed directory: <resolved> is outside <base>` is preserved verbatim, and no command changes what it accepts or rejects. Three rejection MESSAGES are reworded, none of which now reveals a host path it previously hid: `state.cts`'s `<label> path rejected: …` becomes `<label> path validation failed: …`, and the sub-repo and agent-skills warnings name the condition instead of echoing the predicate's error string. (#4653)
--- a/.changeset/gallant-hawks-tumble.md
+++ b/.changeset/gallant-hawks-tumble.md
@@ -0,0 +1,5 @@
+---
+type: Security
+pr: 4672
+---
+**Installed capability skills can no longer be redirected or leaked through a symlink** — the three install paths that confine a capability skill name relied on a lexical check, which cannot see a symlink. A link planted at the destination let `mkdirSync` succeed silently and the SKILL.md write land outside the install root, and a link planted at a capability's own SKILL.md was followed by `statSync` so an outside file's contents were installed as a skill body. All three now refuse to write or read through a link. (#4636)
--- a/CONTEXT.md
+++ b/CONTEXT.md
--- a/docs/explanation/security-model.md
+++ b/docs/explanation/security-model.md
@@ -138,9 +138,22 @@ GSD Core addresses prompt injection at three levels.
 **Input validation (`security.cjs`).** The `gsd-core/bin/lib/security.cjs`
 module is the central security utility. It provides:

- Path traversal prevention: user-supplied file paths (`--text-file`, `--prd`)
-  are validated to resolve within the project directory, with macOS
-  `/var` → `/private/var` symlink resolution handled explicitly
+- Path containment: user-supplied file paths and directories are validated to
+  resolve within a declared root before any filesystem access. One predicate
+  answers this for the whole tree (epic #4636, ADR-4650). The resolution engine
+  is module-internal and resolves symlinks, closes a dangling-symlink existence
+  oracle, and canonicalizes ancestors so a not-yet-created path under a
+  non-canonical base (macOS `/var` → `/private/var`) still resolves. The
+  exported surface is `assertWithinRoot` (throws), `tryWithinRoot` (returns
+  `null`), and `requireSafePath` (a preserved alias of the throwing form).
+  All three return a branded `ContainedPath`: a plain `string` is not assignable
+  to it, so validating one path and then handing a different one to the
+  filesystem is a type error rather than a silent bug. Whether an absolute
+  candidate is considered at all is a named policy — `PathAcceptance.RelativeOnly`
+  or `PathAcceptance.AbsoluteInsideRoot` — and neither relaxes containment: an
+  absolute path resolving outside the root is rejected exactly as a traversal is.
+  A caller may decide how to degrade on rejection, never whether a path is
+  contained.
 - Prompt injection detection: known injection patterns (role overrides,
  instruction bypasses, system tag injections) are scanned in user-supplied
  text before it enters any planning artifact
@@ -149,6 +162,41 @@ module is the central security utility. It provides:
 - Shell argument validation: arguments passed to subshell commands are
  validated before use

+Containment is decided in exactly one place, but resolved two ways. The
+comparison itself — separator-aware, so a sibling merely sharing a prefix is
+never accepted — is internal to `security.cjs` and is the single decision. Two
+exported families sit on it and differ only in how a candidate is resolved
+before that decision: `assertWithinRoot` / `tryWithinRoot` resolve symlinks,
+and `assertWithinRootLexical` / `tryWithinRootLexical` use string resolution
+alone and never touch the filesystem.
+
+The lexical form exists because a realpath-based predicate is the wrong tool
+wherever a symlink must be *preserved* rather than resolved, or where the target
+legitimately does not exist yet. A lexical check **cannot see a symlink**, so a
+caller relying on one for a write-confinement guarantee must pair it with its
+own symlink refusal. Three call sites use it, each for a stated reason.
+
+The backup-restore gate in `gsd-core/bin/gsd-tools.cjs` rejects symlinks outright:
+the canonical predicate accepts a link whose target resolves inside the root, but
+for a restore that is still wrong, because writing through the link overwrites
+whatever it points at instead of materializing a regular file at the backed-up
+path. `isPathConfined` in `src/external-descriptor-trust.cts` is lexical by
+design, because two install callers must validate a destination *before* the
+`mkdirSync` that creates it, where `realpath` cannot resolve. And
+`ensureInsideConfig` in `src/installer-migrations.cts` is lexical because that
+module's contract is that a symlinked managed path is snapshotted, restored and
+backed up *as a link* and never dereferenced — resolving it would dereference
+precisely the links the module exists to preserve, and then reject them for
+escaping the config directory.
+
+A lexical check cannot see a symlink, so callers that rely on one for a
+write-confinement guarantee must pair it with their own symlink refusal. Three
+install call sites did not, and now do: a link planted at a capability skill's
+destination made `mkdirSync` succeed silently and redirected the write outside
+the install root, and a link planted at a capability's own `SKILL.md` was
+followed by `statSync`, so an outside file's contents were installed as a skill
+body.
+
 **Runtime hook: `gsd-prompt-guard.js`.** This hook fires on every Write or
 Edit call that targets `.planning/` files. It scans the content being written
 for injection patterns shared with `gsd-read-injection-scanner.js` through
--- a/gsd-core/bin/gsd-tools.cjs
+++ b/gsd-core/bin/gsd-tools.cjs
@@ -3351,18 +3351,30 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load
    return out;
  }

-  // Why these three checks rather than security.cjs's `validatePath`: that seam
-  // resolves symlinks with realpathSync and then tests containment, so a link
-  // whose target sits inside the config dir passes. For a restore that is still
-  // wrong — writing through any link overwrites whatever it points at instead
-  // of materializing a regular file at the backed-up path. These checks reject
-  // links outright, which is strictly stricter than validatePath, not a
-  // reimplementation of it. Do not "simplify" this to validatePath.
+  // Why these three checks rather than security.cjs's `assertWithinRoot` /
+  // `tryWithinRoot`: that seam resolves symlinks with realpathSync and then
+  // tests containment, so a link whose target sits inside the config dir
+  // passes. For a restore that is still wrong — writing through any link
+  // overwrites whatever it points at instead of materializing a regular file
+  // at the backed-up path. These checks reject links outright, which is
+  // strictly stricter than assertWithinRoot/tryWithinRoot, not a
+  // reimplementation of them. Do not "simplify" this to assertWithinRoot or
+  // tryWithinRoot. Reviewed under epic #4636 Phase 3: the containment
+  // DECISION now routes through the canonical lexical predicate
+  // (`tryWithinRootLexical`, ADR-4650 decision 6); isInsideDir below still
+  // treats target === root as NOT contained via its own extra `!==` check
+  // (unlike every other containment implementation in this repo, which
+  // treats target === root as contained) — that condition is this gate's
+  // own and is layered on top of the shared predicate, not folded into it.

  /** True when `target` resolves strictly inside `root`. */
  function isInsideDir(root, target) {
-    const rel = path.relative(path.resolve(root), path.resolve(target));
-    return rel !== '' && !rel.startsWith('..') && !path.isAbsolute(rel);
+    // Containment decision: canonical lexical predicate (ADR-4650 decision 6).
+    // The extra `!==` condition is this gate's own: a restore must never
+    // target the config directory itself, only something strictly inside it.
+    if (path.resolve(target) === path.resolve(root)) return false;
+    const { tryWithinRootLexical } = require('./lib/security.cjs');
+    return tryWithinRootLexical(target, root) !== null;
  }

  /**
--- a/scripts/check-glossary-refs.cjs
+++ b/scripts/check-glossary-refs.cjs
@@ -37,6 +37,7 @@ const fs = require('node:fs');
 const path = require('node:path');

 const { ExitError, runMain } = require('./lib/cli-exit.cjs');
+const { tryWithinRoot, PathAcceptance } = require('../gsd-core/bin/lib/security.cjs');

 const ROOT = path.resolve(__dirname, '..');
 const CONTEXT_PATH = path.join(ROOT, 'CONTEXT.md');
@@ -131,20 +132,6 @@ function isTracked(token) {
  return TRACKED_EXACT.has(token) || TRACKED_PREFIXES.some((prefix) => token.startsWith(prefix));
 }

-/**
- * True if joining `token` to ROOT stays inside ROOT. `PATH_TOKEN_RE` admits `.`
- * inside a segment, so a token like `src/../../../etc/passwd` matches and (via
- * the `src/` prefix) reads as "tracked" — `path.join(ROOT, token)` would then
- * normalize to an out-of-tree absolute path and `fs.existsSync` would probe it,
- * turning a doc lint into a filesystem-existence oracle on the CI host. A
- * CONTEXT.md reference is always a plain in-repo path, so a `..` escape is never
- * legitimate: confine to ROOT and drop anything that climbs out.
- */
-function isWithinRoot(token) {
-  const resolved = path.resolve(ROOT, token);
-  return resolved === ROOT || resolved.startsWith(ROOT + path.sep);
-}
-
 /**
 * Every distinct, trackable file-path token referenced in `text`, with any
 * trailing `:<line>` suffix stripped.
@@ -167,7 +154,11 @@ function isWithinRoot(token) {
 * (CONTRIBUTING's "Do not compute a next number locally"), never a real path.
 */
 function extractTrackedRefs(text) {
-  const tokens = new Set();
+  // Maps token -> the ContainedPath tryWithinRoot returned for it. ADR-4650:
+  // the value that was validated for containment must be the exact value
+  // that gets probed later — never a path re-derived (e.g. re-joined) from
+  // the token, which could diverge from what was actually checked.
+  const tokens = new Map();
  const add = (raw) => {
    if (!PATH_TOKEN_RE.test(raw)) return;
    const token = raw.replace(/:\d+$/, '');
@@ -177,8 +168,18 @@ function extractTrackedRefs(text) {
    if (!/[A-Za-z0-9_]$/.test(token)) return;
    if (token.includes('NNNN')) return;
    if (!isTracked(token)) return;
-    if (!isWithinRoot(token)) return;
-    tokens.add(token);
+    // Containment decision is the canonical predicate's, per ADR-4650. Carry
+    // the returned ContainedPath forward so checkFileRefs stats the SAME
+    // value that was validated, instead of re-joining `token` onto ROOT.
+    //
+    // The containment ANSWER is unchanged from the retired lexical-only
+    // `isWithinRoot`, but the canonical predicate resolves symlinks, so a
+    // rejected token is now realpath-resolved before being rejected rather
+    // than rejected by string comparison alone; the result is still never
+    // surfaced and the token is never stat'd unless it is contained.
+    const contained = tryWithinRoot(token, ROOT, PathAcceptance.AbsoluteInsideRoot);
+    if (contained === null) return;
+    tokens.set(token, contained);
  };
  const subTokenRe = /[\w.-]+(?:\/[\w.-]+)*/g;
  for (const line of text.split(/\r?\n/)) {
@@ -199,14 +200,17 @@ function extractTrackedRefs(text) {

 /** Check A: every tracked reference must resolve on disk. */
 function checkFileRefs(contextText) {
-  const tokens = [...extractTrackedRefs(contextText)].sort();
+  const entries = [...extractTrackedRefs(contextText)].sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0));
  const findings = [];
-  for (const token of tokens) {
-    if (!fs.existsSync(path.join(ROOT, token))) {
+  for (const [token, contained] of entries) {
+    // Stat the ContainedPath returned by tryWithinRoot — NOT a re-joined
+    // path.join(ROOT, token) — so the path that was validated for
+    // containment is the path that is probed (ADR-4650).
+    if (!fs.existsSync(contained)) {
      findings.push(`CONTEXT.md references \`${token}\` which does not exist in the repo.`);
    }
  }
-  return { findings, checked: tokens.length };
+  return { findings, checked: entries.length };
 }

 /** The glossary's own claim: `Runtime enum: `allRuntimes` (N values: a, b, c)`. */
--- a/src/adr-parser.cts
+++ b/src/adr-parser.cts
@@ -9,7 +9,7 @@

 import fs from 'node:fs';
 import path from 'node:path';
-import { requireSafePath } from './security.cjs';
+import { requireSafePath, PathAcceptance } from './security.cjs';
 import { collectSections } from './markdown-sectionizer.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import cliExitModule = require('./cli-exit.cjs');
@@ -458,7 +458,7 @@ function parseCliArgs(argv: string[]): CliOpts {

 function main(argv: string[]): void {
  const opts = parseCliArgs(argv);
-  const safePath = requireSafePath(opts.input, path.resolve(opts.projectDir), 'ADR input path', { allowAbsolute: true });
+  const safePath = requireSafePath(opts.input, path.resolve(opts.projectDir), 'ADR input path', PathAcceptance.AbsoluteInsideRoot);
  const content = fs.readFileSync(safePath, 'utf8');
  const parsed = parseAdrMarkdown(content, { sourcePath: opts.input ?? undefined, format: opts.format });
  process.stdout.write(JSON.stringify(parsed, null, 2));
--- a/src/audit.cts
+++ b/src/audit.cts
@@ -32,7 +32,7 @@ const { PHASE_NUMBER_TOKEN_SOURCE, scopeToPhase } = phaseIdMod;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import phaseLocator = require('./phase-locator.cjs');
 const { getAllArchivedPhaseDirs } = phaseLocator;
-import { requireSafePath, sanitizeForDisplay, sanitizeLabel } from './security.cjs';
+import { requireSafePath, sanitizeForDisplay, sanitizeLabel, PathAcceptance } from './security.cjs';
 import { platformWriteSync } from './shell-command-projection.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import io = require('./io.cjs');
@@ -447,7 +447,7 @@ function scanDebugSessions(planDir: string): ScanOutcome<DebugSessionItem> {

    let safeFilePath: string;
    try {
-      safeFilePath = requireSafePath(filePath, planDir, 'debug session file', { allowAbsolute: true });
+      safeFilePath = requireSafePath(filePath, planDir, 'debug session file', PathAcceptance.AbsoluteInsideRoot);
    } catch {
      continue;
    }
@@ -563,7 +563,7 @@ function scanQuickTasks(planDir: string): ScanOutcome<QuickTaskItem> {

    let safeTaskDir: string;
    try {
-      safeTaskDir = requireSafePath(taskDir, planDir, 'quick task dir', { allowAbsolute: true });
+      safeTaskDir = requireSafePath(taskDir, planDir, 'quick task dir', PathAcceptance.AbsoluteInsideRoot);
    } catch {
      continue;
    }
@@ -577,7 +577,7 @@ function scanQuickTasks(planDir: string): ScanOutcome<QuickTaskItem> {
    if (summaryPath && fs.existsSync(summaryPath)) {
      let safeSum: string;
      try {
-        safeSum = requireSafePath(summaryPath, planDir, 'quick task summary', { allowAbsolute: true });
+        safeSum = requireSafePath(summaryPath, planDir, 'quick task summary', PathAcceptance.AbsoluteInsideRoot);
      } catch {
        continue;
      }
@@ -658,7 +658,7 @@ function scanThreads(planDir: string): ScanOutcome<ThreadItem> {

    let safeFilePath: string;
    try {
-      safeFilePath = requireSafePath(filePath, planDir, 'thread file', { allowAbsolute: true });
+      safeFilePath = requireSafePath(filePath, planDir, 'thread file', PathAcceptance.AbsoluteInsideRoot);
    } catch {
      continue;
    }
@@ -748,7 +748,7 @@ function scanTodos(todosBase: string): ScanOutcome<TodoItem> {

    let safeFilePath: string;
    try {
-      safeFilePath = requireSafePath(filePath, todosBase, 'todo file', { allowAbsolute: true });
+      safeFilePath = requireSafePath(filePath, todosBase, 'todo file', PathAcceptance.AbsoluteInsideRoot);
    } catch {
      continue;
    }
@@ -829,7 +829,7 @@ function scanSeeds(planDir: string): ScanOutcome<SeedItem> {

    let safeFilePath: string;
    try {
-      safeFilePath = requireSafePath(filePath, planDir, 'seed file', { allowAbsolute: true });
+      safeFilePath = requireSafePath(filePath, planDir, 'seed file', PathAcceptance.AbsoluteInsideRoot);
    } catch {
      continue;
    }
@@ -1014,7 +1014,7 @@ function scanUatGaps(planDir: string, cwd: string): ScanOutcome<UatGapItem> {

      let safeFilePath: string;
      try {
-        safeFilePath = requireSafePath(filePath, planDir, 'UAT file', { allowAbsolute: true });
+        safeFilePath = requireSafePath(filePath, planDir, 'UAT file', PathAcceptance.AbsoluteInsideRoot);
      } catch {
        continue;
      }
@@ -1097,7 +1097,7 @@ function scanVerificationGaps(planDir: string, cwd: string): ScanOutcome<Verific

      let safeFilePath: string;
      try {
-        safeFilePath = requireSafePath(filePath, planDir, 'VERIFICATION file', { allowAbsolute: true });
+        safeFilePath = requireSafePath(filePath, planDir, 'VERIFICATION file', PathAcceptance.AbsoluteInsideRoot);
      } catch {
        continue;
      }
@@ -1166,7 +1166,7 @@ function scanContextQuestions(planDir: string, cwd: string): ScanOutcome<Context

      let safeFilePath: string;
      try {
-        safeFilePath = requireSafePath(filePath, planDir, 'CONTEXT file', { allowAbsolute: true });
+        safeFilePath = requireSafePath(filePath, planDir, 'CONTEXT file', PathAcceptance.AbsoluteInsideRoot);
      } catch {
        continue;
      }
@@ -1262,7 +1262,7 @@ function scanDeferredItems(planDir: string, cwd: string): ScanOutcome<DeferredIt

    let safeFilePath: string;
    try {
-      safeFilePath = requireSafePath(filePath, planDir, 'deferred items file', { allowAbsolute: true });
+      safeFilePath = requireSafePath(filePath, planDir, 'deferred items file', PathAcceptance.AbsoluteInsideRoot);
    } catch {
      continue;
    }
@@ -1667,7 +1667,7 @@ function cmdAuditAcknowledge(cwd: string, args: string[], raw: boolean): void {
      ioError(`no phase directory found for phase "${phase as string}"${archivedMilestone ? ` (archived-milestone "${archivedMilestone}")` : ''}`);
    }
    const filePath = path.join(targetDir as string, file as string);
-    const safeFilePath = requireSafePath(filePath, planDir, 'audit acknowledge target', { allowAbsolute: true });
+    const safeFilePath = requireSafePath(filePath, planDir, 'audit acknowledge target', PathAcceptance.AbsoluteInsideRoot);
    if (!fs.existsSync(safeFilePath)) ioError(`file not found: ${file as string}`);

    if (category === 'deferred_items') {
@@ -1745,19 +1745,19 @@ function cmdAuditAcknowledge(cwd: string, args: string[], raw: boolean): void {

  if (category === 'debug_sessions') {
    if (!slug) ioError('--slug is required for --category debug_sessions');
-    safeFilePath = requireSafePath(path.join(planDir, 'debug', `${slug as string}.md`), planDir, 'audit acknowledge target', { allowAbsolute: true });
+    safeFilePath = requireSafePath(path.join(planDir, 'debug', `${slug as string}.md`), planDir, 'audit acknowledge target', PathAcceptance.AbsoluteInsideRoot);
    if (!fs.existsSync(safeFilePath)) ioError(`file not found: debug/${slug as string}.md`);
    const content = fs.readFileSync(safeFilePath, 'utf-8');
    currentValue = ((extractFrontmatter(content, safeFilePath).status as string) || 'unknown').toLowerCase();
  } else if (category === 'threads') {
    if (!slug) ioError('--slug is required for --category threads');
-    safeFilePath = requireSafePath(path.join(planDir, 'threads', `${slug as string}.md`), planDir, 'audit acknowledge target', { allowAbsolute: true });
+    safeFilePath = requireSafePath(path.join(planDir, 'threads', `${slug as string}.md`), planDir, 'audit acknowledge target', PathAcceptance.AbsoluteInsideRoot);
    if (!fs.existsSync(safeFilePath)) ioError(`file not found: threads/${slug as string}.md`);
    const content = fs.readFileSync(safeFilePath, 'utf-8');
    currentValue = deriveThreadStatus(extractFrontmatter(content, safeFilePath), content);
  } else if (category === 'seeds') {
    if (!seedId) ioError('--seed-id is required for --category seeds');
-    safeFilePath = requireSafePath(path.join(planDir, 'seeds', `${seedId as string}.md`), planDir, 'audit acknowledge target', { allowAbsolute: true });
+    safeFilePath = requireSafePath(path.join(planDir, 'seeds', `${seedId as string}.md`), planDir, 'audit acknowledge target', PathAcceptance.AbsoluteInsideRoot);
    if (!fs.existsSync(safeFilePath)) ioError(`file not found: seeds/${seedId as string}.md`);
    const content = fs.readFileSync(safeFilePath, 'utf-8');
    currentValue = ((extractFrontmatter(content, safeFilePath).status as string) || 'dormant').toLowerCase();
@@ -1768,18 +1768,18 @@ function cmdAuditAcknowledge(cwd: string, args: string[], raw: boolean): void {
    // old workstream-scoped planDir boundary would refuse a root todos file
    // outright, and even a path fix alone would have thrown here.
    const rootTodos = todosDir(cwd);
-    safeFilePath = requireSafePath(path.join(rootTodos, 'pending', filename as string), rootTodos, 'audit acknowledge target', { allowAbsolute: true });
+    safeFilePath = requireSafePath(path.join(rootTodos, 'pending', filename as string), rootTodos, 'audit acknowledge target', PathAcceptance.AbsoluteInsideRoot);
    if (!fs.existsSync(safeFilePath)) ioError(`file not found: todos/pending/${filename as string}`);
    currentValue = ''; // presence-only — see scanTodos
  } else if (category === 'quick_tasks') {
    if (!quickDir) ioError('--dir is required for --category quick_tasks');
-    const taskDir = requireSafePath(path.join(planDir, 'quick', quickDir as string), planDir, 'audit acknowledge target dir', { allowAbsolute: true });
+    const taskDir = requireSafePath(path.join(planDir, 'quick', quickDir as string), planDir, 'audit acknowledge target dir', PathAcceptance.AbsoluteInsideRoot);
    if (!fs.existsSync(taskDir)) ioError(`directory not found: quick/${quickDir as string}`);
    // Shared with scanQuickTasks (#3458 follow-up) so the reader and this
    // writer can never disagree about which file is the task's record.
    const resolvedSummaryPath = resolveQuickTaskSummaryFile(taskDir, quickDir as string);
    if (resolvedSummaryPath) {
-      safeFilePath = requireSafePath(resolvedSummaryPath, planDir, 'audit acknowledge target', { allowAbsolute: true });
+      safeFilePath = requireSafePath(resolvedSummaryPath, planDir, 'audit acknowledge target', PathAcceptance.AbsoluteInsideRoot);
      const content = fs.readFileSync(safeFilePath, 'utf-8');
      currentValue = ((extractFrontmatter(content, safeFilePath).status as string) || 'unknown').toLowerCase();
    } else {
@@ -1789,7 +1789,7 @@ function cmdAuditAcknowledge(cwd: string, args: string[], raw: boolean): void {
      // acknowledgment's own snapshot of "no summary exists yet", which
      // self-invalidates the moment a real SUMMARY.md is written (the
      // scanner then reads THAT file's own status instead).
-      safeFilePath = requireSafePath(path.join(taskDir, `${quickDir as string}-SUMMARY.md`), planDir, 'audit acknowledge target', { allowAbsolute: true });
+      safeFilePath = requireSafePath(path.join(taskDir, `${quickDir as string}-SUMMARY.md`), planDir, 'audit acknowledge target', PathAcceptance.AbsoluteInsideRoot);
      currentValue = 'missing';
      createIfMissing = true;
      fmForCreate = { status: 'missing' };
--- a/src/check-command-router.cts
+++ b/src/check-command-router.cts
@@ -11,7 +11,13 @@ import path from 'node:path';
 import { execFileSync } from 'node:child_process';
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import io = require('./io.cjs');
-const { output, error, ERROR_REASON } = io;
+const { output, ERROR_REASON } = io;
+// Explicitly annotated so TypeScript applies never-return control-flow narrowing.
+// A destructured `const { error } = io` is a const WITHOUT a type annotation, and TS
+// only narrows after a never-returning call when the callee is a function declaration
+// or an annotated const. Without the annotation every `error(...)` guard below would
+// need a dead `throw` after it to convince the checker that the value is non-null.
+const error: typeof io.error = io.error;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import planningWorkspaceMod = require('./planning-workspace.cjs');
 const { planningDir } = planningWorkspaceMod;
@@ -24,7 +30,7 @@ import type { Decision } from './decisions.cjs';
 import frontmatterMod = require('./frontmatter.cjs');
 const { extractFrontmatter } = frontmatterMod;
 import { stripFencedCode, collectSections } from './markdown-sectionizer.cjs';
-import { validatePath } from './security.cjs';
+import { tryWithinRoot, PathAcceptance } from './security.cjs';
 import { checkUiPresence } from './ui-safety-gate.cjs';
 import { hasStaticFrontendEvidence } from './ui-frontend-evidence.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports
@@ -91,11 +97,11 @@ function readIfExists(filePath: string): string {

 function resolvePath(inputPath: string, projectDir: string): string {
  const candidate = path.isAbsolute(inputPath) ? inputPath : path.join(projectDir, inputPath);
-  const check = validatePath(candidate, projectDir, { allowAbsolute: true });
-  if (!check.safe) {
+  const contained = tryWithinRoot(candidate, projectDir, PathAcceptance.AbsoluteInsideRoot);
+  if (contained === null) {
    error(`path escapes its allowed directory: ${inputPath}`, ERROR_REASON.USAGE);
  }
-  return check.resolved;
+  return contained;
 }

 interface WorkflowConfig {
@@ -1271,20 +1277,20 @@ function buildPredicateDeps() {
      ) {
        return null;
      }
-      const directPath = validatePath(artifactSuffix, phaseDir);
-      if (directPath.safe && fs.existsSync(directPath.resolved) && fs.statSync(directPath.resolved).isFile()) {
-        return directPath.resolved;
+      const directContained = tryWithinRoot(artifactSuffix, phaseDir);
+      if (directContained !== null && fs.existsSync(directContained) && fs.statSync(directContained).isFile()) {
+        return directContained;
      }
-      const planningPath = validatePath(path.join('.planning', artifactSuffix), phaseDir);
-      if (planningPath.safe && fs.existsSync(planningPath.resolved) && fs.statSync(planningPath.resolved).isFile()) {
-        return planningPath.resolved;
+      const planningContained = tryWithinRoot(path.join('.planning', artifactSuffix), phaseDir);
+      if (planningContained !== null && fs.existsSync(planningContained) && fs.statSync(planningContained).isFile()) {
+        return planningContained;
      }
      try {
        const files = fs.readdirSync(phaseDir);
        for (const f of files) {
          if (f.endsWith('-' + artifactSuffix) || f === artifactSuffix) {
-            const candidate = validatePath(f, phaseDir);
-            if (candidate.safe && fs.statSync(candidate.resolved).isFile()) return candidate.resolved;
+            const candidateContained = tryWithinRoot(f, phaseDir);
+            if (candidateContained !== null && fs.statSync(candidateContained).isFile()) return candidateContained;
          }
        }
      } catch { /* ignore */ }
--- a/src/commands.cts
+++ b/src/commands.cts
@@ -11,10 +11,14 @@ import path from 'node:path';
 import { normalizeEol } from './text-lines.cjs';
 import { execGit, platformWriteSync, platformReadSync, platformEnsureDir, isSpawnTimeout, retryRenameSync } from './shell-command-projection.cjs';
 import { escapeRegex } from './pattern.cjs';
-import { requireSafePath, sanitizeForDisplay, validatePath } from './security.cjs';
+import { requireSafePath, sanitizeForDisplay, tryWithinRoot, PathAcceptance } from './security.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import ioMod = require('./io.cjs');
-const { output, error, ERROR_REASON } = ioMod;
+const { output, ERROR_REASON } = ioMod;
+// Explicitly annotated so TypeScript applies never-return control-flow narrowing.
+// See the identical note in check-command-router.cts: a destructured const carries no
+// type annotation, so TS will not narrow after `error(...)` without this.
+const error: typeof ioMod.error = ioMod.error;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import configLoaderMod = require('./config-loader.cjs');
 const { loadConfig, isGitIgnored } = configLoaderMod;
@@ -348,7 +352,7 @@ function cmdListSeeds(cwd: string, statusFilter: string | undefined, raw: boolea

    let safeFilePath: string;
    try {
-      safeFilePath = requireSafePath(path.join(seedsDir, entry.name), planDir, 'seed file', { allowAbsolute: true });
+      safeFilePath = requireSafePath(path.join(seedsDir, entry.name), planDir, 'seed file', PathAcceptance.AbsoluteInsideRoot);
    } catch {
      continue;
    }
@@ -401,11 +405,11 @@ function cmdVerifyPathExists(cwd: string, targetPath: string | undefined, raw: b
  }

  // Reject null bytes and validate path does not contain traversal attempts
-  if ((targetPath as string).includes('\0')) {
+  if (targetPath.includes('\0')) {
    error('path contains null bytes');
  }

-  const fullPath = path.isAbsolute(targetPath as string) ? targetPath as string : path.join(cwd, targetPath as string);
+  const fullPath = path.isAbsolute(targetPath) ? targetPath : path.join(cwd, targetPath);

  try {
    const stats = fs.statSync(fullPath);
@@ -541,20 +545,20 @@ function cmdResolveModel(cwd: string, agentType: string | undefined, raw: boolea

  const config = loadConfig(cwd);
  const profile = (config['model_profile'] as string) || 'balanced';
-  const model = resolveModelInternal(cwd, agentType!);
-  const effort = resolveEffortInternal(cwd, agentType!);
+  const model = resolveModelInternal(cwd, agentType);
+  const effort = resolveEffortInternal(cwd, agentType);

  // Own-property guard: agentType is an unvalidated CLI positional, so a
  // prototype-chain value ("toString", "constructor") would otherwise return
  // an inherited truthy member from this plain object and misreport a
  // genuinely unknown agent as known (unknown_agent dropped from the result).
  const agentModelsMap = MODEL_PROFILES as Record<string, unknown>;
-  const agentModels = Object.hasOwn(agentModelsMap, agentType!) ? agentModelsMap[agentType!] : undefined;
+  const agentModels = Object.hasOwn(agentModelsMap, agentType) ? agentModelsMap[agentType] : undefined;
  // #2229: `tier` is additive — existing keys and their values are untouched, so
  // every `--pick model` / `--pick profile` / `--raw` consumer is unaffected. It
  // exists because the model id is deliberately blank under resolve_model_ids:"omit",
  // which leaves a tier-sensitive guard with nothing to read.
-  const tier = resolveTierInternal(cwd, agentType!);
+  const tier = resolveTierInternal(cwd, agentType);
  const result = agentModels
    ? { model, profile, effort, tier }
    : { model, profile, effort, tier, unknown_agent: true };
@@ -567,7 +571,7 @@ function cmdResolveGranularity(cwd: string, phaseType: string | undefined, raw:
  }
  assertValidGranularityOverride(override, error);
  const granularity = resolveGranularityInternal(cwd, phaseType, override);
-  const result = (VALID_PHASE_TYPES).has(phaseType!)
+  const result = (VALID_PHASE_TYPES).has(phaseType)
    ? { granularity, phase_type: phaseType }
    : { granularity, phase_type: phaseType, unknown_phase_type: true };
  output(result, raw, granularity);
@@ -599,8 +603,8 @@ function cmdResolveExecution(cwd: string, agentType: string | undefined, raw: bo
  // explicit attempt routes through the tier ladder. resolveModelForTier itself
  // still falls back to resolveModelInternal when dynamic_routing is off.
  let model = (opts.attempt !== undefined && opts.attempt !== null)
-    ? resolveModelForTier(cwd, agentType!, opts.attempt)
-    : resolveModelInternal(cwd, agentType!);
+    ? resolveModelForTier(cwd, agentType, opts.attempt)
+    : resolveModelInternal(cwd, agentType);

  // #2296: when the caller reports WHY the previous attempt failed, consult the
  // provider-escalation ladder. Only a quota/rate-limit class warrants it — a
@@ -610,7 +614,7 @@ function cmdResolveExecution(cwd: string, agentType: string | undefined, raw: bo
  let escalation: Record<string, unknown> | undefined;
  if (opts.failureClass !== undefined) {
    const applicable = opts.failureClass === AGENT_FAILURE_CLASSES.QUOTA_EXCEEDED;
-    const resolved = resolveProviderEscalation(cwd, agentType!, opts.attempt, applicable);
+    const resolved = resolveProviderEscalation(cwd, agentType, opts.attempt, applicable);
    if (resolved.escalated) model = resolved.to;
    escalation = { class: opts.failureClass, ...resolved };
  }
@@ -622,10 +626,10 @@ function cmdResolveExecution(cwd: string, agentType: string | undefined, raw: bo
  if (typeof opts.fastModeOverride === 'boolean') fastModeOpts['override'] = opts.fastModeOverride;

  const effort = (opts.attempt !== undefined && opts.attempt !== null)
-    ? resolveEffortForTier(cwd, agentType!, opts.attempt)
-    : resolveEffortInternal(cwd, agentType!, effortOpts);
+    ? resolveEffortForTier(cwd, agentType, opts.attempt)
+    : resolveEffortInternal(cwd, agentType, effortOpts);

-  const fastMode = resolveFastModeInternal(cwd, agentType!, fastModeOpts);
+  const fastMode = resolveFastModeInternal(cwd, agentType, fastModeOpts);

  const runtime = (config['runtime'] as string) || 'claude';
  // #3007: pass the resolved model so the per-model advertised-effort ceiling
@@ -681,7 +685,7 @@ function cmdResolveExecution(cwd: string, agentType: string | undefined, raw: bo
  // an inherited truthy member from this plain object and misreport a
  // genuinely unknown agent as known (unknown_agent dropped from the result).
  const agentModelsMap = MODEL_PROFILES as Record<string, unknown>;
-  const agentModels = Object.hasOwn(agentModelsMap, agentType!) ? agentModelsMap[agentType!] : undefined;
+  const agentModels = Object.hasOwn(agentModelsMap, agentType) ? agentModelsMap[agentType] : undefined;
  const result: Record<string, unknown> = {
    model,
    profile,
@@ -2738,7 +2742,7 @@ function cmdCommitToSubrepo(cwd: string, message: string | undefined, files: str
  }

  // Group files by sub-repo prefix
-  const { grouped, unmatched } = groupFilesBySubrepo(files as string[], subRepos as string[]);
+  const { grouped, unmatched } = groupFilesBySubrepo(files, subRepos);

  if (unmatched.length > 0) {
    process.stderr.write(`Warning: ${unmatched.length} file(s) did not match any sub-repo prefix: ${unmatched.join(', ')}\n`);
@@ -2793,8 +2797,8 @@ function cmdCommitToSubrepo(cwd: string, message: string | undefined, files: str
    const isMergeInProgressSub = execGit(['rev-parse', '-q', '--verify', 'MERGE_HEAD'], { cwd: repoCwd }).exitCode === 0;
    const canScopeSub = stagedRelPaths.length > 0 && !isMergeInProgressSub;
    const commitArgs = canScopeSub
-      ? ['commit', '-m', message as string, '--', ...stagedRelPaths]
-      : ['commit', '-m', message as string];
+      ? ['commit', '-m', message, '--', ...stagedRelPaths]
+      : ['commit', '-m', message];
    // #3859 follow-up fix as cmdCommit above (line ~2081) — git 2.39.5 needs
    // this override for pathspec-scoped AND whole-index commits alike.
    const commitEnvSub: Record<string, string> = {
@@ -2866,22 +2870,18 @@ function cmdPrSubrepo(
  if (!commitMessage || commitMessage.startsWith('--')) {
    error('commit message required');
  }
-  if ((branch as string).startsWith('-')) {
+  if (branch.startsWith('-')) {
    error(`Branch name must not start with '-': ${branch}`);
  }

  // 0. Security: validate repo path is contained within the workspace root.
-  //    Uses security.cjs validatePath (symlink-safe realpathSync + startsWith guard)
+  //    Uses security.cjs tryWithinRoot (symlink-safe realpathSync + startsWith guard)
  //    to reject ../escape, absolute paths, and symlink traversal.
-  // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/unbound-method
-  const { validatePath } = require('./security.cjs') as {
-    validatePath(filePath: string, baseDir: string): { safe: boolean; resolved: string; error?: string };
-  };
-  const pathCheck = validatePath(repo as string, cwd);
-  if (!pathCheck.safe) {
-    error(`Sub-repo path is unsafe: ${pathCheck.error}`);
+  const repoContained = tryWithinRoot(repo, cwd);
+  if (repoContained === null) {
+    error(`Sub-repo path is unsafe: resolves outside the workspace root`);
  }
-  const repoCwd = pathCheck.resolved;
+  const repoCwd = repoContained;
  if (!fs.existsSync(repoCwd)) {
    error(`Sub-repo not found: ${repoCwd}`);
  }
@@ -2938,7 +2938,7 @@ function cmdPrSubrepo(
  }

  // 2. Guard: refuse if branch already exists — checkout -b is non-idempotent
-  const branchCheck = execGit(['rev-parse', '--verify', branch as string], { cwd: repoCwd });
+  const branchCheck = execGit(['rev-parse', '--verify', branch], { cwd: repoCwd });
  if (branchCheck.exitCode === 0) {
    error(`Branch already exists in ${repo}: ${branch}. Delete it first or choose a unique name.`);
  }
@@ -2949,7 +2949,7 @@ function cmdPrSubrepo(
  const prevBranchName = prevBranchResult.exitCode === 0 ? prevBranchResult.stdout.trim() : null;

  // 3. Create branch
-  const checkoutResult = execGit(['checkout', '-b', branch as string], { cwd: repoCwd });
+  const checkoutResult = execGit(['checkout', '-b', branch], { cwd: repoCwd });
  if (checkoutResult.exitCode !== 0) {
    error(`Failed to create branch ${branch} in ${repo}: ${checkoutResult.stderr}`);
  }
@@ -2959,7 +2959,7 @@ function cmdPrSubrepo(
    if (prevBranchName) {
      execGit(['checkout', prevBranchName], { cwd: repoCwd });
    }
-    execGit(['branch', '-D', branch as string], { cwd: repoCwd });
+    execGit(['branch', '-D', branch], { cwd: repoCwd });
  };

  // 4. Stage explicit files (never git add -A per universal-anti-patterns.md:44)
@@ -2978,8 +2978,8 @@ function cmdPrSubrepo(
  const isMergeInProgressPr = execGit(['rev-parse', '-q', '--verify', 'MERGE_HEAD'], { cwd: repoCwd }).exitCode === 0;
  const canScopePr = changedFiles.length > 0 && !isMergeInProgressPr;
  const commitArgs = canScopePr
-    ? ['commit', '-m', commitMessage as string, '--', ...changedFiles]
-    : ['commit', '-m', commitMessage as string];
+    ? ['commit', '-m', commitMessage, '--', ...changedFiles]
+    : ['commit', '-m', commitMessage];
  // #3859 follow-up fix as cmdCommit above (line ~2081) — git 2.39.5 needs
  // this override for pathspec-scoped AND whole-index commits alike.
  const commitEnvPr: Record<string, string> = {
@@ -3017,7 +3017,7 @@ function cmdPrSubrepo(
  //    Do NOT rollback on push failure — the commit already exists on the local branch.
  //    Deleting the branch here would destroy the only ref holding the user's work.
  //    Leave the branch in place so the user can retry the push.
-  const pushResult = execGit(['push', '--set-upstream', 'origin', branch as string], { cwd: repoCwd, timeout: 60_000 });
+  const pushResult = execGit(['push', '--set-upstream', 'origin', branch], { cwd: repoCwd, timeout: 60_000 });
  if (pushResult.exitCode !== 0) {
    error(`Failed to push ${branch} in ${repo}: ${pushResult.stderr}\nBranch ${branch} was created locally — retry with: git -C ${repo} push --set-upstream origin ${branch}`);
  }
@@ -3040,7 +3040,7 @@ function cmdSummaryExtract(cwd: string, summaryPath: string | undefined, fields:
    error('summary-path required for summary-extract');
  }

-  const fullPath = path.join(cwd, summaryPath as string);
+  const fullPath = path.join(cwd, summaryPath);

  if (!fs.existsSync(fullPath)) {
    output({ error: 'File not found', path: summaryPath }, raw, undefined);
@@ -3501,7 +3501,7 @@ function cmdTodoComplete(cwd: string, filename: string | undefined, options: Tod
  // `\` explicitly (not just path.basename) matters on POSIX, where a
  // literal backslash is just an ordinary filename character to
  // path.basename but not to path.win32.basename or to the user's intent.
-  const rawFilename = filename as string;
+  const rawFilename = filename;
  if (
    rawFilename === '.' ||
    rawFilename === '..' ||
@@ -3514,23 +3514,23 @@ function cmdTodoComplete(cwd: string, filename: string | undefined, options: Tod
    error(`todo name must be a plain filename inside the pending directory, not a path: ${rawFilename}`, ERROR_REASON.USAGE);
  }

-  const sourcePath = path.join(pendingDir, filename as string);
-  const targetPath = path.join(completedDir, filename as string);
+  const sourcePath = path.join(pendingDir, filename);
+  const targetPath = path.join(completedDir, filename);

-  const sourceCheck = validatePath(sourcePath, todosRoot, { allowAbsolute: true });
-  if (!sourceCheck.safe) {
-    error(`todo file escapes its allowed directory: ${filename as string}`, ERROR_REASON.USAGE);
+  const sourceContained = tryWithinRoot(sourcePath, todosRoot, PathAcceptance.AbsoluteInsideRoot);
+  if (sourceContained === null) {
+    error(`todo file escapes its allowed directory: ${filename}`, ERROR_REASON.USAGE);
  }
-  const targetCheck = validatePath(targetPath, todosRoot, { allowAbsolute: true });
-  if (!targetCheck.safe) {
-    error(`todo file escapes its allowed directory: ${filename as string}`, ERROR_REASON.USAGE);
+  const targetContained = tryWithinRoot(targetPath, todosRoot, PathAcceptance.AbsoluteInsideRoot);
+  if (targetContained === null) {
+    error(`todo file escapes its allowed directory: ${filename}`, ERROR_REASON.USAGE);
  }

-  const resolvedSource = sourceCheck.resolved;
-  const resolvedTarget = targetCheck.resolved;
+  const resolvedSource = sourceContained;
+  const resolvedTarget = targetContained;

  if (!fs.existsSync(resolvedSource)) {
-    error(`Todo not found: ${filename as string}`);
+    error(`Todo not found: ${filename}`);
  }

  // #4652: a name that IS a bare basename can still resolve to something that
@@ -3541,7 +3541,7 @@ function cmdTodoComplete(cwd: string, filename: string | undefined, options: Tod
  // with an absolute-path stack trace where every sibling case gives a clean
  // USAGE rejection.
  if (!fs.statSync(resolvedSource).isFile()) {
-    error(`todo name is not a file: ${filename as string}`, ERROR_REASON.USAGE);
+    error(`todo name is not a file: ${filename}`, ERROR_REASON.USAGE);
  }

  const content = fs.readFileSync(resolvedSource, 'utf-8');
--- a/src/coverage.cts
+++ b/src/coverage.cts
@@ -30,7 +30,7 @@ const { output, error } = io;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import coreUtils = require('./core-utils.cjs');
 const { toPosixPath } = coreUtils;
-import { requireSafePath, sanitizeForDisplay } from './security.cjs';
+import { requireSafePath, sanitizeForDisplay, PathAcceptance } from './security.cjs';

 // ─── Frozen typed-IR surface ────────────────────────────────────────────────

@@ -475,7 +475,7 @@ function cmdClassify(cwd: string, options: { summary?: string; file?: string } =

  let resolvedPath: string;
  try {
-    resolvedPath = requireSafePath(filePath, cwd, 'SUMMARY file', { allowAbsolute: true });
+    resolvedPath = requireSafePath(filePath, cwd, 'SUMMARY file', PathAcceptance.AbsoluteInsideRoot);
  } catch (e) {
    // Emit a structured command error instead of leaking a raw stack trace.
    error(`Invalid SUMMARY path: ${e instanceof Error ? e.message : 'unsafe path'}`);
--- a/src/external-descriptor-trust.cts
+++ b/src/external-descriptor-trust.cts
@@ -20,6 +20,7 @@
 'use strict';

 import path from 'node:path';
+import { tryWithinRootLexical } from './security.cjs';

 /**
 * Pure LEXICAL path-containment check (cross-platform). `target` is confined
@@ -32,11 +33,17 @@ import path from 'node:path';
 * `root`) that would redirect the LEXICALLY-confined path to a physically
 * different, unconfined location on disk. A caller relying on this for a
 * write-confinement guarantee against a symlink-planting attacker must pair
- * it with a symlink check (or refuse to follow symlinks at write time) — see
- * capability-source.cts's install adapters (:491,577,675), which is what
- * currently keeps every caller of this function's callers symlink-safe: they
- * reject symlinks upstream, before a target ever reaches a lexical-only check
- * like this one.
+ * it with a symlink check (or refuse to follow symlinks at write time). Only
+ * the capability-loader.cts route into assertDescriptorConfined gets this for
+ * free today: capability-source.cts's staging path rejects symlinks upstream,
+ * before a target ever reaches a lexical-only check like this one — see
+ * copyDirRecursive's `entry.isSymbolicLink()` throw (capability-source.cts:585-586)
+ * and the post-copy budget-walk re-check (capability-source.cts:671-674). This
+ * does NOT extend to isPathConfined's other callers: install-engine.cts:1608
+ * and install-profiles.cts:755,880 do not go through capability-source.cts's
+ * adapters at all and have no symlink guard of their own here. Of the
+ * remaining callers, only retired-artifact-cleanup.cts:69 carries its own
+ * defense, via a local `lstatSync(destDir).isSymbolicLink()` check at line 77.
 *
 * `opts.pathImpl` (default: the ambient `path` module) lets a caller inject
 * `path.win32` or `path.posix`. This is security-relevant: the win32 branch
@@ -47,6 +54,11 @@ import path from 'node:path';
 * in this repo, e.g. src/shell-command-projection.cts's `opts.platform`
 * (#4641). All existing 2-arg callers are unaffected: the default resolves to
 * the ambient `path`, preserving byte-identical behaviour.
+ *
+ * The containment DECISION here now comes from the canonical predicate in
+ * src/security.cts (`tryWithinRootLexical`, ADR-4650 decision 6) — this
+ * function keeps only the lexical RESOLUTION policy (no realpath, no
+ * filesystem access) as its own choice; the comparison itself is shared.
 */
 export function isPathConfined(
  target: string,
@@ -56,11 +68,7 @@ export function isPathConfined(
  if (typeof target !== 'string' || typeof root !== 'string' || target.length === 0 || root.length === 0) {
    return false;
  }
-  const p = opts.pathImpl ?? path;
-  const rootResolved = p.resolve(root);
-  const targetResolved = p.resolve(root, target);
-  const prefix = rootResolved + p.sep;
-  return targetResolved === rootResolved || targetResolved.startsWith(prefix);
+  return tryWithinRootLexical(target, root, { pathImpl: opts.pathImpl }) !== null;
 }

 export interface DescriptorArtifactKind {
--- a/src/init.cts
+++ b/src/init.cts
@@ -44,7 +44,7 @@ import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs';
 import { resolveReportedRuntime } from './host-runtime-detection.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports -- commands.cjs is an export= CommonJS module
 import commandsMod = require('./commands.cjs');
-import { validatePath, loadTrustedGlobalRoots } from './security.cjs';
+import { tryWithinRoot, loadTrustedGlobalRoots, PathAcceptance } from './security.cjs';
 import { getGlobalSkillDir, getGlobalSkillDisplayPath, getGlobalSkillsBase, getGlobalConfigDir } from './runtime-homes.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports -- frontmatter.cjs is an export= CommonJS module
 import frontmatterMod = require('./frontmatter.cjs');
@@ -4015,12 +4015,11 @@ function buildAgentSkillsBlock(
        );
        continue;
      }
-      const pathCheck = validatePath(globalSkillMd, globalSkillsBase, { allowAbsolute: true }) as unknown as Record<string, unknown>;
-      if (!pathCheck['safe']) {
-        const acceptedViaTrustedRoot = trustedGlobalRoots.some((root) => {
-          const rootCheck = validatePath(globalSkillMd, root, { allowAbsolute: true }) as unknown as Record<string, unknown>;
-          return Boolean(rootCheck['safe']);
-        });
+      const globalSkillMdContained = tryWithinRoot(globalSkillMd, globalSkillsBase, PathAcceptance.AbsoluteInsideRoot);
+      if (globalSkillMdContained === null) {
+        const acceptedViaTrustedRoot = trustedGlobalRoots.some(
+          (root) => tryWithinRoot(globalSkillMd, root, PathAcceptance.AbsoluteInsideRoot) !== null,
+        );
        if (!acceptedViaTrustedRoot) {
          warn(
            `[agent-skills] WARNING: Global skill "${skillName}" failed path check (symlink escape?) — skipping\n`,
@@ -4031,19 +4030,27 @@ function buildAgentSkillsBlock(
        // trace, not a skip, so it must not land in the diagnostics warnings[].
        process.stderr.write(`[agent-skills] NOTE: Global skill "${skillName}" accepted via trusted_global_roots (resolves outside the default skills dir)\n`);
      }
+      // `ref` is an emitted display token, not a path anything reads or writes
+      // through — the containment check above is a gate, not a path producer.
+      // Emitting the validated (realpath-resolved, platform-separator) value
+      // instead of this literal broke symlinked skill dirs and Windows output.
+      // The only filesystem read here (existsSync above) already ran on the
+      // lexical path before containment was checked, so ADR-4650's "use the
+      // validated value" rule doesn't apply to this emission.
      validEntries.push({ kind: 'include', ref: `${globalSkillDir}/SKILL.md`, display: displayPath });
      continue;
    }

-    const pathCheck = validatePath(skillPath, projectRoot) as unknown as Record<string, unknown>;
-    if (!pathCheck['safe']) {
+    const skillPathContained = tryWithinRoot(skillPath, projectRoot);
+    if (skillPathContained === null) {
      warn(
-        `[agent-skills] WARNING: Skipping unsafe path "${skillPath}": ${pathCheck['error'] as string}\n`,
+        `[agent-skills] WARNING: Skipping unsafe path "${skillPath}": not confined to the project directory\n`,
      );
      continue;
    }

-    const skillMdPath = path.join(projectRoot, skillPath, 'SKILL.md');
+    // ADR-4650: the validated value is the value used — never re-derive from raw input.
+    const skillMdPath = path.join(skillPathContained, 'SKILL.md');
    if (!fs.existsSync(skillMdPath)) {
      // #2941: if the bare name matches a global skill, hint at the global: prefix.
      // The bare name resolves as project-relative (which doesn't exist), but the
--- a/src/install-engine.cts
+++ b/src/install-engine.cts
@@ -1610,6 +1610,13 @@ function installOpencodeFamilySkills(
      content = applyOpencodeFamilyPathPrefix(content, runtime, pathPrefix);
      content = processAttribution(content, resolveAttribution(runtime));
      const skillDir = path.join(dest, skillName);
+      // isPathConfined is lexical and cannot see a symlink. mkdirSync({recursive:true})
+      // does NOT throw when skillDir already exists as a symlink to a directory, so a
+      // pre-planted link would redirect the SKILL.md write outside `dest`. Refuse to
+      // write through a link (epic #4636; mirrors retired-artifact-cleanup.cts:77).
+      try {
+        if (installFs().lstatSync(skillDir).isSymbolicLink()) continue;
+      } catch { /* ENOENT: not created yet — the normal case */ }
      installFs().mkdirSync(skillDir, { recursive: true });
      installFs().writeFileSync(path.join(skillDir, 'SKILL.md'), content);
      // #2322 HIGH-3 parity: persist the capability-owned marker so a later
--- a/src/install-profiles.cts
+++ b/src/install-profiles.cts
@@ -755,6 +755,10 @@ function readInstalledCapabilitySkill(stem: string, registry: CapabilityRegistry
  if (!isPathConfined(relSkillPath, capDir)) return null;
  const skillPath = path.join(capDir, relSkillPath);
  try {
+    // statSync follows symlinks; isPathConfined is lexical and cannot see one.
+    // Refuse to read through a link so an outside file's content cannot be
+    // installed as a capability skill (epic #4636).
+    if (fs.lstatSync(skillPath).isSymbolicLink()) return null;
    if (!fs.statSync(skillPath).isFile()) return null;
    return { capId, content: fs.readFileSync(skillPath, 'utf8') };
  } catch {
@@ -879,6 +883,13 @@ function stageSkillsForRuntimeAsSkills(
        const skillName = `${prefix}${stem}`;
        if (!isPathConfined(skillName, stageDir)) continue; // defense-in-depth
        const destDir = path.join(stageDir, skillName);
+        // isPathConfined is lexical and cannot see a symlink. mkdirSync({recursive:true})
+        // does NOT throw when destDir already exists as a symlink to a directory, so a
+        // pre-planted link would redirect the SKILL.md write outside `stageDir`. Refuse
+        // to write through a link (epic #4636; mirrors retired-artifact-cleanup.cts:77).
+        try {
+          if (installFs().lstatSync(destDir).isSymbolicLink()) continue;
+        } catch { /* ENOENT: not created yet — the normal case */ }
        installFs().mkdirSync(destDir, { recursive: true });
        installFs().writeFileSync(path.join(destDir, 'SKILL.md'), found.content);
        // #2322 HIGH-3: persist the capability-owned marker so a later prune
--- a/src/installer-migrations.cts
+++ b/src/installer-migrations.cts
@@ -19,6 +19,7 @@ import {
 import { platformWriteSync, retryRenameSync, posixNormalize } from './shell-command-projection.cjs';
 import { realClock, type Clock } from './clock.cjs';
 import { isInstallScopeId, type InstallScope } from './install-scope.cjs';
+import { tryWithinRootLexical } from './security.cjs';
 // #2874 (ADR-58 cleanup phase): this file is the ~1200-line migration
 // plan/apply/rollback/lock/journal engine — almost none of it is on the
 // installRuntimeArtifacts call tree. Only `readInstallManifest` and
@@ -664,11 +665,30 @@ interface EnsureInsideConfigResult {
  fullPath: string;
 }

+// DELIBERATELY LEXICAL — the RESOLUTION policy stays lexical, never realpath
+// (`assertWithinRoot` / `tryWithinRoot`, src/security.cts).
+//
+// Reviewed under epic #4636 Phase 3 and reverted after the remote matrix proved
+// the realpath collapse wrong. This module's contract is that a symlinked
+// managed path is treated AS A LINK and never dereferenced — it is snapshotted
+// as a link, restored as a link, and backed up as a link. The realpath-based
+// predicate dereferences exactly the symlinks this module exists to preserve
+// and then rejects them for escaping configDir
+// ("migration path escapes configDir: extensions/gsd.cjs"). Four tests in
+// tests/installer-migrations.test.cjs pin that behavior.
+//
+// The containment DECISION now routes through the canonical LEXICAL predicate
+// (`tryWithinRootLexical`, ADR-4650 decision 6) — only the comparison moved;
+// the lexical policy itself remains this module's own required choice, and
+// the thrown message / returned `fullPath` are unchanged.
+//
+// `normalizeRelPath` is the pre-gate: it throws on absolute paths and on any
+// '..' segment BEFORE this runs, so the check below is defense-in-depth over
+// already-traversal-free input rather than the primary boundary.
 function ensureInsideConfig(configDir: string, relPath: string): EnsureInsideConfigResult {
  const normalized = normalizeRelPath(relPath);
  const fullPath = path.resolve(configDir, normalized);
-  const root = path.resolve(configDir);
-  if (fullPath !== root && !fullPath.startsWith(root + path.sep)) {
+  if (tryWithinRootLexical(fullPath, configDir) === null) {
    throw new Error(`migration path escapes configDir: ${relPath}`);
  }
  return { normalized, fullPath };
--- a/src/mcp-catalog.cts
+++ b/src/mcp-catalog.cts
@@ -93,7 +93,7 @@

 import fs from 'node:fs';
 import path from 'node:path';
-import { validatePath } from './security.cjs';
+import { tryWithinRoot } from './security.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports -- workflow-fragments.cjs is a CommonJS module compiled from a sibling .cts source; `import x = require()` reads its module.exports namespace directly.
 import workflowFragments = require('./workflow-fragments.cjs');
 const { composeWorkflow } = workflowFragments;
@@ -506,8 +506,8 @@ function classifyUnindexedUri(uri: string): string {

 /** Second, independent gate (design "Hostile inputs" gate 2): re-validate an INDEXED entry's relPath against the catalog root, catching a symlink planted after the index was built. */
 function readIndexedResource(catalog: Catalog, entry: CatalogResourceEntry): ReadResourceResult {
-  const check = validatePath(entry.relPath, catalog.root);
-  if (!check.safe) {
+  const contained = tryWithinRoot(entry.relPath, catalog.root);
+  if (contained === null) {
    fail(REASON.TRAVERSAL_REFUSED, `indexed resource escapes catalog root: ${entry.relPath}`);
  }
  let raw: string;
--- a/src/milestone.cts
+++ b/src/milestone.cts
@@ -21,7 +21,7 @@ import { transitionCore } from './state-transition.cjs';
 import { writeSetComplete } from './write-set.cjs';
 import type { WriteSet } from './write-set.cjs';
 import { updateTableCell, resetQuickTaskRows, QUICK_TASKS_SECTION_ABSENT } from './markdown-table.cjs';
-import { requireSafePath } from './security.cjs';
+import { requireSafePath, PathAcceptance } from './security.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports -- audit.cjs is an export= CommonJS module
 import auditMod = require('./audit.cjs');
 const { resolveQuickTaskSummaryFile } = auditMod;
@@ -1640,7 +1640,7 @@ function listQuickTaskDirsForArchive(cwd: string): string[] {
  for (const entry of sourceEntries) {
    if (!entry.isDirectory()) continue; // excludes symlinks too — see MAJOR 3 note above
    try {
-      requireSafePath(path.join(quickDir, entry.name), planningBase, 'quick task dir', { allowAbsolute: true });
+      requireSafePath(path.join(quickDir, entry.name), planningBase, 'quick task dir', PathAcceptance.AbsoluteInsideRoot);
    } catch {
      continue; // symlink/escape attempt — never a candidate, in preview OR real run
    }
@@ -1719,7 +1719,7 @@ function archiveQuickTaskDirectories(cwd: string, version: string): { archiveDir
        // rename are two separate filesystem observations, and an entry
        // that was a safe real directory at selection time could in theory
        // be swapped for a symlink before this loop reaches it.
-        safeSrc = requireSafePath(src, planningBase, 'quick task dir', { allowAbsolute: true });
+        safeSrc = requireSafePath(src, planningBase, 'quick task dir', PathAcceptance.AbsoluteInsideRoot);
      } catch {
        continue; // symlink/escape attempt — skip, not archived
      }
--- a/src/planning-inspect.cts
+++ b/src/planning-inspect.cts
@@ -92,6 +92,9 @@ const { parseMarkdownTable, matchTableSchema } = markdownTable;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import coreUtilsMod = require('./core-utils.cjs');
 const { normalizeLineEndings } = coreUtilsMod;
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+import securityMod = require('./security.cjs');
+const { tryWithinRoot, PathAcceptance } = securityMod;

 /**
 * The wire schema version. A consumer MUST reject any value other than this
@@ -212,32 +215,45 @@ function toPosix(value: string): string {
 * directory like `.planning-evil/` that merely shares a string prefix. Pure
 * string comparison, no I/O — callers own their own `fs.realpathSync` call
 * (and its own not-found/broken-symlink handling).
+ *
+ * NOT an independent containment implementation — it is the comparison step
+ * of one. `readDocument` below realpaths target and root itself (to keep its
+ * own exists-vs-escaped tri-state) and calls this directly; `isPathContained`
+ * gets its containment DECISION from the canonical `tryWithinRoot` predicate
+ * instead (ADR-4650 decision 6) and no longer uses this function. Every
+ * caller owns its own resolution.
 */
 function isWithinRoot(resolvedTarget: string, resolvedRoot: string): boolean {
  return resolvedTarget === resolvedRoot || resolvedTarget.startsWith(resolvedRoot + path.sep);
 }

 /**
- * Containment check for a path (file OR directory) that resolves its own
- * `fs.realpathSync`, then delegates the actual boundary comparison to
- * `isWithinRoot`. Used where the caller does not need to distinguish "target
- * vanished / broken symlink" from "target resolved but escapes root" — both
- * degrade the same way at every call site that uses this (an escaped or
- * unresolvable phase directory is treated identically to an unreadable one).
- * `readDocument` below needs that distinction for its own exists/readable
- * tri-state, so it keeps its own inline `realpathSync` calls and calls
- * `isWithinRoot` directly instead of this wrapper.
+ * Containment check for a path (file OR directory), used where the caller
+ * does not need to distinguish "target vanished / broken symlink" from
+ * "target resolved but escapes root" — both degrade the same way at every
+ * call site that uses this (an escaped or unresolvable phase directory is
+ * treated identically to an unreadable one). `readDocument` below needs
+ * that distinction for its own exists/readable tri-state, so it keeps its
+ * own inline `realpathSync` calls and calls `isWithinRoot` directly instead
+ * of this wrapper.
+ *
+ * The containment DECISION comes from the canonical `tryWithinRoot`
+ * predicate (ADR-4650 decision 6: a wrapper may decide HOW to degrade,
+ * never WHETHER a path is contained). Must-exist stays this module's OWN
+ * degradation condition, applied after: `tryWithinRoot` deliberately accepts
+ * a not-yet-created path under the root (ancestor-walk realpath), but every
+ * caller of `isPathContained` guards an `fs` read that is about to happen
+ * against an already-existing directory, so a vanished/unresolvable path
+ * must still degrade the same as an escaped one.
 */
 function isPathContained(target: string, root: string): boolean {
-  let realTarget: string;
-  let realRoot: string;
+  if (tryWithinRoot(target, root, PathAcceptance.AbsoluteInsideRoot) === null) return false;
  try {
-    realTarget = fs.realpathSync(target);
-    realRoot = fs.realpathSync(root);
+    fs.realpathSync(target);
  } catch {
    return false;
  }
-  return isWithinRoot(realTarget, realRoot);
+  return true;
 }

 function readDocument(filePath: string, root: string): { text: string | null; exists: boolean; readable: boolean } {
--- a/src/quick-batch.cts
+++ b/src/quick-batch.cts
@@ -34,7 +34,7 @@

 import fs from 'node:fs';
 import path from 'node:path';
-import { requireSafePath, safeJsonParse } from './security.cjs';
+import { requireSafePath, safeJsonParse, PathAcceptance } from './security.cjs';
 import {
  appendQuickTaskRow,
  parseMarkdownTable,
@@ -305,7 +305,7 @@ function parseTaskListFromFile(cwd: string, filePath: string): Result<QuickBatch
  const root = planningRoot(cwd);
  let safePath: string;
  try {
-    safePath = requireSafePath(filePath, root, 'quick-batch --file', { allowAbsolute: true });
+    safePath = requireSafePath(filePath, root, 'quick-batch --file', PathAcceptance.AbsoluteInsideRoot);
  } catch (err) {
    return { ok: false, reason: err instanceof Error ? err.message : String(err) };
  }
--- a/src/security.cts
+++ b/src/security.cts
@@ -24,11 +24,32 @@ import path from 'node:path';

 // ─── Path Traversal Prevention ──────────────────────────────────────────────

+/**
+ * THE containment comparison — the single place this repo decides whether an
+ * already-resolved path lies inside an already-resolved root (ADR-4650).
+ *
+ * Separator-aware on purpose: comparing the bare strings would accept a
+ * sibling that merely shares a prefix (`<root>-evil` against `<root>`), so both
+ * sides get a trailing separator before the prefix test. `target === root` is
+ * contained.
+ *
+ * `pathImpl` lets a caller supply `path.win32` / `path.posix` instead of the
+ * ambient module, so win32 separator semantics are testable off Windows.
+ */
+function isContainedIn(
+  resolvedTarget: string,
+  resolvedRoot: string,
+  pathImpl: { sep: string } = path,
+): boolean {
+  if (resolvedTarget === resolvedRoot) return true;
+  return (resolvedTarget + pathImpl.sep).startsWith(resolvedRoot + pathImpl.sep);
+}
+
 /**
 * Validate that a file path resolves within an allowed base directory.
 * Prevents path traversal attacks via ../ sequences, symlinks, or absolute paths.
 */
-export function validatePath(filePath: unknown, baseDir: unknown, opts: { allowAbsolute?: boolean } = {}): { safe: boolean; resolved: string; error?: string } {
+function validatePath(filePath: unknown, baseDir: unknown, opts: { allowAbsolute?: boolean } = {}): { safe: boolean; resolved: string; error?: string } {
  if (!filePath || typeof filePath !== 'string') {
    return { safe: false, resolved: '', error: 'Empty or invalid file path' };
  }
@@ -102,9 +123,7 @@ export function validatePath(filePath: unknown, baseDir: unknown, opts: { allowA
      }
    }
  }
-  const normalizedBase = resolvedBase + path.sep;
-  const normalizedPath = resolvedPath + path.sep;
-  if (resolvedPath !== resolvedBase && !normalizedPath.startsWith(normalizedBase)) {
+  if (!isContainedIn(resolvedPath, resolvedBase)) {
    return {
      safe: false,
      resolved: resolvedPath,
@@ -182,16 +201,136 @@ export function loadTrustedGlobalRoots(config: unknown): string[] {
  return result;
 }

+/**
+ * A path proven to resolve inside a declared root.
+ *
+ * A plain `string` is NOT assignable to `ContainedPath` — that asymmetry is
+ * the entire point. The shape being replaced (`validatePath`'s
+ * `{ resolved: string }`) returns a usable-looking path even when the answer
+ * is unsafe (the traversal branch still populates `resolved` with the
+ * escaping path), so a plain string in hand proves nothing. A
+ * `ContainedPath` can only be produced by `assertWithinRoot` /
+ * `tryWithinRoot` on their success paths, so possessing one is proof the
+ * containment check already passed.
+ */
+export type ContainedPath = string & { readonly __containedIn: unique symbol };
+
+/**
+ * Named acceptance policy for what kind of candidate path is even considered.
+ *
+ * This replaces the old per-call-site `{ allowAbsolute: true }` boolean flag.
+ * At a call site, `{ allowAbsolute: true }` reads as "containment is relaxed
+ * here" — which is FALSE. An absolute path that resolves OUTSIDE the root is
+ * still rejected; the flag only ever controlled whether an absolute candidate
+ * was considered at all. `AbsoluteInsideRoot` states the real contract: an
+ * absolute candidate is accepted for consideration, but containment is
+ * enforced exactly as it is for a relative one.
+ */
+export const PathAcceptance = {
+  /** Relative candidates only; an absolute candidate is rejected outright. */
+  RelativeOnly: 'relative-only',
+  /**
+   * An absolute candidate is accepted — but ONLY if it still resolves inside the
+   * root. Containment is NOT relaxed by this policy; an absolute path outside the
+   * root is rejected exactly as a traversal is. This is the distinction the old
+   * `{ allowAbsolute: true }` flag failed to make at its call sites.
+   */
+  AbsoluteInsideRoot: 'absolute-inside-root',
+} as const;
+
+export type PathAcceptancePolicy = (typeof PathAcceptance)[keyof typeof PathAcceptance];
+
 /**
 * Validate a file path and throw on traversal attempt.
 * Convenience wrapper around validatePath for use in CLI commands.
 */
-export function requireSafePath(filePath: unknown, baseDir: unknown, label: string | null | undefined, opts: { allowAbsolute?: boolean } = {}): string {
-  const result = validatePath(filePath, baseDir, opts);
+export function assertWithinRoot(candidate: unknown, root: unknown, label?: string | null, policy: PathAcceptancePolicy = PathAcceptance.RelativeOnly): ContainedPath {
+  const result = validatePath(candidate, root, { allowAbsolute: policy === PathAcceptance.AbsoluteInsideRoot });
  if (!result.safe) {
    throw new Error(`${label || 'Path'} validation failed: ${result.error}`);
  }
-  return result.resolved;
+  return result.resolved as ContainedPath;
+}
+
+/**
+ * Validate a file path and return null on traversal attempt (no throw).
+ *
+ * Returns exactly `null` when unsafe — never `''`, never `result.resolved`.
+ * `validatePath` populates `resolved` with the ESCAPING path on the
+ * traversal branch, so returning it here would reproduce the defect this
+ * narrowing exists to remove.
+ */
+export function tryWithinRoot(candidate: unknown, root: unknown, policy: PathAcceptancePolicy = PathAcceptance.RelativeOnly): ContainedPath | null {
+  const result = validatePath(candidate, root, { allowAbsolute: policy === PathAcceptance.AbsoluteInsideRoot });
+  if (!result.safe) {
+    return null;
+  }
+  return result.resolved as ContainedPath;
+}
+
+/**
+ * Validate a file path and throw on traversal attempt.
+ * Convenience wrapper around validatePath for use in CLI commands.
+ *
+ * Delegates to assertWithinRoot so there is one implementation beneath both
+ * names; its declared return type is ContainedPath (a branded string, still
+ * assignable to string) so existing callers keep compiling untouched.
+ */
+export function requireSafePath(filePath: unknown, baseDir: unknown, label: string | null | undefined, policy: PathAcceptancePolicy = PathAcceptance.RelativeOnly): ContainedPath {
+  return assertWithinRoot(filePath, baseDir, label, policy);
+}
+
+/**
+ * LEXICAL containment — `path.resolve` only, never any filesystem access.
+ *
+ * Shares `isContainedIn` with the realpath-based predicate, so there is ONE
+ * containment decision in this repo; these differ only in how a path is
+ * RESOLVED before that decision, never in the decision itself (ADR-4650
+ * decisions 1 and 6).
+ *
+ * Use this — and say why at the call site — only where a symlink must be
+ * PRESERVED rather than resolved, or where the target legitimately does not
+ * exist yet. Three such cases exist: a destination validated before the
+ * `mkdirSync` that creates it, a migration that snapshots and restores a
+ * symlinked path AS A LINK, and a restore gate that refuses links outright.
+ * Everywhere else the realpath-based `assertWithinRoot` / `tryWithinRoot` is
+ * the correct predicate, because a lexical check CANNOT SEE A SYMLINK: a
+ * caller relying on one for a write-confinement guarantee must pair it with
+ * its own symlink refusal.
+ *
+ * `candidate` is resolved RELATIVE TO `root` (so an absolute candidate is
+ * taken as-is, matching `path.resolve` semantics). `target === root` is
+ * contained.
+ *
+ * DELIBERATELY ABSENT: no NUL-byte rejection here. The existing lexical
+ * callers do not reject NUL at this layer (one of them checks NUL itself,
+ * separately), and adding it here would change their behavior. Callers that
+ * need it keep their own check.
+ */
+export function tryWithinRootLexical(
+  candidate: unknown,
+  root: unknown,
+  opts: { pathImpl?: { resolve(...segments: string[]): string; sep: string } } = {},
+): ContainedPath | null {
+  const p = opts.pathImpl || path;
+  if (typeof candidate !== 'string' || candidate === '') return null;
+  if (typeof root !== 'string' || root === '') return null;
+  const rootResolved = p.resolve(root);
+  const targetResolved = p.resolve(root, candidate);
+  return isContainedIn(targetResolved, rootResolved, p) ? (targetResolved as ContainedPath) : null;
+}
+
+export function assertWithinRootLexical(
+  candidate: unknown,
+  root: unknown,
+  label?: string | null,
+  opts: { pathImpl?: { resolve(...segments: string[]): string; sep: string } } = {},
+): ContainedPath {
+  const contained = tryWithinRootLexical(candidate, root, opts);
+  if (contained === null) {
+    throw new Error(`${label || 'Path'} validation failed: lexical containment check failed`);
+  }
+  return contained;
 }

 // ─── Prompt Injection Detection ────────────────────────────────────────────────────
--- a/src/state.cts
+++ b/src/state.cts
@@ -620,14 +620,11 @@ function readTextArgOrFile(cwd: string, value: string | undefined, filePath: str

  // Path traversal guard: ensure file resolves within project directory
  // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/unbound-method
-  const { validatePath } = require('./security.cjs') as { validatePath(filePath: unknown, baseDir: unknown, opts?: { allowAbsolute?: boolean }): { safe: boolean; resolved: string; error?: string } };
-  const pathCheck = validatePath(filePath, cwd, { allowAbsolute: true });
-  if (!pathCheck.safe) {
-    throw new Error(`${label} path rejected: ${pathCheck.error as string}`);
-  }
+  const { assertWithinRoot, PathAcceptance } = require('./security.cjs') as { assertWithinRoot(filePath: unknown, baseDir: unknown, label?: string | null, policy?: 'relative-only' | 'absolute-inside-root'): string; PathAcceptance: { RelativeOnly: 'relative-only'; AbsoluteInsideRoot: 'absolute-inside-root' } };
+  const contained = assertWithinRoot(filePath, cwd, `${label} path`, PathAcceptance.AbsoluteInsideRoot);

  try {
-    return fs.readFileSync(pathCheck.resolved, 'utf-8').trimEnd();
+    return fs.readFileSync(contained, 'utf-8').trimEnd();
  } catch {
    throw new Error(`${label} file not found: ${filePath}`);
  }
--- a/src/uat.cts
+++ b/src/uat.cts
@@ -38,7 +38,7 @@ const { listMilestonePhaseDirs, getAllArchivedPhaseDirs } = phaseLocator;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import auditMod = require('./audit.cjs');
 const { isAuditItemAcknowledged, deriveUatGapSnapshotValue } = auditMod;
-import { requireSafePath, sanitizeForDisplay } from './security.cjs';
+import { requireSafePath, sanitizeForDisplay, PathAcceptance } from './security.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports -- config-loader.cjs is an export= CommonJS module
 import configLoader = require('./config-loader.cjs');
 const { loadConfig } = configLoader;
@@ -403,7 +403,7 @@ function cmdRenderCheckpoint(cwd: string, options: { file?: string } = {}, raw:
    error('UAT file required: use uat render-checkpoint --file <path>');
  }

-  const resolvedPath = requireSafePath(filePath, cwd, 'UAT file', { allowAbsolute: true });
+  const resolvedPath = requireSafePath(filePath, cwd, 'UAT file', PathAcceptance.AbsoluteInsideRoot);
  if (!fs.existsSync(resolvedPath)) {
    error(`UAT file not found: ${filePath}`);
  }
--- a/src/verify.cts
+++ b/src/verify.cts
@@ -34,7 +34,7 @@ import worktreeSafetyMod = require('./worktree-safety.cjs');
 // codebase-drift --name-status parse loop).
 const { decodeGitQuotedPath } = worktreeSafetyMod;
 import { execGit, platformReadSync as safeReadFile } from './shell-command-projection.cjs';
-import { validatePath } from './security.cjs';
+import { tryWithinRoot } from './security.cjs';
 import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs';
 import { detectSchemaFiles, checkSchemaDrift } from './schema-detect.cjs';
 import { extractTaggedBlocks } from './markdown-sectionizer.cjs';
@@ -1549,11 +1549,11 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi
      // project. Leave sourceContent as null so the existing not-found /
      // pending classification below runs unchanged. Note this guard is
      // narrower than it may look: `from: "."` is a non-empty string, so it
-      // still reaches validatePath and safeReadFile below, and DOES read the
+      // still reaches tryWithinRoot and safeReadFile below, and DOES read the
      // cwd directory (yielding "Source read failed: EISDIR") — this branch
      // only short-circuits the true empty-string case.
-      const fromCheck = validatePath(fromPath, cwd);
-      if (!fromCheck.safe) {
+      const fromContained = tryWithinRoot(fromPath, cwd);
+      if (fromContained === null) {
        // Do not echo result.error — it embeds absolute host paths.
        check['path_rejected'] = 'from';
        check['detail'] = 'Source path rejected — resolves outside the project directory';
@@ -1561,7 +1561,7 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi
        continue;
      }
      try {
-        sourceContent = safeReadFile(fromCheck.resolved);
+        sourceContent = safeReadFile(fromContained);
      } catch (err) {
        // Report the errno only — never the message or path (untrusted `from:`
        // can trigger EISDIR/EACCES, which platformReadSync re-throws for any
@@ -1624,15 +1624,15 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi
              // An empty/missing `to:` is a malformed plan, not a
              // path-confinement violation — only a non-empty path that
              // actually resolves outside the project is path_rejected.
-              const toCheck = validatePath(toPath, cwd);
-              if (!toCheck.safe) {
+              const toContained = tryWithinRoot(toPath, cwd);
+              if (toContained === null) {
                // Do not read a rejected `to:` — treat as no target content
                // and do not echo result.error, which embeds absolute host
                // paths.
                check['path_rejected'] = 'to';
                check['detail'] = `Pattern "${link['pattern'] as string}" not found in source; target path rejected — resolves outside the project directory`;
              } else {
-                targetContent = safeReadFile(toCheck.resolved);
+                targetContent = safeReadFile(toContained);
              }
            }
            if (targetContent && pat.test(targetContent)) {
@@ -2022,8 +2022,8 @@ function resolvePhaseDirByToken(phasesDir: string, phaseArg: string): string | n
  const dirNames = dirEntries.filter((e) => e.isDirectory()).map((e) => e.name);
  const matched = matchPhaseDirs(dirNames, normalizedPhase).matches[0];
  if (matched) return path.join(phasesDir, matched);
-  const check = validatePath(phaseArg, phasesDir);
-  if (check.safe && fs.existsSync(check.resolved)) return check.resolved;
+  const contained = tryWithinRoot(phaseArg, phasesDir);
+  if (contained !== null && fs.existsSync(contained)) return contained;
  return null;
 }

--- a/tests/agent-skills.test.cjs
+++ b/tests/agent-skills.test.cjs
@@ -35,7 +35,7 @@ function runGsdToolsWithStderr(args, cwd, env) {
  };
 }

-const { loadTrustedGlobalRoots, validatePath } = require('../gsd-core/bin/lib/security.cjs');
+const { loadTrustedGlobalRoots, tryWithinRoot, PathAcceptance } = require('../gsd-core/bin/lib/security.cjs');

 // ─── helpers ──────────────────────────────────────────────────────────────────

@@ -890,7 +890,7 @@ describe('loadTrustedGlobalRoots', () => {
 // ─── trusted_global_roots integration guard (#52) ─────────────────────────────
 //
 // NOTE: These tests validate the trusted-root bypass logic by directly calling
-// loadTrustedGlobalRoots + validatePath rather than invoking the full CLI
+// loadTrustedGlobalRoots + tryWithinRoot rather than invoking the full CLI
 // (which would require controlling the runtime HOME path in a way that also
 // triggers a symlink escape scenario through gsd-tools subprocess invocation).
 // Full end-to-end symlink testing would require OS-level symlink setup in tmp
@@ -913,24 +913,24 @@ describe('trusted_global_roots guard logic', () => {
    cleanup(externalDir);
  });

-  test('validatePath rejects skill outside globalSkillsBase (baseline — no trusted roots)', () => {
+  test('tryWithinRoot rejects skill outside globalSkillsBase (baseline — no trusted roots)', () => {
    const skillMd = path.join(externalDir, 'SKILL.md');
-    const result = validatePath(skillMd, tmpDir, { allowAbsolute: true });
-    assert.ok(!result.safe, 'skill outside base must be rejected by validatePath');
+    const result = tryWithinRoot(skillMd, tmpDir, PathAcceptance.AbsoluteInsideRoot);
+    assert.equal(result, null, 'skill outside base must be rejected by tryWithinRoot');
  });

-  test('with trusted root matching real target dir — validatePath accepts', () => {
+  test('with trusted root matching real target dir — tryWithinRoot accepts', () => {
    // Simulate the trusted-root fallback: skill is outside base but inside trusted root
    const skillMd = path.join(externalDir, 'SKILL.md');
-    const baseCheck = validatePath(skillMd, tmpDir, { allowAbsolute: true });
-    assert.ok(!baseCheck.safe, 'base check must fail (prerequisite)');
+    const baseCheck = tryWithinRoot(skillMd, tmpDir, PathAcceptance.AbsoluteInsideRoot);
+    assert.equal(baseCheck, null, 'base check must fail (prerequisite)');

    // Trusted root fallback: check against externalDir
    const config = { agent_skills_security: { trusted_global_roots: [externalDir] } };
    const trustedRoots = loadTrustedGlobalRoots(config);
    const acceptedViaTrustedRoot = trustedRoots.some((root) => {
-      const rootCheck = validatePath(skillMd, root, { allowAbsolute: true });
-      return rootCheck.safe;
+      const rootCheck = tryWithinRoot(skillMd, root, PathAcceptance.AbsoluteInsideRoot);
+      return rootCheck !== null;
    });
    assert.ok(acceptedViaTrustedRoot, 'skill must be accepted when within a trusted root');
  });
@@ -942,8 +942,8 @@ describe('trusted_global_roots guard logic', () => {
      const config = { agent_skills_security: { trusted_global_roots: [unrelatedDir] } };
      const trustedRoots = loadTrustedGlobalRoots(config);
      const acceptedViaTrustedRoot = trustedRoots.some((root) => {
-        const rootCheck = validatePath(skillMd, root, { allowAbsolute: true });
-        return rootCheck.safe;
+        const rootCheck = tryWithinRoot(skillMd, root, PathAcceptance.AbsoluteInsideRoot);
+        return rootCheck !== null;
      });
      assert.ok(!acceptedViaTrustedRoot, 'skill must still be rejected when trusted root is unrelated');
    } finally {
@@ -957,8 +957,8 @@ describe('trusted_global_roots guard logic', () => {
    const trustedRoots = loadTrustedGlobalRoots(config);
    assert.strictEqual(trustedRoots.length, 0, 'no roots loaded');
    const acceptedViaTrustedRoot = trustedRoots.some((root) => {
-      const rootCheck = validatePath(skillMd, root, { allowAbsolute: true });
-      return rootCheck.safe;
+      const rootCheck = tryWithinRoot(skillMd, root, PathAcceptance.AbsoluteInsideRoot);
+      return rootCheck !== null;
    });
    assert.ok(!acceptedViaTrustedRoot, 'skill must be rejected when trusted roots is empty');
  });
--- a/tests/install-write-confinement.test.cjs
+++ b/tests/install-write-confinement.test.cjs
@@ -4380,3 +4380,212 @@ describe('Bug #4135: saveLocalPatches reports honest gsd-pristine coverage on mu
 });
  });
 }
+
+// ─────────────────────────────────────────────────────────────────────────
+// #4636 RED: isPathConfined (external-descriptor-trust.cts) is lexical-only —
+// it never calls realpath and cannot see a symlink. Three call sites rely on
+// it with no independent symlink defense of their own. These three tests
+// pre-plant a REAL symlink at the exact write/read target and assert the
+// OUTCOME (outside content untouched / not leaked) rather than the mechanism.
+// Each must FAIL today — that is the deliverable.
+// ─────────────────────────────────────────────────────────────────────────
+describe('#4636 RED: capability-skill symlink escape (isPathConfined has no realpath defense)', () => {
+  const { createTempDir } = require('./helpers.cjs');
+  const runtimeArtifactLayout = require('../gsd-core/bin/lib/runtime-artifact-layout.cjs');
+  const runtimeArtifactInstallPlan = require('../gsd-core/bin/lib/runtime-artifact-install-plan.cjs');
+  const installProfiles = require('../gsd-core/bin/lib/install-profiles.cjs');
+  const { installOpencodeFamilySkills } = require('../gsd-core/bin/lib/install-engine.cjs');
+
+  let savedGsdHome;
+  beforeEach(() => {
+    savedGsdHome = process.env['GSD_HOME'];
+  });
+  afterEach(() => {
+    if (savedGsdHome === undefined) delete process.env['GSD_HOME'];
+    else process.env['GSD_HOME'] = savedGsdHome;
+  });
+
+  test('[RED #4636] a pre-planted symlink at the skill dir must not redirect the SKILL.md write outside dest (install-engine.cts installOpencodeFamilySkills)', (t) => {
+    const targetDir = createTempDir('gsd-4636-oc-target-');
+    const rawDir = createTempDir('gsd-4636-oc-raw-');
+    const outsideDir = createTempDir('gsd-4636-oc-outside-');
+    const gsdHome = createTempDir('gsd-4636-oc-home-');
+    process.env['GSD_HOME'] = gsdHome;
+
+    const capId = 'evil-cap';
+    const stem = 'victim';
+    const capSkillDir = path.join(gsdHome, '.gsd', 'capabilities', capId, 'skills', stem);
+    fs.mkdirSync(capSkillDir, { recursive: true });
+    fs.writeFileSync(path.join(capSkillDir, 'SKILL.md'), '# safe capability skill\n', 'utf8');
+    // A pre-existing, harmless file in rawDir so the writer does not early-return.
+    fs.writeFileSync(path.join(rawDir, 'other.md'), '# other\n', 'utf8');
+
+    const registry = { capabilityClusters: { [capId]: [stem] } };
+    const resolvedProfile = { skills: new Set([stem]) };
+
+    // Derive the exact write target the production code will compute, via the
+    // SAME real functions it calls — not a reimplementation.
+    const layout = runtimeArtifactLayout.resolveRuntimeArtifactLayout('opencode', targetDir);
+    const skillsKindEntry = layout.kinds.find((k) => k.kind === 'skills');
+    if (!skillsKindEntry) {
+      t.skip('opencode layout declares no skills kind on this build');
+      return;
+    }
+    const installRoot = skillsKindEntry.home ?? targetDir;
+    const dest = runtimeArtifactInstallPlan.assertDestWithinConfigHome(installRoot, skillsKindEntry.destSubpath);
+    const skillName = `${skillsKindEntry.prefix}${stem}`;
+    const skillDirPath = path.join(dest, skillName);
+
+    // installOpencodeFamilySkills creates `dest` and prunes any existing
+    // gsd-*-prefixed entries BEFORE reading rawDir — so a symlink planted
+    // before the call is swept by that prune. The window this defect actually
+    // lives in is AFTER the prune and BEFORE this stem's write: hook the
+    // real fs.readdirSync call that reads rawDir (the first read that happens
+    // once the prune has already run) to plant the symlink deterministically
+    // in that exact window — not a race, a synchronous side effect of a call
+    // the function is already guaranteed to make.
+    const origReaddirSync = fs.readdirSync;
+    let planted = false;
+    fs.readdirSync = (...args) => {
+      const [p] = args;
+      if (!planted && fs.existsSync(dest) && path.resolve(String(p)) === path.resolve(rawDir)) {
+        planted = true;
+        fs.symlinkSync(outsideDir, skillDirPath);
+      }
+      return origReaddirSync.apply(fs, args);
+    };
+
+    let installOpencodeFamilySkillsErr = null;
+    try {
+      installOpencodeFamilySkills('opencode', targetDir, rawDir, '~/.opencode/', undefined, resolvedProfile, registry);
+    } catch (err) {
+      installOpencodeFamilySkillsErr = err;
+    } finally {
+      fs.readdirSync = origReaddirSync;
+    }
+
+    try {
+      assert.strictEqual(
+        installOpencodeFamilySkillsErr,
+        null,
+        `installOpencodeFamilySkills threw unexpectedly: ${installOpencodeFamilySkillsErr && installOpencodeFamilySkillsErr.message}`,
+      );
+      assert.ok(planted, 'the symlink was never planted — this test would pass vacuously');
+      assert.strictEqual(
+        fs.existsSync(path.join(outsideDir, 'SKILL.md')),
+        false,
+        'SKILL.md must not be written outside dest via a pre-planted symlink at the skill dir',
+      );
+    } finally {
+      try { fs.unlinkSync(skillDirPath); } catch { /* may not exist / already a real dir */ }
+      cleanup(targetDir);
+      cleanup(rawDir);
+      cleanup(outsideDir);
+      cleanup(gsdHome);
+    }
+  });
+
+  test('[RED #4636] a pre-planted symlink at the staged skill dir must not redirect the SKILL.md write outside stageDir (install-profiles.cts stageSkillsForRuntimeAsSkills)', () => {
+    const srcCommandsDir = createTempDir('gsd-4636-stage-src-');
+    const outsideDir = createTempDir('gsd-4636-stage-outside-');
+    const gsdHome = createTempDir('gsd-4636-stage-home-');
+    process.env['GSD_HOME'] = gsdHome;
+
+    const capId = 'evil-cap-2';
+    const stem = 'victim2';
+    const capSkillDir = path.join(gsdHome, '.gsd', 'capabilities', capId, 'skills', stem);
+    fs.mkdirSync(capSkillDir, { recursive: true });
+    fs.writeFileSync(path.join(capSkillDir, 'SKILL.md'), '# safe capability skill\n', 'utf8');
+    fs.writeFileSync(path.join(srcCommandsDir, 'other.md'), '# other\n', 'utf8');
+
+    const registry = { capabilityClusters: { [capId]: [stem] } };
+    const resolvedProfile = { skills: new Set([stem]) };
+    const converter = (content, _skillName) => content;
+    const prefix = 'gsd-';
+    const skillName = `${prefix}${stem}`;
+
+    // stageDir is a freshly mkdtemp'd, unpredictable-named directory created
+    // INSIDE the function under test, so it cannot be pre-planted from the
+    // outside before the call. Hook the real fs.mkdtempSync call (the same
+    // deterministic-monkeypatch technique already used elsewhere in this repo,
+    // e.g. tests/install-runtime-artifacts.test.cjs's
+    // "rmSync is called on the tempDir when readFileSync throws") to capture
+    // the exact directory this invocation creates and plant the symlink inside
+    // it immediately after creation, before the function's first write.
+    const origMkdtempSync = fs.mkdtempSync;
+    let stageDirSeen = null;
+    fs.mkdtempSync = (...args) => {
+      const dir = origMkdtempSync.apply(fs, args);
+      stageDirSeen = dir;
+      fs.symlinkSync(outsideDir, path.join(dir, skillName));
+      return dir;
+    };
+
+    let stageDir;
+    let stageErr = null;
+    try {
+      stageDir = installProfiles.stageSkillsForRuntimeAsSkills(srcCommandsDir, resolvedProfile, converter, prefix, false, registry);
+    } catch (err) {
+      stageErr = err;
+    } finally {
+      fs.mkdtempSync = origMkdtempSync;
+    }
+
+    try {
+      assert.strictEqual(stageErr, null, `stageSkillsForRuntimeAsSkills threw unexpectedly: ${stageErr && stageErr.message}`);
+      assert.ok(stageDirSeen, 'the function under test must have created a stageDir via mkdtempSync');
+      assert.strictEqual(
+        fs.existsSync(path.join(outsideDir, 'SKILL.md')),
+        false,
+        'SKILL.md must not be written outside stageDir via a pre-planted symlink at the staged skill dir',
+      );
+    } finally {
+      try { fs.unlinkSync(path.join(stageDirSeen, skillName)); } catch { /* may not exist / already a real dir */ }
+      cleanup(srcCommandsDir);
+      cleanup(outsideDir);
+      cleanup(gsdHome);
+      if (stageDir) cleanup(stageDir);
+    }
+  });
+
+  test('[RED #4636] a symlinked SKILL.md inside a capability skill dir must not leak outside file content (install-profiles.cts readInstalledCapabilitySkill)', (t) => {
+    const gsdHome = createTempDir('gsd-4636-read-home-');
+    process.env['GSD_HOME'] = gsdHome;
+    const outsideDir = createTempDir('gsd-4636-read-outside-');
+
+    const capId = 'evil-cap-3';
+    const stem = 'victim3';
+    const capSkillDir = path.join(gsdHome, '.gsd', 'capabilities', capId, 'skills', stem);
+    fs.mkdirSync(capSkillDir, { recursive: true });
+
+    const CANARY_CONTENT = '# TOP SECRET OUTSIDE FILE — must never be surfaced as a skill\n';
+    const canaryPath = path.join(outsideDir, 'secret.txt');
+    fs.writeFileSync(canaryPath, CANARY_CONTENT, 'utf8');
+
+    const skillMdPath = path.join(capSkillDir, 'SKILL.md');
+    try {
+      fs.symlinkSync(canaryPath, skillMdPath, 'file');
+    } catch (err) {
+      if (err && ['EPERM', 'EACCES', 'ENOTSUP'].includes(err.code)) {
+        t.skip('symlink creation is not available on this platform/privilege');
+        cleanup(gsdHome);
+        cleanup(outsideDir);
+        return;
+      }
+      throw err;
+    }
+
+    try {
+      const registry = { capabilityClusters: { [capId]: [stem] } };
+      const found = installProfiles.readInstalledCapabilitySkill(stem, registry);
+      assert.ok(
+        found === null || !String(found.content).includes(CANARY_CONTENT),
+        'readInstalledCapabilitySkill must not return content read through a symlinked SKILL.md pointing outside the capability dir',
+      );
+    } finally {
+      try { fs.unlinkSync(skillMdPath); } catch { /* already gone */ }
+      cleanup(gsdHome);
+      cleanup(outsideDir);
+    }
+  });
+});
--- a/tests/security-prompt-injection.security.test.cjs
+++ b/tests/security-prompt-injection.security.test.cjs
@@ -95,7 +95,9 @@ const FIXTURE_DIR = path.join(__dirname, 'fixtures', 'adversarial', 'security');
 const {
  scanForInjection,
  sanitizeForPrompt,
-  validatePath,
+  assertWithinRoot,
+  tryWithinRoot,
+  PathAcceptance,
  validateShellArg,
  validatePhaseNumber,
  validateFieldName,
@@ -618,20 +620,24 @@ describe('validatePath: hostile path values are rejected before write', () => {
  afterEach(() => { cleanup(tmpDir); });

  test('parent-directory traversal is rejected', () => {
-    const r = validatePath('../../etc/passwd', path.join(tmpDir, '.planning'));
-    assert.strictEqual(r.safe, false);
-    assert.ok(typeof r.error === 'string' && r.error.length > 0);
+    assert.throws(
+      () => assertWithinRoot('../../etc/passwd', path.join(tmpDir, '.planning'), 'test'),
+      /.+/,
+    );
  });

  test('absolute path outside base is rejected', () => {
-    const r = validatePath('/etc/passwd', path.join(tmpDir, '.planning'), { allowAbsolute: true });
-    assert.strictEqual(r.safe, false);
+    assert.equal(
+      tryWithinRoot('/etc/passwd', path.join(tmpDir, '.planning'), PathAcceptance.AbsoluteInsideRoot),
+      null,
+    );
  });

  test('null byte in path is rejected', () => {
-    const r = validatePath('plan.md', path.join(tmpDir, '.planning'));
-    assert.strictEqual(r.safe, false);
-    assert.match(r.error, /null byte/i);
+    assert.throws(
+      () => assertWithinRoot('plan.md', path.join(tmpDir, '.planning'), 'test'),
+      /null byte/i,
+    );
  });

  test('symlink escaping the base is rejected', () => {
@@ -640,8 +646,7 @@ describe('validatePath: hostile path values are rejected before write', () => {
    const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3596-escape-'));
    const linkInside = path.join(base, 'escape-link');
    fs.symlinkSync(outside, linkInside);
-    const r = validatePath('escape-link/anything', base);
-    assert.strictEqual(r.safe, false,
+    assert.equal(tryWithinRoot('escape-link/anything', base), null,
      'a symlink whose target is outside the base must fail containment');
    // Cleanup the outside dir; the link itself is cleaned by cleanup(tmpDir).
    cleanup(outside);
--- a/tests/security.test.cjs
+++ b/tests/security.test.cjs
@@ -13,7 +13,6 @@ const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
 const fc = require('./helpers/fast-check-setup.cjs');

 const {
-  validatePath,
  requireSafePath,
  scanForInjection,
  sanitizeForPrompt,
@@ -24,76 +23,78 @@ const {
  validateFieldName,
  validateShellArg,
  validatePromptStructure,
+  assertWithinRoot,
+  tryWithinRoot,
+  PathAcceptance,
 } = require('../gsd-core/bin/lib/security.cjs');

 // ─── Path Traversal Prevention ──────────────────────────────────────────────

-describe('validatePath', () => {
+describe('assertWithinRoot / tryWithinRoot — engine invariance', () => {
  const base = '/projects/my-app';

  test('allows relative paths within base', () => {
-    const result = validatePath('src/index.js', base);
-    assert.ok(result.safe);
-    assert.equal(result.resolved, path.resolve(base, 'src/index.js'));
+    const r = tryWithinRoot('src/index.js', base);
+    assert.ok(r !== null);
+    assert.equal(r, path.resolve(base, 'src/index.js'));
  });

  test('allows nested relative paths', () => {
-    const result = validatePath('.planning/phases/01-setup/PLAN.md', base);
-    assert.ok(result.safe);
+    const r = tryWithinRoot('.planning/phases/01-setup/PLAN.md', base);
+    assert.ok(r !== null);
  });

  test('rejects ../ traversal escaping base', () => {
-    const result = validatePath('../../etc/passwd', base);
-    assert.ok(!result.safe);
-    assert.ok(result.error.includes('escapes allowed directory'));
+    assert.throws(
+      () => assertWithinRoot('../../etc/passwd', base, 'test'),
+      /escapes allowed directory/,
+    );
  });

  test('rejects absolute paths by default', () => {
-    const result = validatePath('/etc/passwd', base);
-    assert.ok(!result.safe);
-    assert.ok(result.error.includes('Absolute paths not allowed'));
+    assert.throws(
+      () => assertWithinRoot('/etc/passwd', base, 'test'),
+      /Absolute paths not allowed/,
+    );
  });

  test('allows absolute paths within base when opted in', () => {
-    const result = validatePath(path.join(base, 'src/file.js'), base, { allowAbsolute: true });
-    assert.ok(result.safe);
+    const r = tryWithinRoot(path.join(base, 'src/file.js'), base, PathAcceptance.AbsoluteInsideRoot);
+    assert.ok(r !== null);
  });

  test('rejects absolute paths outside base even when opted in', () => {
-    const result = validatePath('/etc/passwd', base, { allowAbsolute: true });
-    assert.ok(!result.safe);
+    assert.equal(tryWithinRoot('/etc/passwd', base, PathAcceptance.AbsoluteInsideRoot), null);
  });

  test('rejects null bytes', () => {
-    const result = validatePath('src/\0evil.js', base);
-    assert.ok(!result.safe);
-    assert.ok(result.error.includes('null bytes'));
+    assert.throws(
+      () => assertWithinRoot('src/\0evil.js', base, 'test'),
+      /null bytes/,
+    );
  });

  test('rejects empty path', () => {
-    const result = validatePath('', base);
-    assert.ok(!result.safe);
+    assert.equal(tryWithinRoot('', base), null);
  });

  test('rejects non-string path', () => {
-    const result = validatePath(42, base);
-    assert.ok(!result.safe);
+    assert.equal(tryWithinRoot(42, base), null);
  });

  test('handles . and ./ correctly (stays in base)', () => {
-    const result = validatePath('.', base);
-    assert.ok(result.safe);
-    assert.equal(result.resolved, path.resolve(base));
+    const r = tryWithinRoot('.', base);
+    assert.ok(r !== null);
+    assert.equal(r, path.resolve(base));
  });

  test('handles complex traversal like src/../../..', () => {
-    const result = validatePath('src/../../../etc/shadow', base);
-    assert.ok(!result.safe);
+    assert.equal(tryWithinRoot('src/../../../etc/shadow', base), null);
  });

  test('allows path that resolves back into base after ..', () => {
-    const result = validatePath('src/../lib/file.js', base);
-    assert.ok(result.safe);
+    const r = tryWithinRoot('src/../lib/file.js', base);
+    assert.ok(r !== null);
  });

  // ─── Dangling symlink + non-canonical base regression coverage ───────────
@@ -147,9 +148,8 @@ describe('validatePath', () => {
      const linkPath = path.join(scratch, 'link.txt');
      const ok = withSymlinkGuard(t, () => fs.symlinkSync(targetPath, linkPath, 'file'));
      if (ok) {
-        const result = validatePath('link.txt', scratch);
-        assert.ok(result.safe, `expected safe:true, got error: ${result.error}`);
-        assert.equal(result.resolved, fs.realpathSync(targetPath));
+        const r = assertWithinRoot('link.txt', scratch, 'in-project symlink');
+        assert.equal(r, fs.realpathSync(targetPath));
      }
    } finally {
      cleanup(scratch);
@@ -165,8 +165,7 @@ describe('validatePath', () => {
      const linkPath = path.join(scratch, 'evil-link.txt');
      const ok = withSymlinkGuard(t, () => fs.symlinkSync(outsideTarget, linkPath, 'file'));
      if (ok) {
-        const result = validatePath('evil-link.txt', scratch);
-        assert.ok(!result.safe);
+        assert.equal(tryWithinRoot('evil-link.txt', scratch), null);
      }
    } finally {
      cleanup(scratch);
@@ -181,11 +180,9 @@ describe('validatePath', () => {
      const linkPath = path.join(scratch, 'dangling-link.txt');
      const ok = withSymlinkGuard(t, () => fs.symlinkSync(nonExistentOutsideTarget, linkPath, 'file'));
      if (ok) {
-        const result = validatePath('dangling-link.txt', scratch);
-        assert.ok(!result.safe, 'a dangling symlink must not be accepted as an in-project path');
-        assert.ok(
-          result.error && result.error.includes('unresolvable symbolic link'),
-          `expected the unresolvable-symlink error, got: ${result.error}`,
+        assert.throws(
+          () => assertWithinRoot('dangling-link.txt', scratch, 'test'),
+          /unresolvable symbolic link/,
        );
      }
    } finally {
@@ -200,9 +197,8 @@ describe('validatePath', () => {
      nc = makeNonCanonicalBase(scratch, t);
      if (nc) {
        fs.mkdirSync(path.join(nc.canonical, 'existingSub'));
-        const result = validatePath('existingSub/newfile.txt', nc.base);
-        assert.ok(result.safe, `expected safe:true, got error: ${result.error}`);
-        assert.equal(result.resolved, path.join(nc.canonical, 'existingSub', 'newfile.txt'));
+        const r = assertWithinRoot('existingSub/newfile.txt', nc.base, 'existing-subdir');
+        assert.equal(r, path.join(nc.canonical, 'existingSub', 'newfile.txt'));
      }
    } finally {
      cleanup(scratch);
@@ -218,9 +214,8 @@ describe('validatePath', () => {
      if (nc) {
        // Neither 'sub1' nor 'sub1/sub2' exist — the immediate-parent-only
        // fallback fails here, which is exactly the BLOCKER-2 scenario.
-        const result = validatePath('sub1/sub2/newfile.txt', nc.base);
-        assert.ok(result.safe, `expected safe:true, got error: ${result.error}`);
-        assert.equal(result.resolved, path.join(nc.canonical, 'sub1', 'sub2', 'newfile.txt'));
+        const r = assertWithinRoot('sub1/sub2/newfile.txt', nc.base, 'BLOCKER-2');
+        assert.equal(r, path.join(nc.canonical, 'sub1', 'sub2', 'newfile.txt'));
      }
    } finally {
      cleanup(scratch);
@@ -237,8 +232,11 @@ describe('validatePath', () => {
        // sub1/sub2 don't exist, and the .. segments escape not just the
        // not-yet-created subdirs but the base itself — the ancestor walk-up
        // must not turn this into an accepted path.
-        const result = validatePath('sub1/sub2/../../../escape.txt', nc.base);
-        assert.ok(!result.safe, 'escaping via .. through not-yet-created dirs must still be rejected');
+        assert.equal(
+          tryWithinRoot('sub1/sub2/../../../escape.txt', nc.base),
+          null,
+          'escaping via .. through not-yet-created dirs must still be rejected',
+        );
      }
    } finally {
      cleanup(scratch);
@@ -1218,7 +1216,7 @@ describe('SECURE: ASVS level scaling (#1627)', () => {
 // accepted. Both directions are required — a predicate rejecting everything
 // would vacuously satisfy PR1 alone.

-describe('validatePath — containment properties (#4652)', () => {
+describe('containment properties (#4652)', () => {
  // A path SEGMENT: letters/digits/dash/underscore, non-empty, never '.' or '..'
  // by construction so every generated escaping path is escaping ONLY via the
  // deliberately-injected `..` components below (never an accidental one).
@@ -1241,10 +1239,10 @@ describe('validatePath — containment properties (#4652)', () => {
        const upCount = baseSegments.length + extraUp;
        const traversal = path.join(...Array(upCount).fill('..'), ...tailSegments, 'target');

-        const result = validatePath(traversal, root);
+        const result = tryWithinRoot(traversal, root);
        assert.strictEqual(
-          result.safe,
-          false,
+          result,
+          null,
          `traversal ${JSON.stringify(traversal)} against root ${root} must be rejected, got: ${JSON.stringify(result)}`,
        );
      },
@@ -1258,13 +1256,13 @@ describe('validatePath — containment properties (#4652)', () => {
        const root = path.resolve('/gsd-root-anchor-in');
        const relPath = path.join(...segments);

-        const result = validatePath(relPath, root);
+        const result = tryWithinRoot(relPath, root);
        assert.strictEqual(
-          result.safe,
+          result !== null,
          true,
          `in-root path ${JSON.stringify(relPath)} against root ${root} must be accepted, got: ${JSON.stringify(result)}`,
        );
-        assert.strictEqual(result.resolved, path.resolve(root, relPath));
+        assert.strictEqual(result, path.resolve(root, relPath));
      },
    ), { seed: 4652, numRuns: 200 });
  });
@@ -1346,3 +1344,148 @@ describe('cross-boundary containment — shared escaping inputs, same rejection
    });
  }
 });
+
+// ─── #4653: assertWithinRoot / tryWithinRoot — narrowed export ──────────────
+//
+// Phase 3 narrows the public surface: `validatePath` becomes module-internal
+// and two new exports appear, both returning a branded ContainedPath.
+// `assertWithinRoot` throws on escape (requireSafePath becomes a thin alias
+// of it); `tryWithinRoot` returns null on escape. Neither export exists yet
+// — this whole block is RED by construction (missing export, not a typo:
+// verified against the compiled gsd-core/bin/lib/security.cjs export list,
+// which lists only validatePath/loadTrustedGlobalRoots/requireSafePath/
+// scanForInjection/sanitizeForPrompt/sanitizeForDisplay/sanitizeLabel/
+// validateShellArg/safeJsonParse/validatePhaseNumber/validateFieldName/
+// validatePromptStructure).
+
+describe('assertWithinRoot / tryWithinRoot — narrowed export (#4653)', () => {
+  const base = '/projects/my-app';
+
+  describe('assertWithinRoot', () => {
+    test('returns the resolved path for a contained relative input', () => {
+      const resolved = assertWithinRoot('src/index.js', base);
+      assert.equal(resolved, path.resolve(base, 'src/index.js'));
+    });
+
+    test('returns the resolved path for an absolute input INSIDE the root when {allowAbsolute:true}', () => {
+      const resolved = assertWithinRoot(path.join(base, 'src/file.js'), base, null, PathAcceptance.AbsoluteInsideRoot);
+      assert.equal(resolved, path.resolve(base, 'src/file.js'));
+    });
+
+    test('throws on a ../ traversal escaping the root', () => {
+      assert.throws(() => assertWithinRoot('../../etc/passwd', base));
+    });
+
+    test('throws on an absolute path outside the root even with {allowAbsolute:true}', () => {
+      assert.throws(() => assertWithinRoot('/etc/passwd', base, null, PathAcceptance.AbsoluteInsideRoot));
+    });
+
+    test('throws on a null byte', () => {
+      assert.throws(() => assertWithinRoot('src/\0evil.js', base));
+    });
+
+    test('throws on empty input', () => {
+      assert.throws(() => assertWithinRoot('', base));
+    });
+
+    test('throws on non-string input', () => {
+      assert.throws(() => assertWithinRoot(42, base));
+    });
+
+    test('thrown message uses the label, matching requireSafePath\'s "<label> validation failed: <reason>" shape', () => {
+      assert.throws(
+        () => assertWithinRoot('../../etc/passwd', base, 'PRD file'),
+        /PRD file validation failed/,
+      );
+    });
+  });
+
+  describe('tryWithinRoot', () => {
+    test('returns the resolved path for a contained relative input', () => {
+      const resolved = tryWithinRoot('src/index.js', base);
+      assert.equal(resolved, path.resolve(base, 'src/index.js'));
+    });
+
+    test('returns the resolved path for an absolute input INSIDE the root when {allowAbsolute:true}', () => {
+      const resolved = tryWithinRoot(path.join(base, 'src/file.js'), base, PathAcceptance.AbsoluteInsideRoot);
+      assert.equal(resolved, path.resolve(base, 'src/file.js'));
+    });
+
+    test('returns exactly null (not "" and not the escaping path) for a ../ traversal escaping the root', () => {
+      const result = tryWithinRoot('../../etc/passwd', base);
+      assert.strictEqual(result, null);
+    });
+
+    test('returns exactly null for an absolute path outside the root even with {allowAbsolute:true}', () => {
+      const result = tryWithinRoot('/etc/passwd', base, PathAcceptance.AbsoluteInsideRoot);
+      assert.strictEqual(result, null);
+    });
+
+    test('returns exactly null for a null byte, empty input, and non-string input', () => {
+      assert.strictEqual(tryWithinRoot('src/\0evil.js', base), null);
+      assert.strictEqual(tryWithinRoot('', base), null);
+      assert.strictEqual(tryWithinRoot(42, base), null);
+    });
+
+    test('on a traversal escape, the return value does NOT contain the escaping path\'s basename', () => {
+      // Regression guard: the current validatePath shape populates `resolved`
+      // with the ESCAPING path on the traversal branch even when safe:false —
+      // a caller who ignores the boolean gets a usable attacker-controlled
+      // value. tryWithinRoot must not leak that value in any form; asserting
+      // strict null (above) already covers this, but this test additionally
+      // guards against a partial fix that returns '' or a truncated variant
+      // still containing the escaping basename.
+      const result = tryWithinRoot('../../etc/passwd', base);
+      assert.strictEqual(result, null);
+      const resultStr = String(result);
+      assert.ok(!resultStr.includes('passwd'), `leaked escaping path basename: ${resultStr}`);
+    });
+  });
+
+  // ── Parity: the two shapes must never drift ──────────────────────────────
+  //
+  // tryWithinRoot(p, root) returns non-null IFF assertWithinRoot(p, root)
+  // does not throw, and when both succeed the returned values are equal.
+  // Two exported shapes over one engine is a divergence pair by
+  // construction; this is the parity assertion for it. Seeded per this
+  // repo's fast-check convention (see the #4652 containment properties
+  // above in this same file).
+
+  test('parity: tryWithinRoot succeeds IFF assertWithinRoot does not throw, and values agree (#4653)', () => {
+    const root = path.resolve('/gsd-root-anchor-parity');
+    fc.assert(fc.property(
+      fc.string(),
+      (candidate) => {
+        let assertResult;
+        let assertThrew = false;
+        try {
+          assertResult = assertWithinRoot(candidate, root);
+        } catch {
+          assertThrew = true;
+        }
+        const tryResult = tryWithinRoot(candidate, root);
+
+        if (assertThrew) {
+          assert.strictEqual(tryResult, null, `assertWithinRoot threw for ${JSON.stringify(candidate)} but tryWithinRoot returned non-null: ${tryResult}`);
+        } else {
+          assert.notStrictEqual(tryResult, null, `assertWithinRoot succeeded for ${JSON.stringify(candidate)} but tryWithinRoot returned null`);
+          assert.strictEqual(tryResult, assertResult, `assertWithinRoot and tryWithinRoot disagree on resolved value for ${JSON.stringify(candidate)}`);
+        }
+      },
+    ), { seed: 4653, numRuns: 200 });
+  });
+
+  // ── Message-text contract ─────────────────────────────────────────────────
+  //
+  // A user-facing `reason` field elsewhere in the test suite asserts on the
+  // literal string "escapes allowed directory" (see the existing
+  // `validatePath` "rejects ../ traversal escaping base" test above). A
+  // refactor to assertWithinRoot/tryWithinRoot must not reword it.
+
+  test('escape rejection still carries the text "escapes allowed directory" (#4653)', () => {
+    assert.throws(
+      () => assertWithinRoot('../../etc/passwd', base),
+      /escapes allowed directory/,
+    );
+  });
+});