From 889c7efba02be9b375be0aec656fdb4539955af0 Mon Sep 17 00:00:00 2001
From: sim <sim@local>
Date: Sat, 12 Sep 2026 11:54:08 -0400
Subject: [PATCH 01/11] test(#4653): failing-first coverage for the narrowed
 containment export
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Phase 3 of epic #4636. Tests only; no implementation. These MUST fail.

A refactor changes what a good test looks like: the behavior under test must be
IDENTICAL before and after, so most of this phase's safety comes from
invariance rather than new assertions. That safety net already exists and is
untouched here — tests/security.test.cjs already pins the two engine behaviors
a re-derivation would silently lose:

  :177  a DANGLING symlink to a non-existent OUTSIDE target stays safe:false
        (the existence-oracle closure)
  :213  a not-yet-created file in a not-yet-created subdir under a
        non-canonical base stays safe:true (ancestor canonicalization)

plus traversal, absolute in/out, null bytes, empty, non-string, and
requireSafePath's throw. Those 0 deletions are the point: if any of them had to
change, the engine would have changed, and the engine is not supposed to.

What is new is the export surface Phase 3 introduces:

  assertWithinRoot(candidate, root, label?, opts?) -> ContainedPath  (throws)
  tryWithinRoot(candidate, root, opts?)            -> ContainedPath | null

Two shapes rather than one, because several call sites need a NON-throwing
check — findPhaseArtifact probes a direct path, then a .planning/ path, then
each readdir entry, and throwing on the first miss would break it outright.
ADR-4650 names only the throwing form; this is the gap between the ADR and the
call sites, recorded rather than papered over.

Rows that exist because they are the ones nobody enumerates:

- tryWithinRoot must return EXACTLY null on escape, and its return must not
  contain the escaping path's basename. The shape being replaced populates its
  "resolved" field with the escaping path precisely on the traversal branch, so
  a caller who ignores the boolean gets a usable attacker-controlled value.
  That is the defect the narrowing exists to remove, so it is asserted
  directly.
- A seeded parity property: tryWithinRoot returns non-null if and only if
  assertWithinRoot does not throw, and the values agree. Two exported shapes
  over one engine is a divergence pair by construction.
- The rejection text still contains the phrase "escapes allowed directory".
  Another suite surfaces it through a user-facing "reason" field, and a
  refactor is exactly where wording drifts unnoticed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 tests/security.test.cjs | 147 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 147 insertions(+)

diff --git a/tests/security.test.cjs b/tests/security.test.cjs
index f7e34d523..6433bacae 100644
--- a/tests/security.test.cjs
+++ b/tests/security.test.cjs
@@ -24,6 +24,8 @@ const {
   validateFieldName,
   validateShellArg,
   validatePromptStructure,
+  assertWithinRoot,
+  tryWithinRoot,
 } = require('../gsd-core/bin/lib/security.cjs');
 
 // ─── Path Traversal Prevention ──────────────────────────────────────────────
@@ -1346,3 +1348,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, { allowAbsolute: true });
+      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, { allowAbsolute: true }));
+    });
+
+    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, { allowAbsolute: true });
+      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, { allowAbsolute: true });
+      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/,
+    );
+  });
+});

From 4ebcec7750e70fcd1eb81e692d50787b1acc4128 Mon Sep 17 00:00:00 2001
From: sim <sim@local>
Date: Sat, 12 Sep 2026 12:16:24 -0400
Subject: [PATCH 02/11] refactor(#4653): narrow the containment export and
 migrate all 15 validatePath sites
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Phase 3 of epic #4636, stages 1-2. Implements ADR-4650 decisions 1 and 2: one
containment predicate, and an exported shape that cannot hand a caller a usable
path when the answer is unsafe.

THE EXPORT IS NOW A PAIR, BOTH RETURNING A BRANDED TYPE:

  assertWithinRoot(candidate, root, label?, opts?) -> ContainedPath   (throws)
  tryWithinRoot(candidate, root, opts?)            -> ContainedPath | null

ADR-4650 names only the throwing form. That does not survive contact with the
call sites: findPhaseArtifact probes a direct path, then a .planning/ path, then
each readdir entry, and throwing on the first miss breaks it outright. Six of the
fifteen sites need a non-throwing check. Recorded here rather than papered over.

WHY BRANDED. validatePath returns { safe, resolved, error } and populates
resolved with the ESCAPING path on the traversal branch — so a caller who skips
the boolean gets an attacker-controlled value precisely in the dangerous case.
tryWithinRoot returns exactly null there; assertWithinRoot throws. A plain string
is not assignable to ContainedPath, so a migrated site that validates one path
and then passes a different one is now a type error rather than a silent bug.
That is the defect this epic exists to close, and I introduced it twice in
Phase 2.

THE ENGINE IS UNTOUCHED. validatePath's body is not re-derived — the diff shows
zero edits to the dangling-symlink existence-oracle closure, the ancestor
canonicalization (macOS /var vs /private/var), or the separator-aware boundary
test. Each was acquired as a bug fix and a re-derivation would silently lose one.
requireSafePath now delegates to assertWithinRoot, so there is one implementation
beneath both names; its return type is branded, which is why its 13 call sites
compile unchanged.

A TYPESCRIPT LIMITATION, FIXED AT THE ROOT RATHER THAN WORKED AROUND. TS applies
never-return control-flow narrowing only when the callee is a function
declaration or a const with an EXPLICIT type annotation. Both routers do
`const { error } = io` — destructured, unannotated — so `error(...)` did not
narrow ContainedPath | null and four sites wanted a dead
`throw new Error('unreachable')` after it. Annotating the const
(`const error: typeof io.error = io.error`) makes TS narrow properly and the dead
throws are gone.

That annotation has a large, deliberate consequence: with narrowing working,
`@typescript-eslint/no-unnecessary-type-assertion` fires at 37 sites in
commands.cts where `as string` / `!` existed ONLY to paper over the missing
narrowing. They are removed. The rule is type-aware and fires only where the
assertion changes nothing, and both forms erase at compile time, so the emitted
behavior is unchanged — but the module loses 37 unchecked casts over
string | undefined, which is the same class of "trust me" the containment work
is removing. Widening the diff here buys that.

TWO SITES LOSE DIAGNOSTIC TEXT, deliberately. tryWithinRoot has no error channel,
so init.cts's agent-skills warning and cmdPrSubrepo's rejection now name the
condition rather than echoing validatePath's message. verify.cts had already
stopped echoing it on purpose — the message embeds absolute host paths — so this
makes the three agree instead of two-of-three.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/check-command-router.cts | 32 +++++++-----
 src/commands.cts             | 98 ++++++++++++++++++------------------
 src/init.cts                 | 15 +++---
 src/mcp-catalog.cts          |  6 +--
 src/security.cts             | 48 ++++++++++++++++--
 src/state.cts                |  9 ++--
 src/verify.cts               | 18 +++----
 7 files changed, 135 insertions(+), 91 deletions(-)

diff --git a/src/check-command-router.cts b/src/check-command-router.cts
index 576116a57..80e0e7974 100644
--- 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 } 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, { allowAbsolute: true });
+  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 */ }
diff --git a/src/commands.cts b/src/commands.cts
index 663e02b39..95ba378dc 100644
--- 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 } 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;
@@ -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, { allowAbsolute: true });
+  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, { allowAbsolute: true });
+  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');
diff --git a/src/init.cts b/src/init.cts
index 93bfc37fe..9c9206948 100644
--- 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 } 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,11 +4015,10 @@ function buildAgentSkillsBlock(
         );
         continue;
       }
