From a02462e050b5203d8358733e4d78f52546e5f424 Mon Sep 17 00:00:00 2001 From: 0xdhx Date: Tue, 28 Jul 2026 06:05:32 -0500 Subject: [PATCH] test(#2665): fail the suite when it writes into a live config dir The recurrence guard, and #2665's own "Optional hardening". This class is silent by construction: TEST_ENV_BASE cannot see an in-process caller, and CI cannot see the class at all because CI never has these env vars set. It damages the developer's machine and reports nothing -- which is how two prior authors each diagnosed it and fixed only the instance in front of them. run-tests.cjs snapshots GSD's install footprint in every live runtime config dir before the suite and re-checks it after, failing the run on a create or a modify. Roots come from the product's own getGlobalConfigDir, so the guard watches wherever the product actually points, including through an ambient var. Scope is ownership-based, not whole-root: the top-level install footprint plus gsd-prefixed children of dirs GSD shares with the host agent. A config root like ~/.claude is shared, and watching it wholesale would false-positive on the host's own history.jsonl or settings.json -- a guard that cries wolf gets disabled, and then catches nothing. The prefix test is load-bearing: the first version watched only the three top-level entries and MISSED a real leak into skills/gsd-*. It earned its place immediately -- it is what found the fifth in-process leak in runtime-artifact-layout.test.cjs, which no amount of reading the review would have surfaced. Known gap documented in the module: a write to a file GSD does not own is out of scope by construction. Lives in scripts/, deliberately NOT scripts/lib/ -- the installer copies that dir into every user's config dir wholesale while uninstall removes only an allowlist, so a test-only module there would ship to users and survive uninstall. Addresses review finding: Minor 8. --- scripts/live-config-guard.cjs | 245 +++++++++++++++++++++++++++++++ scripts/run-tests.cjs | 29 ++++ tests/live-config-guard.test.cjs | 160 ++++++++++++++++++++ 3 files changed, 434 insertions(+) create mode 100644 scripts/live-config-guard.cjs create mode 100644 tests/live-config-guard.test.cjs diff --git a/scripts/live-config-guard.cjs b/scripts/live-config-guard.cjs new file mode 100644 index 000000000..74a83c433 --- /dev/null +++ b/scripts/live-config-guard.cjs @@ -0,0 +1,245 @@ +#!/usr/bin/env node +'use strict'; + +/** + * #2665 — post-suite hermeticity guard. + * + * The test suite must never write into a runtime's LIVE config directory. Two + * mechanisms defend that, and neither one can see this failure: + * + * - `TEST_ENV_BASE` (tests/helpers.cjs) blanks every config-location env var, + * but only for CHILD processes. A test that calls the installer IN-PROCESS + * is untouched by it. + * - CI is blind to the ambient-env half of the class outright, because CI + * never has `CLAUDE_CONFIG_DIR` and friends set. + * + * So the class is silent by construction: it damages the developer's machine and + * reports nothing. #2665 records two prior authors each diagnosing it and fixing + * only the instance in front of them. This module converts it from silent to + * loud by snapshotting GSD's own install footprint before the suite and + * re-checking it after. + * + * LOCATION — `scripts/`, deliberately NOT `scripts/lib/`. The installer copies + * `scripts/lib/` into every user's config dir wholesale (readdirSync), while + * uninstall removes only an explicit allowlist, so a test-only module placed + * there would ship to users AND survive uninstall. `scripts/affected-tests-lib.cjs` + * is the precedent for a non-shipped helper at this level. + * + * SCOPE — ownership-based, not whole-root. It watches entries GSD unambiguously + * owns: the top-level install footprint (`GSD_OWNED_ENTRIES`) plus `gsd-`-prefixed + * children of the dirs GSD shares with the host agent (`GSD_PREFIXED_PARENTS`). + * It does NOT watch whole config roots. A root such as `~/.claude` is shared with + * the host agent, which may legitimately write `history.jsonl`, `todos/`, or + * `settings.json` while the suite runs; watching the root would turn that into a + * false failure, and a guard that cries wolf gets disabled — after which it + * catches nothing at all. + * + * The ownership test is the prefix, not the location. That distinction is load + * bearing: the first version of this guard watched only the three top-level + * entries and MISSED a real leak into `/skills/gsd-dev-preferences/`. + * + * KNOWN GAP — a leak into a file GSD does not own (e.g. mutating the host's own + * `.claude.json`) is outside this guard by construction. Closing it would require + * watching shared files, which is the false-positive trap above. + */ + +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +/** Top-level entries only a GSD install creates. See SCOPE above before widening. */ +const GSD_OWNED_ENTRIES = ['gsd-core', 'gsd-file-manifest.json', 'gsd-pristine']; + +/** + * Directories GSD SHARES with the host agent. Watching them wholesale would + * false-positive on the host's own writes, so only `gsd-`-prefixed children are + * watched — those are unambiguously ours. + * + * Added after the first version of this guard MISSED a real leak: a raw + * `spawnSync` that sandboxed HOME but inherited an ambient CLAUDE_CONFIG_DIR + * wrote `/skills/gsd-dev-preferences/SKILL.md`, which sits under none of + * the three top-level entries above. + */ +const GSD_PREFIXED_PARENTS = ['agents', 'commands', 'skills']; +const GSD_ARTIFACT_PREFIX = 'gsd-'; + +/** Bounds on the recursive walk, so a pathological tree cannot stall the suite. */ +const MAX_ENTRIES = 20000; +const MAX_DEPTH = 12; + +/** + * Resolve every runtime config root the product could write to, using the REAL + * resolver rather than a reimplementation — the guard must watch wherever the + * product actually points, including through an ambient env var. + * + * @returns {string[]} deduped, sorted roots; empty if the built lib is absent. + */ +function resolveLiveConfigRoots(deps = {}) { + const libDir = deps.libDir || path.join(__dirname, '..', 'gsd-core', 'bin', 'lib'); + let getGlobalConfigDir; + let runtimes; + try { + ({ getGlobalConfigDir } = require(path.join(libDir, 'runtime-homes.cjs'))); + ({ runtimes } = require(path.join(libDir, 'capability-registry.cjs'))); + } catch { + // Unbuilt tree: the guard is advisory infrastructure and must never be the + // reason a test run cannot start. Callers treat [] as "guard unavailable". + return []; + } + + const roots = new Set(); + // 'grok' is a hardcoded branch of getGlobalConfigDir with no registry entry. + for (const runtime of [...Object.keys(runtimes || {}), 'grok']) { + try { + const dir = getGlobalConfigDir(runtime); + if (typeof dir === 'string' && dir.length > 0) roots.add(path.resolve(dir)); + } catch { + // A descriptor the resolver cannot satisfy is not this module's problem. + } + } + return [...roots].sort(); +} + +/** + * Newest mtime within a tree, bounded. Returns `truncated: true` when a bound + * was hit — the caller must NOT report such a result as clean, on the same + * principle that an existence probe passing vacuously is worse than no probe. + */ +function newestMtime(target, budget) { + let newest = 0; + let truncated = false; + + const walk = (current, depth) => { + if (budget.remaining <= 0) { truncated = true; return; } + if (depth > MAX_DEPTH) { truncated = true; return; } + let st; + try { + st = fs.lstatSync(current); + } catch { + return; + } + budget.remaining -= 1; + if (st.mtimeMs > newest) newest = st.mtimeMs; + if (!st.isDirectory()) return; + let entries; + try { + entries = fs.readdirSync(current); + } catch { + return; + } + for (const entry of entries) walk(path.join(current, entry), depth + 1); + }; + + walk(target, 0); + return { newest, truncated }; +} + +/** + * Snapshot GSD-owned entries under each root. + * + * @returns {Record} + * keyed by absolute entry path. + */ +function snapshotLiveConfig(roots) { + const budget = { remaining: MAX_ENTRIES }; + const snap = {}; + + const record = (target) => { + if (!fs.existsSync(target)) { + snap[target] = { exists: false, newest: 0, truncated: false }; + return; + } + const { newest, truncated } = newestMtime(target, budget); + snap[target] = { exists: true, newest, truncated }; + }; + + for (const root of roots) { + for (const entry of GSD_OWNED_ENTRIES) record(path.join(root, entry)); + + // Shared dirs: enumerate only gsd-prefixed children. A child that appears + // between the two snapshots is absent from `before` entirely — diffLiveConfig + // treats after-only paths as created, which is exactly the leak signal. + for (const parent of GSD_PREFIXED_PARENTS) { + const parentDir = path.join(root, parent); + let children; + try { + children = fs.readdirSync(parentDir); + } catch { + continue; // parent absent — nothing of ours can be in it yet + } + for (const child of children) { + if (child.startsWith(GSD_ARTIFACT_PREFIX)) record(path.join(parentDir, child)); + } + } + } + return snap; +} + +/** + * Compare two snapshots. A path is a violation when it was created during the + * run, or when its newest mtime advanced. + * + * @returns {{path: string, kind: 'created'|'modified'|'unverified'}[]} + */ +function diffLiveConfig(before, after) { + const violations = []; + for (const [target, post] of Object.entries(after)) { + const pre = before[target]; + // Absent from `before` entirely: a gsd-prefixed child that did not exist + // when the run started. Both snapshots cover the same roots, so an + // after-only path was created BY the run — never skip it. + if (!pre) { + if (post.exists) violations.push({ path: target, kind: 'created' }); + continue; + } + if (!pre.exists && post.exists) { + violations.push({ path: target, kind: 'created' }); + } else if (pre.exists && post.exists && post.newest > pre.newest) { + violations.push({ path: target, kind: 'modified' }); + } else if (pre.truncated || post.truncated) { + // Bound hit: we cannot attest this path either way, and saying nothing + // would let a truncated scan read as a clean one. + violations.push({ path: target, kind: 'unverified' }); + } + } + return violations; +} + +/** Human-facing report for a non-empty violation set. */ +function formatViolations(violations) { + const lines = [ + '', + 'run-tests: HERMETICITY FAILURE — the suite wrote into a LIVE config directory.', + '', + 'A test resolved a runtime config dir from the ambient environment instead of a', + 'sandbox. The usual cause is an IN-PROCESS install() call: tests/helpers.cjs', + 'TEST_ENV_BASE only scrubs CHILD process env, so an in-process caller must also', + 'use scrubConfigLocationEnv() (see tests/install.test.cjs) alongside its HOME', + 'sandbox. CI cannot catch this class — it never has these env vars set.', + '', + ]; + for (const v of violations) { + const label = v.kind === 'unverified' + ? 'UNVERIFIED (scan bound hit — not attested clean)' + : v.kind.toUpperCase(); + lines.push(` ${label}: ${v.path}`); + } + lines.push(''); + lines.push('Set GSD_SKIP_LIVE_CONFIG_GUARD=1 to bypass (intentionally loud).'); + lines.push(''); + return lines.join('\n'); +} + +module.exports = { + GSD_OWNED_ENTRIES, + GSD_PREFIXED_PARENTS, + GSD_ARTIFACT_PREFIX, + MAX_ENTRIES, + MAX_DEPTH, + resolveLiveConfigRoots, + snapshotLiveConfig, + diffLiveConfig, + formatViolations, + newestMtime, + os, // exported for test seams only +}; diff --git a/scripts/run-tests.cjs b/scripts/run-tests.cjs index 4d9912127..e2eef2ec8 100644 --- a/scripts/run-tests.cjs +++ b/scripts/run-tests.cjs @@ -39,6 +39,12 @@ const { readdirSync, readFileSync } = require('fs'); const { join, basename } = require('path'); const { execFileSync } = require('child_process'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); +const { + resolveLiveConfigRoots, + snapshotLiveConfig, + diffLiveConfig, + formatViolations, +} = require('./live-config-guard.cjs'); const SUITES = ['all', 'unit', 'integration', 'install', 'security', 'slow', 'qa']; @@ -965,6 +971,18 @@ function main() { // job cap. Operator/test override via RUN_TESTS_CHUNK_TIMEOUT_MS. const chunkTimeoutMs = positiveNumberEnv(process.env.RUN_TESTS_CHUNK_TIMEOUT_MS, 600000); + // #2665: snapshot GSD's install footprint in every LIVE runtime config dir + // before a single test runs. The suite must not write there; the check after + // the chunk loop is what makes a violation loud instead of silent. See + // scripts/lib/live-config-guard.cjs for why the scope is narrow. + const liveConfigGuardEnabled = process.env.GSD_SKIP_LIVE_CONFIG_GUARD !== '1'; + let liveConfigRoots = []; + let liveConfigBefore = null; + if (liveConfigGuardEnabled) { + liveConfigRoots = resolveLiveConfigRoots(); + if (liveConfigRoots.length > 0) liveConfigBefore = snapshotLiveConfig(liveConfigRoots); + } + let firstFailureExit = 0; for (let i = 0; i < chunks.length; i++) { if (chunks.length > 1) { @@ -1030,6 +1048,17 @@ function main() { // and the first non-zero exit is reported at the end. } } + // #2665: post-suite hermeticity check. Runs even when tests failed — a leaked + // global install is worth reporting alongside the failure that hid it, and + // suppressing it on red would hide it exactly when the suite is least trusted. + if (liveConfigBefore) { + const violations = diffLiveConfig(liveConfigBefore, snapshotLiveConfig(liveConfigRoots)); + if (violations.length > 0) { + console.error(formatViolations(violations)); + if (firstFailureExit === 0) firstFailureExit = 1; + } + } + if (firstFailureExit !== 0) return firstFailureExit; } diff --git a/tests/live-config-guard.test.cjs b/tests/live-config-guard.test.cjs new file mode 100644 index 000000000..b942342b6 --- /dev/null +++ b/tests/live-config-guard.test.cjs @@ -0,0 +1,160 @@ +'use strict'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const { + GSD_OWNED_ENTRIES, + resolveLiveConfigRoots, + snapshotLiveConfig, + diffLiveConfig, + formatViolations, +} = require('../scripts/live-config-guard.cjs'); + +const { cleanup } = require('./helpers.cjs'); + +function tmpRoot() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'live-config-guard-')); +} + +describe('#2665: live-config hermeticity guard', () => { + test('resolves real runtime config roots via the product resolver', () => { + const roots = resolveLiveConfigRoots(); + // Guards the guard: an empty set would make every downstream assertion + // vacuous, and run-tests.cjs would silently skip the check. + assert.ok(roots.length > 5, `expected many runtime config roots, got ${roots.length}`); + for (const root of roots) { + assert.ok(path.isAbsolute(root), `root must be absolute: ${root}`); + } + }); + + test('a clean run produces no violations', () => { + const root = tmpRoot(); + try { + fs.mkdirSync(path.join(root, 'gsd-core', 'bin'), { recursive: true }); + fs.writeFileSync(path.join(root, 'gsd-core', 'bin', 'x.cjs'), 'x'); + + const before = snapshotLiveConfig([root]); + const after = snapshotLiveConfig([root]); + assert.deepStrictEqual(diffLiveConfig(before, after), []); + } finally { + cleanup(root); + } + }); + + test('detects a global install CREATED during the run', () => { + const root = tmpRoot(); + try { + const before = snapshotLiveConfig([root]); + // Exactly the Blocker 1 shape: an in-process install(true, …) landing a + // full global install in a live config dir that was previously empty. + fs.mkdirSync(path.join(root, 'gsd-core'), { recursive: true }); + fs.writeFileSync(path.join(root, 'gsd-file-manifest.json'), '{}'); + + const violations = diffLiveConfig(before, snapshotLiveConfig([root])); + const kinds = Object.fromEntries(violations.map((v) => [path.basename(v.path), v.kind])); + assert.strictEqual(kinds['gsd-core'], 'created'); + assert.strictEqual(kinds['gsd-file-manifest.json'], 'created'); + } finally { + cleanup(root); + } + }); + + test('detects an existing install MODIFIED during the run', () => { + const root = tmpRoot(); + try { + const target = path.join(root, 'gsd-core', 'bin'); + fs.mkdirSync(target, { recursive: true }); + const file = path.join(target, 'gsd-tools.cjs'); + fs.writeFileSync(file, 'original'); + + const before = snapshotLiveConfig([root]); + // mtime resolution is coarse on some filesystems; set it forward explicitly + // rather than racing the clock with a sleep. + const future = new Date(Date.now() + 10000); + fs.writeFileSync(file, 'clobbered'); + fs.utimesSync(file, future, future); + + const violations = diffLiveConfig(before, snapshotLiveConfig([root])); + assert.strictEqual(violations.length, 1); + assert.strictEqual(violations[0].kind, 'modified'); + assert.strictEqual(path.basename(violations[0].path), 'gsd-core'); + } finally { + cleanup(root); + } + }); + + test('ignores non-GSD writes in a shared config root', () => { + const root = tmpRoot(); + try { + const before = snapshotLiveConfig([root]); + // A concurrent host-agent session writing its own state must NOT trip the + // guard — a guard that cries wolf gets disabled, and then catches nothing. + fs.writeFileSync(path.join(root, 'history.jsonl'), '{}'); + fs.mkdirSync(path.join(root, 'todos'), { recursive: true }); + fs.writeFileSync(path.join(root, 'settings.json'), '{}'); + + assert.deepStrictEqual(diffLiveConfig(before, snapshotLiveConfig([root])), []); + } finally { + cleanup(root); + } + }); + + test('watches exactly the GSD-owned entry set', () => { + const root = tmpRoot(); + try { + const snap = snapshotLiveConfig([root]); + const watched = Object.keys(snap).map((p) => path.basename(p)).sort(); + assert.deepStrictEqual(watched, [...GSD_OWNED_ENTRIES].sort()); + } finally { + cleanup(root); + } + }); + + test('detects a gsd-prefixed artifact written into a SHARED dir', () => { + const root = tmpRoot(); + try { + fs.mkdirSync(path.join(root, 'skills'), { recursive: true }); + const before = snapshotLiveConfig([root]); + // The exact leak the first version of this guard MISSED: a writer that + // sandboxed HOME but inherited an ambient CLAUDE_CONFIG_DIR landed + // /skills/gsd-dev-preferences/SKILL.md, outside the three + // top-level GSD entries. + fs.mkdirSync(path.join(root, 'skills', 'gsd-dev-preferences'), { recursive: true }); + fs.writeFileSync(path.join(root, 'skills', 'gsd-dev-preferences', 'SKILL.md'), '# x'); + + const violations = diffLiveConfig(before, snapshotLiveConfig([root])); + assert.strictEqual(violations.length, 1); + assert.strictEqual(violations[0].kind, 'created'); + assert.strictEqual(path.basename(violations[0].path), 'gsd-dev-preferences'); + } finally { + cleanup(root); + } + }); + + test('ignores NON-gsd artifacts in a shared dir', () => { + const root = tmpRoot(); + try { + fs.mkdirSync(path.join(root, 'skills'), { recursive: true }); + const before = snapshotLiveConfig([root]); + // The host agent's own skills must not trip the guard. + fs.mkdirSync(path.join(root, 'skills', 'my-personal-skill'), { recursive: true }); + fs.writeFileSync(path.join(root, 'skills', 'my-personal-skill', 'SKILL.md'), '# mine'); + + assert.deepStrictEqual(diffLiveConfig(before, snapshotLiveConfig([root])), []); + } finally { + cleanup(root); + } + }); + + test('the report names the path and the remedy', () => { + const out = formatViolations([{ path: '/live/.claude/gsd-core', kind: 'created' }]); + assert.match(out, /HERMETICITY FAILURE/); + assert.match(out, /\/live\/\.claude\/gsd-core/); + assert.match(out, /scrubConfigLocationEnv/); + assert.match(out, /GSD_SKIP_LIVE_CONFIG_GUARD/); + }); +});