From e82a15a8529e55c864e63361ea55800c078aeeb1 Mon Sep 17 00:00:00 2001 From: 0xdhx Date: Thu, 6 Aug 2026 16:31:40 -0500 Subject: [PATCH] fix(#2665): watch the fallback root a scrubbing child actually resolves to resolveLiveConfigRoots resolves what THIS process sees, and getGlobalConfigDir is env-first -- so with an ambient CLAUDE_CONFIG_DIR the guard watched that path. A spawned child does not see it: TEST_ENV_BASE blanks the config-location vars precisely so the child cannot follow them, and a blanked var is falsy, so the child resolves its HOME-derived root instead. A child that blanks the var and does NOT also sandbox HOME therefore writes into the developer's real ~/.claude, which the guard was not watching. That is this PR's own escape route, taken one process deeper -- and the guard is the artifact that is supposed to make it loud. Both resolutions are now unioned: the ambient one, and the fallback one obtained by handing the REAL descriptor resolver an EMPTY env. Deriving it that way is deliberate -- a hand-listed copy of the scrub set inside the guard is a second list to drift, which is the defect this PR spent three rounds closing one layer up. grok resolves through a hardcoded branch rather than a descriptor, so its fallback is stated explicitly for the same reason it is named in the ambient loop. Addresses review finding: Blocker 3. --- scripts/live-config-guard.cjs | 40 +++++++++++++++++++++++++++++++- tests/live-config-guard.test.cjs | 28 ++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/scripts/live-config-guard.cjs b/scripts/live-config-guard.cjs index 8d71341e3..4a6b93514 100644 --- a/scripts/live-config-guard.cjs +++ b/scripts/live-config-guard.cjs @@ -101,14 +101,23 @@ const MAX_DEPTH = 12; * resolver rather than a reimplementation — the guard must watch wherever the * product actually points, including through an ambient env var. * + * TWO resolutions, unioned, because the parent and its children do not resolve + * the same way: the AMBIENT one (what this process sees, env-first) and the + * FALLBACK one (what a child that BLANKED the config-location vars resolves to, + * i.e. HOME-derived). Watching only the first leaves the second unwatched, which + * is where a child that scrubs the var but not HOME actually writes. + * * @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'); + const homedir = (deps.os || os).homedir; let getGlobalConfigDir; + let resolveConfigHomeFromDescriptor; let runtimes; try { - ({ getGlobalConfigDir } = require(path.join(libDir, 'runtime-homes.cjs'))); + ({ getGlobalConfigDir, resolveConfigHomeFromDescriptor } = + 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 @@ -117,6 +126,35 @@ function resolveLiveConfigRoots(deps = {}) { } const roots = new Set(); + + // ── The FALLBACK roots, which the ambient resolution above cannot reach ──── + // + // #2665 round 5: getGlobalConfigDir is env-first, so the loop below resolves + // whatever THIS process sees. A spawned child does not see that — TEST_ENV_BASE + // blanks the config-location vars precisely so the child cannot follow them — + // and a blanked var is falsy, so the child falls back to its HOME-derived root + // instead. A child that blanks the var and does NOT also sandbox HOME therefore + // writes into the developer's real ~/.claude while the guard is watching the + // ambient path, one process shallower. That is the exact escape route this PR + // exists to close, taken one layer down. + // + // Derived, never re-listed: passing an EMPTY env to the real descriptor resolver + // IS "what a child with no config-location vars resolves to". Deriving it this + // way keeps the guard from carrying a second copy of the scrub set to drift + // against -- the defect this PR spent three rounds closing one layer up. + for (const entry of Object.values(runtimes || {})) { + const descriptor = entry?.runtime?.configHome; + if (!descriptor) continue; + try { + const dir = resolveConfigHomeFromDescriptor(descriptor, { env: {}, home: homedir() }); + if (typeof dir === 'string' && dir.length > 0) roots.add(path.resolve(dir)); + } catch { + // Same posture as the ambient loop below. + } + } + // grok resolves through a hardcoded branch rather than a descriptor, so its + // fallback is stated here for the same reason it is named in the loop below. + roots.add(path.resolve(path.join(homedir(), '.agents'))); // 'grok' is a hardcoded branch of getGlobalConfigDir with no registry entry. // // DELIBERATE NON-ROOT: getGlobalSkillsBase(runtime) is NOT added here. The diff --git a/tests/live-config-guard.test.cjs b/tests/live-config-guard.test.cjs index 0ee8b92c1..5f3c22c28 100644 --- a/tests/live-config-guard.test.cjs +++ b/tests/live-config-guard.test.cjs @@ -43,6 +43,34 @@ describe('#2665: live-config hermeticity guard', () => { } }); + test('watches the HOME-derived fallback root, not only the ambient one', () => { + // #2665 round 5: the guard resolves env-first, so it sees the AMBIENT root. + // A child that blanks CLAUDE_CONFIG_DIR (which is exactly what TEST_ENV_BASE + // does) falls back to /.claude instead. Watching only the ambient path + // leaves that fallback unwatched -- the escape route this PR closes, one + // process deeper. + const ambient = tmpRoot(); + const fakeHome = tmpRoot(); + const saved = process.env.CLAUDE_CONFIG_DIR; + try { + process.env.CLAUDE_CONFIG_DIR = ambient; + const roots = resolveLiveConfigRoots({ os: { homedir: () => fakeHome } }); + assert.ok( + roots.includes(path.resolve(ambient)), + `ambient root missing from ${JSON.stringify(roots)}`, + ); + assert.ok( + roots.includes(path.resolve(path.join(fakeHome, '.claude'))), + `HOME-derived fallback root missing from ${JSON.stringify(roots)}`, + ); + } finally { + if (saved === undefined) delete process.env.CLAUDE_CONFIG_DIR; + else process.env.CLAUDE_CONFIG_DIR = saved; + cleanup(ambient); + cleanup(fakeHome); + } + }); + test('a clean run produces no violations', () => { const root = tmpRoot(); try {