From 08021b02c0f036a05b4f69cd11fd0efba2e4fe90 Mon Sep 17 00:00:00 2001 From: 0xdhx Date: Sun, 26 Jul 2026 17:41:40 -0500 Subject: [PATCH] fix(#2665): scrub config-location env vars in every TEST_ENV_BASE declaration TEST_ENV_BASE blanks session-identity variables but none of the three that decide WHERE a child process writes: CLAUDE_CONFIG_DIR, GSD_RUNTIME and CODEX_HOME. The config-home resolver is env-first (runtime-homes.cts, the dot-home case consults the env var before the home-derived fallback), so an ambient CLAUDE_CONFIG_DIR in the developer's shell beats a call site that sandboxes only HOME. The suite then writes into the developer's real config directory -- including a registered skill under /skills/ whose body carries behavioural directives that load into later sessions. Blank all three alongside the session-identity vars. `...env` still spreads last, so the five call sites that already constrain these locally keep winning with their explicit values. TEST_ENV_BASE is re-declared in nine files, so the three lines are added nine times rather than once. Consolidating the nine into a single exported constant -- and fixing the TERM_SESSION / TERM_SESSION_ID drift between the copies -- is deliberately left out of this change; see the PR body. One call site needed adjusting. capability-state.test.cjs's `capability state --runtime claude` CLI test passed no env at all and compared the CHILD's resolved config dir against the PARENT process's getGlobalConfigDir('claude'). That agreed only because the child inherited the developer's ambient CLAUDE_CONFIG_DIR -- i.e. it passed *because of* the leak. It now redirects both runtime homes into the sandbox and asserts against values the test controls, so it is hermetic with the variable set or unset. Regression case folded into the owning module's test file rather than a new bug-NNNN file, per scripts/lint-regression-test-names.cjs. It sets the variable on the PARENT process, which is the actual vector; setting it in the per-call env argument would exercise a path that was never broken. --- tests/agent-skills.test.cjs | 6 +++ tests/api-coverage-gate-e2e.test.cjs | 6 +++ .../assumption-delta-checkpoint-e2e.test.cjs | 6 +++ tests/capability-state.test.cjs | 18 +++++-- .../check-tdd-review-checkpoint-e2e.test.cjs | 6 +++ tests/config-loader.test.cjs | 6 +++ tests/configuration-migrate-config.test.cjs | 6 +++ tests/helpers.cjs | 7 ++- tests/profile-output.test.cjs | 47 ++++++++++++++++++- tests/representative-corpus.test.cjs | 6 +++ tests/run-tests-harness.test.cjs | 6 +++ 11 files changed, 112 insertions(+), 8 deletions(-) diff --git a/tests/agent-skills.test.cjs b/tests/agent-skills.test.cjs index 40063bc1f..8a78e8b9d 100644 --- a/tests/agent-skills.test.cjs +++ b/tests/agent-skills.test.cjs @@ -35,6 +35,12 @@ const TEST_ENV_BASE = { GSD_WORKSTREAM: '', TTY: '', SSH_TTY: '', + // Config-LOCATION vars. Distinct in kind from the session-identity vars + // above: these decide WHERE a child writes, so leaving them ambient lets a + // test that sandboxes HOME still escape into the developer's real config dir. + CLAUDE_CONFIG_DIR: '', + GSD_RUNTIME: '', + CODEX_HOME: '', }; /** diff --git a/tests/api-coverage-gate-e2e.test.cjs b/tests/api-coverage-gate-e2e.test.cjs index c9f49f70f..0407e8a81 100644 --- a/tests/api-coverage-gate-e2e.test.cjs +++ b/tests/api-coverage-gate-e2e.test.cjs @@ -44,6 +44,12 @@ const TEST_ENV_BASE = { ZELLIJ_SESSION_NAME: '', TTY: '', SSH_TTY: '', + // Config-LOCATION vars. Distinct in kind from the session-identity vars + // above: these decide WHERE a child writes, so leaving them ambient lets a + // test that sandboxes HOME still escape into the developer's real config dir. + CLAUDE_CONFIG_DIR: '', + GSD_RUNTIME: '', + CODEX_HOME: '', }; function runTools(args, cwd) { diff --git a/tests/assumption-delta-checkpoint-e2e.test.cjs b/tests/assumption-delta-checkpoint-e2e.test.cjs index 7ae030fb4..da57a3364 100644 --- a/tests/assumption-delta-checkpoint-e2e.test.cjs +++ b/tests/assumption-delta-checkpoint-e2e.test.cjs @@ -40,6 +40,12 @@ const TEST_ENV_BASE = { ZELLIJ_SESSION_NAME: '', TTY: '', SSH_TTY: '', + // Config-LOCATION vars. Distinct in kind from the session-identity vars + // above: these decide WHERE a child writes, so leaving them ambient lets a + // test that sandboxes HOME still escape into the developer's real config dir. + CLAUDE_CONFIG_DIR: '', + GSD_RUNTIME: '', + CODEX_HOME: '', }; function runTools(args, cwd) { diff --git a/tests/capability-state.test.cjs b/tests/capability-state.test.cjs index 4ab47fecc..3bd6dbd11 100644 --- a/tests/capability-state.test.cjs +++ b/tests/capability-state.test.cjs @@ -2108,12 +2108,24 @@ describe('regressions: --runtime override bypasses persisted runtime (#2003)', ( const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-rt-cli-')); try { writePersistedRuntime(tmpDir, 'codex'); - const result = runGsdTools('capability state --runtime claude --raw', tmpDir); + // #2665: redirect both runtime homes into the sandbox so the expectation is + // built from values this test controls. Comparing the child's answer against + // the PARENT process's getGlobalConfigDir() compared two different + // environments -- it agreed only because the child inherited the developer's + // ambient CLAUDE_CONFIG_DIR, which is the leak this test's helper now blocks. + const claudeConfigDir = path.join(tmpDir, 'claude-config'); + const codexHome = path.join(tmpDir, 'codex-home'); + const result = runGsdTools('capability state --runtime claude --raw', tmpDir, { + HOME: tmpDir, + CLAUDE_CONFIG_DIR: claudeConfigDir, + CODEX_HOME: codexHome, + }); assert.ok(result.success, `capability state --runtime should succeed: ${result.error || ''}`); const parsed = JSON.parse(result.output); - const runtimeHomes = require('../gsd-core/bin/lib/runtime-homes.cjs'); - assert.strictEqual(parsed.runtimeConfigDir, runtimeHomes.getGlobalConfigDir('claude'), + assert.strictEqual(parsed.runtimeConfigDir, claudeConfigDir, '`capability state --runtime claude` must resolve to the Claude config dir, not the persisted codex dir'); + assert.notStrictEqual(parsed.runtimeConfigDir, codexHome, + 'must NOT resolve to the codex config dir when --runtime claude is explicit'); } finally { cleanup(tmpDir); } diff --git a/tests/check-tdd-review-checkpoint-e2e.test.cjs b/tests/check-tdd-review-checkpoint-e2e.test.cjs index 1a01e8aa6..3a2a895b4 100644 --- a/tests/check-tdd-review-checkpoint-e2e.test.cjs +++ b/tests/check-tdd-review-checkpoint-e2e.test.cjs @@ -151,6 +151,12 @@ const TEST_ENV_BASE = { ZELLIJ_SESSION_NAME: '', TTY: '', SSH_TTY: '', + // Config-LOCATION vars. Distinct in kind from the session-identity vars + // above: these decide WHERE a child writes, so leaving them ambient lets a + // test that sandboxes HOME still escape into the developer's real config dir. + CLAUDE_CONFIG_DIR: '', + GSD_RUNTIME: '', + CODEX_HOME: '', }; function runTools(args, cwd) { diff --git a/tests/config-loader.test.cjs b/tests/config-loader.test.cjs index 1e3e89cdf..1e41ffe4d 100644 --- a/tests/config-loader.test.cjs +++ b/tests/config-loader.test.cjs @@ -759,6 +759,12 @@ const TEST_ENV_BASE = { ZELLIJ_SESSION_NAME: '', TTY: '', SSH_TTY: '', + // Config-LOCATION vars. Distinct in kind from the session-identity vars + // above: these decide WHERE a child writes, so leaving them ambient lets a + // test that sandboxes HOME still escape into the developer's real config dir. + CLAUDE_CONFIG_DIR: '', + GSD_RUNTIME: '', + CODEX_HOME: '', }; /** diff --git a/tests/configuration-migrate-config.test.cjs b/tests/configuration-migrate-config.test.cjs index d98baea6e..efe602ba0 100644 --- a/tests/configuration-migrate-config.test.cjs +++ b/tests/configuration-migrate-config.test.cjs @@ -34,6 +34,12 @@ const TEST_ENV_BASE = { ZELLIJ_SESSION_NAME: '', TTY: '', SSH_TTY: '', + // Config-LOCATION vars. Distinct in kind from the session-identity vars + // above: these decide WHERE a child writes, so leaving them ambient lets a + // test that sandboxes HOME still escape into the developer's real config dir. + CLAUDE_CONFIG_DIR: '', + GSD_RUNTIME: '', + CODEX_HOME: '', }; function runMigrateConfig(cwd, extraArgs = [], env = {}) { diff --git a/tests/helpers.cjs b/tests/helpers.cjs index 812c257f1..3392ced3b 100644 --- a/tests/helpers.cjs +++ b/tests/helpers.cjs @@ -25,10 +25,9 @@ const TEST_ENV_BASE = { ZELLIJ_SESSION_NAME: '', TTY: '', SSH_TTY: '', - // #2665: blank config-LOCATION vars so npm test never writes into the developer's - // live config directory. The resolver consults these before HOME, so an ambient - // value wins unconditionally over a sandboxed HOME. Per-site overrides still win - // because env is spread last in the child-env merge. + // Config-LOCATION vars. Distinct in kind from the session-identity vars + // above: these decide WHERE a child writes, so leaving them ambient lets a + // test that sandboxes HOME still escape into the developer's real config dir. CLAUDE_CONFIG_DIR: '', GSD_RUNTIME: '', CODEX_HOME: '', diff --git a/tests/profile-output.test.cjs b/tests/profile-output.test.cjs index cc40ba566..849b804a3 100644 --- a/tests/profile-output.test.cjs +++ b/tests/profile-output.test.cjs @@ -12,7 +12,13 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { runGsdTools, createTempProject, createTempGitProject, cleanup } = require('./helpers.cjs'); +const { + runGsdTools, + createTempProject, + createTempGitProject, + cleanup, + withIsolatedProcessState, +} = require('./helpers.cjs'); const { PROFILING_QUESTIONS, @@ -184,6 +190,45 @@ describe('write-profile command', () => { assert.strictEqual(out.profile_path, path.join(codexHome, 'gsd-core', 'USER-PROFILE.md')); }); + test('#2665: ambient CLAUDE_CONFIG_DIR cannot escape a HOME-only sandbox', () => { + // The defect: TEST_ENV_BASE blanked session-identity vars but none of the + // config-LOCATION vars, and the dot-home resolver is env-first. So an + // ambient CLAUDE_CONFIG_DIR in the DEVELOPER'S shell beat `{ HOME: tmpDir }` + // and the suite wrote into their real config directory. Setting it on the + // PARENT process is the actual vector — passing it in the per-call env + // argument would test nothing, because that path was never broken. + const analysis = { + profile_version: '1.0', + dimensions: { communication_style: { rating: 'terse-direct', confidence: 'HIGH' } }, + }; + const analysisPath = path.join(tmpDir, 'analysis.json'); + fs.writeFileSync(analysisPath, JSON.stringify(analysis)); + + const ambientConfigDir = path.join(tmpDir, 'ambient-live-config'); + fs.mkdirSync(ambientConfigDir, { recursive: true }); + + const out = withIsolatedProcessState(() => { + process.env.CLAUDE_CONFIG_DIR = ambientConfigDir; + const result = runGsdTools( + ['write-profile', '--input', analysisPath, '--raw'], + tmpDir, + { HOME: tmpDir } + ); + assert.ok(result.success, `Failed: ${result.error}`); + return JSON.parse(result.output); + }); + + assert.deepStrictEqual( + fs.readdirSync(ambientConfigDir), + [], + 'a call site that sandboxes HOME must not write into an ambient CLAUDE_CONFIG_DIR' + ); + assert.ok( + !out.profile_path.startsWith(ambientConfigDir), + `profile must not resolve under the ambient config dir, got: ${out.profile_path}` + ); + }); + test('errors when --input is missing', () => { const result = runGsdTools('write-profile --raw', tmpDir); assert.ok(!result.success, 'should fail without --input'); diff --git a/tests/representative-corpus.test.cjs b/tests/representative-corpus.test.cjs index 9f4f2971f..7e537e7bd 100644 --- a/tests/representative-corpus.test.cjs +++ b/tests/representative-corpus.test.cjs @@ -68,6 +68,12 @@ const TEST_ENV_BASE = { ZELLIJ_SESSION_NAME: '', TTY: '', SSH_TTY: '', + // Config-LOCATION vars. Distinct in kind from the session-identity vars + // above: these decide WHERE a child writes, so leaving them ambient lets a + // test that sandboxes HOME still escape into the developer's real config dir. + CLAUDE_CONFIG_DIR: '', + GSD_RUNTIME: '', + CODEX_HOME: '', }; function runTools(args, cwd) { diff --git a/tests/run-tests-harness.test.cjs b/tests/run-tests-harness.test.cjs index 07a822f64..9b510aab5 100644 --- a/tests/run-tests-harness.test.cjs +++ b/tests/run-tests-harness.test.cjs @@ -1416,6 +1416,12 @@ describe('bug #969 B — runGsdTools kill-signal discrimination', () => { GSD_SESSION_KEY: '', CODEX_THREAD_ID: '', CLAUDE_SESSION_ID: '', + // Config-LOCATION vars. Distinct in kind from the session-identity vars + // above: these decide WHERE a child writes, so leaving them ambient lets a + // test that sandboxes HOME still escape into the developer's real config dir. + CLAUDE_CONFIG_DIR: '', + GSD_RUNTIME: '', + CODEX_HOME: '', }; try { let result;