From 7efd9032eee62fa1905fadfeec4a591fea9c32ab Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 15 Sep 2026 02:37:18 -0400 Subject: [PATCH] fix(#4544): cover hooks/, scripts/, CHANGELOG and the manifest in codex rollback (#4760) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#4544): failing-first rollback-coverage tests for codex install * test(#4544): scope the rollback suite to its own requires The appended describe used bare describe/test/os/cleanup and the folded block's runCodexInstall — none visible at file scope, so the whole test file failed to load. Wrap it in its own block with local requires and a local harness copy, matching the folded-block idiom. * test(#4544): import beforeEach/afterTest hooks into the suite scope * fix(#4544): restore manifest-tracked files and hooks/ on codex rollback restoreCodexSnapshot and _codexPreConfigRollback knew only the five #3245 targets (config.toml, hooks.json, skills/gsd-*, agents/gsd-*, VERSION). A Codex install also writes hooks/, gsd-core/CHANGELOG.md, gsd-core/.gsd-runtime, scripts/changeset|lib + the standalone scripts, and rewrites gsd-file-manifest.json -- none were captured, so any rollback left the new payload in place for all of them. The pre-install capture now records every path the PRIOR install's own gsd-file-manifest.json lists (bytes when present, absence-marker when not, so a path deleted between installs is re-deleted rather than resurrected), the manifest file itself, and the whole hooks/ tree -- wholesale, because the Codex manifest deliberately omits hooks/ and hooks/ is shared space, so restore returns user files that predated the install and drops everything the failed install staged. Both rollback paths share one restore closure; malformed or missing prior state degrades to today's behavior. Known residual, documented: files the FAILED install adds under manifest-tracked dirs survive a rollback that fires before the new manifest is written (they are named by no prior state). The five original targets and hooks/ have no residual. * test(#4544): align malformed-manifest fixtures with pre-install-state semantics Row 7 seeded VERSION and then asserted its absence -- but a seeded VERSION is pre-install state the fix must restore, not remove. Row 8 asserted a pre-existing array-shaped manifest must not survive, when restoring those exact bytes IS the contract. Both were fixture bugs; the probe-verified installer behavior was correct. * docs(#4544): add Fixed changeset for codex rollback coverage * fix(#4544): harden snapshot per adversarial review — minimal mode, symlinks, clean installs Review (three independent passes) found five defects and one coverage gap in the first cut; all fixed: - BLOCKER: the capture gate is off in minimal mode but the restore call was not, so a minimal-mode rollback wholesale-deleted the user's entire hooks/ directory (empirically confirmed by the reviewer). The restore now consults a captured flag: no snapshot means do nothing. - MAJOR: the hooks/ walk followed file symlinks — a repo-shipped .codex/hooks symlink to a FIFO would hang the installer, to /dev/zero exhaust memory, or to private data copy that data into the snapshot. The walk lstats every entry and captures only true regular files; anything else marks the capture incomplete. - Incomplete captures now downgrade the restore to per-file: put back what was captured, remove only the names GSD itself stages (the hoisted CODEX_HOOKS_TO_COPY set + CommonJS marker), never wholesale- delete a tree the snapshot did not fully see. GSD-owned removal runs before the restore so a name in both sets keeps its pre-install bytes. - A pre-existing hooks FILE (not directory) is left alone instead of deleted. - readInstallManifest now rejects a manifest whose files field is a JSON array (typeof [] === 'object'), which previously produced numeric-key paths. - Clean FIRST installs: with no prior manifest nothing recorded the payload, so a failed clean install rolled back to a half-written tree. Capture now enumerates the same source directories the installer copies plus the two standalone files (CHANGELOG.md from the repo root, generated .gsd-runtime) and records absence — a failed clean install now rolls back to actually nothing. Tests: minimal-mode preservation regression, symlink never-followed regression, helpers.cjs temp dirs, the injected-failure message is asserted, residue assertions made unconditional. * test(#4544): pin symlink-downgrade semantics the final run exposed The symlink itself marks the capture incomplete, so the restore takes the per-file downgrade — which preserves uncaptured pre-install state (the link) rather than wholesale-dropping it. The probe run verified exactly this; the assertion guessed the wholesale branch. Pin the verified behavior: referent untouched, link preserved and resolving, no leak. * docs(#4544): backfill changeset PR number --------- Co-authored-by: sim --- .changeset/zesty-pumas-hum.md | 5 + bin/install.js | 265 ++++++++++++++++++++++++- src/installer-migrations.cts | 9 +- tests/codex-config-hooks.test.cjs | 317 ++++++++++++++++++++++++++++++ 4 files changed, 587 insertions(+), 9 deletions(-) create mode 100644 .changeset/zesty-pumas-hum.md diff --git a/.changeset/zesty-pumas-hum.md b/.changeset/zesty-pumas-hum.md new file mode 100644 index 000000000..c70b17cc3 --- /dev/null +++ b/.changeset/zesty-pumas-hum.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4760 +--- +**Codex rollback no longer leaves a partially-installed payload behind** — the installer's rollback snapshot now covers every file the previous install's manifest recorded (CHANGELOG.md, scripts/, .gsd-runtime, the manifest itself) plus the whole `hooks/` directory, instead of only config.toml, hooks.json, skills/, agents/ and VERSION, so a failed install reverts to the true pre-install state. (#4544) diff --git a/bin/install.js b/bin/install.js index f7c5db688..3cff2c902 100755 --- a/bin/install.js +++ b/bin/install.js @@ -438,6 +438,17 @@ const GSD_CHANGESET_FILES = [ ]; const GSD_SCRIPTS_LIB_FILES = ['cli-exit.cjs', 'allowlist-ratchet.cjs', 'drift-scan.cjs', 'alias-drift-families.cjs', 'exit-code-registry.cjs', 'ndjson-reporter.cjs', 'ci-job-timing.cjs', 'shellcheck-fetch.cjs', 'npm-version-check-diagnosis.cjs', 'platform-conformance-tier.generated.cjs', 'suite-detection.cjs', 'macos-conformance-tier.generated.cjs']; +// #4544 — the Codex hook payload the install stages into /hooks/. +// Hoisted to module scope (the #3184 precedent above) so the rollback's +// incomplete-capture path can name exactly the files GSD owns without a +// second copy of the list drifting away from the staging site, which lives +// inside the Codex config block where the constant used to be declared. +const CODEX_HOOKS_TO_COPY = [ + 'gsd-check-update.js', + 'gsd-check-update-worker.js', + 'managed-hooks-registry.cjs', +]; + /** * Resolve a runtime's shared-hooks directory name from its descriptor. * @@ -812,6 +823,7 @@ const { applyInstallerMigrationPlan, discoverInstallerMigrations, MANIFEST_SCHEMA_VERSION, + readInstallManifest, runInstallerMigrations, } = require(path.join(_gsdLibDir, 'installer-migrations.cjs')); const { @@ -10842,7 +10854,43 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // Map — content snapshot of each pre-existing gsd-* agent file. const codexPreInstallAgentContents = new Map(); let codexPreInstallVersionBytes = null; + // #4544 — manifest-driven snapshot state (captured in the block below): + // codexPreInstallManagedFiles — Map; one + // entry per path the PRIOR install's gsd-file-manifest.json recorded. + // null means the path did not exist pre-install, so rollback re-deletes + // whatever this install put there instead of resurrecting it. + // codexPreInstallManifestBytes — Buffer (or null) of the prior manifest file + // itself, which a reinstall rewrites. + // codexPreInstallHooksTree — Map, a full recursive + // snapshot of /hooks/. The Codex manifest deliberately omits + // hooks/ (the !isCodex gate on shared-hooks tracking), and hooks/ is + // shared space, so the restore is wholesale: user files that predate + // the install are in the snapshot and come back; anything the failed + // install staged does not. + const codexPreInstallManagedFiles = new Map(); + let codexPreInstallManifestBytes = null; + const codexPreInstallHooksTree = new Map(); + // #4544 (review) — capture-state flags the restore must consult: + // codexManagedSnapshotCaptured — the capture gate ran at all. When + // false (non-Codex runtimes, minimal mode) NO pre-install state was + // recorded, and the only safe restore is no restore: an empty + // snapshot must never be read as "hooks/ was absent". + // codexPreInstallHooksDirPreExisted — hooks/ existed as a DIRECTORY + // pre-install. A pre-existing hooks FILE is left alone on rollback + // rather than deleted. + // codexPreInstallHooksCaptureIncomplete — some part of the hooks/ tree + // could not be read (permissions, special files). The restore + // downgrades to per-file so an uncapturable user file is never + // destroyed by a wholesale delete whose snapshot lacked it. + let codexManagedSnapshotCaptured = false; + // null = the gate never ran; true/false = the gate ran and hooks/ (did|did + // not) exist as a directory pre-install. Three states are load-bearing: a + // clean first install records false, so its rollback removes the staged + // hooks/ tree entirely; minimal mode records null, so rollback does nothing. + let codexPreInstallHooksDirPreExisted = null; + let codexPreInstallHooksCaptureIncomplete = false; if (_hostBehaviors(runtime).tomlConfigInstall && !isMinimalMode(_effectiveInstallMode)) { + codexManagedSnapshotCaptured = true; const _preSkillsDir = _resolveSkillsRootDir(runtime, targetDir, _installScopeId); if (fs.existsSync(_preSkillsDir)) { for (const entry of fs.readdirSync(_preSkillsDir, { withFileTypes: true })) { @@ -10884,18 +10932,212 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { if (fs.existsSync(_preVersionPath)) { try { codexPreInstallVersionBytes = fs.readFileSync(_preVersionPath); } catch (_) { /* best-effort */ } } + // #4544 — capture the manifest-driven surfaces, same best-effort + // conventions as the skills/ snapshot above. readInstallManifest is the + // same hardened reader installer-migrations uses (array/ garbage shapes + // degrade to an empty file set — rollback then simply covers less, never + // crashes), and resolveInstallRelativePath keeps a hostile manifest key + // from turning into a write outside the install root. + const _priorManifest = readInstallManifest(targetDir); + for (const rel of Object.keys(_priorManifest.files)) { + const resolved = resolveInstallRelativePath(targetDir, rel); + if (!resolved) continue; + try { + codexPreInstallManagedFiles.set(resolved.relPath, fs.readFileSync(resolved.fullPath)); + } catch (_) { + // Listed but absent/unreadable pre-install: snapshot absence, so + // rollback re-deletes instead of resurrecting. + codexPreInstallManagedFiles.set(resolved.relPath, null); + } + } + const _preManifestPath = path.join(targetDir, MANIFEST_NAME); + if (fs.existsSync(_preManifestPath)) { + try { codexPreInstallManifestBytes = fs.readFileSync(_preManifestPath); } catch (_) { /* best-effort */ } + } + // #4544 (review) — a clean FIRST install has no prior manifest, so nothing + // above records the payload this install is about to write, and a failed + // clean install would roll back to a half-written tree. Enumerate the SAME + // source directories the installer copies (a directory walk tracks the + // source tree automatically — no second file list to keep in parity) and + // record every path as absent-pre-install. On a reinstall most of these + // already carry entries from the prior manifest; any that do not (files + // new in this version) snapshot their pre-install bytes or absence exactly + // like the rest, which also closes the new-version-file residual. + const _recordWritePlanTree = (srcDir, relPrefix) => { + let children; + try { children = fs.readdirSync(srcDir, { withFileTypes: true }); } catch (_) { return; } + for (const child of children) { + const rel = relPrefix ? `${relPrefix}/${child.name}` : child.name; + if (child.isDirectory()) { + _recordWritePlanTree(path.join(srcDir, child.name), rel); + } else if (child.isFile()) { + if (codexPreInstallManagedFiles.has(rel)) continue; + // USER_OWNED_ARTIFACTS are manifest-relative to gsd-core/ (#2771): + // they are durably staged across reinstalls and must never enter a + // rollback delete-set. + const manifestRel = rel.startsWith('gsd-core/') ? rel.slice('gsd-core/'.length) : rel; + if (USER_OWNED_ARTIFACTS.includes(manifestRel)) continue; + const resolved = resolveInstallRelativePath(targetDir, rel); + if (!resolved) continue; + try { + codexPreInstallManagedFiles.set(rel, fs.existsSync(resolved.fullPath) ? fs.readFileSync(resolved.fullPath) : null); + } catch (_) { + codexPreInstallManagedFiles.set(rel, null); + } + } + } + }; + _recordWritePlanTree(path.join(src, 'gsd-core'), 'gsd-core'); + _recordWritePlanTree(path.join(src, 'scripts', 'changeset'), 'scripts/changeset'); + _recordWritePlanTree(path.join(src, 'scripts', 'lib'), 'scripts/lib'); + // gsd-core/CHANGELOG.md is sourced from the repo root (not src/gsd-core) + // and gsd-core/.gsd-runtime is generated at install time — neither appears + // in the directory walks, so record them explicitly. + for (const standalone of ['gsd-core/CHANGELOG.md', 'gsd-core/.gsd-runtime', 'scripts/fix-slash-commands.cjs', 'scripts/gen-capability-registry.cjs', 'scripts/gen-loop-host-contract.cjs']) { + if (codexPreInstallManagedFiles.has(standalone)) continue; + const resolved = resolveInstallRelativePath(targetDir, standalone); + if (!resolved) continue; + try { + codexPreInstallManagedFiles.set(standalone, fs.existsSync(resolved.fullPath) ? fs.readFileSync(resolved.fullPath) : null); + } catch (_) { + codexPreInstallManagedFiles.set(standalone, null); + } + } + // hooks/ — full recursive snapshot, but never blind: lstat every entry so + // a symlink under hooks/ is neither followed (a link to a FIFO would hang + // the installer, /dev/zero would exhaust memory, and a link to private + // data would copy that data into the snapshot — hooks/ is user-writable + // shared space and, for local installs, repo-controllable) nor restored + // as a link. Anything unreadable or special marks the capture INCOMPLETE + // so the restore downgrades to per-file instead of wholesale-deleting a + // tree it never fully saw. A pre-existing hooks FILE (not directory) is + // recorded as such and left alone on rollback. + const _preHooksPath = path.join(targetDir, 'hooks'); + let _preHooksStat = null; + try { _preHooksStat = fs.lstatSync(_preHooksPath); } catch (_) { /* absent */ } + codexPreInstallHooksDirPreExisted = Boolean(_preHooksStat && _preHooksStat.isDirectory()); + if (codexPreInstallHooksDirPreExisted) { + const _snapshotHooksDir = (dir, relBase) => { + let children; + try { children = fs.readdirSync(dir, { withFileTypes: true }); } catch (_) { + codexPreInstallHooksCaptureIncomplete = true; + return; + } + for (const child of children) { + const relPath = relBase ? `${relBase}/${child.name}` : child.name; + const fullPath = path.join(dir, child.name); + let st = null; + try { st = fs.lstatSync(fullPath); } catch (_) { + codexPreInstallHooksCaptureIncomplete = true; + continue; + } + if (st.isDirectory()) { + _snapshotHooksDir(fullPath, relPath); + } else if (st.isFile()) { + try { codexPreInstallHooksTree.set(relPath, fs.readFileSync(fullPath)); } catch (_) { + codexPreInstallHooksCaptureIncomplete = true; + } + } else { + codexPreInstallHooksCaptureIncomplete = true; + } + } + }; + _snapshotHooksDir(_preHooksPath, ''); + } } + // #4544 — shared restore for the manifest-driven surfaces. Called by BOTH + // rollback paths: _codexPreConfigRollback (CHANGELOG.md, scripts/, the + // initial manifest write AND — via installer migrations' stale-hook removal + // — hooks/ itself are all mutated BEFORE config.toml is touched, so the + // early path must cover them) and the full restoreCodexSnapshot() below. + // Best-effort throughout, matching the #3245 convention: restore failures + // never mask the original install error. + const restoreCodexManagedSnapshot = () => { + // #4544 (review) — if the capture never ran (non-Codex runtimes, minimal + // mode), no pre-install state was recorded. The only safe action is NONE: + // an empty snapshot must never be read as "hooks/ was absent", or a + // minimal-mode rollback would delete the user's entire hooks/ tree. + if (!codexManagedSnapshotCaptured) return; + // hooks/ — the pre-install tree is restored wholesale: a user file that + // predated the install is IN the snapshot and comes back; anything the + // failed install staged is not, and goes away with the tree. When the + // capture was INCOMPLETE, wholesale deletion would permanently destroy a + // file whose bytes were never captured, so the restore downgrades to + // per-file: put back what was captured and remove only the names GSD + // itself stages (the hoisted CODEX_HOOKS_TO_COPY set plus the CommonJS + // marker). hooks/lib/ is left untouched in that mode — its contents are + // transitive and cannot be enumerated safely without the capture. + if (codexPreInstallHooksDirPreExisted !== null) { + const _hooksRestoreDir = path.join(targetDir, 'hooks'); + if (!codexPreInstallHooksDirPreExisted) { + // Clean first install: nothing pre-existed under hooks/, so nothing + // the failed install staged may survive either. + try { fs.rmSync(_hooksRestoreDir, { recursive: true, force: true }); } catch (_) { /* best-effort */ } + } else if (!codexPreInstallHooksCaptureIncomplete) { + try { fs.rmSync(_hooksRestoreDir, { recursive: true, force: true }); } catch (_) { /* best-effort */ } + for (const [relPath, buf] of codexPreInstallHooksTree) { + const destFile = path.join(_hooksRestoreDir, relPath); + try { + fs.mkdirSync(path.dirname(destFile), { recursive: true }); + fs.writeFileSync(destFile, buf); + } catch (_) { /* best-effort */ } + } + } else { + // GSD-owned names are removed FIRST: several of them are also + // legitimate pre-install files the snapshot just restored, and a + // removal pass after the restore would delete the restored bytes. + for (const hookName of CODEX_HOOKS_TO_COPY) { + try { fs.rmSync(path.join(_hooksRestoreDir, hookName), { force: true }); } catch (_) { /* best-effort */ } + } + try { fs.rmSync(path.join(_hooksRestoreDir, 'package.json'), { force: true }); } catch (_) { /* best-effort */ } + for (const [relPath, buf] of codexPreInstallHooksTree) { + const destFile = path.join(_hooksRestoreDir, relPath); + try { + fs.mkdirSync(path.dirname(destFile), { recursive: true }); + fs.writeFileSync(destFile, buf); + } catch (_) { /* best-effort */ } + } + } + } + // Every GSD-owned path the prior manifest recorded (plus the clean-install + // write plan): restore bytes, or re-delete a path that was absent + // pre-install. + for (const [relPath, buf] of codexPreInstallManagedFiles) { + const resolved = resolveInstallRelativePath(targetDir, relPath); + if (!resolved) continue; + try { + if (buf !== null) { + fs.mkdirSync(path.dirname(resolved.fullPath), { recursive: true }); + fs.writeFileSync(resolved.fullPath, buf); + } else if (fs.existsSync(resolved.fullPath)) { + fs.rmSync(resolved.fullPath, { force: true }); + } + } catch (_) { /* best-effort */ } + } + // The prior manifest file itself: reinstall rewrites it; rollback returns + // the previous install's manifest (or removes it on a clean first install). + const _manifestRestorePath = path.join(targetDir, MANIFEST_NAME); + if (codexPreInstallManifestBytes !== null) { + try { fs.writeFileSync(_manifestRestorePath, codexPreInstallManifestBytes); } catch (_) { /* best-effort */ } + } else if (fs.existsSync(_manifestRestorePath)) { + try { fs.unlinkSync(_manifestRestorePath); } catch (_) { /* best-effort */ } + } + }; + // #3245 CR finding 2 — Rollback coverage extends to ALL post-snapshot operations, // not just the Codex config/hook error paths. Any throw between snapshot capture and // the Codex config block (skills copy, agents copy, VERSION write, manifest write, etc.) // must also trigger rollback so the caller is never left in a partially-installed state. // - // _codexPreConfigRollback covers the four surfaces that can be mutated before - // config.toml is touched: skills/, agents/, gsd-core/VERSION, and orphaned + // _codexPreConfigRollback covers the surfaces that can be mutated before + // config.toml is touched: skills/, agents/, gsd-core/VERSION, the manifest- + // driven surfaces (#4544 — CHANGELOG.md, scripts/, .gsd-runtime and the + // manifest itself are all rewritten in this window), and orphaned // atomic-write temp files. It is safe to call before any writes have happened. // The full restoreCodexSnapshot() (defined inside the config block) additionally - // handles config.toml, which is not yet touched at this point in the pipeline. + // handles config.toml and the staged hooks/ tree, which are not yet touched + // at this point in the pipeline. const _codexPreConfigRollback = !_hostBehaviors(runtime).tomlConfigInstall || isMinimalMode(_effectiveInstallMode) ? null : () => { rollbackInstallerMigrations(); // skills/gsd-* — pass 1: restore snapshot entries (may be absent if deleted mid-install). @@ -10956,6 +11198,11 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } else if (fs.existsSync(_earlyVersionPath)) { try { fs.unlinkSync(_earlyVersionPath); } catch (_) { /* best-effort */ } } + // #4544 — manifest-driven surfaces (CHANGELOG.md, scripts/, the initial + // manifest write, and — via installer migrations' stale-hook removal — + // hooks/ itself are all mutated in this window). The shared restore is + // also idempotent against an untouched tree. + restoreCodexManagedSnapshot(); // Orphaned atomic-write temp files. const _earlyTmpPattern = /\.tmp-\d+-\d+$/; function _earlyCleanTmpFiles(dir) { @@ -12256,6 +12503,11 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { try { fs.unlinkSync(_rollbackVersionPath); } catch (_) { /* best-effort */ } } + // 4b. #4544 — manifest-driven surfaces: the staged hooks/ tree, every + // GSD-owned path the prior manifest recorded (scripts/, gsd-core/ + // payload), and the prior manifest file itself. + restoreCodexManagedSnapshot(); + // 5. Orphaned atomic-write temp files (.tmp--) in targetDir. // These can accumulate if an atomic write fails mid-rename. Best-effort scan. // @@ -12329,11 +12581,8 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // ENOENT -> allow(undefined), every invocation, every event, no exceptions). // A pre-#2586 install's stale copy + hooks.json registrations are cleaned // up below (see the CODEX_EXTENDED_HOOK_EVENTS loop), not re-added here. - const CODEX_HOOKS_TO_COPY = [ - 'gsd-check-update.js', - 'gsd-check-update-worker.js', - 'managed-hooks-registry.cjs', - ]; + // CODEX_HOOKS_TO_COPY itself lives at module scope (#4544) — the rollback's + // incomplete-capture path must name the same set without a second literal. const codexHooksSrc = path.join(src, 'hooks', 'dist'); if (fs.existsSync(codexHooksSrc)) { const codexHooksDest = path.join(targetDir, 'hooks'); diff --git a/src/installer-migrations.cts b/src/installer-migrations.cts index 56f8ebce6..c28fe270a 100644 --- a/src/installer-migrations.cts +++ b/src/installer-migrations.cts @@ -319,7 +319,14 @@ function readInstallManifest(configDir: string): InstallManifest { version: typeof m.version === 'string' ? m.version : null, timestamp: typeof m.timestamp === 'string' ? m.timestamp : null, mode: typeof m.mode === 'string' ? m.mode : null, - files: m.files && typeof m.files === 'object' ? m.files as Record : {}, + // #4544 (review): `typeof [] === 'object'` — a manifest whose `files` is a + // JSON array passed the object-shape guard, and Object.keys() then yielded + // "0","1",... as install-relative file paths. Consumers iterate these keys, + // so an array shape must degrade to the empty set exactly like a + // non-object shape does. + files: m.files && typeof m.files === 'object' && !Array.isArray(m.files) + ? m.files as Record + : {}, manifestVersion: normalizeManifestVersion(m.manifestVersion), runtime: normalizeReportedRuntime(rawRuntime), scope: isInstallScopeId(m.scope) ? m.scope : null, diff --git a/tests/codex-config-hooks.test.cjs b/tests/codex-config-hooks.test.cjs index 1b35c543e..9fe3bdf26 100644 --- a/tests/codex-config-hooks.test.cjs +++ b/tests/codex-config-hooks.test.cjs @@ -3627,3 +3627,320 @@ describe('#3427 + #3433 — Codex installer avoids duplicate skills and mixed ho }); }); } + + +{ + const { test, describe, beforeEach, afterEach } = require('node:test'); + const assert = require('node:assert/strict'); + const { cleanup, createTempDir } = require('./helpers.cjs'); + const { install: installFor4544 } = require('../bin/install.js'); + const installModule = require('../bin/install.js'); + + // Harness copy of runCodexInstall — the canonical one lives inside a folded + // block above and is not visible at this scope. + function runCodexInstall(codexHome) { + const previousCodexHome = process.env.CODEX_HOME; + const previousCwd = process.cwd(); + const previousHome = process.env.HOME; + const previousUserProfile = process.env.USERPROFILE; + process.env.CODEX_HOME = codexHome; + process.env.HOME = codexHome; + process.env.USERPROFILE = codexHome; + try { + process.chdir(path.join(__dirname, '..')); + return installFor4544(true, 'codex'); + } finally { + process.chdir(previousCwd); + if (previousCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodexHome; + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + if (previousUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = previousUserProfile; + } + } + + describe('#4544 — manifest-driven rollback covers hooks/, scripts/, gsd-core payload, and the manifest', { concurrency: false }, () => { + let tmpDir; + let codexHome; + + beforeEach(() => { + tmpDir = createTempDir('gsd-4544-rollback-'); + codexHome = path.join(tmpDir, 'codex-home'); + }); + + afterEach(() => { + delete installModule.__codexSchemaValidator; + cleanup(tmpDir); + }); + + /** Seed a shape-valid prior-install manifest listing `relPaths`. */ + function seedPriorManifest(relPaths) { + const files = {}; + for (const rel of relPaths) files[rel] = 'prior-install-hash'; + fs.writeFileSync( + path.join(codexHome, 'gsd-file-manifest.json'), + JSON.stringify({ manifestVersion: 2, version: '1.12.0', files }), + 'utf8', + ); + } + + function forceValidationFailure() { + installModule.__codexSchemaValidator = () => ({ + ok: false, + reason: 'simulated failure for #4544 rollback test', + }); + } + + function runFailingInstall() { + let err = null; + try { runCodexInstall(codexHome); } catch (e) { err = e; } + assert.ok(err, 'install must throw when validation fails'); + assert.match( + String(err && err.message), + /post-write Codex schema validation failed/, + 'the throw must be the injected validation failure, not an unrelated error' + ); + } + + test('restores prior-manifest files the failed install overwrote (#4544 must-have)', () => { + fs.mkdirSync(codexHome, { recursive: true }); + const sentinels = new Map([ + ['gsd-core/CHANGELOG.md', 'SENTINEL-CHANGELOG'], + ['gsd-core/.gsd-runtime', 'SENTINEL-RUNTIME'], + ['scripts/lib/drift-scan.cjs', 'SENTINEL-LIB'], + ]); + fs.mkdirSync(path.join(codexHome, 'gsd-core'), { recursive: true }); + fs.mkdirSync(path.join(codexHome, 'scripts', 'lib'), { recursive: true }); + for (const [rel, body] of sentinels) { + fs.writeFileSync(path.join(codexHome, rel), body, 'utf8'); + } + seedPriorManifest([...sentinels.keys()]); + + forceValidationFailure(); + runFailingInstall(); + + for (const [rel, body] of sentinels) { + assert.strictEqual( + fs.readFileSync(path.join(codexHome, rel), 'utf8'), + body, + `rollback must restore the pre-install bytes of ${rel}` + ); + } + }); + + test('re-deletes a prior-manifest path that was absent before the install', () => { + fs.mkdirSync(codexHome, { recursive: true }); + // Listed by the prior install but deleted before this one: the null + // snapshot branch must remove whatever the failed install recreates. + seedPriorManifest(['scripts/lib/drift-scan.cjs']); + assert.strictEqual(fs.existsSync(path.join(codexHome, 'scripts')), false, + 'fixture precondition: the path must not exist pre-install'); + + forceValidationFailure(); + runFailingInstall(); + + assert.strictEqual( + fs.existsSync(path.join(codexHome, 'scripts', 'lib', 'drift-scan.cjs')), + false, + 'rollback must re-delete a prior-manifest path the user had removed' + ); + }); + + test('wholesale-restores the hooks/ directory (the Codex manifest omits it)', () => { + fs.mkdirSync(path.join(codexHome, 'hooks'), { recursive: true }); + fs.writeFileSync(path.join(codexHome, 'hooks', 'gsd-check-update.js'), + 'SENTINEL-USER-EDITED-HOOK', 'utf8'); + + forceValidationFailure(); + runFailingInstall(); + + assert.strictEqual( + fs.readFileSync(path.join(codexHome, 'hooks', 'gsd-check-update.js'), 'utf8'), + 'SENTINEL-USER-EDITED-HOOK', + 'rollback must restore the pre-install hook bytes the install overwrote' + ); + assert.strictEqual( + fs.existsSync(path.join(codexHome, 'hooks', 'package.json')), + false, + 'the CommonJS marker the failed install staged must not survive rollback' + ); + assert.strictEqual( + fs.existsSync(path.join(codexHome, 'hooks', 'managed-hooks-registry.cjs')), + false, + 'a staged hook file that did not pre-exist must not survive rollback' + ); + assert.strictEqual( + fs.existsSync(path.join(codexHome, 'hooks', 'lib')), + false, + 'transitive hooks/lib/ helpers staged by the failed install must not survive rollback' + ); + }); + + test('preserves pre-existing user files under hooks/', () => { + fs.mkdirSync(path.join(codexHome, 'hooks'), { recursive: true }); + fs.writeFileSync(path.join(codexHome, 'hooks', 'my-own.sh'), '#!/bin/sh\necho mine\n', 'utf8'); + + forceValidationFailure(); + runFailingInstall(); + + assert.strictEqual( + fs.readFileSync(path.join(codexHome, 'hooks', 'my-own.sh'), 'utf8'), + '#!/bin/sh\necho mine\n', + 'a user-owned hooks/ file that predated the install must survive rollback' + ); + }); + + test('restores the prior gsd-file-manifest.json the failed install rewrote', () => { + fs.mkdirSync(codexHome, { recursive: true }); + const priorManifest = JSON.stringify({ + manifestVersion: 2, version: '1.12.0', + files: { 'gsd-core/CHANGELOG.md': 'prior-hash' }, + }, null, 2); + fs.writeFileSync(path.join(codexHome, 'gsd-file-manifest.json'), priorManifest, 'utf8'); + fs.mkdirSync(path.join(codexHome, 'gsd-core'), { recursive: true }); + fs.writeFileSync(path.join(codexHome, 'gsd-core', 'CHANGELOG.md'), 'SENTINEL', 'utf8'); + + forceValidationFailure(); + runFailingInstall(); + + assert.strictEqual( + fs.readFileSync(path.join(codexHome, 'gsd-file-manifest.json'), 'utf8'), + priorManifest, + 'rollback must restore the prior manifest bytes' + ); + assert.strictEqual( + fs.readFileSync(path.join(codexHome, 'gsd-core', 'CHANGELOG.md'), 'utf8'), + 'SENTINEL', + 'the manifest-listed file must be restored alongside the manifest itself' + ); + }); + + test('clean-first-install rollback leaves no manifest or hooks residue', () => { + fs.mkdirSync(codexHome, { recursive: true }); + + forceValidationFailure(); + runFailingInstall(); + + assert.strictEqual( + fs.existsSync(path.join(codexHome, 'gsd-file-manifest.json')), + false, + 'no prior manifest existed, so the manifest the failed install wrote must be gone' + ); + assert.strictEqual( + fs.existsSync(path.join(codexHome, 'hooks')), + false, + 'hooks/ must not exist at all after a clean-first-install rollback (nothing pre-existed)' + ); + }); + + test('a malformed prior manifest degrades gracefully', () => { + // No VERSION pre-exists, so the five-target restore must still remove the + // one the failed install wrote -- an unreadable prior manifest must cost + // the original restores nothing. + fs.mkdirSync(codexHome, { recursive: true }); + fs.writeFileSync(path.join(codexHome, 'gsd-file-manifest.json'), 'garbage{{{', 'utf8'); + + forceValidationFailure(); + runFailingInstall(); + + assert.strictEqual(fs.existsSync(path.join(codexHome, 'gsd-core', 'VERSION')), false, + 'VERSION rollback must still work when the prior manifest is unreadable'); + }); + + test('a prior manifest that is a JSON array degrades gracefully', () => { + fs.mkdirSync(codexHome, { recursive: true }); + fs.writeFileSync(path.join(codexHome, 'gsd-file-manifest.json'), '[]', 'utf8'); + + forceValidationFailure(); + runFailingInstall(); + + // The array file PRE-EXISTED, so it is pre-install state: rollback + // restores those exact bytes rather than deleting them. What must be gone + // is any manifest the failed install wrote over it. + assert.strictEqual(fs.readFileSync(path.join(codexHome, 'gsd-file-manifest.json'), 'utf8'), '[]', + 'an array-shaped prior manifest is restored as pre-install state; rollback must not crash on it'); + }); + + test('a traversal-shaped manifest key is skipped, not written', () => { + fs.mkdirSync(codexHome, { recursive: true }); + const canary = path.join(tmpDir, 'evil.txt'); + fs.writeFileSync(canary, 'do-not-touch', 'utf8'); + // `../evil.txt` resolves OUTSIDE codexHome — resolveInstallRelativePath + // must reject it, and the snapshotter must skip the key entirely. + seedPriorManifest(['../evil.txt']); + + forceValidationFailure(); + runFailingInstall(); + + assert.strictEqual(fs.readFileSync(canary, 'utf8'), 'do-not-touch', + 'a traversal manifest key must never cause a write outside the install root'); + }); + + test('very early failure: the new restores stay idempotent before any capture', () => { + fs.mkdirSync(codexHome, { recursive: true }); + + forceValidationFailure(); + runFailingInstall(); + + assert.strictEqual(fs.existsSync(path.join(codexHome, 'gsd-file-manifest.json')), false, + 'an empty home must still be empty after rollback'); + assert.strictEqual(fs.existsSync(path.join(codexHome, 'hooks')), false, + 'an empty home must have no hooks/ residue after rollback'); + }); + + test('minimal-mode rollback never touches a pre-existing hooks/ tree (capture gate)', () => { + // BLOCKER regression (#4544 review): the capture gate is off in minimal + // mode, and the restore must treat "no snapshot" as "do nothing" — never + // as "hooks/ was absent". Seeds the profile marker so the in-process + // install runs minimal without a validator-unreachable subprocess. + fs.mkdirSync(path.join(codexHome, 'hooks'), { recursive: true }); + fs.writeFileSync(path.join(codexHome, 'hooks', 'gsd-check-update.js'), 'SENTINEL-OLD-HOOK', 'utf8'); + fs.writeFileSync(path.join(codexHome, 'hooks', 'user-backup.js'), 'PRECIOUS-USER-DATA', 'utf8'); + fs.writeFileSync(path.join(codexHome, '.gsd-profile'), 'core', 'utf8'); + + forceValidationFailure(); + runFailingInstall(); + + assert.strictEqual( + fs.readFileSync(path.join(codexHome, 'hooks', 'gsd-check-update.js'), 'utf8'), + 'SENTINEL-OLD-HOOK', + 'minimal-mode rollback must preserve the pre-existing hook file' + ); + assert.strictEqual( + fs.readFileSync(path.join(codexHome, 'hooks', 'user-backup.js'), 'utf8'), + 'PRECIOUS-USER-DATA', + 'minimal-mode rollback must preserve user files under hooks/ (empty snapshot means do nothing)' + ); + }); + + test('a symlink under hooks/ is never followed; downgrade preserves it uncaptured', (t) => { + const canaryDir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'gsd-4544-canary-')); + t.after(() => cleanup(canaryDir)); + const canary = path.join(canaryDir, 'secret.txt'); + fs.writeFileSync(canary, 'DO-NOT-READ', 'utf8'); + + fs.mkdirSync(path.join(codexHome, 'hooks'), { recursive: true }); + const linkPath = path.join(codexHome, 'hooks', 'evil-link'); + try { + fs.symlinkSync(canary, linkPath); + } catch (_) { + t.skip('symlinks unavailable on this platform/filesystem'); + return; + } + + forceValidationFailure(); + runFailingInstall(); + + assert.strictEqual(fs.readFileSync(canary, 'utf8'), 'DO-NOT-READ', + 'the symlink referent must never be read into the snapshot or written over'); + // The link makes the capture incomplete, so the restore downgrades to + // per-file: nothing uncaptured is deleted — the pre-existing link is + // pre-install state and survives, still pointing at its referent. + assert.strictEqual(fs.readFileSync(linkPath, 'utf8'), 'DO-NOT-READ', + 'the preserved link must still resolve to the untouched referent'); + assert.strictEqual(fs.existsSync(path.join(codexHome, 'hooks', 'secret.txt')), false, + 'the referent bytes must not leak into the install tree as a regular file'); + }); +}); +}