diff --git a/scripts/live-config-guard.cjs b/scripts/live-config-guard.cjs index 5794718bf..b0e054bc4 100644 --- a/scripts/live-config-guard.cjs +++ b/scripts/live-config-guard.cjs @@ -111,6 +111,50 @@ function resolveLiveConfigRoots(deps = {}) { return [...roots].sort(); } +/** + * Watch targets that are NOT runtime config roots, and so cannot be expressed as + * `root x GSD_OWNED_ENTRIES`. + * + * #2665 round 3: resolveLiveConfigRoots enumerates getGlobalConfigDir per registry + * runtime plus grok. Two live write surfaces are invisible to that shape, so a leak + * on either passed through this guard — the PR's own safety net — silently: + * + * $GSD_HOME/.gsd — GSD's user-owned store (consent.json, defaults.json, capability + * overlays). Watched WHOLESALE: unlike ~/.claude this root is + * exclusively ours, so the shared-root false-positive trap in + * SCOPE above does not apply and an ownership filter would only + * narrow the guard for nothing. + * /config.toml — the file GSD writes its native [[hooks]] block into + * (resolveKimiHooksTomlDir, KIMI_SHARE_DIR). The INVERSE case: + * ~/.kimi belongs to Kimi CLI, so only the one file GSD writes is + * watched, never the root. This is the KNOWN GAP above accepted + * deliberately in one direction — GSD demonstrably writes this + * file (bin/install.js calls resolveKimiHooksTomlDir at two sites), + * so a concurrent Kimi CLI write is the only false positive, and + * Kimi is not running during the suite. + * + * @returns {string[]} absolute paths; empty if the built lib is absent. + */ +function resolveExtraWatchTargets(deps = {}) { + const libDir = deps.libDir || path.join(__dirname, '..', 'gsd-core', 'bin', 'lib'); + const env = deps.env || process.env; + const homedir = (deps.os || os).homedir; + + const targets = [path.resolve(path.join(env.GSD_HOME || homedir(), '.gsd'))]; + + try { + const { resolveKimiHooksTomlDir } = require(path.join(libDir, 'runtime-homes.cjs')); + // Thread the SAME injected env/home the GSD_HOME line above uses. Calling it + // bare reads process.env and os.homedir() regardless of `deps`, which leaves + // the seam untestable and the two targets resolved against different worlds. + const kimiDir = resolveKimiHooksTomlDir({ env, home: homedir() }); + targets.push(path.resolve(path.join(kimiDir, 'config.toml'))); + } catch { + // Unbuilt tree — same posture as resolveLiveConfigRoots: advisory, never fatal. + } + return targets; +} + /** * 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 @@ -151,7 +195,7 @@ function newestMtime(target, budget) { * @returns {Record} * keyed by absolute entry path. */ -function snapshotLiveConfig(roots) { +function snapshotLiveConfig(roots, extraTargets = []) { const budget = { remaining: MAX_ENTRIES }; const snap = {}; @@ -164,6 +208,12 @@ function snapshotLiveConfig(roots) { snap[target] = { exists: true, newest, truncated }; }; + // Non-root targets (resolveExtraWatchTargets) are recorded verbatim — they are + // already the exact path to watch, whole-dir or single-file. Passed explicitly + // rather than resolved here so a caller testing a fixture root does not silently + // pull the developer's real ~/.gsd into its snapshot. + for (const target of extraTargets) record(path.resolve(target)); + for (const root of roots) { for (const entry of GSD_OWNED_ENTRIES) record(path.join(root, entry)); @@ -251,6 +301,7 @@ module.exports = { MAX_ENTRIES, MAX_DEPTH, resolveLiveConfigRoots, + resolveExtraWatchTargets, snapshotLiveConfig, diffLiveConfig, formatViolations, diff --git a/scripts/run-tests.cjs b/scripts/run-tests.cjs index 1633c7d78..0fabccd13 100644 --- a/scripts/run-tests.cjs +++ b/scripts/run-tests.cjs @@ -41,6 +41,7 @@ const { execFileSync } = require('child_process'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); const { resolveLiveConfigRoots, + resolveExtraWatchTargets, snapshotLiveConfig, diffLiveConfig, formatViolations, @@ -977,10 +978,19 @@ function main() { // 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 liveConfigExtras = []; let liveConfigBefore = null; if (liveConfigGuardEnabled) { liveConfigRoots = resolveLiveConfigRoots(); - if (liveConfigRoots.length > 0) liveConfigBefore = snapshotLiveConfig(liveConfigRoots); + // #2665 round 3: $GSD_HOME/.gsd and kimi's native config.toml are live write + // surfaces that are not runtime config ROOTS, so they are invisible to the + // line above. Watched independently — and note the OR: the extras alone are + // reason enough to snapshot, so an unbuilt tree that yields zero roots no + // longer silently disables the whole guard. + liveConfigExtras = resolveExtraWatchTargets(); + if (liveConfigRoots.length > 0 || liveConfigExtras.length > 0) { + liveConfigBefore = snapshotLiveConfig(liveConfigRoots, liveConfigExtras); + } } let firstFailureExit = 0; @@ -1052,7 +1062,10 @@ function main() { // 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)); + const violations = diffLiveConfig( + liveConfigBefore, + snapshotLiveConfig(liveConfigRoots, liveConfigExtras), + ); if (violations.length > 0) { console.error(formatViolations(violations)); // Reports by default; fails only under opt-in strict mode. See the diff --git a/tests/live-config-guard.test.cjs b/tests/live-config-guard.test.cjs index 0a757795a..9760f4767 100644 --- a/tests/live-config-guard.test.cjs +++ b/tests/live-config-guard.test.cjs @@ -6,12 +6,17 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); +const fc = require('fast-check'); + const { GSD_OWNED_ENTRIES, + MAX_DEPTH, resolveLiveConfigRoots, + resolveExtraWatchTargets, snapshotLiveConfig, diffLiveConfig, formatViolations, + newestMtime, } = require('../scripts/live-config-guard.cjs'); const { cleanup } = require('./helpers.cjs'); @@ -20,6 +25,13 @@ function tmpRoot() { return fs.mkdtempSync(path.join(os.tmpdir(), 'live-config-guard-')); } +/** Create `n` flat files under a fresh dir; returns [dir, entryCount-including-dir]. */ +function treeWithEntries(n) { + const dir = tmpRoot(); + for (let i = 0; i < n; i++) fs.writeFileSync(path.join(dir, `f${i}`), 'x'); + return [dir, n + 1]; // +1: the directory itself is lstat'd and costs budget +} + describe('#2665: live-config hermeticity guard', () => { test('resolves real runtime config roots via the product resolver', () => { const roots = resolveLiveConfigRoots(); @@ -159,3 +171,209 @@ describe('#2665: live-config hermeticity guard', () => { assert.match(out, /GSD_STRICT_LIVE_CONFIG_GUARD/); }); }); + +// ── Round 3: the two write surfaces that are not runtime config ROOTS ──────── +describe('#2665: guard watches non-root write surfaces', () => { + test('resolveExtraWatchTargets covers $GSD_HOME/.gsd and kimi config.toml', () => { + const home = tmpRoot(); + const share = tmpRoot(); + try { + const targets = resolveExtraWatchTargets({ + env: { GSD_HOME: home, KIMI_SHARE_DIR: share }, + os: { homedir: () => home }, + }); + assert.ok( + targets.includes(path.resolve(path.join(home, '.gsd'))), + `expected $GSD_HOME/.gsd in ${JSON.stringify(targets)}`, + ); + assert.ok( + targets.some((t) => t === path.resolve(path.join(share, 'config.toml'))), + `expected kimi config.toml in ${JSON.stringify(targets)}`, + ); + } finally { + cleanup(home); + cleanup(share); + } + }); + + test('GSD_HOME falls back to homedir when unset', () => { + const home = tmpRoot(); + try { + const targets = resolveExtraWatchTargets({ env: {}, os: { homedir: () => home } }); + assert.ok(targets.includes(path.resolve(path.join(home, '.gsd')))); + } finally { + cleanup(home); + } + }); + + test('detects a consent/defaults write into $GSD_HOME/.gsd', () => { + const home = tmpRoot(); + try { + const target = path.join(home, '.gsd'); + const before = snapshotLiveConfig([], [target]); + // The Blocker-1 shape one family over: an ambient GSD_HOME sends real + // consent records and defaults.json into the developer's own store. + fs.mkdirSync(target, { recursive: true }); + fs.writeFileSync(path.join(target, 'consent.json'), '{}'); + + const violations = diffLiveConfig(before, snapshotLiveConfig([], [target])); + assert.strictEqual(violations.length, 1); + assert.strictEqual(violations[0].kind, 'created'); + } finally { + cleanup(home); + } + }); + + test('detects a [[hooks]] write into kimi config.toml', () => { + const share = tmpRoot(); + try { + const target = path.join(share, 'config.toml'); + const before = snapshotLiveConfig([], [target]); + fs.writeFileSync(target, '[[hooks]]\n'); + + const violations = diffLiveConfig(before, snapshotLiveConfig([], [target])); + assert.strictEqual(violations.length, 1); + assert.strictEqual(violations[0].kind, 'created'); + } finally { + cleanup(share); + } + }); + + test('NEGATIVE CONTROL: without the extras both leaks are silent', () => { + const home = tmpRoot(); + try { + // This is the pre-round-3 guard shape — roots only. It is what let a leak + // on either variable pass through the PR's own safety net unreported. + const before = snapshotLiveConfig([]); + fs.mkdirSync(path.join(home, '.gsd'), { recursive: true }); + fs.writeFileSync(path.join(home, '.gsd', 'consent.json'), '{}'); + + assert.deepStrictEqual(diffLiveConfig(before, snapshotLiveConfig([])), []); + } finally { + cleanup(home); + } + }); + + test('a whole-dir extra target does not watch unrelated siblings', () => { + const home = tmpRoot(); + try { + const target = path.join(home, '.gsd'); + fs.mkdirSync(target, { recursive: true }); + const before = snapshotLiveConfig([], [target]); + // A sibling of .gsd is outside the watched target entirely. + fs.writeFileSync(path.join(home, 'unrelated.json'), '{}'); + + assert.deepStrictEqual(diffLiveConfig(before, snapshotLiveConfig([], [target])), []); + } finally { + cleanup(home); + } + }); +}); + +// ── Round 3: the truncation budget — the module's own safety-critical case ─── +describe('#2665: scan-budget truncation', () => { + test('boundary: limit-1 truncates, limit and limit+1 do not', () => { + const [dir, entries] = treeWithEntries(24); + try { + // RULESET.TESTS.boundary-coverage: N in {limit-1, limit, limit+1}. The + // budget is injected, so the boundary is exercised at a real threshold + // without materialising MAX_ENTRIES files. + assert.strictEqual( + newestMtime(dir, { remaining: entries - 1 }).truncated, + true, + 'one entry short of the tree size MUST truncate', + ); + assert.strictEqual( + newestMtime(dir, { remaining: entries }).truncated, + false, + 'a budget exactly equal to the tree size must NOT truncate', + ); + assert.strictEqual( + newestMtime(dir, { remaining: entries + 1 }).truncated, + false, + 'a budget above the tree size must NOT truncate', + ); + } finally { + cleanup(dir); + } + }); + + test('exceeding MAX_DEPTH truncates', () => { + const dir = tmpRoot(); + try { + let deep = dir; + for (let i = 0; i <= MAX_DEPTH + 1; i++) deep = path.join(deep, `d${i}`); + fs.mkdirSync(deep, { recursive: true }); + + const res = newestMtime(dir, { remaining: 1e6 }); + assert.strictEqual(res.truncated, true, 'a tree deeper than MAX_DEPTH must truncate'); + } finally { + cleanup(dir); + } + }); + + test('a truncated scan reports UNVERIFIED, never clean', () => { + const [dir, entries] = treeWithEntries(10); + try { + // The safety-critical branch named in this module's own docstring: a scan + // that hit a bound must not read as an attestation of cleanliness. + const snap = { [dir]: { exists: true, newest: 1, truncated: true } }; + const violations = diffLiveConfig(snap, { + [dir]: { exists: true, newest: 1, truncated: true }, + }); + assert.strictEqual(violations.length, 1); + assert.strictEqual(violations[0].kind, 'unverified'); + assert.match(formatViolations(violations), /UNVERIFIED \(scan bound hit/); + assert.ok(entries > 0); + } finally { + cleanup(dir); + } + }); + + test('a modified path outranks unverified (a real leak is never downgraded)', () => { + const p = '/live/.claude/gsd-core'; + const violations = diffLiveConfig( + { [p]: { exists: true, newest: 1, truncated: true } }, + { [p]: { exists: true, newest: 2, truncated: true } }, + ); + assert.strictEqual(violations[0].kind, 'modified'); + }); + + test('property: truncation is monotone in the budget (boundary containment)', () => { + const [dir, entries] = treeWithEntries(12); + try { + fc.assert( + fc.property(fc.integer({ min: 1, max: entries * 3 }), (budget) => { + const { truncated } = newestMtime(dir, { remaining: budget }); + // The invariant: a budget at or above the tree size never truncates, + // and one below it always does. A regression flipping `truncated` to + // false on an exhausted budget — the exact silent-clean failure the + // module warns about — breaks this for every budget < entries. + return budget >= entries ? truncated === false : truncated === true; + }), + { numRuns: 100 }, + ); + } finally { + cleanup(dir); + } + }); + + test('property: newest mtime never exceeds the true maximum', () => { + const [dir, entries] = treeWithEntries(8); + try { + const trueMax = Math.max( + ...fs.readdirSync(dir).map((f) => fs.lstatSync(path.join(dir, f)).mtimeMs), + fs.lstatSync(dir).mtimeMs, + ); + fc.assert( + fc.property(fc.integer({ min: 1, max: entries * 2 }), (budget) => { + const { newest } = newestMtime(dir, { remaining: budget }); + return newest <= trueMax; + }), + { numRuns: 50 }, + ); + } finally { + cleanup(dir); + } + }); +});