-      const pathCheck = validatePath(globalSkillMd, globalSkillsBase, { allowAbsolute: true }) as unknown as Record<string, unknown>;
-      if (!pathCheck['safe']) {
+      const globalSkillMdContained = tryWithinRoot(globalSkillMd, globalSkillsBase, { allowAbsolute: true });
+      if (globalSkillMdContained === null) {
         const acceptedViaTrustedRoot = trustedGlobalRoots.some((root) => {
-          const rootCheck = validatePath(globalSkillMd, root, { allowAbsolute: true }) as unknown as Record<string, unknown>;
-          return Boolean(rootCheck['safe']);
+          return tryWithinRoot(globalSkillMd, root, { allowAbsolute: true }) !== null;
         });
         if (!acceptedViaTrustedRoot) {
           warn(
@@ -4035,10 +4034,10 @@ function buildAgentSkillsBlock(
       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}": resolves outside the project directory\n`,
       );
       continue;
     }
diff --git a/src/mcp-catalog.cts b/src/mcp-catalog.cts
index a4aec05ae..c74d11375 100644
--- 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;
diff --git a/src/security.cts b/src/security.cts
index 2821bdb97..0c696a2e2 100644
--- a/src/security.cts
+++ b/src/security.cts
@@ -182,16 +182,58 @@ 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 };
+
 /**
  * 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, opts: { allowAbsolute?: boolean } = {}): ContainedPath {
+  const result = validatePath(candidate, root, opts);
   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, opts: { allowAbsolute?: boolean } = {}): ContainedPath | null {
+  const result = validatePath(candidate, root, opts);
+  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, opts: { allowAbsolute?: boolean } = {}): ContainedPath {
+  return assertWithinRoot(filePath, baseDir, label, opts);
 }
 
 // ─── Prompt Injection Detection ────────────────────────────────────────────────────
diff --git a/src/state.cts b/src/state.cts
index c294aee79..9bad609d5 100644
--- 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 } = require('./security.cjs') as { assertWithinRoot(filePath: unknown, baseDir: unknown, label?: string | null, opts?: { allowAbsolute?: boolean }): string };
+  const contained = assertWithinRoot(filePath, cwd, `${label} path`, { allowAbsolute: true });
 
   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}`);
   }
diff --git a/src/verify.cts b/src/verify.cts
index d8e1ef3e5..0a3c4e8db 100644
--- 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';
@@ -1552,8 +1552,8 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi
       // still reaches validatePath 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;
 }
 

From 26384ca988d573848a9a914eb9238b57a3d70cda Mon Sep 17 00:00:00 2001
From: sim <sim@local>
Date: Sat, 12 Sep 2026 12:22:22 -0400
Subject: [PATCH 03/11] refactor(#4653): make validatePath module-internal
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Phase 3 of epic #4636, stage 3a. ADR-4650 decision 2: the engine stops being a
public shape. The only exported containment surface is now assertWithinRoot /
tryWithinRoot / requireSafePath, none of which can hand a caller a usable path
when the answer is unsafe.

The src/security.cts diff is one keyword. The engine body is byte-identical —
the dangling-symlink existence-oracle closure, the ancestor canonicalization and
the separator-aware boundary test are untouched, which is the whole constraint
this phase operates under.

WHAT THE TRANSLATION COST, AND THE RULE THAT KEPT IT AT ZERO. Roughly thirty test
call sites consumed validatePath directly, including the two BLOCKER regressions
that are this refactor's safety net. Translating them all to
`tryWithinRoot(...) === null` would have looked correct and silently destroyed
one of them: BLOCKER-1 asserts the rejection reason contains "unresolvable
symbolic link", which is what distinguishes a DANGLING symlink from an ordinary
escape. tryWithinRoot returns a bare null and cannot tell those apart, so that
assertion would have degenerated into "it failed somehow" — and the
existence-oracle closure could regress with the test still green.

So the rule applied throughout is: an assertion on the rejection REASON goes
through assertWithinRoot, whose throw carries the engine's message verbatim; only
assertions on the boolean go through tryWithinRoot. Under that rule no coverage
is lost. BLOCKER-1 still pins "unresolvable symbolic link" and BLOCKER-2 still
pins the exact canonicalized resolved value.

Three success-path tests came out BETTER than they went in. They previously
carried `expected safe:true, got error: ${result.error}` as an assertion message;
routing them through assertWithinRoot means an engine regression now surfaces the
real reason in the failure itself rather than as a hand-built string.

The two describe blocks named after validatePath are renamed — a block named for
a symbol the module no longer exports is a false signpost.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/security.cts            |   2 +-
 tests/agent-skills.test.cjs |  28 +++++-----
 tests/security.test.cjs     | 107 +++++++++++++++++-------------------
 3 files changed, 66 insertions(+), 71 deletions(-)

diff --git a/src/security.cts b/src/security.cts
index 0c696a2e2..84e9d9c87 100644
--- a/src/security.cts
+++ b/src/security.cts
@@ -28,7 +28,7 @@ import path from 'node:path';
  * 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' };
   }
diff --git a/tests/agent-skills.test.cjs b/tests/agent-skills.test.cjs
index bc0e77aa4..f1f935c02 100644
--- 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 } = 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, { allowAbsolute: true });
+    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, { allowAbsolute: true });
+    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, { allowAbsolute: true });
+      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, { allowAbsolute: true });
+        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, { allowAbsolute: true });
+      return rootCheck !== null;
     });
     assert.ok(!acceptedViaTrustedRoot, 'skill must be rejected when trusted roots is empty');
   });
diff --git a/tests/security.test.cjs b/tests/security.test.cjs
index 6433bacae..be8cfa297 100644
--- 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,
@@ -30,72 +29,71 @@ const {
 
 // ─── 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, { allowAbsolute: true });
+    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, { allowAbsolute: true }), 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 ───────────
@@ -149,9 +147,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);
@@ -167,8 +164,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);
@@ -183,11 +179,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 {
@@ -202,9 +196,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);
@@ -220,9 +213,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);
@@ -239,8 +231,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);
@@ -1220,7 +1215,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).
@@ -1243,10 +1238,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)}`,
         );
       },
@@ -1260,13 +1255,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 });
   });

From 7e7a239a652d6b29f4c5773c443447b24d189910 Mon Sep 17 00:00:00 2001
From: sim <sim@local>
Date: Sat, 12 Sep 2026 12:31:01 -0400
Subject: [PATCH 04/11] refactor(#4653): replace the allowAbsolute flag with a
 named acceptance policy
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Phase 3 of epic #4636, stage 3b. Satisfies #4653's criterion that
`opts.allowAbsolute` become "a named acceptance policy on the predicate, not a
per-call-site boolean".

The flag was actively misleading at the call site. `{ allowAbsolute: true }`
reads as "containment is relaxed here". It never was: an absolute path that
resolves outside the root is rejected exactly as a traversal is. The flag only
ever controlled whether an absolute candidate was CONSIDERED. On a security
predicate that is the wrong thing for a reviewer to have to infer, and 31 call
sites were asking them to infer it.

    PathAcceptance.RelativeOnly         relative candidates only
    PathAcceptance.AbsoluteInsideRoot   absolute accepted, containment unchanged

The three exported wrappers take the policy and translate it inward.
validatePath keeps its internal `{ allowAbsolute }` opts and its body untouched —
the engine is not re-derived here either, only the exported surface is renamed.

MEASURED, NOT ESTIMATED. 31 call sites across 10 files, counted by walking the
AST with the repo's own @typescript-eslint/parser rather than grepping: a text
match would have folded in the options-type declaration, default parameter
values and comments. All 31 pass the literal `true`; none passes `false` or a
dynamic value, so the migration is uniform and `RelativeOnly` is purely the
existing default made nameable. audit.cts alone holds 18 of them.

This migration is compiler-verified in a way the containment-value migration in
the previous commit was not: the parameter type changed from an object to a
string union, so any missed site is a build error rather than a silent
behavioral difference. That is why a 31-site mechanical edit is acceptable in
the phase whose stated risk is the width of mechanical change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/adr-parser.cts           |  4 ++--
 src/audit.cts                | 38 ++++++++++++++++++------------------
 src/check-command-router.cts |  4 ++--
 src/commands.cts             |  8 ++++----
 src/coverage.cts             |  4 ++--
 src/init.cts                 |  6 +++---
 src/milestone.cts            |  6 +++---
 src/quick-batch.cts          |  4 ++--
 src/security.cts             | 37 +++++++++++++++++++++++++++++------
 src/state.cts                |  4 ++--
 src/uat.cts                  |  4 ++--
 tests/agent-skills.test.cjs  | 12 ++++++------
 tests/security.test.cjs      | 13 ++++++------
 13 files changed, 85 insertions(+), 59 deletions(-)

diff --git a/src/adr-parser.cts b/src/adr-parser.cts
index 0337d609f..fe6110373 100644
--- 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));
diff --git a/src/audit.cts b/src/audit.cts
index 50da84a5c..f983b4c4b 100644
--- 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' };
diff --git a/src/check-command-router.cts b/src/check-command-router.cts
index 80e0e7974..1c1ddfcd9 100644
--- a/src/check-command-router.cts
+++ b/src/check-command-router.cts
@@ -30,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 { tryWithinRoot } 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
@@ -97,7 +97,7 @@ function readIfExists(filePath: string): string {
 
 function resolvePath(inputPath: string, projectDir: string): string {
   const candidate = path.isAbsolute(inputPath) ? inputPath : path.join(projectDir, inputPath);
-  const contained = tryWithinRoot(candidate, projectDir, { allowAbsolute: true });
+  const contained = tryWithinRoot(candidate, projectDir, PathAcceptance.AbsoluteInsideRoot);
   if (contained === null) {
     error(`path escapes its allowed directory: ${inputPath}`, ERROR_REASON.USAGE);
   }
diff --git a/src/commands.cts b/src/commands.cts
index 95ba378dc..c7413596f 100644
--- a/src/commands.cts
+++ b/src/commands.cts
@@ -11,7 +11,7 @@ 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, tryWithinRoot } 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_REASON } = ioMod;
@@ -352,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;
     }
@@ -3517,11 +3517,11 @@ function cmdTodoComplete(cwd: string, filename: string | undefined, options: Tod
   const sourcePath = path.join(pendingDir, filename);
   const targetPath = path.join(completedDir, filename);
 
-  const sourceContained = tryWithinRoot(sourcePath, todosRoot, { allowAbsolute: true });
+  const sourceContained = tryWithinRoot(sourcePath, todosRoot, PathAcceptance.AbsoluteInsideRoot);
   if (sourceContained === null) {
     error(`todo file escapes its allowed directory: ${filename}`, ERROR_REASON.USAGE);
   }
-  const targetContained = tryWithinRoot(targetPath, todosRoot, { allowAbsolute: true });
+  const targetContained = tryWithinRoot(targetPath, todosRoot, PathAcceptance.AbsoluteInsideRoot);
   if (targetContained === null) {
     error(`todo file escapes its allowed directory: ${filename}`, ERROR_REASON.USAGE);
   }
diff --git a/src/coverage.cts b/src/coverage.cts
index 3295c7834..6ec9962c0 100644
--- 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'}`);
diff --git a/src/init.cts b/src/init.cts
index 9c9206948..0888cf0c0 100644
--- 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 { tryWithinRoot, 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,10 +4015,10 @@ function buildAgentSkillsBlock(
         );
         continue;
       }
-      const globalSkillMdContained = tryWithinRoot(globalSkillMd, globalSkillsBase, { allowAbsolute: true });
+      const globalSkillMdContained = tryWithinRoot(globalSkillMd, globalSkillsBase, PathAcceptance.AbsoluteInsideRoot);
       if (globalSkillMdContained === null) {
         const acceptedViaTrustedRoot = trustedGlobalRoots.some((root) => {
-          return tryWithinRoot(globalSkillMd, root, { allowAbsolute: true }) !== null;
+          return tryWithinRoot(globalSkillMd, root, PathAcceptance.AbsoluteInsideRoot) !== null;
         });
         if (!acceptedViaTrustedRoot) {
           warn(
diff --git a/src/milestone.cts b/src/milestone.cts
index ce78418a4..b29fe2d9e 100644
--- 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
       }
diff --git a/src/quick-batch.cts b/src/quick-batch.cts
index 4b48f0e3f..75dae3ab8 100644
--- 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) };
   }
diff --git a/src/security.cts b/src/security.cts
index 84e9d9c87..260832f3c 100644
--- a/src/security.cts
+++ b/src/security.cts
@@ -196,12 +196,37 @@ export function loadTrustedGlobalRoots(config: unknown): string[] {
  */
 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 assertWithinRoot(candidate: unknown, root: unknown, label?: string | null, opts: { allowAbsolute?: boolean } = {}): ContainedPath {
-  const result = validatePath(candidate, root, 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}`);
   }
@@ -216,8 +241,8 @@ export function assertWithinRoot(candidate: unknown, root: unknown, label?: stri
  * traversal branch, so returning it here would reproduce the defect this
  * narrowing exists to remove.
  */
-export function tryWithinRoot(candidate: unknown, root: unknown, opts: { allowAbsolute?: boolean } = {}): ContainedPath | null {
-  const result = validatePath(candidate, root, opts);
+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;
   }
@@ -232,8 +257,8 @@ export function tryWithinRoot(candidate: unknown, root: unknown, opts: { allowAb
  * 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, opts: { allowAbsolute?: boolean } = {}): ContainedPath {
-  return assertWithinRoot(filePath, baseDir, label, opts);
+export function requireSafePath(filePath: unknown, baseDir: unknown, label: string | null | undefined, policy: PathAcceptancePolicy = PathAcceptance.RelativeOnly): ContainedPath {
+  return assertWithinRoot(filePath, baseDir, label, policy);
 }
 
 // ─── Prompt Injection Detection ────────────────────────────────────────────────────
diff --git a/src/state.cts b/src/state.cts
index 9bad609d5..6ccb5a6dd 100644
--- a/src/state.cts
+++ b/src/state.cts
@@ -620,8 +620,8 @@ 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 { assertWithinRoot } = require('./security.cjs') as { assertWithinRoot(filePath: unknown, baseDir: unknown, label?: string | null, opts?: { allowAbsolute?: boolean }): string };
-  const contained = assertWithinRoot(filePath, cwd, `${label} path`, { allowAbsolute: true });
+  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(contained, 'utf-8').trimEnd();
diff --git a/src/uat.cts b/src/uat.cts
index 117baf281..8a77f53e4 100644
--- 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}`);
   }
diff --git a/tests/agent-skills.test.cjs b/tests/agent-skills.test.cjs
index f1f935c02..dfe718cb6 100644
--- a/tests/agent-skills.test.cjs
+++ b/tests/agent-skills.test.cjs
@@ -35,7 +35,7 @@ function runGsdToolsWithStderr(args, cwd, env) {
   };
 }
 
-const { loadTrustedGlobalRoots, tryWithinRoot } = require('../gsd-core/bin/lib/security.cjs');
+const { loadTrustedGlobalRoots, tryWithinRoot, PathAcceptance } = require('../gsd-core/bin/lib/security.cjs');
 
 // ─── helpers ──────────────────────────────────────────────────────────────────
 
@@ -915,21 +915,21 @@ describe('trusted_global_roots guard logic', () => {
 
   test('tryWithinRoot rejects skill outside globalSkillsBase (baseline — no trusted roots)', () => {
     const skillMd = path.join(externalDir, 'SKILL.md');
-    const result = tryWithinRoot(skillMd, tmpDir, { allowAbsolute: true });
+    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 — tryWithinRoot accepts', () => {
     // Simulate the trusted-root fallback: skill is outside base but inside trusted root
     const skillMd = path.join(externalDir, 'SKILL.md');
-    const baseCheck = tryWithinRoot(skillMd, tmpDir, { allowAbsolute: true });
+    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 = tryWithinRoot(skillMd, root, { allowAbsolute: true });
+      const rootCheck = tryWithinRoot(skillMd, root, PathAcceptance.AbsoluteInsideRoot);
       return rootCheck !== null;
     });
     assert.ok(acceptedViaTrustedRoot, 'skill must be accepted when within a trusted root');
@@ -942,7 +942,7 @@ 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 = tryWithinRoot(skillMd, root, { allowAbsolute: true });
+        const rootCheck = tryWithinRoot(skillMd, root, PathAcceptance.AbsoluteInsideRoot);
         return rootCheck !== null;
       });
       assert.ok(!acceptedViaTrustedRoot, 'skill must still be rejected when trusted root is unrelated');
@@ -957,7 +957,7 @@ 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 = tryWithinRoot(skillMd, root, { allowAbsolute: true });
+      const rootCheck = tryWithinRoot(skillMd, root, PathAcceptance.AbsoluteInsideRoot);
       return rootCheck !== null;
     });
     assert.ok(!acceptedViaTrustedRoot, 'skill must be rejected when trusted roots is empty');
diff --git a/tests/security.test.cjs b/tests/security.test.cjs
index be8cfa297..5c65a84db 100644
--- a/tests/security.test.cjs
+++ b/tests/security.test.cjs
@@ -25,6 +25,7 @@ const {
   validatePromptStructure,
   assertWithinRoot,
   tryWithinRoot,
+  PathAcceptance,
 } = require('../gsd-core/bin/lib/security.cjs');
 
 // ─── Path Traversal Prevention ──────────────────────────────────────────────
