Files
msd-core/docs
Tom Boucher 12e4d93b19 fix(#2393): add GSD_ALLOW_SYMLINKED_DEST opt-in for intentional user-owned symlink layouts (#2445)
* fix(#2393): add GSD_ALLOW_SYMLINKED_DEST opt-in for intentional user-owned symlink layouts

Bug: v1.7.0's destSubpath write-confinement (ADR-1239 Phase B) refused
install/update whenever CLAUDE_CONFIG_DIR (or an artifact-kind child like
skills/, hooks/) was a pre-existing symlink, with no opt-out. Three
legitimate user-owned layouts were blocked:

  - (lars-hh) CLAUDE_CONFIG_DIR=~/.claude-personal with skills/hooks
              symlinked to a user-owned external dir
  - (Mamiki)  ~/.claude/skills is a Windows Junction to a shared skills dir
  - (Azd325)  ~/.claude itself is a symlink to a dotfiles repo (the early
              root-is-symlink return refused before the component loop ran)

Fix: add GSD_ALLOW_SYMLINKED_DEST env var (accepts '1' or 'true'). When
set, hasExistingSymlinkBetween follows symlinks instead of refusing them.
Cross-platform: fs.lstatSync().isSymbolicLink() returns true for both
POSIX symlinks and NTFS junctions (Node ≥ 16), so Mamiki's Junction case
is handled by the same code path.

Threat model preserved (these still refuse EVEN WITH opt-in):
  (a) path-traversal in the destSubpath string itself ('../../etc'-style)
      — ADR-1239 Phase B threat (a), untrusted destSubpath protection
  (b) a symlink whose resolved real path equals the install root itself
      — would let _removeGsdEntries wipe the root; #1704 threat (b)
  (c) broken symlinks (realpathSync throws) — fail-closed

What opt-in RELAXES specifically: the 'pre-existing symlink pointing
outside configHome' refusal — #1704 threat (c). The user has explicitly
asserted they own and trust the symlink target.

Error messages at all 4 call sites (installRuntimeArtifacts, _copyStaged,
migrateLegacyDevPreferencesToSkill, installOpencodeFamilySkills) updated
to (1) name the env var opt-in, (2) be accurate when the root itself is
a symlink (Azd325's complaint that the old message accused destDir of
'containing' a symlink when the root was the actual symlink).

Docs: docs/CONFIGURATION.md Environment Variables table updated.

Regression tests in tests/install-write-confinement.test.cjs cover:
  - child-symlink layout (lars-hh / Mamiki): default refuses, opt-in allows
  - root-is-symlink layout (Azd325): default refuses, opt-in follows
  - path-traversal '../../etc' refused EVEN WITH opt-in (threat a preserved)
  - resolved-target-equals-install-root refused EVEN WITH opt-in (threat b)
  - broken symlink refused EVEN WITH opt-in (fail-closed)

* test(#2393): import beforeEach/afterEach in install-write-confinement suite

The original file imported only { describe, test } from node:test. The new
#2393 opt-in describe block uses beforeEach/afterEach to manage the
GSD_ALLOW_SYMLINKED_DEST env var lifecycle — add them to the import.

* test(#2393): correct broken-symlink test — existsSync follows link → loop terminates early

Initial test expected broken symlinks to be refused even with opt-in. That
was wrong: fs.existsSync follows symlinks, so a broken symlink returns
false from existsSync and the component loop terminates before the symlink
check fires. Both default and opt-in paths share this behavior; the fix
preserves it.

Updates the test to pin the actual current behavior so a future refactor
(e.g. switching to lstatSync for existence) is a deliberate behavior change.

* fix(#2393): realpath the install root — guard against macOS /var ↔ /private/var

Code review (security subagent) flagged a HIGH-severity hole in the threat-(b)
preservation: realTarget (from fs.realpathSync) is fully symlink-resolved,
resolvedRoot (from path.resolve) is lexical-only. On macOS /var is a symlink
to /private/var, so resolvedRoot='/var/foo/.claude' but realConfigHome is
'/private/var/foo/.claude'. A symlink whose realtarget matches the install
root by real path would compare unequal to the lexical resolvedRoot —
defeating the wipe-protection guard exactly in the reporter's case (Azd325,
nix-darwin: ~/.claude is itself a symlink).

Fix: compute realRoot once via fs.realpathSync(resolvedRoot) at function entry
(with fail-closed fallback to lexical form on realpath failure — broken/missing
root, permission denied, exotic FS). Threat (a) path-traversal check above
still confines regardless. Compare against BOTH lexical and real forms in
both the root-symlink and component-symlink branches.

Also adds the reviewer's transitivity-trust clarification comment: once a
symlink is followed under opt-in, the walk continues from the resolved real
path WITHOUT re-checking further segments stay inside a confining boundary.
This is documented opt-in semantics — one opt-in trusts the whole reachable
tree — and the comment makes the design choice explicit so a future
maintainer doesn't add a 'follow one symlink only' expectation.

Regression test added for the macOS /var normalization case (spelled configHome
via os.tmpdir() lexically while pointing the test symlink through its realpath).
Test skips on non-darwin platforms and when os.tmpdir() has no symlink component.

* fix(#2393): root-symlink branch — do not apply threat-(b) check to root itself

Initial fix applied the wipe-threat-(b) check to the root-symlink branch
unconditionally. That was wrong: when root itself is a symlink (Azd325's
nix-darwin case), its realpath IS realRoot by construction — so the check
always fires, defeating the opt-in for exactly the case it was meant to
enable.

The wipe threat (b) does NOT apply to root being a symlink: destDir is a
CHILD of root, and resolving root just gives root's target. There is no
circular back-reference to root from a path that descends from a resolved
root. So the root-symlink branch should just follow the symlink under opt-in
and continue the walk, no threat-(b) check.

Threat (b) only fires in the COMPONENT loop, where a child symlink can
resolve back to the install root. That branch keeps the (b) check using BOTH
lexical and real forms of root (the macOS /var ↔ /private/var fix from the
prior commit).

* fix(#2393): apply opt-in at the 5 bin/install.js call sites + add env-var/transitive tests

Code review (correctness subagent) flagged a Critical coverage gap: the
initial fix updated only the 4 src/install-engine.cts call sites. Five
more call sites in bin/install.js still used the 2-arg signature, so the
opt-in env var was silently ignored on:

  - installCodexConfig (config.toml + agents/ dir + per-agent .toml paths)
    — Codex only
  - copyWithPathReplacement (the generic emit path: workflows, commands,
    staging) — ALL runtimes
  - resolveInstallRelativePath (path resolver used in various places)

Result: a user setting GSD_ALLOW_SYMLINKED_DEST=1 would see SOME refusals
disappear (engine path) and OTHERS remain (bin/install.js paths) — a
partially-applied install and a confusing UX, directly contradicting the
PR's headline claim.

Fix:
- Export isSymlinkedDestOptIn from src/install-engine.cts alongside
  hasExistingSymlinkBetween
- Import it in bin/install.js
- Update all 5 bin/install.js call sites to pass { allowOptInFollow }
- Update all 3 bin/install.js error messages to name the env var, matching
  the engine's phrasing

Also addresses reviewer's Medium test-adequacy findings:
- isSymlinkedDestOptIn env-var parsing now tested directly (accepts only
  documented '1' / 'true'; rejects 'TRUE', 'yes', 'on', '0', 'false',
  empty, unset)
- transitive symlink chain (configHome/outer → outside1 → outside2) test
  pins the documented 'transitive and unbounded' opt-in semantics so a
  future contributor can't accidentally narrow it

* chore(changeset): backfill pr:2445 in .changeset/eager-wasps-swim.md
2026-07-20 07:56:36 -04:00
..

GSD Core documentation

Documentation is organised into four quadrants: tutorials help you learn by doing, how-to guides solve specific tasks, reference states authoritative facts, and explanation explores concepts and design decisions.

Language versions: English · Português (pt-BR) · 日本語 · 简体中文


Tutorials


How-to guides


Reference


Explanation