From 6f0e5ccf85d538798ddca0a3020deee3ad10e347 Mon Sep 17 00:00:00 2001
From: sim <sim@local>
Date: Sat, 12 Sep 2026 13:59:52 -0400
Subject: [PATCH] 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);