@@ -58,12 +59,12 @@ describe('assertWithinRoot / tryWithinRoot — engine invariance', () => {
   });
 
   test('allows absolute paths within base when opted in', () => {
-    const r = tryWithinRoot(path.join(base, 'src/file.js'), base, { allowAbsolute: true });
+    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', () => {
-    assert.equal(tryWithinRoot('/etc/passwd', base, { allowAbsolute: true }), null);
+    assert.equal(tryWithinRoot('/etc/passwd', base, PathAcceptance.AbsoluteInsideRoot), null);
   });
 
   test('rejects null bytes', () => {
@@ -1367,7 +1368,7 @@ describe('assertWithinRoot / tryWithinRoot — narrowed export (#4653)', () => {
     });
 
     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, { allowAbsolute: true });
+      const resolved = assertWithinRoot(path.join(base, 'src/file.js'), base, null, PathAcceptance.AbsoluteInsideRoot);
       assert.equal(resolved, path.resolve(base, 'src/file.js'));
     });
 
@@ -1376,7 +1377,7 @@ describe('assertWithinRoot / tryWithinRoot — narrowed export (#4653)', () => {
     });
 
     test('throws on an absolute path outside the root even with {allowAbsolute:true}', () => {
-      assert.throws(() => assertWithinRoot('/etc/passwd', base, null, { allowAbsolute: true }));
+      assert.throws(() => assertWithinRoot('/etc/passwd', base, null, PathAcceptance.AbsoluteInsideRoot));
     });
 
     test('throws on a null byte', () => {
@@ -1406,7 +1407,7 @@ describe('assertWithinRoot / tryWithinRoot — narrowed export (#4653)', () => {
     });
 
     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, { allowAbsolute: true });
+      const resolved = tryWithinRoot(path.join(base, 'src/file.js'), base, PathAcceptance.AbsoluteInsideRoot);
       assert.equal(resolved, path.resolve(base, 'src/file.js'));
     });
 
@@ -1416,7 +1417,7 @@ describe('assertWithinRoot / tryWithinRoot — narrowed export (#4653)', () => {
     });
 
     test('returns exactly null for an absolute path outside the root even with {allowAbsolute:true}', () => {
-      const result = tryWithinRoot('/etc/passwd', base, { allowAbsolute: true });
+      const result = tryWithinRoot('/etc/passwd', base, PathAcceptance.AbsoluteInsideRoot);
       assert.strictEqual(result, null);
     });
 

From cd58aaabf4683bee87d77bd0b22f4e1f97d79dba Mon Sep 17 00:00:00 2001
From: sim <sim@local>
Date: Sat, 12 Sep 2026 13:29:54 -0400
Subject: [PATCH 05/11] refactor(#4653): drain the containment duplicates and
 record the two rulings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Phase 3 of epic #4636, stage 3c. ADR-4650 decision 6: a wrapper may decide HOW
to degrade, never WHETHER a path is contained. Four implementations are drained
on that rule; two are retained, with the reasons recorded rather than assumed.

DRAINED — the containment decision now comes from the canonical predicate:

  scripts/check-glossary-refs.cjs   local isWithinRoot deleted outright.
  src/installer-migrations.cts      ensureInsideConfig keeps its throw and its
                                    lexical fullPath; only the decision moves.
  src/planning-inspect.cts          isPathContained keeps must-exist as its own
                                    condition; only the decision moves.

Two of those are wrappers rather than deletions, and each is a wrapper for a
reason that would have been a silent behavior change if collapsed naively:

- `isPathContained` returns FALSE for a path that does not exist, because
  fs.realpathSync throws ENOENT and its catch swallows it. The canonical
  predicate does the opposite: for a missing target it walks up to the nearest
  existing ancestor and ACCEPTS a not-yet-created path under the root. Its
  callers at planning-inspect.cts:747 and :839 guard a phaseDir immediately
  before readdirSync, so under a naive swap a missing phaseDir would stop
  reporting scope UNREADABLE and start throwing ENOENT out of readdirSync.
  Existence is therefore kept as an explicit local requirement.

- `ensureInsideConfig` returns a LEXICAL fullPath that both callers consume for
  existsSync and for journal entries. The canonical predicate realpath-resolves,
  so if configDir is itself a symlink the two differ. The decision is canonical;
  the returned value stays lexical. Its message is likewise preserved verbatim,
  which is why this uses tryWithinRoot plus an explicit throw rather than
  assertWithinRoot.

`isWithinRoot` in planning-inspect is left in place and documented: it is a pure
comparison over paths the CALLER has already resolved, which readDocument does
inline specifically to keep a third degradation shape (exists-but-unreadable vs
absent) that neither isPathContained nor the canonical predicate expresses. It
is the comparison step of one implementation, not a second implementation.

RETAINED, DELIBERATELY — gsd-core/bin/gsd-tools.cjs. My own design document said
"collapse" and that was wrong. The file carries an explicit comment forbidding
it, and the comment is correct: its three checks reject symlinks OUTRIGHT, which
is strictly stricter than the canonical predicate, not a reimplementation of it.
The canonical predicate accepts a link whose target lands inside the root — for
a restore that is still wrong, because writing through the link overwrites
whatever it points at instead of materializing a regular file. Collapsing would
have reintroduced that hole. The comment is updated to name the current exported
predicate, to record that this was reviewed under this phase and deliberately
not collapsed, and to note that isInsideDir treats target === root as NOT
contained — the one implementation in the repo that does.

THE configHome RULING — retained lexical, and a false safety claim corrected.
isPathConfined stays lexical because two of its callers must validate a
destSubpath BEFORE the mkdirSync that creates it (install-engine.cts:1608,
install-profiles.cts:880), where realpath cannot resolve and a realpath-based
predicate would reject every legitimate install.

Its docstring's justification, however, did not survive being checked. It cited
capability-source.cts:491,577,675 as the upstream symlink rejection that made
the lexical form safe. Read directly: :491 is a blank line before assertSafeId's
JSDoc and :577 is an entry-count budget check. Neither is a symlink check. The
real guards are :585-586 and :671-674. Worse than stale line numbers, the claim
that this "keeps every caller of this function's callers symlink-safe" is false:
that rejection lives in capability-source's staging path and covers only the
capability-loader route to assertDescriptorConfined. Three other callers do not
reach it, and only retired-artifact-cleanup.cts:69 carries its own defense
(its lstatSync check at :77). The docstring now states what is actually true and
cites the lines that actually exist.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 gsd-core/bin/gsd-tools.cjs        | 20 +++++++++-----
 scripts/check-glossary-refs.cjs   | 18 +++----------
 src/external-descriptor-trust.cts | 16 +++++++----
 src/installer-migrations.cts      | 12 +++++++--
 src/planning-inspect.cts          | 44 +++++++++++++++++++++----------
 5 files changed, 67 insertions(+), 43 deletions(-)

diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs
index d575db4e3..7378564f0 100755
--- a/gsd-core/bin/gsd-tools.cjs
+++ b/gsd-core/bin/gsd-tools.cjs
@@ -3351,13 +3351,19 @@ 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 and deliberately NOT
+  // collapsed. Note also: isInsideDir below treats target === root as NOT
+  // contained (it requires a non-empty relative path), unlike every other
+  // containment implementation in this repo, which treats target === root as
+  // contained.
 
   /** True when `target` resolves strictly inside `root`. */
   function isInsideDir(root, target) {
diff --git a/scripts/check-glossary-refs.cjs b/scripts/check-glossary-refs.cjs
index d0bcd4d95..4a87a671b 100644
--- 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.
@@ -177,7 +164,8 @@ function extractTrackedRefs(text) {
     if (!/[A-Za-z0-9_]$/.test(token)) return;
     if (token.includes('NNNN')) return;
     if (!isTracked(token)) return;
-    if (!isWithinRoot(token)) return;
+    // Containment decision is the canonical predicate's, per ADR-4650.
+    if (tryWithinRoot(token, ROOT, PathAcceptance.AbsoluteInsideRoot) === null) return;
     tokens.add(token);
   };
   const subTokenRe = /[\w.-]+(?:\/[\w.-]+)*/g;
diff --git a/src/external-descriptor-trust.cts b/src/external-descriptor-trust.cts
index 4bd346361..ce3ee854f 100644
--- a/src/external-descriptor-trust.cts
+++ b/src/external-descriptor-trust.cts
@@ -32,11 +32,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
diff --git a/src/installer-migrations.cts b/src/installer-migrations.cts
index 4a2eb0a9e..775877acd 100644
--- 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 { tryWithinRoot, PathAcceptance } 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
@@ -666,9 +667,16 @@ interface EnsureInsideConfigResult {
 
 function ensureInsideConfig(configDir: string, relPath: string): EnsureInsideConfigResult {
   const normalized = normalizeRelPath(relPath);
+  // fullPath stays the LEXICAL path.resolve result (not the canonical
+  // predicate's realpath-resolved value): both callers (readJson's
+  // ensureInsideConfig call and the migration-apply loop) use fullPath for
+  // fs.existsSync checks and journal entries, and those must not shift if
+  // configDir happens to be a symlink. Per ADR-4650 decision 6, the
+  // containment DECISION (whether fullPath is inside configDir) is owned by
+  // the canonical predicate — this wrapper only decides how to degrade
+  // (throw with this file's existing message), never whether contained.
   const fullPath = path.resolve(configDir, normalized);
-  const root = path.resolve(configDir);
-  if (fullPath !== root && !fullPath.startsWith(root + path.sep)) {
+  if (tryWithinRoot(fullPath, configDir, PathAcceptance.AbsoluteInsideRoot) === null) {
     throw new Error(`migration path escapes configDir: ${relPath}`);
   }
   return { normalized, fullPath };
diff --git a/src/planning-inspect.cts b/src/planning-inspect.cts
index fe60b61e8..a6d175e50 100644
--- 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 } {

From 7086dcbbc989168858ad5fdd22aab867615c9651 Mon Sep 17 00:00:00 2001
From: sim <sim@local>
Date: Sat, 12 Sep 2026 13:30:16 -0400
Subject: [PATCH 06/11] test(#4636): failing-first coverage for symlink escape
 past lexical confinement
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Tests only; no fix. These MUST fail.

Found while draining the containment duplicates in Phase 3, not by looking for
it: isPathConfined's docstring claims its callers are kept symlink-safe
upstream, and checking that claim showed it does not hold for three of its four
call sites. isPathConfined is lexical by design and structurally cannot see a
symlink, so where the claim fails there is no defense at all.

  install-engine.cts:1608     mkdirSync(recursive) + writeFileSync under dest
  install-profiles.cts:880    the same shape under stageDir
  install-profiles.cts:755    statSync/readFileSync, and statSync FOLLOWS links

SEVERITY, STATED HONESTLY, because the three are not equal and the first
impression was wrong.

  :755 is the real one and needs no race. It reads
  <capDir>/skills/<stem>/SKILL.md. Nothing prunes that path and nothing
  randomizes it, so a symlink planted there is followed and its content is
  returned — and then written into the install tree as a skill body.

  :1608 and :880 are defense-in-depth against a local race, not plain
  write-through. _removeGsdEntries (install-engine.cts:851-854) rmSyncs every
  entry whose name starts with kind.prefix, and the skill names ARE
  prefix+stem — so a symlink planted before the call is unlinked before the
  write loop reaches it. :880's stageDir is a fresh mkdtempSync path, so its
  name cannot be guessed in advance either. Both require winning a window.

That is also why the first two tests do not simply pre-plant a link and call
the function: written that way they PASS today, against vulnerable code, for
the wrong reason. They instead open the window deterministically by hooking a
call the function is guaranteed to make — the technique CLAUDE.md section 4
prescribes for injecting filesystem conditions, and the one already used in
tests/install-runtime-artifacts.test.cjs. No sleeps, no concurrency, no
flakiness: the window is opened by a synchronous side effect, not raced for.

Each test asserts the OUTCOME rather than the mechanism — the canary file
outside the root is byte-identical afterwards, or the installed content does
not contain it — so a fix is free to close the hole any way it likes. Symlink
creation is guarded so the Windows lanes skip rather than fail, and no test
uses chmod 0o000, which is vacuous on a bench running as root.

Folded into tests/install-write-confinement.test.cjs, the owning suite, whose
existing F2/M1 coverage coincidentally proves the gap: it tests the case where
the destination DIRECTORY is a symlink, which is already defended, and never
the per-entry case.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 tests/install-write-confinement.test.cjs | 208 +++++++++++++++++++++++
 1 file changed, 208 insertions(+)

diff --git a/tests/install-write-confinement.test.cjs b/tests/install-write-confinement.test.cjs
index 663c7236c..5bb453585 100644
--- a/tests/install-write-confinement.test.cjs
+++ b/tests/install-write-confinement.test.cjs
@@ -4380,3 +4380,211 @@ 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.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);
+    }
+  });
+});

