diff --git a/.changeset/proud-cats-click.md b/.changeset/proud-cats-click.md new file mode 100644 index 0000000000..2020dc1330 --- /dev/null +++ b/.changeset/proud-cats-click.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 5300 +--- +**Installs, updates and uninstalls no longer replace a symlinked `settings.json` with a plain file** — without `GSD_ALLOW_SYMLINKED_DEST` they stop with a message naming it, and with it the write goes through to the link's target. (#5037) diff --git a/CONTEXT.md b/CONTEXT.md index c0296b3c4b..6be568d647 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -295,7 +295,7 @@ Pure, additive, no-I/O Module owning the versioned, negotiated contract over the Host-integration hook (`hooks/gsd-statusline.js`) that renders the session status line: model name, context-window meter, workspace directory, and the GSD-state segment (`formatGsdState()` projecting `.planning/` STATE.md). `readGsdState()` is workstream-aware (#2850): when the walk-up finds no flat `.planning/STATE.md` but lands on a `.planning/workstreams/` directory, it resolves the active workstream via `resolveActiveWorkstream` (`active-workstream-store.cts`), called with an empty args array — only its env>store precedence applies for this caller, since the CLI leg is inert without argv — and `planningPaths`/`listAvailableWorkstreams` (`planning-workspace.cts`) for path/mode resolution, the same seams every other workstream-aware command uses, and reads that workstream's `STATE.md` instead. The store tier is `peekActiveWorkstream`, a read-only sibling of `getActiveWorkstream` that never deletes a stale/invalid pointer file — a renderer invoked on every prompt must never mutate persistent state as a side effect of drawing a screen (`getActiveWorkstream`'s self-heal is correct for a command, not a render). When workstream mode is detected but nothing resolves, it returns a `{noActiveWorkstream:true}` sentinel that `formatGsdState`/`formatGsdStateCompact` render as `"no active workstream"` — observable, never silent emptiness. Opt-in segments are gated by `.planning/config.json` keys (`statusline.show_last_command`, `statusline.context_position`, plus the approved `statusline.show_context_tokens`, `statusline.state_format`, `statusline.show_git` and `statusline.show_state_freshness`), each registered across `gsd-core/bin/shared/config-schema.manifest.json` + `src/config.cts` + the `loadConfig` whitelist + `docs/CONFIGURATION.md`. The compact GSD-state format consumes the canonical status vocabulary from `normalizeStateStatus()` (STATE.md Document Module) rather than a parallel keyword list. Config resolution for every `statusline.*` key is centralized in `resolveStatuslineOptions(cfg)` — the two entry points (`runStatusline()`, the stdin path, and `renderStatusline(data)`, the test-facing renderer) previously duplicated it byte-for-byte, and a single resolver is what keeps a newly-added key from reaching only one of them (#2734). The opt-in **STATE.md freshness marker** (`statusline.show_state_freshness`, #2734) renders `state ~N commits back` in both renderers when STATE.md's `state_head` stamp is at least `STATE_HEAD_ADVISORY_COMMITS` (20) commits behind HEAD — the same threshold `validate.health`'s W024 uses, not `> 0`, because `commit_docs: true` advances HEAD by one on every STATE sync and a `> 0` threshold would alarm permanently on a fresh project. It follows the git segment's impure-reader → pure-IR → pure-formatter shape (`readStateHeadCommits` → `deriveStateFreshness` → `formatStateFreshness`), spends exactly one bounded `git rev-list --left-right --count` per render (ancestry and distance in one spawn) and none when disabled, and mirrors `src/state.cts`'s hash fence and `projectOwnsItsRepo`/`sub_repos` degradation guards hook-side rather than requiring `state.cjs` on the per-render path; `tests/gsd-statusline.test.cjs` binds the two copies by **behavioral** differential parity against `readStateHeadFreshness`, not a source comparison. Every degradation yields the tri-state *unknown* (marker absent), never a "fresh" claim the project cannot substantiate. **Data-source boundary (ADR-2164):** the statusline sources only local, read-only data — it refines the stdin payload Claude Code already sends and may add a new *local* source (e.g. `git`), but does not read credentials or call external/network APIs for data; account/usage/platform-level state is out of scope. ### Install Engine Module -Module owning the layout-driven runtime-artifact install pipeline — `installRuntimeArtifacts`, `uninstallRuntimeArtifacts`, `installOpencodeFamilySkills`, and their cluster helpers (`_copyStaged`, `_snapshotDir`/`_restoreDir`, legacy-migration + GSD-entry pruning, user-artifact preserve/restore). Extracted from the 12k-line `bin/install.js` (ADR-1239 Phase B, #1679) so adapters import the engine instead of reaching into the installer. Commit-attribution resolution stays in `bin/install.js` and is injected via a `resolveAttribution` parameter (the engine takes no config I/O). Source: `src/install-engine.cts` -> `gsd-core/bin/lib/install-engine.cjs`. +Module owning the layout-driven runtime-artifact install pipeline — `installRuntimeArtifacts`, `uninstallRuntimeArtifacts`, `installOpencodeFamilySkills`, and their cluster helpers (`_copyStaged`, `_snapshotDir`/`_restoreDir`, legacy-migration + GSD-entry pruning, user-artifact preserve/restore). Extracted from the 12k-line `bin/install.js` (ADR-1239 Phase B, #1679) so adapters import the engine instead of reaching into the installer. Commit-attribution resolution stays in `bin/install.js` and is injected via a `resolveAttribution` parameter (the engine takes no config I/O). It also owns the symlinked-destination guard the install writers share: `hasExistingSymlinkBetween` with the `isSymlinkedDestOptIn` (`GSD_ALLOW_SYMLINKED_DEST`, #2393) opt-in, and `resolveSymlinkedLeafWritePath` (#5037), which applies that policy to one file a temp-file-and-rename save is about to write. A symlinked file is refused without the opt-in and resolved to its real target with it, so the rename cannot replace the link; a dangling link is refused either way, and with the opt-in already set the refusal names the missing target (or the install root, for a link resolving to it) instead of asking for the opt-in again. `writeSettings` (Installer Module) and the Codex `hooks.json` writer (Runtime Hooks Surface Module) both go through it with the same behavior. Source: `src/install-engine.cts` -> `gsd-core/bin/lib/install-engine.cjs`. ### CommonJS Marker Module Module owning the `{"type":"commonjs"}` module-type marker GSD writes beside its own staged `.js` files (#2544) — `markerPathFor`, `classifyMarker`, `ensureCommonJsMarker`, `removeCommonJsMarker`, and the `COMMONJS_MARKER` / `COMMONJS_MARKER_CONTENT` constants. Exists so two rules are enforced in exactly one place: **write only where GSD owns the contents** (`hooks/`, and the `nativePlugin.dir` for runtimes declaring one — never the shared runtime config root, which on OpenCode and Kilo is documented, user-writable territory for local-plugin npm dependencies), and **never overwrite a file GSD did not write**. Ownership is decided by exact content match, the same predicate the uninstall path always used; `classifyMarker` is the shared seam behind both the write and the remove path so install and uninstall cannot drift apart again. Fails **closed** throughout: `lstat` (not `existsSync`) so a dangling symlink is never classified `absent`; anything that is not a regular file is `foreign`; a present-but-unreadable file is `foreign`, never downgraded to the permissive answer; the write uses `flag:'wx'` so anything appearing between classify and write yields `preserved-foreign` rather than a follow-or-overwrite. `ensureCommonJsMarker` **never throws** — an unwritable directory returns `failed` and both call sites warn and continue, matching the best-effort posture of every other marker interaction. The stale pre-#2544 config-root marker is retired by `src/installer-migrations/007-retire-config-root-commonjs-marker.cts` (and, for kimi's out-of-configDir root, by `bin/install.js` directly). Source: `src/commonjs-marker.cts` -> `gsd-core/bin/lib/commonjs-marker.cjs`. Test anchor: `tests/commonjs-marker.test.cjs`. @@ -304,7 +304,7 @@ Module owning the `{"type":"commonjs"}` module-type marker GSD writes beside its Module owning validation for Installer Migration Module records and planned actions. It enforces migration metadata, explicit install scopes, ownership evidence for destructive/config actions, and runtime contract citations for runtime config rewrites before a migration can enter planning or apply. ### Installer Module -Primary installer for all runtimes. Single production file: `bin/install.js` (hand-authored JS — it is NOT generated from `src/*.cts`; ADR-1508 keeps it hand-authored deliberately, and no `npm run build` step emits it). Exports: `install(isGlobal, runtime[, configDir])` → typed result `{ runtime, configDir, settingsPath, settings, statuslineCommand, updateBannerCommand, configuredEntrypoints }`; `uninstall(isGlobal, runtime[, configDir])`; `installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile)`; `uninstallRuntimeArtifacts(runtime, configDir, scope)`; `writeManifest(configDir, runtime)`. Runtime enum: `allRuntimes` (18 values: claude, antigravity, augment, cline, codebuddy, codex, copilot, cursor, hermes, kimi, kimi-code, kilo, opencode, pi, qwen, trae, windsurf, zcode). Directory helpers: `getDirName(runtime)` → local dir name; `getConfigDirFromHome(runtime, isGlobal)` → shell-quoted path fragment. Per-runtime global config-dir resolution is delegated to `gsd-core/bin/lib/runtime-homes.cjs:getGlobalConfigDir(runtime[, explicitDir])` — the canonical, env-var–aware projection (`explicitDir` override + opencode/kilo `*_CONFIG` file-path precedence); the legacy in-installer `getGlobalDir`/`getOpencodeGlobalDir`/`getKiloGlobalDir` were retired into it (#56). The same module exposes `detectAntigravityDirAmbiguity(opts)` — a side-effect-free probe reporting whether multiple `~/.gemini/antigravity{,-ide,-cli}` dirs coexist and which one GSD's `gsd-core/VERSION` marker (the `dot-home-nested` `probeExists`) resolves to, for installer / `/gsd-update` operator guidance when a pre-#217 install landed in the wrong sibling dir (#1441). Runtime-specific helpers: `resolveKiloConfigPath(configDir)`, `configureKiloPermissions(isGlobal[, explicitDir])`. Claude-specific permission helpers: `mergeClaudePermissions(settings)` — non-destructively appends GSD-owned allow entries (see `GSD_CLAUDE_ALLOW_PERMISSIONS`) to a Claude Code settings object and filters out the retired legacy forms (`GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS` #2278 — also retires the unscoped `Bash(npx gsd-core *)` rule (#5054); `GSD_CLAUDE_LEGACY_DENY_PERMISSIONS` #4221 — the `Read(.env*)` deny rules are retired in favor of the managed `gsd-secret-read-guard.js` hook, and an emptied `deny` array is deleted); called from `finishInstall` for `runtime === 'claude'` only; uninstall removes exactly these entries (#768). Layout-driven artifact copy/removal delegates to `gsd-core/bin/lib/runtime-artifact-layout.cjs:resolveRuntimeArtifactLayout` (throws `TypeError` for unknown runtimes). Five runtimes with non-recursive skill loaders (cline, qwen, hermes, augment, trae) use a nested router layout: 6 `gsd-ns-*` router bundles emitted as top-level skills, with concrete skills nested at `/skills//SKILL.md` (hermes prefix='': `skills/gsd/ns-*/…`). claude (reverted from nested per #924 — the Skill tool errors on unrouted names) and antigravity (one-level scan, but concrete skills must be top-level discoverable) plus the remaining skills-runtimes (cursor, codex, copilot, windsurf, codebuddy, opencode, kilo) use the flat `skills/gsd-/` layout. See Skill Surface Budget Module and Runtime Artifact Layout Module. `finishInstall` calls `assertConfiguredEntrypoints` (Runtime Hooks Surface Module, dedup'd by `(configPath, scriptPath)` first) before its own `writeSettings` — a failing settings-json-surface install/update never persists that write. `installAllRuntimes`'s `finalize()` runs the same check once over the aggregate set before its `printSummaries()` loop calls `finishInstall` per runtime, so the per-runtime slice is checked twice; the second pass is a dozen-odd `statSync`/`accessSync` calls and is left unconditional rather than gated on a caller-supplied bypass flag. Codex/Cursor/Windsurf/Kimi/Cline writers persist their own hooks.json/config.toml/`.clinerules/hooks/PreToolUse` directly inside `install()`, ahead of `installAllRuntimes`'s aggregate `assertConfiguredEntrypoints` call, so a validation failure there is reported (and blocks the completion message) before that file is reverted where a rollback exists. Codex's `install()` result exposes two rollbacks: `rollbackInstallerMigrations` — like every other runtime's — is the installer-migrations-only closure the name describes, and the Codex-only `rollbackPreInstallSnapshot` binds `restoreCodexSnapshot` (#3245's full pre-install snapshot restore — config.toml, hooks.json, skills/, agents/, VERSION). `installAllRuntimes`'s `rollbackFinalizedInstallerMigrations` picks `rollbackPreInstallSnapshot` only when the triggering error is tagged `configuredEntrypointValidation` (set by `assertConfiguredEntrypoints`); any other finalize-stage exception gets `rollbackInstallerMigrations`, so an unrelated sibling failure (e.g. EACCES on another runtime's permission write) cannot un-install a Codex install that already succeeded. The discriminator is the error's KIND, not which runtime owns the bad entrypoint: the aggregate gate is all-or-nothing, so ANY runtime's invalid entrypoint reverts the Codex snapshot — including when Codex's own entrypoints were fine and its "Done!" summary already printed (#4249). Cursor/Windsurf/Kimi/Cline have no equivalent snapshot, so their result binds only the narrow rollback and a validation failure leaves their just-written file unrevertable (#4249). `bannerOpts.configuredEntrypoints` is the only source `finishInstall` validates — a caller other than `installAllRuntimes` that omits it gets no entrypoint validation at all. +Primary installer for all runtimes. Single production file: `bin/install.js` (hand-authored JS — it is NOT generated from `src/*.cts`; ADR-1508 keeps it hand-authored deliberately, and no `npm run build` step emits it). Exports: `install(isGlobal, runtime[, configDir])` → typed result `{ runtime, configDir, settingsPath, settings, statuslineCommand, updateBannerCommand, configuredEntrypoints }`; `uninstall(isGlobal, runtime[, configDir])`; `installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile)`; `uninstallRuntimeArtifacts(runtime, configDir, scope)`; `writeManifest(configDir, runtime)`. Runtime enum: `allRuntimes` (18 values: claude, antigravity, augment, cline, codebuddy, codex, copilot, cursor, hermes, kimi, kimi-code, kilo, opencode, pi, qwen, trae, windsurf, zcode). Directory helpers: `getDirName(runtime)` → local dir name; `getConfigDirFromHome(runtime, isGlobal)` → shell-quoted path fragment. Per-runtime global config-dir resolution is delegated to `gsd-core/bin/lib/runtime-homes.cjs:getGlobalConfigDir(runtime[, explicitDir])` — the canonical, env-var–aware projection (`explicitDir` override + opencode/kilo `*_CONFIG` file-path precedence); the legacy in-installer `getGlobalDir`/`getOpencodeGlobalDir`/`getKiloGlobalDir` were retired into it (#56). The same module exposes `detectAntigravityDirAmbiguity(opts)` — a side-effect-free probe reporting whether multiple `~/.gemini/antigravity{,-ide,-cli}` dirs coexist and which one GSD's `gsd-core/VERSION` marker (the `dot-home-nested` `probeExists`) resolves to, for installer / `/gsd-update` operator guidance when a pre-#217 install landed in the wrong sibling dir (#1441). Runtime-specific helpers: `resolveKiloConfigPath(configDir)`, `configureKiloPermissions(isGlobal[, explicitDir])`. Claude-specific permission helpers: `mergeClaudePermissions(settings)` — non-destructively appends GSD-owned allow entries (see `GSD_CLAUDE_ALLOW_PERMISSIONS`) to a Claude Code settings object and filters out the retired legacy forms (`GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS` #2278 — also retires the unscoped `Bash(npx gsd-core *)` rule (#5054); `GSD_CLAUDE_LEGACY_DENY_PERMISSIONS` #4221 — the `Read(.env*)` deny rules are retired in favor of the managed `gsd-secret-read-guard.js` hook, and an emptied `deny` array is deleted); called from `finishInstall` for `runtime === 'claude'` only; uninstall removes exactly these entries (#768). Layout-driven artifact copy/removal delegates to `gsd-core/bin/lib/runtime-artifact-layout.cjs:resolveRuntimeArtifactLayout` (throws `TypeError` for unknown runtimes). Five runtimes with non-recursive skill loaders (cline, qwen, hermes, augment, trae) use a nested router layout: 6 `gsd-ns-*` router bundles emitted as top-level skills, with concrete skills nested at `/skills//SKILL.md` (hermes prefix='': `skills/gsd/ns-*/…`). claude (reverted from nested per #924 — the Skill tool errors on unrouted names) and antigravity (one-level scan, but concrete skills must be top-level discoverable) plus the remaining skills-runtimes (cursor, codex, copilot, windsurf, codebuddy, opencode, kilo) use the flat `skills/gsd-/` layout. See Skill Surface Budget Module and Runtime Artifact Layout Module. `finishInstall` calls `assertConfiguredEntrypoints` (Runtime Hooks Surface Module, dedup'd by `(configPath, scriptPath)` first) before its own `writeSettings` — a failing settings-json-surface install/update never persists that write. `writeSettings` resolves its target through Install Engine Module's `resolveSymlinkedLeafWritePath` (#5037), the policy it shares with the Codex `hooks.json` writer: a symlinked settings file is refused unless `GSD_ALLOW_SYMLINKED_DEST` is set, and with it the write lands on the link's real target, so `atomicWriteFileSync`'s final rename cannot replace the link; a dangling link is refused either way. The #338 local-settings migration calls `assertSettingsWriteAllowed` on the shared `settings.json` before it writes `settings.local.json`, so a refusal leaves both files untouched, and `uninstall` calls it before its first removal, but only when its settings cleanup (`stripGsdFromSettings`, run once on a throwaway copy) would write the file, so a refusal there leaves every GSD file in place instead of leaving settings entries that point at deleted hooks, while a settings.json with no GSD entries, a dangling link or an unparseable file never blocks an uninstall. For Codex it applies the same policy to `hooks.json` before the first removal, and both that check and the removal run only when the file holds GSD hooks or cannot be read (Runtime Hooks Surface Module's `codexHooksJsonHasManagedHooks`). `installAllRuntimes`'s `finalize()` runs the same check once over the aggregate set before its `printSummaries()` loop calls `finishInstall` per runtime, so the per-runtime slice is checked twice; the second pass is a dozen-odd `statSync`/`accessSync` calls and is left unconditional rather than gated on a caller-supplied bypass flag. Codex/Cursor/Windsurf/Kimi/Cline writers persist their own hooks.json/config.toml/`.clinerules/hooks/PreToolUse` directly inside `install()`, ahead of `installAllRuntimes`'s aggregate `assertConfiguredEntrypoints` call, so a validation failure there is reported (and blocks the completion message) before that file is reverted where a rollback exists. Codex's `install()` result exposes two rollbacks: `rollbackInstallerMigrations` — like every other runtime's — is the installer-migrations-only closure the name describes, and the Codex-only `rollbackPreInstallSnapshot` binds `restoreCodexSnapshot` (#3245's full pre-install snapshot restore — config.toml, hooks.json, skills/, agents/, VERSION). `installAllRuntimes`'s `rollbackFinalizedInstallerMigrations` picks `rollbackPreInstallSnapshot` only when the triggering error is tagged `configuredEntrypointValidation` (set by `assertConfiguredEntrypoints`); any other finalize-stage exception gets `rollbackInstallerMigrations`, so an unrelated sibling failure (e.g. EACCES on another runtime's permission write) cannot un-install a Codex install that already succeeded. The discriminator is the error's KIND, not which runtime owns the bad entrypoint: the aggregate gate is all-or-nothing, so ANY runtime's invalid entrypoint reverts the Codex snapshot — including when Codex's own entrypoints were fine and its "Done!" summary already printed (#4249). Cursor/Windsurf/Kimi/Cline have no equivalent snapshot, so their result binds only the narrow rollback and a validation failure leaves their just-written file unrevertable (#4249). `bannerOpts.configuredEntrypoints` is the only source `finishInstall` validates — a caller other than `installAllRuntimes` that omits it gets no entrypoint validation at all. ### I/O Module Module owning the tool's CLI I/O primitives: `output()` result emission (with large-payload temp-file spillover via `GSD_TEMP_DIR`/`ensureGsdTempDir`/`reapStaleTempFiles`), `error()` stderr emission with exit-code mapping, and the JSON-error-mode toggle (`setJsonErrorMode`/`getJsonErrorMode`, `ERROR_REASON`). **Degraded result vs fault (ADR-2980, #2980):** the two emitters are a deliberate two-channel failure contract, not a drift. A **fault** is `error(message, reason)` — stderr, exit **1**, structured `{ok:false,reason,message}` envelope under `--json-errors`. A **degraded result** is `output({ error: … })` — stdout, exit **0**, `--json-errors` does not apply — and means the command ran to completion and is reporting a condition (absent artifact, and in practice also missing-argument and unusable-input cases) through its result; a caller detects it by inspecting the payload, never by exit code. Ratified across **60 sites in 9 modules** (`state` 25, `verify` 8, `workstream` 7, `frontmatter` 6, `commands` 5, `template` 3, `gsd2-import` 2, `phase` 2, `roadmap` 2) because normalizing them to exit 1 is a Hyrum's Law break over a CRITICAL radius (`get_impact(cmdStateSnapshot)`; `output` has 170 direct callers). #2966/#2980 record "42 sites" — that counts only literals whose FIRST key is `error` (the `output\(\{\s*error:` regex); 18 more put another key first (`{found:false, error}`) and are identical in contract, so 60 is the population and 42 is a subset. New code prefers the fault path or a named-field result (`{updated:false, reason}`), not a 61st site. Known cost carried by the decision: the exit code does not distinguish absent from unusable, which is ADR-1411's "corrupt is not absent" open edge. Docs: `docs/json-errors.md` → "Degraded results vs faults". Extracted from the Core module per ADR-857 rollout phase 1 (#859) so feature modules (`graphify`, `intel`, `audit`, `profile-pipeline`) depend on a small I/O seam instead of the core god-module; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/io.cjs` (generated from `src/io.cts`). @@ -516,7 +516,7 @@ A per-agent narrative entry written by `mempalace_diary_write`. GSD's `gsd-mempa The `mempalace.memory_mode` config key controlling how authoritative MemPalace is during recall/capture relative to GSD's native memory. Three wired values: `augment` (default — palace is an additive recall layer; native memory stays authoritative; lowest coupling), `kg_backend` (knowledge-graph queries resolve against MemPalace's temporal graph as the primary source, `.planning/graphs/` as fallback; non-KG drawer recall stays additive), `replace` (recall resolves through the palace as the source of truth, native artifacts as fallback). Every mode is `onError:skip` and default-resilient — an unreachable palace degrades to native memory and GSD keeps writing `.planning/graphs/`, so no mode loses memory. Read at hook-render time; switching is a config change, not a reinstall. Cross-mode migration of existing `.planning/graphs/` into the palace is a separate, not-yet-implemented concern (PRD/ADR §17 open question). See MemPalace Settings in `docs/CONFIGURATION.md`. ### Runtime Hooks Surface Module -Standalone hook-surface writer module extracted from `bin/install.js` as ADR-857 phase 5f-1 (behavior-preserving relocation, no logic change). Owns: Cline rules-body/agents-md/pre-tool-use hook generation (`buildClineRulesBody`, `buildClineAgentsMdBody`, `buildClinePreToolUseHook`, `mergeGsdAgentsMd`, `writeClineArtifacts`); Cursor `hooks.json` lifecycle (`buildCursorHookEntry`, `isManagedCursorHookEntry`, `reconcileCursorHooksJson`, `writeCursorHooksJson`, `removeCursorHooksJson`); Copilot session-hook config (`buildCopilotHookConfig`, `writeCopilotHookConfig`); Codex hook-block and event management (`buildCodexHookBlock`, `rewriteLegacyCodexHookBlock`, `reconcileCodexHooksJsonEvent`, `reconcileCodexHooksJsonSessionStart`, `ensureCodexHooksJsonSessionStart`, `removeCodexHooksJsonEvent`, `removeCodexHooksJsonSessionStart`, `buildCodexHookWindowsShimIR`, `cleanupOrphanedCodexContextMonitorScript`, `isGsdOwnedCodexContextMonitorScript`, `hooksJsonReferencesCodexContextMonitor` — #2586, the last three replacing the deleted `ensureCodexHooksJsonEvent`: GSD no longer adds Codex context-monitor hook-event registrations, only recognizes and removes stale ones left by a pre-#2586 install, then deletes the orphaned script/`.cmd` shim once unreferenced and content-verified as GSD-owned); Kimi native config.toml `[[hooks]]` lifecycle (`buildKimiHooksTomlBlock`, `stripKimiHooksTomlBlock`, `writeKimiHooksToml`, `removeKimiHooksToml` — #2095 EoS/kimi Upgrade 1, the first genuinely NEW hook surface added post-relocation rather than a behavior-preserving move: kimi's `[[hooks]]` array lives in its own native `config.toml`, resolved by `resolveKimiHooksTomlDir` in Runtime Homes Module to a directory deliberately separate from kimi's GSD configDir, wrapped in `# GSD Hooks BEGIN`/`END` marker comments for idempotent reinstall); and shared hook command helpers (`buildHookCommand`, `rewriteLegacyManagedNodeHookCommands`, `normalizeNodePath`, `resolveNodeRunner`, and — #3662 — `buildNodeRunnerChainToken`, the POSIX-sh runner token that resolves node at hook-fire time for non-portable managed JS hooks, plus the `NODE_RUNNER_RESOLVER_HOOK` basename of the staged `hooks/gsd-node-runner.sh` resolver that portable installs route through with the baked node path as its first argument). `bin/install.js` delegates to this module via thin wrappers and re-exports its functions unchanged so existing tests require no modification. Also owns post-install entrypoint validation (#4154/#4249): `buildHookCommand` and every writer's `recordConfiguredHookCommand` record a `ConfiguredEntrypoint {runtime, configPath, scriptPath, interpreterCandidates?, selfExecutable?, platform, command?}` for each hook they compute — including an already-registered hook whose register-only-if-absent guard leaves its on-disk `command` stale, since `scriptPath`/`interpreterCandidates` are derived from `configDir`/`hookName`, not from that command string. `validateConfiguredEntrypoints(entries)` runs up to three checks per entry, none of which execute anything. First, always: `statSync`s `scriptPath` (must be a file) then `accessSync(R_OK)`s it — required even for a self-executable shebang script, since its kernel-invoked interpreter still opens and reads it, not just execs it. Second, only when `selfExecutable: true`: `accessSync(X_OK)`. Every producer that needs this sets the flag explicitly, rather than it being inferred from an absent `interpreterCandidates` — a Windows-Claude `.sh` hook direct-invoked via `shellHookOmitsBashRunner`, Codex's Windows `.cmd` shim (no shebang; relies on Windows' own extension-based dispatch), and Cline's hybrid `#!/usr/bin/env node` hook all set it; the check is skipped entirely when `entry.platform` is `win32` (POSIX mode bits don't mean executable there — mirroring `resolveExecutableBinary`'s own X_OK carve-out), so Cline is the only one of the three where this check runs. Third, only when `interpreterCandidates` is present: at least one must resolve via `resolveExecutableBinary(candidate, {requireExecutable:true})` (for a Node hook, the first candidate is `normalizeNodePath`'d — the SAME stable version-manager alias `buildNodeRunnerChainToken` bakes as its own first choice, not the raw, always-resolving `process.execPath`; a `.sh` hook's candidate is its resolved `bash` path instead; Cline's is `['node']`, since unlike every other GSD JS hook — which bakes an absolute install-time-resolved node path specifically to avoid this — its interpreter is looked up on PATH by `env` at hook-fire time). Failures carry `reason: 'missing'|'unreadable'|'wrong-file-type'|'unresolved-interpreter'|'not-executable'` (`unreadable` for an `EACCES` `statSync`, distinct from a genuinely absent path). See Installer Module for the caller-side gate, which deduplicates entries by `(configPath, scriptPath)` first (writers can push the same pair more than once, e.g. Kimi's context-monitor hook across several events). Source: `src/runtime-hooks-surface.cts`. Built output: `gsd-core/bin/lib/runtime-hooks-surface.cjs`. +Standalone hook-surface writer module extracted from `bin/install.js` as ADR-857 phase 5f-1 (behavior-preserving relocation, no logic change). Owns: Cline rules-body/agents-md/pre-tool-use hook generation (`buildClineRulesBody`, `buildClineAgentsMdBody`, `buildClinePreToolUseHook`, `mergeGsdAgentsMd`, `writeClineArtifacts`); Cursor `hooks.json` lifecycle (`buildCursorHookEntry`, `isManagedCursorHookEntry`, `reconcileCursorHooksJson`, `writeCursorHooksJson`, `removeCursorHooksJson`); Copilot session-hook config (`buildCopilotHookConfig`, `writeCopilotHookConfig`); Codex hook-block and event management (`buildCodexHookBlock`, `rewriteLegacyCodexHookBlock`, `reconcileCodexHooksJsonEvent`, `reconcileCodexHooksJsonSessionStart`, `ensureCodexHooksJsonSessionStart`, `removeCodexHooksJsonEvent`, `removeCodexHooksJsonSessionStart`, `codexHooksJsonHasManagedHooks`, `buildCodexHookWindowsShimIR`, `cleanupOrphanedCodexContextMonitorScript`, `isGsdOwnedCodexContextMonitorScript`, `hooksJsonReferencesCodexContextMonitor` — #2586, the last three replacing the deleted `ensureCodexHooksJsonEvent`: GSD no longer adds Codex context-monitor hook-event registrations, only recognizes and removes stale ones left by a pre-#2586 install, then deletes the orphaned script/`.cmd` shim once unreferenced and content-verified as GSD-owned); Kimi native config.toml `[[hooks]]` lifecycle (`buildKimiHooksTomlBlock`, `stripKimiHooksTomlBlock`, `writeKimiHooksToml`, `removeKimiHooksToml` — #2095 EoS/kimi Upgrade 1, the first genuinely NEW hook surface added post-relocation rather than a behavior-preserving move: kimi's `[[hooks]]` array lives in its own native `config.toml`, resolved by `resolveKimiHooksTomlDir` in Runtime Homes Module to a directory deliberately separate from kimi's GSD configDir, wrapped in `# GSD Hooks BEGIN`/`END` marker comments for idempotent reinstall); and shared hook command helpers (`buildHookCommand`, `rewriteLegacyManagedNodeHookCommands`, `normalizeNodePath`, `resolveNodeRunner`, and — #3662 — `buildNodeRunnerChainToken`, the POSIX-sh runner token that resolves node at hook-fire time for non-portable managed JS hooks, plus the `NODE_RUNNER_RESOLVER_HOOK` basename of the staged `hooks/gsd-node-runner.sh` resolver that portable installs route through with the baked node path as its first argument). `bin/install.js` delegates to this module via thin wrappers and re-exports its functions unchanged so existing tests require no modification. Also owns post-install entrypoint validation (#4154/#4249): `buildHookCommand` and every writer's `recordConfiguredHookCommand` record a `ConfiguredEntrypoint {runtime, configPath, scriptPath, interpreterCandidates?, selfExecutable?, platform, command?}` for each hook they compute — including an already-registered hook whose register-only-if-absent guard leaves its on-disk `command` stale, since `scriptPath`/`interpreterCandidates` are derived from `configDir`/`hookName`, not from that command string. `validateConfiguredEntrypoints(entries)` runs up to three checks per entry, none of which execute anything. First, always: `statSync`s `scriptPath` (must be a file) then `accessSync(R_OK)`s it — required even for a self-executable shebang script, since its kernel-invoked interpreter still opens and reads it, not just execs it. Second, only when `selfExecutable: true`: `accessSync(X_OK)`. Every producer that needs this sets the flag explicitly, rather than it being inferred from an absent `interpreterCandidates` — a Windows-Claude `.sh` hook direct-invoked via `shellHookOmitsBashRunner`, Codex's Windows `.cmd` shim (no shebang; relies on Windows' own extension-based dispatch), and Cline's hybrid `#!/usr/bin/env node` hook all set it; the check is skipped entirely when `entry.platform` is `win32` (POSIX mode bits don't mean executable there — mirroring `resolveExecutableBinary`'s own X_OK carve-out), so Cline is the only one of the three where this check runs. Third, only when `interpreterCandidates` is present: at least one must resolve via `resolveExecutableBinary(candidate, {requireExecutable:true})` (for a Node hook, the first candidate is `normalizeNodePath`'d — the SAME stable version-manager alias `buildNodeRunnerChainToken` bakes as its own first choice, not the raw, always-resolving `process.execPath`; a `.sh` hook's candidate is its resolved `bash` path instead; Cline's is `['node']`, since unlike every other GSD JS hook — which bakes an absolute install-time-resolved node path specifically to avoid this — its interpreter is looked up on PATH by `env` at hook-fire time). Failures carry `reason: 'missing'|'unreadable'|'wrong-file-type'|'unresolved-interpreter'|'not-executable'` (`unreadable` for an `EACCES` `statSync`, distinct from a genuinely absent path). See Installer Module for the caller-side gate, which deduplicates entries by `(configPath, scriptPath)` first (writers can push the same pair more than once, e.g. Kimi's context-monitor hook across several events). Source: `src/runtime-hooks-surface.cts`. Built output: `gsd-core/bin/lib/runtime-hooks-surface.cjs`. ### Runtime Config Adapter Registry Module owning the explicit per-runtime config-mutation dispatch table for the installer. `resolveRuntimeConfigIntent(runtime)` projects a typed config intent — `installSurface` (`settings-json` | `codex-toml` | `copilot-instructions` | `cline-rules` | `cursor-hooks-json` | `profile-marker-only`), `writesSharedSettings` (the `finishInstall` shared-settings write gate), and `finishPermissionWriter` (`opencode` | `kilo` | `antigravity` | none) — that `bin/install.js` dispatches on instead of inline `runtime === '...'` branching. Owns adapter selection only: it performs no filesystem IO and does not execute config mutations (the install/finishInstall handlers and the per-runtime writers do that). Unknown runtimes fail loudly with a `TypeError`, guarded by an `Object.hasOwn` own-property check so prototype-chain keys (`__proto__`, `constructor`) also throw. Also exports `resolveInstallPlan(runtime)` — the ADR-58 `InstallPlan` capstone — which collects the install-level descriptor axes (`installSurface`, `writesSharedSettings`, `finishPermissionWriter`, `hookEvents`, `extendedHookEvents`, `hooksSurface`, `sandboxTier`) into one typed `InstallPlan` value consumed by `install()` and `finishInstall()` in `bin/install.js`. `sandboxTier` (`none` | `codex-agent-sandbox`) gates per-agent `sandbox_mode` emission in the codex TOML path and fails loud on a missing/invalid value (#1151). The spatial axes (`configHome`, `artifactLayout`, `commandStyle`) remain behind their self-resolving adapter modules and are not part of the plan; they are the execution adapters. Realizes both the adapter-selection and plan-collection halves of the Runtime Install Policy Module boundary. Source: `gsd-core/bin/lib/runtime-config-adapter-registry.cjs`. See ADR-58, #60. diff --git a/bin/install.js b/bin/install.js index 9cae7681ef..d6c50edd7e 100755 --- a/bin/install.js +++ b/bin/install.js @@ -841,6 +841,7 @@ const { _installNativePluginIfDeclared, hasExistingSymlinkBetween, isSymlinkedDestOptIn, + resolveSymlinkedLeafWritePath, migrateLegacyDevPreferencesToSkill, USER_OWNED_ARTIFACTS, _snapshotDir, @@ -1399,6 +1400,12 @@ function removeCodexHooksJsonSessionStart(targetDir) { return hooksSurface.removeCodexHooksJsonSessionStart(targetDir); } +// Whether uninstall's step 1a would remove a GSD hook from hooks.json: the +// same events it removes, SessionStart plus CODEX_EXTENDED_HOOK_EVENTS. +function codexHooksJsonHasGsdHooks(targetDir) { + return hooksSurface.codexHooksJsonHasManagedHooks(targetDir, ['SessionStart', ...CODEX_EXTENDED_HOOK_EVENTS]); +} + /** * Build a hook command path using forward slashes for cross-platform compatibility. * On Windows, $HOME is not expanded by cmd.exe/PowerShell, so we use the actual path. @@ -1498,8 +1505,11 @@ function stripJsonComments(text) { * Supports JSONC (JSON with comments) — many CLI tools allow comments in * their settings files, so we strip them before parsing to avoid silent * data loss from JSON.parse failures. + * + * `quiet` skips the malformed-file warning, for a read ahead of the one that + * reports it, so the warning is printed once. */ -function readSettings(settingsPath) { +function readSettings(settingsPath, { quiet = false } = {}) { if (fs.existsSync(settingsPath)) { try { const raw = fs.readFileSync(settingsPath, 'utf8'); @@ -1510,7 +1520,7 @@ function readSettings(settingsPath) { return parsed === null ? {} : parsed; // valid JSON null = empty settings, not malformed } catch (e) { // If even JSONC stripping fails, warn instead of silently returning {} - console.warn(' ' + yellow + '⚠' + reset + ' Warning: Could not parse ' + settingsPath + ' — file may be malformed. Existing settings preserved.'); + if (!quiet) console.warn(' ' + yellow + '⚠' + reset + ' Warning: Could not parse ' + settingsPath + ' — file may be malformed. Existing settings preserved.'); return null; } } @@ -1529,7 +1539,33 @@ function readSettings(settingsPath) { * at call time, after module evaluation, so the ordering is safe. */ function writeSettings(settingsPath, settings) { - atomicWriteFileSync(settingsPath, JSON.stringify(settings, null, 2) + '\n', 'utf8'); + atomicWriteFileSync(resolveSettingsWritePath(settingsPath), JSON.stringify(settings, null, 2) + '\n', 'utf8'); +} + +/** + * The file writeSettings should actually write for `settingsPath` (#5037): + * a symlinked settings file is refused unless GSD_ALLOW_SYMLINKED_DEST is + * set, and with it the link's real target is written so the link stays in + * place. The policy lives in install-engine's resolveSymlinkedLeafWritePath, + * shared with the Codex hooks.json writer. The install root is the settings + * file's own directory, as the hooks.json writer passes its own. + */ +function resolveSettingsWritePath(settingsPath) { + return resolveSymlinkedLeafWritePath( + path.dirname(path.resolve(settingsPath)), + settingsPath, + 'writeSettings: settings file', + ); +} + +/** + * Throws writeSettings' symlink refusal for `settingsPath` without writing + * anything. The #338 migration writes settings.local.json before it rewrites + * the shared settings.json, so it checks the shared file first: a refusal + * then leaves both files as they were instead of half-migrated. + */ +function assertSettingsWriteAllowed(settingsPath) { + resolveSettingsWritePath(settingsPath); } // #2875 Part 2 (J8): model-override resolution (readGsdGlobalModelOverrides / @@ -8429,6 +8465,157 @@ function reclaimKimiHooksRoot(kimiHooksRoot) { return steps; } +/** + * Uninstall step 6's settings.json cleanup, applied in memory to `settings` + * for `runtime` installed at `targetDir`. Returns whether anything changed, + * which is exactly when step 6 writes the file. Each removal is reported + * through `log`. uninstall() also runs it on a throwaway copy with a silent + * `log` before its first removal, to check a symlinked settings.json only + * when step 6 would write it (#5037). Sharing one function keeps that check + * and step 6 from disagreeing about whether a write happens. + */ +function stripGsdFromSettings(settings, runtime, targetDir, log) { + let settingsModified = false; + + // Remove GSD statusline if it references our hook + if (settings.statusLine && settings.statusLine.command && + settings.statusLine.command.includes('gsd-statusline')) { + delete settings.statusLine; + settingsModified = true; + log(` ${green}✓${reset} Removed GSD statusline from settings`); + } + + // Remove GSD hooks from settings — per-hook granularity to preserve + // user hooks that share an entry with a GSD hook (#1755 followup). + // Includes the 3 Qwen-only events added in #788 (SubagentStop, Stop, + // PreCompact, also registered for Claude in #770), the 3 Antigravity-only + // events added in #776 (BeforeAgent, AfterAgent, BeforeModel), and the + // Claude-only FileChanged event added in #770 — safe to iterate for all + // runtimes; installs that don't register these events simply find no + // entries and skip. + for (const eventName of ['SessionStart', 'PostToolUse', 'AfterTool', 'PreToolUse', 'BeforeTool', 'SubagentStop', 'Stop', 'PreCompact', 'BeforeAgent', 'AfterAgent', 'BeforeModel', 'FileChanged']) { + if (settings.hooks && settings.hooks[eventName]) { + const before = JSON.stringify(settings.hooks[eventName]); + settings.hooks[eventName] = settings.hooks[eventName] + .map(entry => { + if (!entry || typeof entry !== 'object' || !Array.isArray(entry.hooks)) return entry; + // Filter out individual GSD hooks, keep user hooks + entry.hooks = entry.hooks.filter((h) => { + if (!h || typeof h.command !== 'string') return true; + return !isManagedHookCommand(h.command, { + surface: 'settings-json', + }); + }); + return entry.hooks.length > 0 ? entry : null; + }) + .filter(Boolean); + if (JSON.stringify(settings.hooks[eventName]) !== before) { + settingsModified = true; + } + if (settings.hooks[eventName].length === 0) { + delete settings.hooks[eventName]; + } + } + } + if (settingsModified) { + log(` ${green}✓${reset} Removed GSD hooks from settings`); + } + + // Clean up empty hooks object + if (settings.hooks && Object.keys(settings.hooks).length === 0) { + delete settings.hooks; + } + + // #768 — Remove GSD-owned Claude permissions from settings.json. + // Applies only to Claude uninstalls. Filter only the exact GSD-owned entries + // to preserve any user-added allow/deny entries. + // Uses a local flag to avoid the shared `settingsModified` producing a false + // "Removed GSD permissions" message when only hooks/statusline changed. + if (hostBehaviorsFor(runtime).permissionsSchema === 'claude' && settings.permissions) { + let permissionsModified = false; + if (Array.isArray(settings.permissions.allow)) { + const before = settings.permissions.allow.length; + // #2278 — filter against the union of the current allow-rule forms + // AND the retired legacy forms, so uninstall still cleans up + // pre-fix installs that still carry the stale `Write(...)` entries. + settings.permissions.allow = settings.permissions.allow.filter( + (e) => !GSD_CLAUDE_ALLOW_PERMISSIONS.includes(e) && !GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS.includes(e) + ); + if (settings.permissions.allow.length !== before) { + permissionsModified = true; + // #4221: an array this filter emptied was GSD-only \u2014 remove the + // key rather than leave an empty array behind (Antigravity symmetry). + if (settings.permissions.allow.length === 0) { + delete settings.permissions.allow; + } + } + } + if (Array.isArray(settings.permissions.deny)) { + const before = settings.permissions.deny.length; + // #4221: the deny rules are retired, so this is a legacy-only filter. + settings.permissions.deny = settings.permissions.deny.filter( + (e) => !GSD_CLAUDE_LEGACY_DENY_PERMISSIONS.includes(e) + ); + if (settings.permissions.deny.length !== before) { + permissionsModified = true; + if (settings.permissions.deny.length === 0) { + delete settings.permissions.deny; + } + } + } + if (permissionsModified && Object.keys(settings.permissions).length === 0) { + delete settings.permissions; + } + if (permissionsModified) { + settingsModified = true; + log(` ${green}✓${reset} Removed GSD permissions from settings.json`); + } + } + + // #2096 Phase B Upgrade 1 — Remove GSD-owned Antigravity permissions.allow + // rules from settings.json. Symmetric to the Claude branch above: filters + // only the exact GSD-owned rule strings (regenerated from the current + // configDir) to preserve any user-added allow entries and all deny/ask. + if (resolveInstallPlan(runtime).finishPermissionWriter === 'antigravity' && settings.permissions) { + let antigravityPermissionsModified = false; + if (Array.isArray(settings.permissions.allow)) { + const gsdRules = new Set(buildAntigravityAllowRules(targetDir)); + const before = settings.permissions.allow.length; + settings.permissions.allow = settings.permissions.allow.filter((e) => !gsdRules.has(e)); + if (settings.permissions.allow.length !== before) { + antigravityPermissionsModified = true; + } + if (settings.permissions.allow.length === 0) { + delete settings.permissions.allow; + } + } + if (Object.keys(settings.permissions).length === 0) { + delete settings.permissions; + } + if (antigravityPermissionsModified) { + settingsModified = true; + log(` ${green}✓${reset} Removed GSD permissions from settings.json`); + } + } + + // #2097 UPGRADE 3 — Remove the MCP companion entry from settings.json for + // runtimes that host MCP there (Augment), symmetric to the mcp_config.json + // removal for Antigravity below. Only the GSD-owned mcpServers.gsd key is + // removed — any other user-configured MCP servers are preserved. + if (hostBehaviorsFor(runtime).mcpCompanion === 'settings-json' && + settings.mcpServers && typeof settings.mcpServers === 'object' && + settings.mcpServers.gsd !== undefined) { + delete settings.mcpServers.gsd; + if (Object.keys(settings.mcpServers).length === 0) { + delete settings.mcpServers; + } + settingsModified = true; + log(` ${green}✓${reset} Removed GSD MCP companion server from settings.json`); + } + + return settingsModified; +} + /** * Uninstall GSD from the specified directory for a specific runtime * Removes only GSD-specific files/directories, preserves user content @@ -8474,6 +8661,27 @@ function uninstall(isGlobal, runtime = DEFAULT_RUNTIME) { console.log(` Uninstalling GSD from ${cyan}${runtimeLabel}${reset} at ${cyan}${locationLabel}${reset}\n`); + // Step 6 rewrites settings.json after every GSD file is gone, so a symlinked + // settings.json that writeSettings would refuse (#5037) has to stop the + // uninstall here, before the first removal. Refusing at step 6 would leave + // settings entries pointing at hooks that were already deleted. Step 6's + // own cleanup runs here on a throwaway copy, and only a settings.json it + // would write is checked: one without GSD entries, a dangling link or an + // unparseable file is never written, so it must not block the uninstall. + const settingsPath = path.join(targetDir, 'settings.json'); + if (fs.existsSync(settingsPath)) { + const settingsPreview = readSettings(settingsPath, { quiet: true }); + if (settingsPreview !== null && stripGsdFromSettings(settingsPreview, runtime, targetDir, () => {})) { + assertSettingsWriteAllowed(settingsPath); + } + } + // Codex's hooks.json goes through the same policy at step 1a, after skills, + // agents and config.toml are already handled, so it is checked here too, + // and likewise only when step 1a has GSD hooks to remove from it. + if (hostBehaviorsFor(runtime).tomlConfigInstall && codexHooksJsonHasGsdHooks(targetDir)) { + resolveSymlinkedLeafWritePath(targetDir, path.join(targetDir, 'hooks.json'), 'hooks.json at'); + } + // #786: AGENTS.md lives at the repo root (outside targetDir) for local Copilot // installs, so its cleanup must run even when .github (targetDir) was already // removed — i.e. BEFORE the "target directory missing" early-return below. @@ -8601,20 +8809,25 @@ function uninstall(isGlobal, runtime = DEFAULT_RUNTIME) { } } - const hooksJsonCleanup = removeCodexHooksJsonSessionStart(targetDir); - if (hooksJsonCleanup.changed) { - removedCount++; - console.log(` ${green}✓${reset} Removed managed Codex SessionStart hook from hooks.json`); - } - - // #772/#2088: remove every managed Codex extended hook-event registration. - // Shares CODEX_EXTENDED_HOOK_EVENTS with the install loop — removal set == - // registration set, so no managed event is ever orphaned. - for (const eventName of CODEX_EXTENDED_HOOK_EVENTS) { - const eventCleanup = removeCodexHooksJsonEvent(targetDir, eventName); - if (eventCleanup.changed) { + // The removals write hooks.json through the symlink policy (#5037), which + // refuses a dangling link even with GSD_ALLOW_SYMLINKED_DEST, so a + // hooks.json with no GSD hooks to remove is not touched at all. + if (codexHooksJsonHasGsdHooks(targetDir)) { + const hooksJsonCleanup = removeCodexHooksJsonSessionStart(targetDir); + if (hooksJsonCleanup.changed) { removedCount++; - console.log(` ${green}✓${reset} Removed managed Codex ${eventName} hook from hooks.json`); + console.log(` ${green}✓${reset} Removed managed Codex SessionStart hook from hooks.json`); + } + + // #772/#2088: remove every managed Codex extended hook-event registration. + // Shares CODEX_EXTENDED_HOOK_EVENTS with the install loop — removal set == + // registration set, so no managed event is ever orphaned. + for (const eventName of CODEX_EXTENDED_HOOK_EVENTS) { + const eventCleanup = removeCodexHooksJsonEvent(targetDir, eventName); + if (eventCleanup.changed) { + removedCount++; + console.log(` ${green}✓${reset} Removed managed Codex ${eventName} hook from hooks.json`); + } } } // #2586: uninstall's own symmetric half of the orphaned-script cleanup — @@ -9192,152 +9405,13 @@ function uninstall(isGlobal, runtime = DEFAULT_RUNTIME) { } // 6. Clean up settings.json (remove GSD hooks and statusline) - const settingsPath = path.join(targetDir, 'settings.json'); if (fs.existsSync(settingsPath)) { let settings = readSettings(settingsPath); if (settings === null) { console.log(` ${yellow}i${reset} Skipping settings.json cleanup — file could not be parsed`); settings = {}; // prevent downstream crashes, but don't write back } - let settingsModified = false; - - // Remove GSD statusline if it references our hook - if (settings.statusLine && settings.statusLine.command && - settings.statusLine.command.includes('gsd-statusline')) { - delete settings.statusLine; - settingsModified = true; - console.log(` ${green}✓${reset} Removed GSD statusline from settings`); - } - - // Remove GSD hooks from settings — per-hook granularity to preserve - // user hooks that share an entry with a GSD hook (#1755 followup). - // Includes the 3 Qwen-only events added in #788 (SubagentStop, Stop, - // PreCompact, also registered for Claude in #770), the 3 Antigravity-only - // events added in #776 (BeforeAgent, AfterAgent, BeforeModel), and the - // Claude-only FileChanged event added in #770 — safe to iterate for all - // runtimes; installs that don't register these events simply find no - // entries and skip. - for (const eventName of ['SessionStart', 'PostToolUse', 'AfterTool', 'PreToolUse', 'BeforeTool', 'SubagentStop', 'Stop', 'PreCompact', 'BeforeAgent', 'AfterAgent', 'BeforeModel', 'FileChanged']) { - if (settings.hooks && settings.hooks[eventName]) { - const before = JSON.stringify(settings.hooks[eventName]); - settings.hooks[eventName] = settings.hooks[eventName] - .map(entry => { - if (!entry || typeof entry !== 'object' || !Array.isArray(entry.hooks)) return entry; - // Filter out individual GSD hooks, keep user hooks - entry.hooks = entry.hooks.filter((h) => { - if (!h || typeof h.command !== 'string') return true; - return !isManagedHookCommand(h.command, { - surface: 'settings-json', - }); - }); - return entry.hooks.length > 0 ? entry : null; - }) - .filter(Boolean); - if (JSON.stringify(settings.hooks[eventName]) !== before) { - settingsModified = true; - } - if (settings.hooks[eventName].length === 0) { - delete settings.hooks[eventName]; - } - } - } - if (settingsModified) { - console.log(` ${green}✓${reset} Removed GSD hooks from settings`); - } - - // Clean up empty hooks object - if (settings.hooks && Object.keys(settings.hooks).length === 0) { - delete settings.hooks; - } - - // #768 — Remove GSD-owned Claude permissions from settings.json. - // Applies only to Claude uninstalls. Filter only the exact GSD-owned entries - // to preserve any user-added allow/deny entries. - // Uses a local flag to avoid the shared `settingsModified` producing a false - // "Removed GSD permissions" message when only hooks/statusline changed. - if (hostBehaviorsFor(runtime).permissionsSchema === 'claude' && settings.permissions) { - let permissionsModified = false; - if (Array.isArray(settings.permissions.allow)) { - const before = settings.permissions.allow.length; - // #2278 — filter against the union of the current allow-rule forms - // AND the retired legacy forms, so uninstall still cleans up - // pre-fix installs that still carry the stale `Write(...)` entries. - settings.permissions.allow = settings.permissions.allow.filter( - (e) => !GSD_CLAUDE_ALLOW_PERMISSIONS.includes(e) && !GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS.includes(e) - ); - if (settings.permissions.allow.length !== before) { - permissionsModified = true; - // #4221: an array this filter emptied was GSD-only \u2014 remove the - // key rather than leave an empty array behind (Antigravity symmetry). - if (settings.permissions.allow.length === 0) { - delete settings.permissions.allow; - } - } - } - if (Array.isArray(settings.permissions.deny)) { - const before = settings.permissions.deny.length; - // #4221: the deny rules are retired, so this is a legacy-only filter. - settings.permissions.deny = settings.permissions.deny.filter( - (e) => !GSD_CLAUDE_LEGACY_DENY_PERMISSIONS.includes(e) - ); - if (settings.permissions.deny.length !== before) { - permissionsModified = true; - if (settings.permissions.deny.length === 0) { - delete settings.permissions.deny; - } - } - } - if (permissionsModified && Object.keys(settings.permissions).length === 0) { - delete settings.permissions; - } - if (permissionsModified) { - settingsModified = true; - console.log(` ${green}✓${reset} Removed GSD permissions from settings.json`); - } - } - - // #2096 Phase B Upgrade 1 — Remove GSD-owned Antigravity permissions.allow - // rules from settings.json. Symmetric to the Claude branch above: filters - // only the exact GSD-owned rule strings (regenerated from the current - // configDir) to preserve any user-added allow entries and all deny/ask. - if (resolveInstallPlan(runtime).finishPermissionWriter === 'antigravity' && settings.permissions) { - let antigravityPermissionsModified = false; - if (Array.isArray(settings.permissions.allow)) { - const gsdRules = new Set(buildAntigravityAllowRules(targetDir)); - const before = settings.permissions.allow.length; - settings.permissions.allow = settings.permissions.allow.filter((e) => !gsdRules.has(e)); - if (settings.permissions.allow.length !== before) { - antigravityPermissionsModified = true; - } - if (settings.permissions.allow.length === 0) { - delete settings.permissions.allow; - } - } - if (Object.keys(settings.permissions).length === 0) { - delete settings.permissions; - } - if (antigravityPermissionsModified) { - settingsModified = true; - console.log(` ${green}✓${reset} Removed GSD permissions from settings.json`); - } - } - - // #2097 UPGRADE 3 — Remove the MCP companion entry from settings.json for - // runtimes that host MCP there (Augment), symmetric to the mcp_config.json - // removal for Antigravity below. Only the GSD-owned mcpServers.gsd key is - // removed — any other user-configured MCP servers are preserved. - if (hostBehaviorsFor(runtime).mcpCompanion === 'settings-json' && - settings.mcpServers && typeof settings.mcpServers === 'object' && - settings.mcpServers.gsd !== undefined) { - delete settings.mcpServers.gsd; - if (Object.keys(settings.mcpServers).length === 0) { - delete settings.mcpServers; - } - settingsModified = true; - console.log(` ${green}✓${reset} Removed GSD MCP companion server from settings.json`); - } - - if (settingsModified) { + if (stripGsdFromSettings(settings, runtime, targetDir, console.log)) { writeSettings(settingsPath, settings); removedCount++; } @@ -13471,6 +13545,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } } } + assertSettingsWriteAllowed(sharedSettingsPath); fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); writeSettings(settingsPath, localRaw); @@ -14949,6 +15024,8 @@ module.exports = { install, installAllRuntimes, uninstall, + // #5037: uninstall's settings.json cleanup, shared by its pre-check and step 6 + stripGsdFromSettings, // #3023 — shared hook bundle directory name, descriptor-driven SHARED_HOOKS_DIR_DEFAULT, resolveSharedHooksDirName, diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index e4116c8807..280abc71e7 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -2286,7 +2286,7 @@ Use `provider: "generic"` (or `"custom"`) for OpenRouter, LiteLLM, local gateway | `GSD_PROJECT` | Override project root for multi-project workspace support (v1.32) | | `GSD_SKIP_SCHEMA_CHECK` | Skip schema drift detection during execute-phase (v1.31) | | `GSD_EXIT_CONTRACT` | Select the exit-code projection: `v1` (default) or `v2`. See [Exit-code contract](#exit-code-contract-gsd_exit_contract) below. | -| `GSD_ALLOW_SYMLINKED_DEST` | Set to `1` (or `true`) to permit install/update when `CLAUDE_CONFIG_DIR` (or any artifact-kind child like `skills/`, `hooks/`) is an **intentional, user-owned symlink** pointing outside the install root. v1.7.x write-confinement (ADR-1239 Phase B) refuses such layouts by default to prevent untrusted `destSubpath` traversal. Opt in only if you manage configHome via symlinked external dirs, multi-account config layouts (`~/.claude-personal`, `~/.claude-team`), or dotfiles-managed configHome (nix-darwin, etc.). Two refusals remain load-bearing even with opt-in: path-traversal in `destSubpath` (`../../etc`-style), and a symlink whose resolved target equals the install root itself (would let the prune pass wipe it). | +| `GSD_ALLOW_SYMLINKED_DEST` | Set to `1` (or `true`) to permit install/update when `CLAUDE_CONFIG_DIR` (or any artifact-kind child like `skills/`, `hooks/`) is an **intentional, user-owned symlink** pointing outside the install root. v1.7.x write-confinement (ADR-1239 Phase B) refuses such layouts by default to prevent untrusted `destSubpath` traversal. Opt in only if you manage configHome via symlinked external dirs, multi-account config layouts (`~/.claude-personal`, `~/.claude-team`), or dotfiles-managed configHome (nix-darwin, etc.). A symlinked settings file (for example a dotfiles-managed `settings.json`) follows the same rule: without the opt-in the install refuses to write it, and with the opt-in GSD writes to the link's target and leaves the link in place. A settings link whose target is missing is refused either way rather than replaced. Two refusals remain load-bearing even with opt-in: path-traversal in `destSubpath` (`../../etc`-style), and a symlink whose resolved target equals the install root itself (would let the prune pass wipe it). | | `NODE_DISABLE_COMPILE_CACHE` | Set (to any value) to turn off Node's compile cache, which `gsd-tools` and the hooks that load `gsd-core/bin/lib` enable to start faster. The cache lives in `node-compile-cache` under the OS temp directory, or in `NODE_COMPILE_CACHE` if you set it. Node reads both variables itself. | | `WSL_DISTRO_NAME` | Detected by installer for WSL path handling | diff --git a/src/install-engine.cts b/src/install-engine.cts index 3c9fa2ce51..d1becd7e93 100644 --- a/src/install-engine.cts +++ b/src/install-engine.cts @@ -471,6 +471,65 @@ function hasExistingSymlinkBetween( return false; } +/** + * Whether a symlink's target is genuinely absent. Only ENOENT and ENOTDIR mean + * that: a link loop (ELOOP) or an unreadable directory (EACCES) also fail to + * resolve, but saying "the target does not exist" there would be wrong. + */ +function linkTargetIsMissing(linkPath: string): boolean { + try { + installFs().realpathSync(linkPath); + return false; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + return code === 'ENOENT' || code === 'ENOTDIR'; + } +} + +/** + * The path a temp-file-and-rename save (`atomicWriteFileSync`, + * `platformWriteSync`) should be handed for `targetPath`, a file under + * `installRoot` that may be a user-owned symlink (#5037). + * + * A rename onto a symlink replaces the link with a plain file, so the user's + * real file silently stops receiving changes. A symlinked `targetPath` is + * therefore refused unless `GSD_ALLOW_SYMLINKED_DEST` is set, and with the + * opt-in the link's real target is returned, so the rename lands there and the + * link stays in place. A plain or missing file is returned unchanged. A + * dangling link is refused either way (`tryLstat` sees it where `existsSync` + * would not), the same as `hasExistingSymlinkBetween` treats one. + * + * `label` starts the refusal message and names the writer, e.g. + * `writeSettings: settings file`. When the opt-in is already set, telling the + * user to re-run with it would send them in a circle, so those refusals say + * what is actually wrong instead: the link's target is missing, or it resolves + * to the install root itself (which `hasExistingSymlinkBetween` always refuses). + */ +function resolveSymlinkedLeafWritePath(installRoot: string, targetPath: string, label: string): string { + const stat = tryLstat(targetPath); + if (!stat || !stat.isSymbolicLink()) return targetPath; + const optIn = isSymlinkedDestOptIn(); + if (hasExistingSymlinkBetween(installRoot, targetPath, { allowOptInFollow: optIn })) { + if (optIn && linkTargetIsMissing(targetPath)) { + throw new Error( + `${label} "${targetPath}" is a symlink whose target does not exist, so it was not written. ` + + 'Point the link at an existing file or remove it, then re-run.', + ); + } + if (optIn) { + throw new Error( + `${label} "${targetPath}" is a symlink GSD will not follow even with GSD_ALLOW_SYMLINKED_DEST set, ` + + `because it resolves to the install root "${installRoot}" itself or cannot be resolved, so it was not written.`, + ); + } + throw new Error( + `${label} "${targetPath}" is a symlink the install root "${installRoot}" does not trust, so it was not written. ` + + 'If this is an intentional user-owned symlink layout, re-run with GSD_ALLOW_SYMLINKED_DEST=1.', + ); + } + return installFs().realpathSync(targetPath); +} + // --------------------------------------------------------------------------- // User-artifact staging root // --------------------------------------------------------------------------- @@ -2210,6 +2269,7 @@ export = { _copyStaged, hasExistingSymlinkBetween, isSymlinkedDestOptIn, + resolveSymlinkedLeafWritePath, _resolveUserArtifactStagingRoot, _tryResolveUserArtifactStagingRoot, migrateLegacyDevPreferencesToSkill, diff --git a/src/runtime-hooks-surface.cts b/src/runtime-hooks-surface.cts index c2fae0c359..c712d9aa95 100644 --- a/src/runtime-hooks-surface.cts +++ b/src/runtime-hooks-surface.cts @@ -13,6 +13,7 @@ * Codex hooks.json: ensureCodexHooksJsonSessionStart, ensureCodexHooksJsonEvent, * reconcileCodexHooksJsonEvent, reconcileCodexHooksJsonSessionStart, * removeCodexHooksJsonEvent, removeCodexHooksJsonSessionStart, + * codexHooksJsonHasManagedHooks, * buildCodexHookWindowsShimIR, buildCodexHookBlock, rewriteLegacyCodexHookBlock * Shared: buildHookCommand, rewriteLegacyManagedNodeHookCommands * @@ -949,14 +950,14 @@ interface ReconcileResult { } /** - * Lazily require install-engine.cjs's `hasExistingSymlinkBetween` / - * `isSymlinkedDestOptIn` — mirrors user-artifact-staging.cts's - * `_installEngineSymlinkGuard` (same call-time-require rationale: avoid a - * static circular require between install-engine.cts and this module). + * Lazily require install-engine.cjs's `resolveSymlinkedLeafWritePath`, which + * applies the `hasExistingSymlinkBetween` / `isSymlinkedDestOptIn` policy to a + * single file. Mirrors user-artifact-staging.cts's `_installEngineSymlinkGuard` + * (same call-time-require rationale: avoid a static circular require between + * install-engine.cts and this module). */ interface InstallEngineSymlinkGuard { - hasExistingSymlinkBetween: (root: string, fullPath: string, options?: { allowOptInFollow?: boolean }) => boolean; - isSymlinkedDestOptIn: () => boolean; + resolveSymlinkedLeafWritePath: (installRoot: string, targetPath: string, label: string) => string; } function _installEngineSymlinkGuard(): InstallEngineSymlinkGuard { @@ -965,6 +966,21 @@ function _installEngineSymlinkGuard(): InstallEngineSymlinkGuard { return mod; } +/** + * Whether one hook object in a Codex hooks.json event entry is GSD's. The + * removal in reconcileCodexHooksJsonEvent and the read-only + * codexHooksJsonHasManagedHooks both ask this, so they agree on what a + * removal would delete. + */ +function isManagedCodexHooksJsonHook(hook: unknown, targetDir: string): boolean { + const cmd = hook && typeof hook === 'object' ? (hook as Record)['command'] : null; + return isManagedHookCommand(cmd as string | null | undefined, { + surface: 'codex-hooks-json', + includeLegacyAliases: true, + configDir: targetDir, + }); +} + function reconcileCodexHooksJsonEvent(targetDir: string, eventName: string, opts: ReconcileCodexOpts = {}): ReconcileResult { const hooksJsonPath = path.join(targetDir, 'hooks.json'); const managedCommand = typeof opts.managedCommand === 'string' ? opts.managedCommand : null; @@ -972,40 +988,19 @@ function reconcileCodexHooksJsonEvent(targetDir: string, eventName: string, opts const matcher = typeof opts.matcher === 'string' ? opts.matcher : undefined; const timeout = typeof opts.timeout === 'number' ? opts.timeout : undefined; // #2586 Major 2: every Codex hooks.json writer funnels through this one - // function, and atomicWriteFileSync's final step is a rename(2) onto - // `hooksJsonPath` — which, when that path is a symlink, REPLACES the - // symlink with a plain file rather than writing through it. Refuse (with - // the same GSD_ALLOW_SYMLINKED_DEST opt-in every other install call site - // honors) before reading or writing, so a symlinked hooks.json is neither - // silently destroyed nor left the caller no escape hatch. - const symlinkGuard = _installEngineSymlinkGuard(); - // The path this function actually reads/writes. Defaults to the nominal - // hooks.json path; reassigned below to the symlink's real target when the - // opt-in is active, so the write lands on the file the user's symlink - // points at instead of clobbering the symlink itself (see note below). - let effectiveHooksJsonPath = hooksJsonPath; - if (fs.existsSync(hooksJsonPath) && fs.lstatSync(hooksJsonPath).isSymbolicLink()) { - if ( - symlinkGuard.hasExistingSymlinkBetween(targetDir, hooksJsonPath, { - allowOptInFollow: symlinkGuard.isSymlinkedDestOptIn(), - }) - ) { - throw new Error( - `hooks.json at "${hooksJsonPath}" contains a symlink the install root "${targetDir}" does not trust — ` + - 'refusing to read or write it. If this is an intentional user-owned symlink layout, re-run with ' + - 'GSD_ALLOW_SYMLINKED_DEST=1.', - ); - } - // hasExistingSymlinkBetween returned false only because the opt-in is - // active (a symlinked leaf always trips it otherwise) — so this IS a - // symlink and we are cleared to follow it. atomicWriteFileSync's final - // step is a rename(2) onto its target, which REPLACES an existing - // symlink at that path rather than writing through it; resolving to the - // real path here makes the read AND the write operate on the symlink's - // target, leaving the symlink itself untouched, matching what "follow" - // is supposed to mean. - effectiveHooksJsonPath = fs.realpathSync(hooksJsonPath); - } + // function, and atomicWriteFileSync's final step is a rename(2) onto its + // target, which replaces a symlink at that path with a plain file instead + // of writing through it. install-engine's resolveSymlinkedLeafWritePath, + // shared with writeSettings (#5037), refuses a symlinked hooks.json unless + // GSD_ALLOW_SYMLINKED_DEST is set, and with the opt-in returns the link's + // real target, so the read AND the write below operate on the file the + // user's symlink points at and the link itself stays in place. A dangling + // hooks.json link is refused rather than replaced, the same as writeSettings. + const effectiveHooksJsonPath = _installEngineSymlinkGuard().resolveSymlinkedLeafWritePath( + targetDir, + hooksJsonPath, + 'hooks.json at', + ); let parsed: Record = {}; let currentContent: string | null = null; if (fs.existsSync(effectiveHooksJsonPath)) { @@ -1060,12 +1055,7 @@ function reconcileCodexHooksJsonEvent(targetDir: string, eventName: string, opts continue; } const keptHooks = originalHooks.filter((hook) => { - const cmd = hook && typeof hook === 'object' ? (hook as Record)['command'] : null; - const managed = isManagedHookCommand(cmd as string | null | undefined, { - surface: 'codex-hooks-json', - includeLegacyAliases: true, - configDir: targetDir, - }); + const managed = isManagedCodexHooksJsonHook(hook, targetDir); if (managed) removedLegacy = true; return !managed; }); @@ -1302,6 +1292,49 @@ function removeCodexHooksJsonSessionStart(targetDir: string): ReconcileResult { return reconcileCodexHooksJsonSessionStart(targetDir, { managedCommand: null }); } +/** + * Read-only: whether removing `eventNames` from `targetDir`'s hooks.json would + * delete a GSD hook. The removal writes hooks.json through the symlink policy + * (#5037), which refuses a symlinked file without GSD_ALLOW_SYMLINKED_DEST and + * a dangling one either way, so uninstall asks this first and leaves alone a + * hooks.json it has nothing to remove from. A dangling link reads as absent. + * Entries are found where the removal finds them, under the nested `hooks` + * table or as a legacy top-level event array. A file that cannot be read or + * parsed counts as holding GSD hooks, so the removal still runs and reports + * the parse error as before. + */ +function codexHooksJsonHasManagedHooks(targetDir: string, eventNames: readonly string[]): boolean { + const hooksJsonPath = path.join(targetDir, 'hooks.json'); + if (!fs.existsSync(hooksJsonPath)) return false; + let parsed: unknown; + try { + const raw = fs.readFileSync(hooksJsonPath, 'utf8'); + if (!raw.trim()) return false; + parsed = JSON.parse(raw); + } catch { + return true; + } + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return false; + const root = parsed as Record; + const nested = root['hooks'] && typeof root['hooks'] === 'object' && !Array.isArray(root['hooks']) + ? (root['hooks'] as Record) + : {}; + for (const eventName of eventNames) { + for (const table of [root, nested]) { + const entries = table[eventName]; + if (!Array.isArray(entries)) continue; + for (const entry of entries) { + if (!entry || typeof entry !== 'object' || Array.isArray(entry)) continue; + const hooks = (entry as Record)['hooks']; + if (Array.isArray(hooks) && hooks.some((hook) => isManagedCodexHooksJsonHook(hook, targetDir))) { + return true; + } + } + } + } + return false; +} + // --------------------------------------------------------------------------- // #2586: cleanupOrphanedCodexContextMonitorScript // --------------------------------------------------------------------------- @@ -3299,6 +3332,7 @@ export = { ensureCodexHooksJsonEvent, removeCodexHooksJsonEvent, removeCodexHooksJsonSessionStart, + codexHooksJsonHasManagedHooks, buildCodexHookWindowsShimIR, cleanupOrphanedCodexContextMonitorScript, isGsdOwnedCodexContextMonitorScript, diff --git a/tests/codex-config-install.test.cjs b/tests/codex-config-install.test.cjs index afb6f32bfa..a16b53e538 100644 --- a/tests/codex-config-install.test.cjs +++ b/tests/codex-config-install.test.cjs @@ -2606,6 +2606,7 @@ describe('#2586 Codex context-monitor: stop installing, clean up on reinstall', cleanupOrphanedCodexContextMonitorScript, isGsdOwnedCodexContextMonitorScript, hooksJsonReferencesCodexContextMonitor, + codexHooksJsonHasManagedHooks, } = require('../gsd-core/bin/lib/runtime-hooks-surface.cjs'); let codexHome; @@ -2809,6 +2810,119 @@ describe('#2586 Codex context-monitor: stop installing, clean up on reinstall', assert.ok(fs.existsSync(realHooksJson), 'the symlink target must have been written through'); }); + // A dangling hooks.json link is refused, never replaced, the same as a + // dangling settings.json link (#5037). With the opt-in already set, the + // refusal names the missing target instead of asking for the opt-in again. + for (const optIn of [false, true]) { + test(`a dangling hooks.json link is refused and left in place (GSD_ALLOW_SYMLINKED_DEST=${optIn ? '1' : 'unset'}) (#5037)`, () => { + runCodexInstall(codexHome); + const missingTarget = path.join(codexHome, 'missing-hooks.json'); + fs.unlinkSync(hooksJsonPath(codexHome)); + fs.symlinkSync(missingTarget, hooksJsonPath(codexHome)); + if (optIn) process.env.GSD_ALLOW_SYMLINKED_DEST = '1'; + + assert.throws(() => runCodexInstall(codexHome), optIn ? /target does not exist/ : /GSD_ALLOW_SYMLINKED_DEST=1/); + + assert.ok(fs.lstatSync(hooksJsonPath(codexHome)).isSymbolicLink(), 'the dangling link must survive'); + assert.strictEqual(fs.readlinkSync(hooksJsonPath(codexHome)), missingTarget, 'the link must still point at the same target'); + assert.strictEqual(fs.existsSync(missingTarget), false, 'nothing may be created at the missing target'); + }); + } + + const UNINSTALL_HOOK_EVENTS = ['SessionStart', ...CODEX_EXTENDED_HOOK_EVENTS]; + + // Uninstall reaches hooks.json only after removing skills and agents, so a + // hooks.json it would write and the policy refuses must stop it before the + // first removal (#5037). + test('uninstall over a symlinked hooks.json holding GSD hooks without GSD_ALLOW_SYMLINKED_DEST refuses before removing any skill (#5037)', () => { + runCodexInstall(codexHome); + const skillsBefore = fs.readdirSync(codexSkillsRoot(codexHome)).sort(); + assert.ok(skillsBefore.length > 0, 'setup: the install must have created skills'); + const target = path.join(codexHome, 'real-hooks.json'); + fs.copyFileSync(hooksJsonPath(codexHome), target); + assert.ok(codexHooksJsonHasManagedHooks(codexHome, UNINSTALL_HOOK_EVENTS), 'setup: hooks.json must hold GSD hooks'); + fs.unlinkSync(hooksJsonPath(codexHome)); + fs.symlinkSync(target, hooksJsonPath(codexHome)); + + assert.throws(() => runCodexUninstall(codexHome), /symlink/); + + assert.deepStrictEqual(fs.readdirSync(codexSkillsRoot(codexHome)).sort(), skillsBefore, + 'no skill may be removed before the refusal'); + assert.ok(fs.lstatSync(hooksJsonPath(codexHome)).isSymbolicLink(), 'hooks.json must still be a symlink'); + }); + + // Asserts the uninstall removed every skill the install had created. + function assertSkillsRemoved(skillsBefore) { + const remaining = fs.existsSync(codexSkillsRoot(codexHome)) ? fs.readdirSync(codexSkillsRoot(codexHome)) : []; + assert.deepStrictEqual(skillsBefore.filter((skill) => remaining.includes(skill)), [], + 'uninstall must remove every GSD skill'); + } + + // A hooks.json holding no GSD hook for uninstall to remove is never written, + // so a link GSD would not write must not block the uninstall (#5037). + test('uninstall over a symlinked hooks.json holding no GSD hooks without GSD_ALLOW_SYMLINKED_DEST succeeds and leaves it untouched (#5037)', () => { + runCodexInstall(codexHome); + const skillsBefore = fs.readdirSync(codexSkillsRoot(codexHome)).sort(); + assert.ok(skillsBefore.length > 0, 'setup: the install must have created skills'); + const target = path.join(codexHome, 'real-hooks.json'); + // Compact on purpose: any rewrite would reformat it. + const before = JSON.stringify({ hooks: { UnrelatedEvent: [{ hooks: [{ type: 'command', command: 'echo unrelated' }] }] } }); + fs.writeFileSync(target, before, 'utf8'); + fs.unlinkSync(hooksJsonPath(codexHome)); + fs.symlinkSync(target, hooksJsonPath(codexHome)); + + assert.doesNotThrow(() => runCodexUninstall(codexHome)); + + assertSkillsRemoved(skillsBefore); + assert.ok(fs.lstatSync(hooksJsonPath(codexHome)).isSymbolicLink(), 'hooks.json must still be a symlink'); + assert.strictEqual(fs.readlinkSync(hooksJsonPath(codexHome)), target, 'the link must still point at the same target'); + assert.strictEqual(fs.readFileSync(target, 'utf8'), before, 'the link target must be byte-for-byte unchanged'); + }); + + for (const optIn of [false, true]) { + test(`uninstall over a dangling hooks.json link succeeds and leaves it in place (GSD_ALLOW_SYMLINKED_DEST=${optIn ? '1' : 'unset'}) (#5037)`, () => { + runCodexInstall(codexHome); + const skillsBefore = fs.readdirSync(codexSkillsRoot(codexHome)).sort(); + assert.ok(skillsBefore.length > 0, 'setup: the install must have created skills'); + const missingTarget = path.join(codexHome, 'missing-hooks.json'); + fs.unlinkSync(hooksJsonPath(codexHome)); + fs.symlinkSync(missingTarget, hooksJsonPath(codexHome)); + if (optIn) process.env.GSD_ALLOW_SYMLINKED_DEST = '1'; + + assert.doesNotThrow(() => runCodexUninstall(codexHome)); + + assertSkillsRemoved(skillsBefore); + assert.ok(fs.lstatSync(hooksJsonPath(codexHome)).isSymbolicLink(), 'the dangling link must survive'); + assert.strictEqual(fs.readlinkSync(hooksJsonPath(codexHome)), missingTarget, 'the link must still point at the same target'); + assert.strictEqual(fs.existsSync(missingTarget), false, 'nothing may be created at the missing target'); + }); + } + + test('codexHooksJsonHasManagedHooks finds GSD hooks where the removal does, and only there (#5037)', () => { + assert.strictEqual(codexHooksJsonHasManagedHooks(codexHome, UNINSTALL_HOOK_EVENTS), false, 'no hooks.json'); + runCodexInstall(codexHome); + const installed = readHooksJson(codexHome); + assert.ok(Array.isArray(installed.hooks.SessionStart), 'setup: the install must register SessionStart'); + assert.strictEqual(codexHooksJsonHasManagedHooks(codexHome, UNINSTALL_HOOK_EVENTS), true, 'nested GSD hook'); + assert.strictEqual(codexHooksJsonHasManagedHooks(codexHome, ['UnrelatedEvent']), false, + 'a GSD hook under an event the caller does not remove does not count'); + + // The legacy flat shape, which the removal lifts into the nested table. + fs.writeFileSync(hooksJsonPath(codexHome), JSON.stringify({ SessionStart: installed.hooks.SessionStart }), 'utf8'); + assert.strictEqual(codexHooksJsonHasManagedHooks(codexHome, UNINSTALL_HOOK_EVENTS), true, 'top-level GSD hook'); + + fs.writeFileSync(hooksJsonPath(codexHome), JSON.stringify({ hooks: { SessionStart: [{ hooks: [{ type: 'command', command: 'echo mine' }] }] } }), 'utf8'); + assert.strictEqual(codexHooksJsonHasManagedHooks(codexHome, UNINSTALL_HOOK_EVENTS), false, 'user hook only'); + + fs.writeFileSync(hooksJsonPath(codexHome), '{ not json', 'utf8'); + assert.strictEqual(codexHooksJsonHasManagedHooks(codexHome, UNINSTALL_HOOK_EVENTS), true, + 'an unparseable file counts, so the removal still reports the parse error'); + + fs.unlinkSync(hooksJsonPath(codexHome)); + fs.symlinkSync(path.join(codexHome, 'missing-hooks.json'), hooksJsonPath(codexHome)); + assert.strictEqual(codexHooksJsonHasManagedHooks(codexHome, UNINSTALL_HOOK_EVENTS), false, 'a dangling link reads as absent'); + }); + test('cleanupOrphanedCodexContextMonitorScript keeps the hooks.json deregistration when script deletion fails', () => { runCodexInstall(codexHome); seedPreExisting2586Install(codexHome); diff --git a/tests/install-regressions.test.cjs b/tests/install-regressions.test.cjs index 3f4658acd8..0ab89950c9 100644 --- a/tests/install-regressions.test.cjs +++ b/tests/install-regressions.test.cjs @@ -36,7 +36,7 @@ const { INSTALL_TIMEOUT_MS, FIXTURE_HOOK_TIMEOUT_SECONDS } = require('./helpers/ // (120000ms), so kept as its own constant rather than raised to that bound. const SCOPED_INSTALL_TIMEOUT_MS = 60_000; -const { createTempDir, cleanup, mockPartialWriteThenThrow } = require('./helpers.cjs'); +const { createTempDir, cleanup, mockPartialWriteThenThrow, installSpawnEnv } = require('./helpers.cjs'); const { loadSkillsManifest, resolveProfile, @@ -2148,6 +2148,282 @@ describe('#1874 F6 adjacent: malformed settings.local.json does not crash the in }); }); +// ─── #5037: a symlinked settings.json is never replaced with a plain file ─── +// A dotfiles setup links settings.json to a file kept elsewhere. Every install +// used to replace that link with a plain file and leave the target stale. Both +// installer writes of the file, the primary settings write and the #338 +// local-install migration, must refuse the link without GSD_ALLOW_SYMLINKED_DEST +// and write through to its target with it. + +describe('#5037: the installer never replaces a symlinked settings.json', () => { + const USER_SETTINGS = { env: { MY_TOKEN: 'keep-me' } }; + + // The settings writer's own refusal. Exit code 1 and an untouched link alone + // would also hold for an install that failed earlier for an unrelated reason, + // so the refusal cases require this message, which also names the opt-in. + const SETTINGS_SYMLINK_REFUSAL = + /writeSettings: settings file "[^"]*settings\.json" is a symlink the install root "[^"]*" does not trust[^\n]*re-run with GSD_ALLOW_SYMLINKED_DEST=1/; + + function runInstaller(root, args, { optIn }) { + const env = installSpawnEnv({ + HOME: root, + USERPROFILE: root, + ...(optIn ? { GSD_ALLOW_SYMLINKED_DEST: '1' } : {}), + }); + delete env.GSD_TEST_MODE; + return runNode([INSTALL_SCRIPT, ...args], { cwd: root, env, timeoutMs: SCOPED_INSTALL_TIMEOUT_MS }); + } + + // Writes `content` to /dotfiles/settings.json and links `linkPath` to it. + // Returns null when this platform or account cannot create symlinks. + function linkSettings(root, linkPath, content) { + const targetPath = path.join(root, 'dotfiles', 'settings.json'); + fs.mkdirSync(path.dirname(targetPath), { recursive: true }); + fs.mkdirSync(path.dirname(linkPath), { recursive: true }); + fs.writeFileSync(targetPath, content); + try { + fs.symlinkSync(targetPath, linkPath, 'file'); + } catch { + return null; + } + return targetPath; + } + + function hasGsdCheckUpdateHook(settings) { + const sessionStart = (settings.hooks && settings.hooks.SessionStart) || []; + return sessionStart.some( + entry => entry && Array.isArray(entry.hooks) && + entry.hooks.some(h => h && typeof h.command === 'string' && h.command.includes('gsd-check-update')) + ); + } + + describe('primary settings write (global install)', () => { + for (const optIn of [false, true]) { + test(optIn + ? 'with GSD_ALLOW_SYMLINKED_DEST=1 the install writes through and the link survives' + : 'without GSD_ALLOW_SYMLINKED_DEST the install refuses and leaves the link and its target unchanged', (t) => { + const root = createTempDir('gsd-5037-global-'); + t.after(() => cleanup(root)); + const claudeDir = path.join(root, '.claude'); + const settingsPath = path.join(claudeDir, 'settings.json'); + const before = JSON.stringify(USER_SETTINGS, null, 2) + '\n'; + const targetPath = linkSettings(root, settingsPath, before); + if (!targetPath) { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + + const result = runInstaller(root, ['--claude', '--global', '--config-dir', claudeDir], { optIn }); + + assert.ok(fs.lstatSync(settingsPath).isSymbolicLink(), 'settings.json must still be a symlink'); + assert.strictEqual(fs.readlinkSync(settingsPath), targetPath, 'the link must still point at the same target'); + if (optIn) { + assert.strictEqual(result.exitCode, 0, `installer exited ${result.exitCode}\n${result.stdout}\n${result.stderr}`); + const after = JSON.parse(fs.readFileSync(targetPath, 'utf8')); + assert.deepStrictEqual(after.env, USER_SETTINGS.env, 'user settings must survive the write-through'); + assert.ok(hasGsdCheckUpdateHook(after), 'GSD hooks must land in the link target'); + } else { + // 1, not merely non-zero: a timed-out or killed installer reports null. + assert.strictEqual(result.exitCode, 1, + `a refused settings write must fail the install\n${result.stdout}\n${result.stderr}`); + assert.match(result.stderr, SETTINGS_SYMLINK_REFUSAL, + 'the install must fail on the settings symlink refusal, not on some earlier error'); + assert.strictEqual(fs.readFileSync(targetPath, 'utf8'), before, 'the link target must be byte-for-byte unchanged'); + } + }); + } + }); + + describe('#338 migration write (local install with GSD entries in a linked settings.json)', () => { + for (const optIn of [false, true]) { + test(optIn + ? 'with GSD_ALLOW_SYMLINKED_DEST=1 the GSD entries move out of the link target and the link survives' + : 'without GSD_ALLOW_SYMLINKED_DEST the install refuses before writing either settings file', (t) => { + const root = createTempDir('gsd-5037-local-'); + t.after(() => cleanup(root)); + const claudeDir = path.join(root, '.claude'); + const sharedSettingsPath = path.join(claudeDir, 'settings.json'); + const sharedBefore = JSON.stringify({ + ...USER_SETTINGS, + hooks: { + SessionStart: [ + { hooks: [{ type: 'command', command: `${process.execPath} ${path.join(claudeDir, 'hooks', 'gsd-check-update.js')}` }] }, + ], + }, + }, null, 2) + '\n'; + const targetPath = linkSettings(root, sharedSettingsPath, sharedBefore); + if (!targetPath) { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + const localSettingsPath = path.join(claudeDir, 'settings.local.json'); + const localBefore = JSON.stringify({ permissions: { allow: ['Bash(npm test)'] } }, null, 2) + '\n'; + fs.writeFileSync(localSettingsPath, localBefore); + + const result = runInstaller(root, ['--claude', '--local'], { optIn }); + + assert.ok(fs.lstatSync(sharedSettingsPath).isSymbolicLink(), 'settings.json must still be a symlink'); + assert.strictEqual(fs.readlinkSync(sharedSettingsPath), targetPath, 'the link must still point at the same target'); + if (optIn) { + assert.strictEqual(result.exitCode, 0, `installer exited ${result.exitCode}\n${result.stdout}\n${result.stderr}`); + const sharedAfter = JSON.parse(fs.readFileSync(targetPath, 'utf8')); + assert.deepStrictEqual(sharedAfter.env, USER_SETTINGS.env, 'user settings must survive in the link target'); + assert.strictEqual(hasGsdCheckUpdateHook(sharedAfter), false, 'GSD entries must be stripped from the link target'); + const localAfter = JSON.parse(fs.readFileSync(localSettingsPath, 'utf8')); + assert.ok(hasGsdCheckUpdateHook(localAfter), 'GSD entries must be relocated to settings.local.json'); + assert.ok(localAfter.permissions.allow.includes('Bash(npm test)'), + 'user permissions in settings.local.json must survive'); + } else { + // 1, not merely non-zero: a timed-out or killed installer reports null. + assert.strictEqual(result.exitCode, 1, + `a refused settings write must fail the install\n${result.stdout}\n${result.stderr}`); + assert.match(result.stderr, SETTINGS_SYMLINK_REFUSAL, + 'the install must fail on the settings symlink refusal, not on some earlier error'); + assert.strictEqual(fs.readFileSync(targetPath, 'utf8'), sharedBefore, 'the link target must be byte-for-byte unchanged'); + assert.strictEqual(fs.readFileSync(localSettingsPath, 'utf8'), localBefore, + 'settings.local.json must not be half-migrated when the shared write is refused'); + } + }); + } + }); + + // Uninstall removes GSD's files and only then rewrites settings.json, so it + // checks a symlinked settings.json before the first removal. A refusal at + // the end would leave settings entries pointing at hooks already deleted. + describe('uninstall', () => { + for (const optIn of [false, true]) { + test(optIn + ? 'with GSD_ALLOW_SYMLINKED_DEST=1 uninstall cleans the link target and the link survives' + : 'without GSD_ALLOW_SYMLINKED_DEST uninstall refuses before removing any GSD file', (t) => { + const root = createTempDir('gsd-5037-uninstall-'); + t.after(() => cleanup(root)); + const claudeDir = path.join(root, '.claude'); + const settingsPath = path.join(claudeDir, 'settings.json'); + fs.mkdirSync(claudeDir, { recursive: true }); + fs.writeFileSync(settingsPath, JSON.stringify(USER_SETTINGS, null, 2) + '\n'); + + const installed = runInstaller(root, ['--claude', '--global', '--config-dir', claudeDir], { optIn: false }); + assert.strictEqual(installed.exitCode, 0, `setup install exited ${installed.exitCode}\n${installed.stdout}\n${installed.stderr}`); + const gsdCoreDir = path.join(claudeDir, 'gsd-core'); + assert.ok(fs.existsSync(gsdCoreDir), 'setup: the install must have created gsd-core/'); + + // Move the installed settings.json into a dotfiles dir and link it back. + const installedSettings = fs.readFileSync(settingsPath, 'utf8'); + fs.unlinkSync(settingsPath); + const targetPath = linkSettings(root, settingsPath, installedSettings); + if (!targetPath) { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + assert.ok(hasGsdCheckUpdateHook(JSON.parse(installedSettings)), 'setup: the link target must hold GSD hooks'); + + const result = runInstaller(root, ['--claude', '--global', '--uninstall', '--config-dir', claudeDir], { optIn }); + + assert.ok(fs.lstatSync(settingsPath).isSymbolicLink(), 'settings.json must still be a symlink'); + assert.strictEqual(fs.readlinkSync(settingsPath), targetPath, 'the link must still point at the same target'); + if (optIn) { + assert.strictEqual(result.exitCode, 0, `uninstall exited ${result.exitCode}\n${result.stdout}\n${result.stderr}`); + assert.strictEqual(fs.existsSync(gsdCoreDir), false, 'uninstall must remove gsd-core/'); + const after = JSON.parse(fs.readFileSync(targetPath, 'utf8')); + assert.deepStrictEqual(after.env, USER_SETTINGS.env, 'user settings must survive in the link target'); + assert.strictEqual(hasGsdCheckUpdateHook(after), false, 'GSD hooks must be removed from the link target'); + } else { + // 1, not merely non-zero: a timed-out or killed uninstall reports null. + assert.strictEqual(result.exitCode, 1, + `a refused settings write must fail the uninstall\n${result.stdout}\n${result.stderr}`); + assert.match(result.stderr, SETTINGS_SYMLINK_REFUSAL, + 'the uninstall must fail on the settings symlink refusal, not on some earlier error'); + assert.ok(fs.existsSync(gsdCoreDir), 'no GSD file may be removed before the refusal'); + assert.strictEqual(fs.readFileSync(targetPath, 'utf8'), installedSettings, + 'the link target must be byte-for-byte unchanged'); + } + }); + } + + // The check covers only a settings.json uninstall would write. A link + // holding no GSD entries, or a dangling one, is never written, so it must + // not block the uninstall, with or without the opt-in. + function installClaudeGlobal(root) { + const claudeDir = path.join(root, '.claude'); + const installed = runInstaller(root, ['--claude', '--global', '--config-dir', claudeDir], { optIn: false }); + assert.strictEqual(installed.exitCode, 0, `setup install exited ${installed.exitCode}\n${installed.stdout}\n${installed.stderr}`); + const gsdCoreDir = path.join(claudeDir, 'gsd-core'); + assert.ok(fs.existsSync(gsdCoreDir), 'setup: the install must have created gsd-core/'); + return { claudeDir, settingsPath: path.join(claudeDir, 'settings.json'), gsdCoreDir }; + } + + test('without GSD_ALLOW_SYMLINKED_DEST uninstall succeeds over a linked settings.json with no GSD entries and leaves it untouched', (t) => { + const root = createTempDir('gsd-5037-uninstall-clean-'); + t.after(() => cleanup(root)); + const { claudeDir, settingsPath, gsdCoreDir } = installClaudeGlobal(root); + const before = JSON.stringify(USER_SETTINGS, null, 2) + '\n'; + fs.unlinkSync(settingsPath); + const targetPath = linkSettings(root, settingsPath, before); + if (!targetPath) { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + + const result = runInstaller(root, ['--claude', '--global', '--uninstall', '--config-dir', claudeDir], { optIn: false }); + + assert.strictEqual(result.exitCode, 0, `uninstall exited ${result.exitCode}\n${result.stdout}\n${result.stderr}`); + assert.strictEqual(fs.existsSync(gsdCoreDir), false, 'uninstall must remove gsd-core/'); + assert.ok(fs.lstatSync(settingsPath).isSymbolicLink(), 'settings.json must still be a symlink'); + assert.strictEqual(fs.readlinkSync(settingsPath), targetPath, 'the link must still point at the same target'); + assert.strictEqual(fs.readFileSync(targetPath, 'utf8'), before, 'the link target must be byte-for-byte unchanged'); + }); + + for (const optIn of [false, true]) { + test(`uninstall succeeds over a dangling settings.json link and leaves it in place (GSD_ALLOW_SYMLINKED_DEST=${optIn ? '1' : 'unset'})`, (t) => { + const root = createTempDir('gsd-5037-uninstall-dangling-'); + t.after(() => cleanup(root)); + const { claudeDir, settingsPath, gsdCoreDir } = installClaudeGlobal(root); + const missingTarget = path.join(root, 'dotfiles', 'settings.json'); + fs.unlinkSync(settingsPath); + try { + fs.symlinkSync(missingTarget, settingsPath, 'file'); + } catch { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + + const result = runInstaller(root, ['--claude', '--global', '--uninstall', '--config-dir', claudeDir], { optIn }); + + assert.strictEqual(result.exitCode, 0, `uninstall exited ${result.exitCode}\n${result.stdout}\n${result.stderr}`); + assert.strictEqual(fs.existsSync(gsdCoreDir), false, 'uninstall must remove gsd-core/'); + assert.ok(fs.lstatSync(settingsPath).isSymbolicLink(), 'the dangling link must survive'); + assert.strictEqual(fs.readlinkSync(settingsPath), missingTarget, 'the link must still point at the same target'); + assert.strictEqual(fs.existsSync(missingTarget), false, 'nothing may be created at the missing target'); + }); + } + }); + + // uninstall() runs the settings cleanup once silently to decide whether to + // check the link, then again for real, so its return value must be exactly + // "step 6 would write". + describe('stripGsdFromSettings', () => { + const { stripGsdFromSettings } = installExports || {}; + const claudeDir = path.join(path.sep, 'home', 'u', '.claude'); + const gsdHook = { type: 'command', command: `node ${path.join(claudeDir, 'hooks', 'gsd-check-update.js')}` }; + const userHook = { type: 'command', command: 'echo mine' }; + + test('reports and strips GSD entries, keeping the user hook beside them', () => { + const settings = { ...USER_SETTINGS, hooks: { SessionStart: [{ hooks: [gsdHook, userHook] }] } }; + assert.strictEqual(stripGsdFromSettings(settings, 'claude', claudeDir, () => {}), true); + assert.deepStrictEqual(settings, { ...USER_SETTINGS, hooks: { SessionStart: [{ hooks: [userHook] }] } }); + }); + + test('reports no change and logs nothing for settings without GSD entries', () => { + const settings = { ...USER_SETTINGS, hooks: { SessionStart: [{ hooks: [userHook] }] } }; + const snapshot = JSON.stringify(settings); + const lines = []; + assert.strictEqual(stripGsdFromSettings(settings, 'claude', claudeDir, (line) => lines.push(line)), false); + assert.strictEqual(JSON.stringify(settings), snapshot, 'settings must be left as they were'); + assert.deepStrictEqual(lines, []); + }); + }); +}); + // ─── #1874 F18 — ~/.gsd/defaults.json is machine-global: lock it, write it once ─── // // Every runtime and project on the box reads this file. The read-modify-write diff --git a/tests/install-write-confinement.test.cjs b/tests/install-write-confinement.test.cjs index 01b75d2133..bcc76ea182 100644 --- a/tests/install-write-confinement.test.cjs +++ b/tests/install-write-confinement.test.cjs @@ -4672,3 +4672,202 @@ describe('#4636 RED: capability-skill symlink escape (isPathConfined has no real } }); }); + +// --------------------------------------------------------------------------- +// resolveSymlinkedLeafWritePath (#5037): the symlinked-file policy shared by +// writeSettings and the Codex hooks.json writer +// --------------------------------------------------------------------------- + +describe('resolveSymlinkedLeafWritePath (#5037)', () => { + const { resolveSymlinkedLeafWritePath } = require('../gsd-core/bin/lib/install-engine.cjs'); + const LABEL = 'test writer: file'; + + let savedOptIn; + beforeEach(() => { + savedOptIn = process.env.GSD_ALLOW_SYMLINKED_DEST; + }); + afterEach(() => { + if (savedOptIn === undefined) delete process.env.GSD_ALLOW_SYMLINKED_DEST; + else process.env.GSD_ALLOW_SYMLINKED_DEST = savedOptIn; + }); + + function setOptIn(value) { + if (value === undefined) delete process.env.GSD_ALLOW_SYMLINKED_DEST; + else process.env.GSD_ALLOW_SYMLINKED_DEST = value; + } + + // An install root holding `file.json`, optionally a link into a separate + // dir. Returns null when this platform or account cannot create symlinks. + function layout(t, { link = false, targetExists = true } = {}) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5037-root-')); + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5037-outside-')); + t.after(() => { + cleanup(root); + cleanup(outside); + }); + const filePath = path.join(root, 'file.json'); + const targetPath = path.join(outside, 'file.json'); + if (!link) { + if (targetExists) fs.writeFileSync(filePath, '{}\n'); + return { root, filePath, targetPath }; + } + if (targetExists) fs.writeFileSync(targetPath, '{}\n'); + try { + fs.symlinkSync(targetPath, filePath, 'file'); + } catch { + return null; + } + return { root, filePath, targetPath }; + } + + for (const optIn of [undefined, '1']) { + const mode = `GSD_ALLOW_SYMLINKED_DEST=${optIn ?? 'unset'}`; + + test(`a plain file is returned unchanged (${mode})`, (t) => { + setOptIn(optIn); + const { root, filePath } = layout(t); + assert.strictEqual(resolveSymlinkedLeafWritePath(root, filePath, LABEL), filePath); + }); + + test(`a missing file is returned unchanged (${mode})`, (t) => { + setOptIn(optIn); + const { root, filePath } = layout(t, { targetExists: false }); + assert.strictEqual(resolveSymlinkedLeafWritePath(root, filePath, LABEL), filePath); + }); + + test(`a dangling link is refused and left in place (${mode})`, (t) => { + setOptIn(optIn); + const l = layout(t, { link: true, targetExists: false }); + if (!l) { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + // With the opt-in already set, asking for it again would not help, so + // that refusal names the missing target instead. + assert.throws(() => resolveSymlinkedLeafWritePath(l.root, l.filePath, LABEL), + optIn ? /target does not exist/ : /GSD_ALLOW_SYMLINKED_DEST=1/); + assert.ok(fs.lstatSync(l.filePath).isSymbolicLink(), 'the dangling link must survive'); + assert.strictEqual(fs.existsSync(l.targetPath), false, 'nothing may be created at the missing target'); + }); + + // Both writers that share this helper must treat a dangling link the same + // way: refuse it and leave it in place. + test(`writeSettings and the Codex hooks.json writer both refuse a dangling link (${mode})`, (t) => { + setOptIn(optIn); + const { writeSettings } = require('../bin/install.js'); + const { reconcileCodexHooksJsonEvent } = require('../gsd-core/bin/lib/runtime-hooks-surface.cjs'); + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5037-parity-')); + t.after(() => cleanup(root)); + const settingsPath = path.join(root, 'settings.json'); + const hooksJsonPath = path.join(root, 'hooks.json'); + const missingSettings = path.join(root, 'missing-settings.json'); + const missingHooks = path.join(root, 'missing-hooks.json'); + try { + fs.symlinkSync(missingSettings, settingsPath, 'file'); + fs.symlinkSync(missingHooks, hooksJsonPath, 'file'); + } catch { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + const refusal = optIn ? /target does not exist/ : /GSD_ALLOW_SYMLINKED_DEST=1/; + + assert.throws(() => writeSettings(settingsPath, { hooks: {} }), refusal); + assert.throws( + () => reconcileCodexHooksJsonEvent(root, 'SessionStart', { managedCommand: 'node /gsd/hooks/gsd-context-monitor.js' }), + refusal, + ); + + for (const [link, target] of [[settingsPath, missingSettings], [hooksJsonPath, missingHooks]]) { + assert.ok(fs.lstatSync(link).isSymbolicLink(), `${path.basename(link)} must still be a symlink`); + assert.strictEqual(fs.existsSync(target), false, `nothing may be created at ${path.basename(target)}`); + } + }); + } + + test('a chain of links is refused without the opt-in, and resolves to the final real file with it', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5037-chain-root-')); + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5037-chain-outside-')); + t.after(() => { + cleanup(root); + cleanup(outside); + }); + const finalTarget = path.join(outside, 'real.json'); + const middleLink = path.join(outside, 'middle.json'); + const filePath = path.join(root, 'file.json'); + fs.writeFileSync(finalTarget, '{}\n'); + try { + fs.symlinkSync(finalTarget, middleLink, 'file'); + fs.symlinkSync(middleLink, filePath, 'file'); + } catch { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + + setOptIn(undefined); + assert.throws(() => resolveSymlinkedLeafWritePath(root, filePath, LABEL), /GSD_ALLOW_SYMLINKED_DEST=1/); + + setOptIn('1'); + assert.strictEqual(resolveSymlinkedLeafWritePath(root, filePath, LABEL), fs.realpathSync(finalTarget)); + assert.strictEqual(fs.readlinkSync(filePath), middleLink, 'resolving must not touch the first link'); + assert.strictEqual(fs.readlinkSync(middleLink), finalTarget, 'resolving must not touch the middle link'); + }); + + test('with the opt-in, a link loop is refused as unresolvable, not as a missing target', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5037-loop-')); + t.after(() => cleanup(root)); + const filePath = path.join(root, 'file.json'); + try { + fs.symlinkSync(filePath, filePath, 'file'); + } catch { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + setOptIn('1'); + assert.throws(() => resolveSymlinkedLeafWritePath(root, filePath, LABEL), /cannot be resolved/); + assert.ok(fs.lstatSync(filePath).isSymbolicLink(), 'the link must be left alone'); + }); + + test('a link whose target is the install root itself is refused even with the opt-in', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5037-rootlink-')); + t.after(() => cleanup(root)); + const filePath = path.join(root, 'file.json'); + try { + fs.symlinkSync(root, filePath, 'dir'); + } catch { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + + setOptIn(undefined); + assert.throws(() => resolveSymlinkedLeafWritePath(root, filePath, LABEL), /GSD_ALLOW_SYMLINKED_DEST=1/); + + // The opt-in never relaxes this case (a write there would land on the + // install root), so the refusal must not ask for the opt-in again. + setOptIn('1'); + assert.throws(() => resolveSymlinkedLeafWritePath(root, filePath, LABEL), /resolves to the install root/); + assert.strictEqual(fs.readlinkSync(filePath), root, 'the link must be left alone'); + }); + + test('a symlinked file is refused without the opt-in, and the link is left alone', (t) => { + setOptIn(undefined); + const l = layout(t, { link: true }); + if (!l) { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + assert.throws(() => resolveSymlinkedLeafWritePath(l.root, l.filePath, LABEL), /GSD_ALLOW_SYMLINKED_DEST=1/); + assert.ok(fs.lstatSync(l.filePath).isSymbolicLink(), 'the link must survive'); + assert.strictEqual(fs.readlinkSync(l.filePath), l.targetPath, 'the link must still point at the same target'); + }); + + test("with GSD_ALLOW_SYMLINKED_DEST=1 a symlinked file resolves to the link's real target", (t) => { + setOptIn('1'); + const l = layout(t, { link: true }); + if (!l) { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + assert.strictEqual(resolveSymlinkedLeafWritePath(l.root, l.filePath, LABEL), fs.realpathSync(l.targetPath)); + assert.ok(fs.lstatSync(l.filePath).isSymbolicLink(), 'resolving must not touch the link'); + }); +}); diff --git a/tests/settings-jsonc.test.cjs b/tests/settings-jsonc.test.cjs index 4e48f62990..e1f6820126 100644 --- a/tests/settings-jsonc.test.cjs +++ b/tests/settings-jsonc.test.cjs @@ -8,7 +8,7 @@ * Closes: #1461 */ -const { test, describe } = require('node:test'); +const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const os = require('os'); @@ -37,7 +37,7 @@ let installExports; process.stdout.write = _origWrite; } } -const { readSettings, writeSettings, stripJsonComments } = installExports; +const { readSettings, writeSettings, stripJsonComments, configureAntigravityPermissions } = installExports; // ─── tests ─────────────────────────────────────────────────────────────────── @@ -434,3 +434,164 @@ describe('writeSettings durability (#1874 F5)', () => { }); }); }); + +// ─── writeSettings on a symlinked settings file (#5037) ────────────────────── +// +// Dotfiles setups link settings.json to a file kept elsewhere. The temp+rename +// write above replaces such a link with a plain file, after which the link's +// target silently stops receiving changes. The writer now follows the policy +// of the Codex hooks.json writer: refuse a symlinked settings file unless +// GSD_ALLOW_SYMLINKED_DEST is set, and with it write through to the target. + +describe('writeSettings on a symlinked settings file (#5037)', () => { + + const PRIOR = JSON.stringify({ env: { MY_TOKEN: 'keep-me' } }, null, 2) + '\n'; + const NEXT = { hooks: { SessionStart: [] }, env: { MY_TOKEN: 'keep-me' } }; + + let savedOptIn; + + beforeEach(() => { + savedOptIn = process.env.GSD_ALLOW_SYMLINKED_DEST; + }); + + afterEach(() => { + if (savedOptIn === undefined) delete process.env.GSD_ALLOW_SYMLINKED_DEST; + else process.env.GSD_ALLOW_SYMLINKED_DEST = savedOptIn; + }); + + function setOptIn(value) { + if (value === undefined) delete process.env.GSD_ALLOW_SYMLINKED_DEST; + else process.env.GSD_ALLOW_SYMLINKED_DEST = value; + } + + // A config dir whose settings.json links to a file in a separate dotfiles + // dir. Returns null when this platform or account cannot create symlinks. + function linkedLayout(t, { targetExists = true } = {}) { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5037-config-')); + const dotfilesDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5037-dotfiles-')); + t.after(() => { + cleanup(configDir); + cleanup(dotfilesDir); + }); + const settingsPath = path.join(configDir, 'settings.json'); + const targetPath = path.join(dotfilesDir, 'settings.json'); + if (targetExists) fs.writeFileSync(targetPath, PRIOR); + try { + fs.symlinkSync(targetPath, settingsPath, 'file'); + } catch { + return null; + } + return { configDir, dotfilesDir, settingsPath, targetPath }; + } + + test('without the opt-in the write is refused, naming GSD_ALLOW_SYMLINKED_DEST, and nothing changes', (t) => { + setOptIn(undefined); + const layout = linkedLayout(t); + if (!layout) { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + const { configDir, dotfilesDir, settingsPath, targetPath } = layout; + + assert.throws(() => writeSettings(settingsPath, NEXT), /GSD_ALLOW_SYMLINKED_DEST=1/); + + assert.ok(fs.lstatSync(settingsPath).isSymbolicLink(), 'settings.json must still be a symlink'); + assert.strictEqual(fs.readlinkSync(settingsPath), targetPath, 'the link must still point at the same target'); + assert.strictEqual(fs.readFileSync(targetPath, 'utf8'), PRIOR, 'the link target must be byte-for-byte unchanged'); + assert.deepStrictEqual(fs.readdirSync(configDir), ['settings.json'], 'no temp residue beside the link'); + assert.deepStrictEqual(fs.readdirSync(dotfilesDir), ['settings.json'], 'no temp residue beside the target'); + }); + + test('with GSD_ALLOW_SYMLINKED_DEST=1 the write lands on the target and the link stays a link', (t) => { + setOptIn('1'); + const layout = linkedLayout(t); + if (!layout) { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + const { configDir, dotfilesDir, settingsPath, targetPath } = layout; + + writeSettings(settingsPath, NEXT); + + assert.ok(fs.lstatSync(settingsPath).isSymbolicLink(), 'settings.json must still be a symlink, not a plain file'); + assert.strictEqual(fs.readlinkSync(settingsPath), targetPath, 'the link must still point at the same target'); + assert.ok(fs.lstatSync(targetPath).isFile(), 'the target must still be a regular file'); + assert.deepStrictEqual(readSettings(targetPath), NEXT, 'the new settings must land in the link target'); + assert.deepStrictEqual(fs.readdirSync(configDir), ['settings.json'], 'no temp residue beside the link'); + assert.deepStrictEqual(fs.readdirSync(dotfilesDir), ['settings.json'], 'no temp residue beside the target'); + }); + + // One test per opt-in state, so each test builds one layout and registers + // one cleanup. + for (const optIn of [undefined, '1']) { + const mode = `GSD_ALLOW_SYMLINKED_DEST=${optIn ?? 'unset'}`; + + test(`a dangling settings.json link is refused, never replaced (${mode})`, (t) => { + setOptIn(optIn); + const layout = linkedLayout(t, { targetExists: false }); + if (!layout) { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + const { settingsPath, targetPath } = layout; + + // With the opt-in already set, asking for it again would not help, so + // that refusal names the missing target instead. + assert.throws(() => writeSettings(settingsPath, NEXT), + optIn ? /target does not exist/ : /GSD_ALLOW_SYMLINKED_DEST=1/, + 'a dangling link must be refused'); + + assert.ok(fs.lstatSync(settingsPath).isSymbolicLink(), 'the dangling link must survive'); + assert.strictEqual(fs.existsSync(targetPath), false, 'nothing may be created at the missing target'); + }); + + test(`a plain settings.json is written in place (${mode})`, (t) => { + setOptIn(optIn); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-5037-plain-')); + t.after(() => cleanup(dir)); + const settingsPath = path.join(dir, 'settings.json'); + fs.writeFileSync(settingsPath, PRIOR); + + writeSettings(settingsPath, NEXT); + + assert.ok(fs.lstatSync(settingsPath).isFile(), 'a plain settings.json must stay a regular file'); + assert.deepStrictEqual(readSettings(settingsPath), NEXT, 'the plain file must receive the write'); + assert.deepStrictEqual(fs.readdirSync(dir), ['settings.json'], 'no temp residue'); + }); + } + + // configureAntigravityPermissions saves the Antigravity settings.json through + // writeSettings, so it inherits the same refusal and write-through. + test('the Antigravity permissions write refuses a symlinked settings.json without the opt-in', (t) => { + setOptIn(undefined); + const layout = linkedLayout(t); + if (!layout) { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + const { configDir, settingsPath, targetPath } = layout; + + assert.throws(() => configureAntigravityPermissions(true, configDir), /GSD_ALLOW_SYMLINKED_DEST=1/); + + assert.ok(fs.lstatSync(settingsPath).isSymbolicLink(), 'settings.json must still be a symlink'); + assert.strictEqual(fs.readFileSync(targetPath, 'utf8'), PRIOR, 'the link target must be byte-for-byte unchanged'); + }); + + test('with GSD_ALLOW_SYMLINKED_DEST=1 the Antigravity permissions write lands on the link target', (t) => { + setOptIn('1'); + const layout = linkedLayout(t); + if (!layout) { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + const { configDir, settingsPath, targetPath } = layout; + + configureAntigravityPermissions(true, configDir); + + assert.ok(fs.lstatSync(settingsPath).isSymbolicLink(), 'settings.json must still be a symlink'); + const after = readSettings(targetPath); + assert.deepStrictEqual(after.env, { MY_TOKEN: 'keep-me' }, 'user settings must survive the write-through'); + assert.ok(Array.isArray(after.permissions && after.permissions.allow) && after.permissions.allow.length > 0, + 'the Antigravity allow rules must land in the link target'); + }); +});