From 5967f1939a8d51b2e7cbe028edc198ae3c95f336 Mon Sep 17 00:00:00 2001
From: sim <sim@local>
Date: Sat, 12 Sep 2026 13:34:11 -0400
Subject: [PATCH 07/11] docs(#4653): record the path-containment seam in
 CONTEXT.md and add the changeset
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The glossary had no entry for the containment predicate at all, which is the
epic's actual deliverable. The new entry states the engine/export split, the
branded type, the named acceptance policy, the preserved message contract, and
— the part most likely to be undone by a later cleanup — the two
implementations deliberately NOT collapsed and why each is stricter or
narrower rather than duplicative.

Glossary gate re-run: 269 refs, exit 0. Install-tree goldens regenerated and
confirmed byte-identical rather than assumed unchanged; lint:ci exits 0, so no
conformance-tier drift either.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .changeset/eager-badgers-bark.md | 5 +++++
 CONTEXT.md                       | 3 +++
 2 files changed, 8 insertions(+)
 create mode 100644 .changeset/eager-badgers-bark.md

diff --git a/.changeset/eager-badgers-bark.md b/.changeset/eager-badgers-bark.md
new file mode 100644
index 000000000..dd85c7e85
--- /dev/null
+++ b/.changeset/eager-badgers-bark.md
@@ -0,0 +1,5 @@
+---
+type: Changed
+pr: 0
+---
+**The path-containment predicate is now a single exported seam** — `security.cjs` no longer exports `validatePath`; `assertWithinRoot` (throws), `tryWithinRoot` (returns null) and `requireSafePath` are the only containment exports, and all three 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. Rejection message text and every command's observable behavior are unchanged. (#4653)
diff --git a/CONTEXT.md b/CONTEXT.md
index 00e53adab..01e25fac1 100644
--- a/CONTEXT.md
+++ b/CONTEXT.md
@@ -329,6 +329,9 @@ Module owning install-time staging and content-rewrite selection for a pre-resol
 ### Install Fs Adapter Module
 Narrow, enumerated fs seam for the `installRuntimeArtifacts` call tree (`src/install-engine.cts`) — lands ADR-58's never-shipped `cleanup` rollout step (`registry → adapter → helpers → cleanup`, #2874, epic #2866 Phase 5). `installRuntimeArtifacts` now returns the executed plan it ran (`{ runtime, scope, kinds: [{kind, sourceDir, destDir, preserved}], cleanup: [{dir, ok}], postSteps }`) instead of `void`, including on the `combinedFamilyInstall` (OpenCode/Kilo) early-return path — no path may return `undefined` after this phase. Failure is unchanged: stage/rewrite errors still throw rather than becoming an `ok:false` value, so a caller cannot read success-shaped data off a failure path. Delivery is an ambient single mutable adapter (`current`), swapped for the duration of one synchronous install via `withInstallFs(deps.fs, fn)` and always restored in a `finally` — a `deps` parameter threaded through every function on the call tree (`install-profiles.cts`, `runtime-artifact-conversion.cts`, `commonjs-marker.cts`, `installer-migrations.cts`'s two reachable entry points) was rejected as a dozen+-site touch for no behavioral gain over the ambient swap, extending rather than replacing `createRuntimeArtifactInstallPlan`'s existing `deps` bag precedent (Runtime Artifact Install Plan Module). An injected `deps.fs` is a PARTIAL adapter merged over real `node:fs`; any method it omits — except `realpathSync`, the one method documented to degrade gracefully — now THROWS immediately if actually called, naming the missing method, rather than silently resolving to the real filesystem (#2875 defect fix: the prior silent fall-through let a fake adapter missing e.g. `rmSync` perform real destructive IO unnoticed). **Routes destination IO only, by design**: every write/probe against the install destination (copies, removals, snapshot/restore of preserved skill dirs, the manifest read) is fake-able; locating this package's own source tree (`findInstallSourceRoot`/`findAgentsSourceRoot`'s walk-up-from-`__dirname`, `readGsdCommandNames`) stays real and unrouted — a destination-fake's store starts empty and was never seeded with the repo's own paths, so routing that lookup would make every fake-adapter install throw instead of staging. The symlink-escape guard (`hasExistingSymlinkBetween`) and `assertDestWithinConfigHome` keep their REFUSAL DECISIONS outside this seam — only their `existsSync`/`lstatSync`/`realpathSync` probes route through it, so an injected fake can change what a probe observes but never flip the security decision itself. Writes remain byte-identical to pre-#2874 (AC4/AC5); existing `void`-ignoring callers (`bin/install.js`) are unaffected. Source: `gsd-core/bin/lib/install-fs-adapter.cjs` (generated from `src/install-fs-adapter.cts`). See Runtime Artifact Install Plan Module, ADR-58.
 
+### Path Containment Seam
+One predicate answers "does this path resolve inside this root?" for the whole tree (epic #4636, ADR-4650). `src/security.cts` owns the resolution engine — realpath-based, with a closed dangling-symlink existence oracle and ancestor canonicalization so a not-yet-created path under a non-canonical base (macOS `/var` vs `/private/var`) still resolves — and that engine is NOT exported: the module-internal `validatePath` returns `{ safe, resolved, error }` and populates `resolved` with the ESCAPING path on the traversal branch, so a caller who skipped the boolean would get an attacker-controlled value precisely in the dangerous case. The exported surface is three shapes that cannot do that: `assertWithinRoot(candidate, root, label?, policy?)` throws, `tryWithinRoot(candidate, root, policy?)` returns exactly `null`, and `requireSafePath` is a preserved alias of the throwing form. All three return the 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. `policy` is the named `PathAcceptance` acceptance policy (`RelativeOnly` | `AbsoluteInsideRoot`) that replaced a per-call-site `{ allowAbsolute: true }` boolean, which read as "containment is relaxed here" when in fact an absolute path outside the root was always rejected. The rejection text `Path escapes allowed directory: <resolved> is outside <base>` is an observable CLI contract, surfaced through command `reason` fields and asserted by `tests/quick-batch.test.cjs`. **A wrapper may decide HOW to degrade, never WHETHER a path is contained** (ADR-4650 decision 6): `planning-inspect.cts`'s `isPathContained` takes its decision from the predicate but keeps must-exist as its own separate condition, because the canonical engine deliberately ACCEPTS a not-yet-created path under the root while its callers guard an `fs` read that is about to happen; `installer-migrations.cts`'s `ensureInsideConfig` likewise takes the decision but preserves its own message and its LEXICAL `fullPath`, which its callers consume for `existsSync` and journal entries. Two implementations are deliberately NOT collapsed, and both are stricter or narrower rather than duplicative: `gsd-core/bin/gsd-tools.cjs`'s restore gate rejects symlinks OUTRIGHT (the canonical predicate accepts a link resolving inside the root, which for a restore still writes through the link) and treats `target === root` as NOT contained; and `external-descriptor-trust.cts`'s `isPathConfined` is lexical by design because two install callers validate a `destSubpath` BEFORE the `mkdirSync` that creates it, where realpath cannot resolve. Source of truth: `gsd-core/bin/lib/security.cjs` (generated from `src/security.cts`). See Gate Predicate Evaluator Module, Runtime Artifact Install Plan Module, Real Home Guard Module.
+
 ### Real Home Guard Module
 Refuses an in-process install whose destination would land in the developer's REAL home while a Node test runner is in scope (#3712, ADR-1239/#2088 territory). Interface: `assertTestHomeSandboxed(operation, runtime, kinds, deps?)` (throws), `isTestHomeGuardRefusal(err)`, `SANDBOX_MARKER`. Exists because a runtime kind may declare a global `home` override resolved from `os.homedir()` rather than from the caller's `configDir` — codex's skills kind (`home: ".agents"`) and, since #3738, antigravity's global skills AND agents kinds (`home: ".gemini/config"`, the dir AGY scans for global discovery; configHome stays `~/.gemini/antigravity` for settings/runtime files) are the live cases — so sandboxing `configDir`/`targetDir` does NOT contain it, and `assertDestWithinConfigHome` (Runtime Artifact Install Plan Module) structurally cannot see the class: that gate confines a `destSubpath` to whatever root it is handed, and here the root IS the escaped home. The observed failure was silent destruction — a test file that sandboxed only `targetDir` pruned all 71 `gsd-*` skills from a real `~/.agents/skills` via `_removeGsdEntries` while the suite exited 0 and the manifest still reported a healthy install. **Six writers** resolve a kind `home` and then write or destroy under it and therefore carry a call: `installRuntimeArtifacts`, `uninstallRuntimeArtifacts` (Install Engine Module), `applySurface` (Surface Module), and `migrateLegacyDevPreferencesToSkill` — which CREATES rather than prunes, which is why it was missed on the first pass, and which runs from `_runLegacyInstallMigrations` BEFORE `installRuntimeArtifacts`' own assertion, so it guards the destination it already resolved rather than resolving a second time (generative-fix divergence) — plus `installOpencodeFamilySkills` and `installAgentsKindStandalone`, guarded against a future descriptor change rather than a present escape since no combined-family or agents kind declares a `home` today. Each of the four reachable writers carries an optional `deps: { os?, env? }` tail parameter — the guard's trigger condition is "HOME equals the passwd home", which cannot be simulated without pointing at the developer's real home — so `tests/install-write-confinement.test.cjs` drives the REAL entrypoint for each one and deleting any guard call site turns a row red. `migrateLegacyDevPreferencesToSkill` guards only when `target.hasHomeOverride` — that same resolution's own answer to "did the skills kind declare a `home`?", never inferred from `installRoot !== targetDir`, which is FALSE whenever the override resolves onto `targetDir` itself (a configDir of `$HOME/.agents`, exactly where codex's override points); both directions of that condition are pinned, so widening it to refuse ordinary confined migrations fails a row too. **Gated on `NODE_TEST_CONTEXT`** (set by `node --test`), never on `GSD_TEST_MODE` — several in-process test files, including the one that caused #3712, never set the latter — so ordinary installs outside a Node test context are untouched and codex still installs to `$HOME/.agents` normally. **The predicate asks about the DESTINATION, not about HOME state**, and about the path the write RESOLVES to rather than how it is spelled: a stale layout captured before `sandboxHome()` still names the real `~/.agents` while HOME is sandboxed, so a HOME-state check would wave it through, and an aliased `<sandbox>/.agents` symlink or junction would otherwise redirect an allowed path into the real home. Canonicalization itself fails CLOSED: a component that cannot be resolved (`EACCES`/`EPERM` on a directory whose mode changed, `ELOOP` on a symlink cycle, `EIO` on a failing mount) is REFUSED rather than falling back to the lexical spelling — that fallback was a second, unnamed fail-open, and precisely the ALLOW an unresolvable alias needs, since the sandbox spelling then satisfies the nested-sandbox exemption (Codex review of #3725). Only `ENOENT`/`ENOTDIR` walk up, matching `identify`'s own errno split rather than inventing a second one: both mean "no such path as named", which is the ordinary shape of a destination no install has created yet. A destination inside the real home is exempted only when all three hold — HOME differs from the passwd home by filesystem identity, the passwd home is not itself beneath that HOME (`/Users`, `C:\Users`), and the destination resolves beneath it. That exemption is not a softening: on Windows `os.tmpdir()` is under `%USERPROFILE%`, so EVERY test sandbox is a descendant of the real home and containment alone cannot separate the safe case from the dangerous one — without it the whole Windows matrix refuses. Every containment decision compares by filesystem identity (`st_dev`+`st_ino`), never pathname (the one pathname comparison in the module is `sameDirectory`'s equality shortcut on the marker branch, described below): `path.resolve` resolves neither symlinks nor case and `realpath` returns a canonical pathname two routes to one directory can disagree on. **Fails CLOSED**, with one named exception: where no passwd entry is readable (some CI images) it falls back to a path-valued marker set by `sandboxHome()`/`installSpawnEnv()` in `tests/helpers.cjs`, which must identify, must equal the home in effect, AND must contain every resolved destination. All three are load-bearing: matching HOME attests that a caller sandboxed HOME and says nothing about where these destinations resolve, so on its own the marker waved through the very stale-layout shape the primary branch refuses; and `sameDirectory`'s pathname-equality shortcut can answer yes for two identical UNidentifiable paths, which is not enough to place a destination against. It remains a deliberate weakening, since nothing on such a host can contradict a marker naming the real home. `installSpawnEnv` re-points the marker at an explicitly overridden HOME, so a spawn that supplies its own sandbox is not refused by a marker still naming the helper's default one. Refusals are STAMPED so `bin/install.js` can rethrow them without running `_codexPreConfigRollback()`, which deletes and recreates every snapshotted `gsd-*` directory in the resolved skills root — otherwise the guard's own refusal would provoke the mutation it exists to prevent. **Known limits, stated rather than implied**: a subordinate bind mount of the real `~/.agents` into the sandbox is not closed (a bind mount is not a link, so realpath keeps the mount-point spelling; closing it needs non-portable mount-table introspection), and a cross-process TOCTOU swap between check and write is out of reach. Source: `gsd-core/bin/lib/real-home-guard.cjs` (generated from `src/real-home-guard.cts`). See Install Engine Module, Surface Module, Runtime Artifact Install Plan Module, Runtime Artifact Layout Module.
 

From 9953d02184ef65170a6477e59412a34c3b2fa5ff Mon Sep 17 00:00:00 2001
From: sim <sim@local>
Date: Sat, 12 Sep 2026 13:41:40 -0400
Subject: [PATCH 08/11] docs(#4653): describe the consolidated path-containment
 seam in the security model
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The security model's input-validation section still described path traversal as
a per-call check with a macOS symlink footnote. It now describes what actually
exists: one predicate, a module-internal engine, three exported shapes none of
which can hand back a usable path when the answer is unsafe, the branded return
type, and the named acceptance policy — including the point the old wording
invited a reader to get wrong, that allowing an absolute candidate does not
relax containment.

Also records the two checks deliberately NOT routed through the predicate and
why each is narrower or stricter rather than a second opinion, so a later
cleanup pass does not read them as stragglers.

Required by the Changed changeset: scripts/lint-docs-required.cjs makes
Added/Changed/Deprecated/Removed fragments demand a file under docs/, and
CONTEXT.md is at the repo root, so the glossary entry alone would not have
satisfied it. The lint currently reports invalid_pr against the mandated pr:0
placeholder and becomes meaningful after backfill.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 docs/explanation/security-model.md | 31 +++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/docs/explanation/security-model.md b/docs/explanation/security-model.md
index e4073a874..8117efa1a 100644
--- 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,18 @@ module is the central security utility. It provides:
 - Shell argument validation: arguments passed to subshell commands are
   validated before use
 
+Two containment checks elsewhere in the tree are deliberately NOT routed through
+this predicate, because each is narrower or stricter rather than a second opinion.
+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. And `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. A lexical check
+cannot see a symlink, so callers that rely on it for a write-confinement
+guarantee must pair it with their own symlink refusal.
+
 **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

From 6f0e5ccf85d538798ddca0a3020deee3ad10e347 Mon Sep 17 00:00:00 2001
From: sim <sim@local>
Date: Sat, 12 Sep 2026 13:59:52 -0400
Subject: [PATCH 09/11] fix(#4636,#4653): close the symlink hole, revert a
 wrong collapse, fix six review findings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The RED checkpoint and two orthogonal reviews found eight defects. All fixed here.

THE COLLAPSE THAT WAS WRONG — installer-migrations. Routing ensureInsideConfig's
containment decision through the realpath-based canonical predicate broke four
tests, and the failure message says it plainly: "migration path escapes
configDir: extensions/gsd.cjs". That module's entire contract is that a
symlinked managed path is snapshotted, restored and backed up AS A LINK and
never dereferenced. The canonical predicate dereferences, then rejects the
result for escaping configDir — so it destroys exactly the thing the module
exists to preserve. Reverted to lexical, with the ruling recorded above the
function so it is not collapsed a third time. normalizeRelPath is the real
pre-gate there; it throws on absolute paths and '..' before this check runs.

That makes THREE deliberately-retained implementations, not two, and they share
one shape worth naming: a realpath-based predicate is the wrong tool wherever a
symlink must be PRESERVED rather than resolved. CONTEXT.md and
docs/explanation/security-model.md are corrected — both previously described
ensureInsideConfig as collapsed.

THE MISSED CONSUMER. tests/security-prompt-injection.security.test.cjs
destructures validatePath from the compiled lib; un-exporting it turned five
tests into TypeError. It appeared in my own earlier search output and I did not
follow it up. Translated under the same rule as the rest: assertions on the
rejection REASON go through assertWithinRoot, boolean-only through
tryWithinRoot.

VALIDATE-ONE-PATH-USE-ANOTHER, FOUND TWICE MORE. This is the fourth and fifth
occurrence in this epic of the exact defect it exists to prevent.
  - scripts/check-glossary-refs.cjs decided containment on `token` and then
    stat'd a separately re-joined path.join(ROOT, token). The ContainedPath is
    now carried through to the probe, so the validated value is the probed one.
  - src/init.cts computed skillPathContained and DISCARDED it, re-joining from
    the raw input for the existsSync and read. The branded type exists to make
    that a type error and here it was inert.

AND THE OVER-CORRECTION OF THAT FIX, caught before it shipped. The first attempt
also substituted the validated value into the EMITTED `ref` for a global skill.
That value is a display token, not a path anything reads through — the only fs
access in that branch runs on the lexical path beforehand — so substituting it
changed emitted output two ways: it is realpath-resolved, so a symlinked global
skills directory would have emitted its resolved target instead of the user's
own path, and it came from path.join, so Windows would have emitted a backslash
where the template has a literal '/'. Restored, with the distinction recorded:
the containment check there is a GATE, not a path producer.

A TEST THAT COULD NOT FAIL. The first symlink regression planted its symlink
from inside a hooked fs.readdirSync and never asserted the planting happened —
if the hook did not fire, the "nothing was written outside" assertion passed
trivially, green against vulnerable code. It now asserts the plant, matching its
sibling. The other two were re-checked: one already asserted its equivalent, the
other plants synchronously and cannot silently no-op.

THE SYMLINK FIX ITSELF, now that the tests are proven red on the matrix.
isPathConfined is lexical by design and structurally cannot see a symlink; three
callers relied on it with no defense of their own. install-engine.cts:1608 and
install-profiles.cts:880 refuse to mkdir/write through a link — mkdirSync with
recursive:true does NOT throw on an existing symlink-to-directory, so a planted
link redirected the SKILL.md write outside the install root.
install-profiles.cts:755 refuses to read through one — statSync FOLLOWS links,
so an outside file's contents were returned and installed as a skill body. Each
mirrors the guard retired-artifact-cleanup.cts:77 already uses.

Severity stated accurately rather than dramatically: only the read at :755 needs
no race. _removeGsdEntries sweeps a pre-planted link at :1608 before the write
loop, and :880's stageDir is a fresh mkdtemp, so both of those require winning a
window. They are fixed as defense-in-depth, not as live exploits.

ALSO: the Changed changeset claimed "every command's observable behavior [is]
unchanged". Three rejection messages are reworded. It now says so, and says that
none of them reveals a host path it previously hid. A stale comment in
verify.cts still named validatePath; an init.cts warning hardcoded "resolves
outside the project directory" for a check that also rejects absolute paths, NUL
bytes and empty strings; and the rationale deleted with check-glossary-refs'
retired helper is restored, noting honestly that a rejected token is now
realpath-resolved before rejection rather than rejected by string comparison.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .changeset/eager-badgers-bark.md              |  2 +-
 .changeset/gallant-hawks-tumble.md            |  5 +++
 CONTEXT.md                                    |  2 +-
 docs/explanation/security-model.md            | 24 +++++++++++---
 scripts/check-glossary-refs.cjs               | 32 ++++++++++++++-----
 src/init.cts                                  | 18 ++++++++---
 src/install-engine.cts                        |  7 ++++
 src/install-profiles.cts                      | 11 +++++++
 src/installer-migrations.cts                  | 27 ++++++++++------
 src/verify.cts                                |  2 +-
 tests/install-write-confinement.test.cjs      |  1 +
 ...ecurity-prompt-injection.security.test.cjs | 27 +++++++++-------
 12 files changed, 116 insertions(+), 42 deletions(-)
 create mode 100644 .changeset/gallant-hawks-tumble.md

diff --git a/.changeset/eager-badgers-bark.md b/.changeset/eager-badgers-bark.md
index dd85c7e85..7882a6145 100644
--- a/.changeset/eager-badgers-bark.md
+++ b/.changeset/eager-badgers-bark.md
@@ -2,4 +2,4 @@
 type: Changed
 pr: 0
 ---
-**The path-containment predicate is now a single exported seam** — `security.cjs` no longer exports `validatePath`; `assertWithinRoot` (throws), `tryWithinRoot` (returns null) and `requireSafePath` are the only containment exports, and all three 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. Rejection message text and every command's observable behavior are unchanged. (#4653)
+**The path-containment predicate is now a single exported seam** — `security.cjs` no longer exports `validatePath`; `assertWithinRoot` (throws), `tryWithinRoot` (returns null) and `requireSafePath` are the only containment exports, and all three 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)
diff --git a/.changeset/gallant-hawks-tumble.md b/.changeset/gallant-hawks-tumble.md
new file mode 100644
index 000000000..124cd9a50
--- /dev/null
+++ b/.changeset/gallant-hawks-tumble.md
@@ -0,0 +1,5 @@
+---
+type: Security
+pr: 0
+---
+**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)
diff --git a/CONTEXT.md b/CONTEXT.md
index 01e25fac1..66052e7d9 100644
--- a/CONTEXT.md
+++ b/CONTEXT.md
@@ -330,7 +330,7 @@ Module owning install-time staging and content-rewrite selection for a pre-resol
 Narrow, enumerated fs seam for the `installRuntimeArtifacts` call tree (`src/install-engine.cts`) — lands ADR-58's never-shipped `cleanup` rollout step (`registry → adapter → helpers → cleanup`, #2874, epic #2866 Phase 5). `installRuntimeArtifacts` now returns the executed plan it ran (`{ runtime, scope, kinds: [{kind, sourceDir, destDir, preserved}], cleanup: [{dir, ok}], postSteps }`) instead of `void`, including on the `combinedFamilyInstall` (OpenCode/Kilo) early-return path — no path may return `undefined` after this phase. Failure is unchanged: stage/rewrite errors still throw rather than becoming an `ok:false` value, so a caller cannot read success-shaped data off a failure path. Delivery is an ambient single mutable adapter (`current`), swapped for the duration of one synchronous install via `withInstallFs(deps.fs, fn)` and always restored in a `finally` — a `deps` parameter threaded through every function on the call tree (`install-profiles.cts`, `runtime-artifact-conversion.cts`, `commonjs-marker.cts`, `installer-migrations.cts`'s two reachable entry points) was rejected as a dozen+-site touch for no behavioral gain over the ambient swap, extending rather than replacing `createRuntimeArtifactInstallPlan`'s existing `deps` bag precedent (Runtime Artifact Install Plan Module). An injected `deps.fs` is a PARTIAL adapter merged over real `node:fs`; any method it omits — except `realpathSync`, the one method documented to degrade gracefully — now THROWS immediately if actually called, naming the missing method, rather than silently resolving to the real filesystem (#2875 defect fix: the prior silent fall-through let a fake adapter missing e.g. `rmSync` perform real destructive IO unnoticed). **Routes destination IO only, by design**: every write/probe against the install destination (copies, removals, snapshot/restore of preserved skill dirs, the manifest read) is fake-able; locating this package's own source tree (`findInstallSourceRoot`/`findAgentsSourceRoot`'s walk-up-from-`__dirname`, `readGsdCommandNames`) stays real and unrouted — a destination-fake's store starts empty and was never seeded with the repo's own paths, so routing that lookup would make every fake-adapter install throw instead of staging. The symlink-escape guard (`hasExistingSymlinkBetween`) and `assertDestWithinConfigHome` keep their REFUSAL DECISIONS outside this seam — only their `existsSync`/`lstatSync`/`realpathSync` probes route through it, so an injected fake can change what a probe observes but never flip the security decision itself. Writes remain byte-identical to pre-#2874 (AC4/AC5); existing `void`-ignoring callers (`bin/install.js`) are unaffected. Source: `gsd-core/bin/lib/install-fs-adapter.cjs` (generated from `src/install-fs-adapter.cts`). See Runtime Artifact Install Plan Module, ADR-58.
 
 ### Path Containment Seam
-One predicate answers "does this path resolve inside this root?" for the whole tree (epic #4636, ADR-4650). `src/security.cts` owns the resolution engine — realpath-based, with a closed dangling-symlink existence oracle and ancestor canonicalization so a not-yet-created path under a non-canonical base (macOS `/var` vs `/private/var`) still resolves — and that engine is NOT exported: the module-internal `validatePath` returns `{ safe, resolved, error }` and populates `resolved` with the ESCAPING path on the traversal branch, so a caller who skipped the boolean would get an attacker-controlled value precisely in the dangerous case. The exported surface is three shapes that cannot do that: `assertWithinRoot(candidate, root, label?, policy?)` throws, `tryWithinRoot(candidate, root, policy?)` returns exactly `null`, and `requireSafePath` is a preserved alias of the throwing form. All three return the 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. `policy` is the named `PathAcceptance` acceptance policy (`RelativeOnly` | `AbsoluteInsideRoot`) that replaced a per-call-site `{ allowAbsolute: true }` boolean, which read as "containment is relaxed here" when in fact an absolute path outside the root was always rejected. The rejection text `Path escapes allowed directory: <resolved> is outside <base>` is an observable CLI contract, surfaced through command `reason` fields and asserted by `tests/quick-batch.test.cjs`. **A wrapper may decide HOW to degrade, never WHETHER a path is contained** (ADR-4650 decision 6): `planning-inspect.cts`'s `isPathContained` takes its decision from the predicate but keeps must-exist as its own separate condition, because the canonical engine deliberately ACCEPTS a not-yet-created path under the root while its callers guard an `fs` read that is about to happen; `installer-migrations.cts`'s `ensureInsideConfig` likewise takes the decision but preserves its own message and its LEXICAL `fullPath`, which its callers consume for `existsSync` and journal entries. Two implementations are deliberately NOT collapsed, and both are stricter or narrower rather than duplicative: `gsd-core/bin/gsd-tools.cjs`'s restore gate rejects symlinks OUTRIGHT (the canonical predicate accepts a link resolving inside the root, which for a restore still writes through the link) and treats `target === root` as NOT contained; and `external-descriptor-trust.cts`'s `isPathConfined` is lexical by design because two install callers validate a `destSubpath` BEFORE the `mkdirSync` that creates it, where realpath cannot resolve. Source of truth: `gsd-core/bin/lib/security.cjs` (generated from `src/security.cts`). See Gate Predicate Evaluator Module, Runtime Artifact Install Plan Module, Real Home Guard Module.
+One predicate answers "does this path resolve inside this root?" for the whole tree (epic #4636, ADR-4650). `src/security.cts` owns the resolution engine — realpath-based, with a closed dangling-symlink existence oracle and ancestor canonicalization so a not-yet-created path under a non-canonical base (macOS `/var` vs `/private/var`) still resolves — and that engine is NOT exported: the module-internal `validatePath` returns `{ safe, resolved, error }` and populates `resolved` with the ESCAPING path on the traversal branch, so a caller who skipped the boolean would get an attacker-controlled value precisely in the dangerous case. The exported surface is three shapes that cannot do that: `assertWithinRoot(candidate, root, label?, policy?)` throws, `tryWithinRoot(candidate, root, policy?)` returns exactly `null`, and `requireSafePath` is a preserved alias of the throwing form. All three return the 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. `policy` is the named `PathAcceptance` acceptance policy (`RelativeOnly` | `AbsoluteInsideRoot`) that replaced a per-call-site `{ allowAbsolute: true }` boolean, which read as "containment is relaxed here" when in fact an absolute path outside the root was always rejected. The rejection text `Path escapes allowed directory: <resolved> is outside <base>` is an observable CLI contract, surfaced through command `reason` fields and asserted by `tests/quick-batch.test.cjs`. **A wrapper may decide HOW to degrade, never WHETHER a path is contained** (ADR-4650 decision 6): `planning-inspect.cts`'s `isPathContained` takes its decision from the predicate but keeps must-exist as its own separate condition, because the canonical engine deliberately ACCEPTS a not-yet-created path under the root while its callers guard an `fs` read that is about to happen. **Three implementations are deliberately NOT collapsed, and each is stricter or narrower rather than duplicative** — a realpath-based predicate is the wrong tool wherever a symlink must be preserved rather than resolved: `gsd-core/bin/gsd-tools.cjs`'s restore gate rejects symlinks OUTRIGHT (the canonical predicate accepts a link resolving inside the root, which for a restore still writes through the link) and treats `target === root` as NOT contained; `external-descriptor-trust.cts`'s `isPathConfined` is lexical by design because two install callers validate a `destSubpath` BEFORE the `mkdirSync` that creates it, where realpath cannot resolve; and `installer-migrations.cts`'s `ensureInsideConfig` 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 — routing it through the canonical predicate dereferenced exactly those links and then rejected them for escaping `configDir`, which the remote matrix caught as four failing tests. `normalizeRelPath` is its real pre-gate, throwing on absolute paths and `..` segments before the containment check runs. Source of truth: `gsd-core/bin/lib/security.cjs` (generated from `src/security.cts`). See Gate Predicate Evaluator Module, Runtime Artifact Install Plan Module, Real Home Guard Module.
 
 ### Real Home Guard Module
 Refuses an in-process install whose destination would land in the developer's REAL home while a Node test runner is in scope (#3712, ADR-1239/#2088 territory). Interface: `assertTestHomeSandboxed(operation, runtime, kinds, deps?)` (throws), `isTestHomeGuardRefusal(err)`, `SANDBOX_MARKER`. Exists because a runtime kind may declare a global `home` override resolved from `os.homedir()` rather than from the caller's `configDir` — codex's skills kind (`home: ".agents"`) and, since #3738, antigravity's global skills AND agents kinds (`home: ".gemini/config"`, the dir AGY scans for global discovery; configHome stays `~/.gemini/antigravity` for settings/runtime files) are the live cases — so sandboxing `configDir`/`targetDir` does NOT contain it, and `assertDestWithinConfigHome` (Runtime Artifact Install Plan Module) structurally cannot see the class: that gate confines a `destSubpath` to whatever root it is handed, and here the root IS the escaped home. The observed failure was silent destruction — a test file that sandboxed only `targetDir` pruned all 71 `gsd-*` skills from a real `~/.agents/skills` via `_removeGsdEntries` while the suite exited 0 and the manifest still reported a healthy install. **Six writers** resolve a kind `home` and then write or destroy under it and therefore carry a call: `installRuntimeArtifacts`, `uninstallRuntimeArtifacts` (Install Engine Module), `applySurface` (Surface Module), and `migrateLegacyDevPreferencesToSkill` — which CREATES rather than prunes, which is why it was missed on the first pass, and which runs from `_runLegacyInstallMigrations` BEFORE `installRuntimeArtifacts`' own assertion, so it guards the destination it already resolved rather than resolving a second time (generative-fix divergence) — plus `installOpencodeFamilySkills` and `installAgentsKindStandalone`, guarded against a future descriptor change rather than a present escape since no combined-family or agents kind declares a `home` today. Each of the four reachable writers carries an optional `deps: { os?, env? }` tail parameter — the guard's trigger condition is "HOME equals the passwd home", which cannot be simulated without pointing at the developer's real home — so `tests/install-write-confinement.test.cjs` drives the REAL entrypoint for each one and deleting any guard call site turns a row red. `migrateLegacyDevPreferencesToSkill` guards only when `target.hasHomeOverride` — that same resolution's own answer to "did the skills kind declare a `home`?", never inferred from `installRoot !== targetDir`, which is FALSE whenever the override resolves onto `targetDir` itself (a configDir of `$HOME/.agents`, exactly where codex's override points); both directions of that condition are pinned, so widening it to refuse ordinary confined migrations fails a row too. **Gated on `NODE_TEST_CONTEXT`** (set by `node --test`), never on `GSD_TEST_MODE` — several in-process test files, including the one that caused #3712, never set the latter — so ordinary installs outside a Node test context are untouched and codex still installs to `$HOME/.agents` normally. **The predicate asks about the DESTINATION, not about HOME state**, and about the path the write RESOLVES to rather than how it is spelled: a stale layout captured before `sandboxHome()` still names the real `~/.agents` while HOME is sandboxed, so a HOME-state check would wave it through, and an aliased `<sandbox>/.agents` symlink or junction would otherwise redirect an allowed path into the real home. Canonicalization itself fails CLOSED: a component that cannot be resolved (`EACCES`/`EPERM` on a directory whose mode changed, `ELOOP` on a symlink cycle, `EIO` on a failing mount) is REFUSED rather than falling back to the lexical spelling — that fallback was a second, unnamed fail-open, and precisely the ALLOW an unresolvable alias needs, since the sandbox spelling then satisfies the nested-sandbox exemption (Codex review of #3725). Only `ENOENT`/`ENOTDIR` walk up, matching `identify`'s own errno split rather than inventing a second one: both mean "no such path as named", which is the ordinary shape of a destination no install has created yet. A destination inside the real home is exempted only when all three hold — HOME differs from the passwd home by filesystem identity, the passwd home is not itself beneath that HOME (`/Users`, `C:\Users`), and the destination resolves beneath it. That exemption is not a softening: on Windows `os.tmpdir()` is under `%USERPROFILE%`, so EVERY test sandbox is a descendant of the real home and containment alone cannot separate the safe case from the dangerous one — without it the whole Windows matrix refuses. Every containment decision compares by filesystem identity (`st_dev`+`st_ino`), never pathname (the one pathname comparison in the module is `sameDirectory`'s equality shortcut on the marker branch, described below): `path.resolve` resolves neither symlinks nor case and `realpath` returns a canonical pathname two routes to one directory can disagree on. **Fails CLOSED**, with one named exception: where no passwd entry is readable (some CI images) it falls back to a path-valued marker set by `sandboxHome()`/`installSpawnEnv()` in `tests/helpers.cjs`, which must identify, must equal the home in effect, AND must contain every resolved destination. All three are load-bearing: matching HOME attests that a caller sandboxed HOME and says nothing about where these destinations resolve, so on its own the marker waved through the very stale-layout shape the primary branch refuses; and `sameDirectory`'s pathname-equality shortcut can answer yes for two identical UNidentifiable paths, which is not enough to place a destination against. It remains a deliberate weakening, since nothing on such a host can contradict a marker naming the real home. `installSpawnEnv` re-points the marker at an explicitly overridden HOME, so a spawn that supplies its own sandbox is not refused by a marker still naming the helper's default one. Refusals are STAMPED so `bin/install.js` can rethrow them without running `_codexPreConfigRollback()`, which deletes and recreates every snapshotted `gsd-*` directory in the resolved skills root — otherwise the guard's own refusal would provoke the mutation it exists to prevent. **Known limits, stated rather than implied**: a subordinate bind mount of the real `~/.agents` into the sandbox is not closed (a bind mount is not a link, so realpath keeps the mount-point spelling; closing it needs non-portable mount-table introspection), and a cross-process TOCTOU swap between check and write is out of reach. Source: `gsd-core/bin/lib/real-home-guard.cjs` (generated from `src/real-home-guard.cts`). See Install Engine Module, Surface Module, Runtime Artifact Install Plan Module, Runtime Artifact Layout Module.
diff --git a/docs/explanation/security-model.md b/docs/explanation/security-model.md
index 8117efa1a..bbcc839d2 100644
--- a/docs/explanation/security-model.md
+++ b/docs/explanation/security-model.md
@@ -162,17 +162,31 @@ module is the central security utility. It provides:
 - Shell argument validation: arguments passed to subshell commands are
   validated before use
 
-Two containment checks elsewhere in the tree are deliberately NOT routed through
+Three containment checks elsewhere in the tree are deliberately NOT routed through
 this predicate, because each is narrower or stricter rather than a second opinion.
+The common thread is that a realpath-based predicate is the wrong tool wherever a
+symlink must be *preserved* rather than resolved.
+
 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. And `isPathConfined` in `src/external-descriptor-trust.cts` is lexical by
+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. A lexical check
-cannot see a symlink, so callers that rely on it for a write-confinement
-guarantee must pair it with their own symlink refusal.
+`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
diff --git a/scripts/check-glossary-refs.cjs b/scripts/check-glossary-refs.cjs
index 4a87a671b..1ff2f339b 100644
--- a/scripts/check-glossary-refs.cjs
+++ b/scripts/check-glossary-refs.cjs
@@ -154,7 +154,11 @@ function isTracked(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+$/, '');
@@ -164,9 +168,18 @@ function extractTrackedRefs(text) {
     if (!/[A-Za-z0-9_]$/.test(token)) return;
     if (token.includes('NNNN')) return;
     if (!isTracked(token)) return;
-    // Containment decision is the canonical predicate's, per ADR-4650.
-    if (tryWithinRoot(token, ROOT, PathAcceptance.AbsoluteInsideRoot) === null) 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/)) {
@@ -187,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)`. */
diff --git a/src/init.cts b/src/init.cts
index 0888cf0c0..a6dc04269 100644
--- a/src/init.cts
+++ b/src/init.cts
@@ -4017,9 +4017,9 @@ function buildAgentSkillsBlock(
       }
       const globalSkillMdContained = tryWithinRoot(globalSkillMd, globalSkillsBase, PathAcceptance.AbsoluteInsideRoot);
       if (globalSkillMdContained === null) {
-        const acceptedViaTrustedRoot = trustedGlobalRoots.some((root) => {
-          return tryWithinRoot(globalSkillMd, root, PathAcceptance.AbsoluteInsideRoot) !== 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`,
@@ -4030,6 +4030,13 @@ 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;
     }
@@ -4037,12 +4044,13 @@ function buildAgentSkillsBlock(
     const skillPathContained = tryWithinRoot(skillPath, projectRoot);
     if (skillPathContained === null) {
       warn(
-        `[agent-skills] WARNING: Skipping unsafe path "${skillPath}": resolves outside the project directory\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
diff --git a/src/install-engine.cts b/src/install-engine.cts
index c6c8ac199..a108dab2e 100644
--- 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
diff --git a/src/install-profiles.cts b/src/install-profiles.cts
index 073aff418..8d3ffb599 100644
--- 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
diff --git a/src/installer-migrations.cts b/src/installer-migrations.cts
index 775877acd..69e3f1a61 100644
--- a/src/installer-migrations.cts
+++ b/src/installer-migrations.cts
@@ -19,7 +19,6 @@ 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 { tryWithinRoot, PathAcceptance } 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
@@ -665,18 +664,26 @@ interface EnsureInsideConfigResult {
   fullPath: string;
 }
 
+// DELIBERATELY LEXICAL — do not route this through the canonical containment
+// predicate (`assertWithinRoot` / `tryWithinRoot`, src/security.cts).
+//
+// Reviewed under epic #4636 Phase 3 and reverted after the remote matrix proved
+// the 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 canonical predicate
+// realpath-resolves, so it 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.
+//
+// `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);
-  // fullPath stays the LEXICAL path.resolve result (not the canonical
-  // predicate's realpath-resolved value): both callers (readJson's
-  // ensureInsideConfig call and the migration-apply loop) use fullPath for
-  // fs.existsSync checks and journal entries, and those must not shift if
-  // configDir happens to be a symlink. Per ADR-4650 decision 6, the
-  // containment DECISION (whether fullPath is inside configDir) is owned by
-  // the canonical predicate — this wrapper only decides how to degrade
-  // (throw with this file's existing message), never whether contained.
   const fullPath = path.resolve(configDir, normalized);
-  if (tryWithinRoot(fullPath, configDir, PathAcceptance.AbsoluteInsideRoot) === null) {
+  const root = path.resolve(configDir);
+  if (fullPath !== root && !fullPath.startsWith(root + path.sep)) {
     throw new Error(`migration path escapes configDir: ${relPath}`);
   }
   return { normalized, fullPath };
diff --git a/src/verify.cts b/src/verify.cts
index 0a3c4e8db..91e6015d6 100644
--- a/src/verify.cts
+++ b/src/verify.cts
@@ -1549,7 +1549,7 @@ 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 fromContained = tryWithinRoot(fromPath, cwd);
diff --git a/tests/install-write-confinement.test.cjs b/tests/install-write-confinement.test.cjs
index 5bb453585..1397b25c0 100644
--- a/tests/install-write-confinement.test.cjs
+++ b/tests/install-write-confinement.test.cjs
@@ -4470,6 +4470,7 @@ describe('#4636 RED: capability-skill symlink escape (isPathConfined has no real
         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,
diff --git a/tests/security-prompt-injection.security.test.cjs b/tests/security-prompt-injection.security.test.cjs
index 19287448b..d2f372997 100644
--- 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);

From bbc3f131be8983c6f9d921d2da53ee2424a198d4 Mon Sep 17 00:00:00 2001
From: sim <sim@local>
Date: Sat, 12 Sep 2026 18:33:01 -0400
Subject: [PATCH 10/11] refactor(#4653): make containment ONE decision,
 resolved two ways
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Satisfies #4653 DW1 and DW9, which were the phase's outstanding acceptance
criteria: every other implementation must be deleted or route its containment
DECISION through the canonical predicate, and no surviving wrapper may decide
WHETHER a path is contained.

Three implementations were being retained with their own comparisons, on the
argument that each needs LEXICAL resolution — a realpath-based predicate is the
wrong tool wherever a symlink must be preserved rather than resolved. That
argument is correct about RESOLUTION and was being used to justify owning the
DECISION too. Those are separable, and separating them is what closes the
criteria honestly rather than by reinterpretation.

  isContainedIn(resolvedTarget, resolvedRoot, pathImpl?)   module-internal

is now the single place this repo decides containment. It is separator-aware, so
a sibling merely sharing a prefix (`<root>-evil` against `<root>`) is still
rejected. Two exported families sit on it and differ ONLY in how a candidate is
resolved before the decision:

  assertWithinRoot / tryWithinRoot                realpath-resolving
  assertWithinRootLexical / tryWithinRootLexical  path.resolve only, no I/O

The lexical pair carries `opts.pathImpl`, so win32 separator semantics stay
testable off Windows — that seam already existed in isPathConfined and would
have been lost by a naive collapse.

The three call sites now take their decision from the predicate and keep only
what is genuinely theirs:

  external-descriptor-trust isPathConfined   delegates outright; pathImpl forwarded
  installer-migrations ensureInsideConfig    delegates; keeps its own message and
                                             its LEXICAL fullPath, which callers
                                             consume for existsSync and journal rows
  gsd-tools.cjs isInsideDir                  delegates; keeps its own `target !==
                                             root` condition, and the separate
                                             symlink refusal above it stands

DW5 is not weakened by this. That criterion binds the symlink oracle and the
ancestor canonicalization; both are untouched. The only change inside
validatePath is three comparison lines becoming one call, and the rejection
string `Path escapes allowed directory: <resolved> is outside <base>` stays
byte-identical because it is an observable CLI contract.

What this does NOT do, stated plainly: the lexical family still cannot see a
symlink. That is a property of lexical resolution, not a gap in the seam, and
the three callers that need it are the three that must pair it with their own
symlink refusal — which is exactly what the fix earlier in this phase added at
the install sites. The doc comment says so at the definition, and CONTEXT.md and
docs/explanation/security-model.md are corrected: they previously described
these three as deliberately NOT routed through the predicate, which is no longer
true.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 CONTEXT.md                         |  2 +-
 docs/explanation/security-model.md | 17 +++++--
 gsd-core/bin/gsd-tools.cjs         | 20 +++++---
 src/external-descriptor-trust.cts  | 12 +++--
 src/installer-migrations.cts       | 23 +++++----
 src/security.cts                   | 78 ++++++++++++++++++++++++++++--
 6 files changed, 123 insertions(+), 29 deletions(-)

diff --git a/CONTEXT.md b/CONTEXT.md
index 66052e7d9..dd11b68aa 100644
--- a/CONTEXT.md
+++ b/CONTEXT.md
@@ -330,7 +330,7 @@ Module owning install-time staging and content-rewrite selection for a pre-resol
 Narrow, enumerated fs seam for the `installRuntimeArtifacts` call tree (`src/install-engine.cts`) — lands ADR-58's never-shipped `cleanup` rollout step (`registry → adapter → helpers → cleanup`, #2874, epic #2866 Phase 5). `installRuntimeArtifacts` now returns the executed plan it ran (`{ runtime, scope, kinds: [{kind, sourceDir, destDir, preserved}], cleanup: [{dir, ok}], postSteps }`) instead of `void`, including on the `combinedFamilyInstall` (OpenCode/Kilo) early-return path — no path may return `undefined` after this phase. Failure is unchanged: stage/rewrite errors still throw rather than becoming an `ok:false` value, so a caller cannot read success-shaped data off a failure path. Delivery is an ambient single mutable adapter (`current`), swapped for the duration of one synchronous install via `withInstallFs(deps.fs, fn)` and always restored in a `finally` — a `deps` parameter threaded through every function on the call tree (`install-profiles.cts`, `runtime-artifact-conversion.cts`, `commonjs-marker.cts`, `installer-migrations.cts`'s two reachable entry points) was rejected as a dozen+-site touch for no behavioral gain over the ambient swap, extending rather than replacing `createRuntimeArtifactInstallPlan`'s existing `deps` bag precedent (Runtime Artifact Install Plan Module). An injected `deps.fs` is a PARTIAL adapter merged over real `node:fs`; any method it omits — except `realpathSync`, the one method documented to degrade gracefully — now THROWS immediately if actually called, naming the missing method, rather than silently resolving to the real filesystem (#2875 defect fix: the prior silent fall-through let a fake adapter missing e.g. `rmSync` perform real destructive IO unnoticed). **Routes destination IO only, by design**: every write/probe against the install destination (copies, removals, snapshot/restore of preserved skill dirs, the manifest read) is fake-able; locating this package's own source tree (`findInstallSourceRoot`/`findAgentsSourceRoot`'s walk-up-from-`__dirname`, `readGsdCommandNames`) stays real and unrouted — a destination-fake's store starts empty and was never seeded with the repo's own paths, so routing that lookup would make every fake-adapter install throw instead of staging. The symlink-escape guard (`hasExistingSymlinkBetween`) and `assertDestWithinConfigHome` keep their REFUSAL DECISIONS outside this seam — only their `existsSync`/`lstatSync`/`realpathSync` probes route through it, so an injected fake can change what a probe observes but never flip the security decision itself. Writes remain byte-identical to pre-#2874 (AC4/AC5); existing `void`-ignoring callers (`bin/install.js`) are unaffected. Source: `gsd-core/bin/lib/install-fs-adapter.cjs` (generated from `src/install-fs-adapter.cts`). See Runtime Artifact Install Plan Module, ADR-58.
 
 ### Path Containment Seam
-One predicate answers "does this path resolve inside this root?" for the whole tree (epic #4636, ADR-4650). `src/security.cts` owns the resolution engine — realpath-based, with a closed dangling-symlink existence oracle and ancestor canonicalization so a not-yet-created path under a non-canonical base (macOS `/var` vs `/private/var`) still resolves — and that engine is NOT exported: the module-internal `validatePath` returns `{ safe, resolved, error }` and populates `resolved` with the ESCAPING path on the traversal branch, so a caller who skipped the boolean would get an attacker-controlled value precisely in the dangerous case. The exported surface is three shapes that cannot do that: `assertWithinRoot(candidate, root, label?, policy?)` throws, `tryWithinRoot(candidate, root, policy?)` returns exactly `null`, and `requireSafePath` is a preserved alias of the throwing form. All three return the 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. `policy` is the named `PathAcceptance` acceptance policy (`RelativeOnly` | `AbsoluteInsideRoot`) that replaced a per-call-site `{ allowAbsolute: true }` boolean, which read as "containment is relaxed here" when in fact an absolute path outside the root was always rejected. The rejection text `Path escapes allowed directory: <resolved> is outside <base>` is an observable CLI contract, surfaced through command `reason` fields and asserted by `tests/quick-batch.test.cjs`. **A wrapper may decide HOW to degrade, never WHETHER a path is contained** (ADR-4650 decision 6): `planning-inspect.cts`'s `isPathContained` takes its decision from the predicate but keeps must-exist as its own separate condition, because the canonical engine deliberately ACCEPTS a not-yet-created path under the root while its callers guard an `fs` read that is about to happen. **Three implementations are deliberately NOT collapsed, and each is stricter or narrower rather than duplicative** — a realpath-based predicate is the wrong tool wherever a symlink must be preserved rather than resolved: `gsd-core/bin/gsd-tools.cjs`'s restore gate rejects symlinks OUTRIGHT (the canonical predicate accepts a link resolving inside the root, which for a restore still writes through the link) and treats `target === root` as NOT contained; `external-descriptor-trust.cts`'s `isPathConfined` is lexical by design because two install callers validate a `destSubpath` BEFORE the `mkdirSync` that creates it, where realpath cannot resolve; and `installer-migrations.cts`'s `ensureInsideConfig` 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 — routing it through the canonical predicate dereferenced exactly those links and then rejected them for escaping `configDir`, which the remote matrix caught as four failing tests. `normalizeRelPath` is its real pre-gate, throwing on absolute paths and `..` segments before the containment check runs. Source of truth: `gsd-core/bin/lib/security.cjs` (generated from `src/security.cts`). See Gate Predicate Evaluator Module, Runtime Artifact Install Plan Module, Real Home Guard Module.
+One predicate answers "does this path resolve inside this root?" for the whole tree (epic #4636, ADR-4650). `src/security.cts` owns the resolution engine — realpath-based, with a closed dangling-symlink existence oracle and ancestor canonicalization so a not-yet-created path under a non-canonical base (macOS `/var` vs `/private/var`) still resolves — and that engine is NOT exported: the module-internal `validatePath` returns `{ safe, resolved, error }` and populates `resolved` with the ESCAPING path on the traversal branch, so a caller who skipped the boolean would get an attacker-controlled value precisely in the dangerous case. The exported surface is three shapes that cannot do that: `assertWithinRoot(candidate, root, label?, policy?)` throws, `tryWithinRoot(candidate, root, policy?)` returns exactly `null`, and `requireSafePath` is a preserved alias of the throwing form. All three return the 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. `policy` is the named `PathAcceptance` acceptance policy (`RelativeOnly` | `AbsoluteInsideRoot`) that replaced a per-call-site `{ allowAbsolute: true }` boolean, which read as "containment is relaxed here" when in fact an absolute path outside the root was always rejected. The rejection text `Path escapes allowed directory: <resolved> is outside <base>` is an observable CLI contract, surfaced through command `reason` fields and asserted by `tests/quick-batch.test.cjs`. **A wrapper may decide HOW to degrade, never WHETHER a path is contained** (ADR-4650 decision 6): `planning-inspect.cts`'s `isPathContained` takes its decision from the predicate but keeps must-exist as its own separate condition, because the canonical engine deliberately ACCEPTS a not-yet-created path under the root while its callers guard an `fs` read that is about to happen. **Containment is decided in exactly one place, but resolved two ways.** `isContainedIn` — the separator-aware comparison, so a sibling sharing a prefix (`<root>-evil`) is never accepted — is module-internal 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` realpath-resolve, and `assertWithinRootLexical`/`tryWithinRootLexical` use `path.resolve` alone and never touch the filesystem (they accept `opts.pathImpl` so win32 separator semantics are testable off Windows). **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 legitimately need the lexical resolution and route their DECISION through it: `gsd-core/bin/gsd-tools.cjs`'s restore gate, which additionally rejects symlinks OUTRIGHT (the realpath predicate accepts a link resolving inside the root, which for a restore still writes through it) and treats `target === root` as NOT contained via its own extra check; `external-descriptor-trust.cts`'s `isPathConfined`, because two install callers validate a `destSubpath` BEFORE the `mkdirSync` that creates it, where realpath cannot resolve; and `installer-migrations.cts`'s `ensureInsideConfig`, because that module snapshots, restores and backs up a symlinked managed path AS A LINK — resolving it dereferenced exactly those links and rejected them for escaping `configDir`, which the remote matrix caught as four failing tests. `normalizeRelPath` is that one's real pre-gate, throwing on absolute paths and `..` before containment runs. Source of truth: `gsd-core/bin/lib/security.cjs` (generated from `src/security.cts`). See Gate Predicate Evaluator Module, Runtime Artifact Install Plan Module, Real Home Guard Module.
 
 ### Real Home Guard Module
 Refuses an in-process install whose destination would land in the developer's REAL home while a Node test runner is in scope (#3712, ADR-1239/#2088 territory). Interface: `assertTestHomeSandboxed(operation, runtime, kinds, deps?)` (throws), `isTestHomeGuardRefusal(err)`, `SANDBOX_MARKER`. Exists because a runtime kind may declare a global `home` override resolved from `os.homedir()` rather than from the caller's `configDir` — codex's skills kind (`home: ".agents"`) and, since #3738, antigravity's global skills AND agents kinds (`home: ".gemini/config"`, the dir AGY scans for global discovery; configHome stays `~/.gemini/antigravity` for settings/runtime files) are the live cases — so sandboxing `configDir`/`targetDir` does NOT contain it, and `assertDestWithinConfigHome` (Runtime Artifact Install Plan Module) structurally cannot see the class: that gate confines a `destSubpath` to whatever root it is handed, and here the root IS the escaped home. The observed failure was silent destruction — a test file that sandboxed only `targetDir` pruned all 71 `gsd-*` skills from a real `~/.agents/skills` via `_removeGsdEntries` while the suite exited 0 and the manifest still reported a healthy install. **Six writers** resolve a kind `home` and then write or destroy under it and therefore carry a call: `installRuntimeArtifacts`, `uninstallRuntimeArtifacts` (Install Engine Module), `applySurface` (Surface Module), and `migrateLegacyDevPreferencesToSkill` — which CREATES rather than prunes, which is why it was missed on the first pass, and which runs from `_runLegacyInstallMigrations` BEFORE `installRuntimeArtifacts`' own assertion, so it guards the destination it already resolved rather than resolving a second time (generative-fix divergence) — plus `installOpencodeFamilySkills` and `installAgentsKindStandalone`, guarded against a future descriptor change rather than a present escape since no combined-family or agents kind declares a `home` today. Each of the four reachable writers carries an optional `deps: { os?, env? }` tail parameter — the guard's trigger condition is "HOME equals the passwd home", which cannot be simulated without pointing at the developer's real home — so `tests/install-write-confinement.test.cjs` drives the REAL entrypoint for each one and deleting any guard call site turns a row red. `migrateLegacyDevPreferencesToSkill` guards only when `target.hasHomeOverride` — that same resolution's own answer to "did the skills kind declare a `home`?", never inferred from `installRoot !== targetDir`, which is FALSE whenever the override resolves onto `targetDir` itself (a configDir of `$HOME/.agents`, exactly where codex's override points); both directions of that condition are pinned, so widening it to refuse ordinary confined migrations fails a row too. **Gated on `NODE_TEST_CONTEXT`** (set by `node --test`), never on `GSD_TEST_MODE` — several in-process test files, including the one that caused #3712, never set the latter — so ordinary installs outside a Node test context are untouched and codex still installs to `$HOME/.agents` normally. **The predicate asks about the DESTINATION, not about HOME state**, and about the path the write RESOLVES to rather than how it is spelled: a stale layout captured before `sandboxHome()` still names the real `~/.agents` while HOME is sandboxed, so a HOME-state check would wave it through, and an aliased `<sandbox>/.agents` symlink or junction would otherwise redirect an allowed path into the real home. Canonicalization itself fails CLOSED: a component that cannot be resolved (`EACCES`/`EPERM` on a directory whose mode changed, `ELOOP` on a symlink cycle, `EIO` on a failing mount) is REFUSED rather than falling back to the lexical spelling — that fallback was a second, unnamed fail-open, and precisely the ALLOW an unresolvable alias needs, since the sandbox spelling then satisfies the nested-sandbox exemption (Codex review of #3725). Only `ENOENT`/`ENOTDIR` walk up, matching `identify`'s own errno split rather than inventing a second one: both mean "no such path as named", which is the ordinary shape of a destination no install has created yet. A destination inside the real home is exempted only when all three hold — HOME differs from the passwd home by filesystem identity, the passwd home is not itself beneath that HOME (`/Users`, `C:\Users`), and the destination resolves beneath it. That exemption is not a softening: on Windows `os.tmpdir()` is under `%USERPROFILE%`, so EVERY test sandbox is a descendant of the real home and containment alone cannot separate the safe case from the dangerous one — without it the whole Windows matrix refuses. Every containment decision compares by filesystem identity (`st_dev`+`st_ino`), never pathname (the one pathname comparison in the module is `sameDirectory`'s equality shortcut on the marker branch, described below): `path.resolve` resolves neither symlinks nor case and `realpath` returns a canonical pathname two routes to one directory can disagree on. **Fails CLOSED**, with one named exception: where no passwd entry is readable (some CI images) it falls back to a path-valued marker set by `sandboxHome()`/`installSpawnEnv()` in `tests/helpers.cjs`, which must identify, must equal the home in effect, AND must contain every resolved destination. All three are load-bearing: matching HOME attests that a caller sandboxed HOME and says nothing about where these destinations resolve, so on its own the marker waved through the very stale-layout shape the primary branch refuses; and `sameDirectory`'s pathname-equality shortcut can answer yes for two identical UNidentifiable paths, which is not enough to place a destination against. It remains a deliberate weakening, since nothing on such a host can contradict a marker naming the real home. `installSpawnEnv` re-points the marker at an explicitly overridden HOME, so a spawn that supplies its own sandbox is not refused by a marker still naming the helper's default one. Refusals are STAMPED so `bin/install.js` can rethrow them without running `_codexPreConfigRollback()`, which deletes and recreates every snapshotted `gsd-*` directory in the resolved skills root — otherwise the guard's own refusal would provoke the mutation it exists to prevent. **Known limits, stated rather than implied**: a subordinate bind mount of the real `~/.agents` into the sandbox is not closed (a bind mount is not a link, so realpath keeps the mount-point spelling; closing it needs non-portable mount-table introspection), and a cross-process TOCTOU swap between check and write is out of reach. Source: `gsd-core/bin/lib/real-home-guard.cjs` (generated from `src/real-home-guard.cts`). See Install Engine Module, Surface Module, Runtime Artifact Install Plan Module, Runtime Artifact Layout Module.
diff --git a/docs/explanation/security-model.md b/docs/explanation/security-model.md
index bbcc839d2..99a67d377 100644
--- a/docs/explanation/security-model.md
+++ b/docs/explanation/security-model.md
@@ -162,10 +162,19 @@ module is the central security utility. It provides:
 - Shell argument validation: arguments passed to subshell commands are
   validated before use
 
-Three containment checks elsewhere in the tree are deliberately NOT routed through
-this predicate, because each is narrower or stricter rather than a second opinion.
-The common thread is that a realpath-based predicate is the wrong tool wherever a
-symlink must be *preserved* rather than resolved.
+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
diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs
index 7378564f0..9c6a23ff2 100755
--- a/gsd-core/bin/gsd-tools.cjs
+++ b/gsd-core/bin/gsd-tools.cjs
@@ -3359,16 +3359,22 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load
   // 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 and deliberately NOT
-  // collapsed. Note also: isInsideDir below treats target === root as NOT
-  // contained (it requires a non-empty relative path), unlike every other
-  // containment implementation in this repo, which treats target === root as
-  // contained.
+  // 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;
   }
 
   /**
diff --git a/src/external-descriptor-trust.cts b/src/external-descriptor-trust.cts
index ce3ee854f..e34c99378 100644
--- 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
@@ -53,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,
@@ -62,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 {
diff --git a/src/installer-migrations.cts b/src/installer-migrations.cts
index 69e3f1a61..f08923fde 100644
--- 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,26 +665,30 @@ interface EnsureInsideConfigResult {
   fullPath: string;
 }
 
-// DELIBERATELY LEXICAL — do not route this through the canonical containment
-// predicate (`assertWithinRoot` / `tryWithinRoot`, src/security.cts).
+// 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 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 canonical predicate
-// realpath-resolves, so it dereferences exactly the symlinks this module exists
-// to preserve and then rejects them for escaping configDir
+// 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 };
diff --git a/src/security.cts b/src/security.cts
index 260832f3c..950f42f46 100644
--- a/src/security.cts
+++ b/src/security.cts
@@ -24,6 +24,27 @@ 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.
@@ -102,9 +123,7 @@ function validatePath(filePath: unknown, baseDir: unknown, opts: { allowAbsolute
       }
     }
   }
-  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,
@@ -261,6 +280,59 @@ export function requireSafePath(filePath: unknown, baseDir: unknown, label: stri
   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 ────────────────────────────────────────────────────
 
 /**

From 2a6f74027fa61f1e994547285ff62964ac9770a2 Mon Sep 17 00:00:00 2001
From: sim <sim@local>
Date: Sat, 12 Sep 2026 18:49:48 -0400
Subject: [PATCH 11/11] chore(#4653): backfill PR 4672 into both changesets
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Also corrects the Changed fragment: it named three containment exports as the
only ones, which stopped being true when the lexical family was added to close
DW1/DW9. It now describes one decision resolved two ways, and says why the
lexical pair exists rather than leaving a reader to assume it is a weaker
alternative to the realpath form.

scripts/lint-docs-required.cjs now passes (ok_docs_updated) — it could not
evaluate against the mandated pr:0 placeholder.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .changeset/eager-badgers-bark.md   | 4 ++--
 .changeset/gallant-hawks-tumble.md | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/.changeset/eager-badgers-bark.md b/.changeset/eager-badgers-bark.md
index 7882a6145..ab8071b6a 100644
--- a/.changeset/eager-badgers-bark.md
+++ b/.changeset/eager-badgers-bark.md
@@ -1,5 +1,5 @@
 ---
 type: Changed
-pr: 0
+pr: 4672
 ---
-**The path-containment predicate is now a single exported seam** — `security.cjs` no longer exports `validatePath`; `assertWithinRoot` (throws), `tryWithinRoot` (returns null) and `requireSafePath` are the only containment exports, and all three 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)
+**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)
diff --git a/.changeset/gallant-hawks-tumble.md b/.changeset/gallant-hawks-tumble.md
index 124cd9a50..95dd7409b 100644
--- a/.changeset/gallant-hawks-tumble.md
+++ b/.changeset/gallant-hawks-tumble.md
@@ -1,5 +1,5 @@
 ---
 type: Security
-pr: 0
+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)