From 6e681cb36c2687c54e62bb1ac8b545f1793853c7 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 29 Aug 2026 18:20:14 -0400 Subject: [PATCH] fix(#3901): commit-docs-guard suites isolate children from a global core.hooksPath (#4063) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#3901): the guard suite must survive a developer's global core.hooksPath (failing first) * fix(#3901): the commit-docs-guard suite isolates children from the host git config A developer's GLOBAL core.hooksPath (~/.gitconfig) applies to every fresh repo, so the guard's correct refusal — it will not install a hook git would never run — failed this suite's beforeEach and 18 tests read as a commit-docs-guard regression on any machine that centralizes commit hooks. CI never saw it (runners have no global git config). before() pins GIT_CONFIG_GLOBAL to an empty file for the whole suite (runGsdTools and the git helpers propagate process.env to every child). A LOCAL repo value cannot isolate this: any non-empty local value trips the same refusal, and an empty local value makes rev-parse --git-path hooks resolve to ./ instead of .git/hooks. The regression test pins the seam: the isolation file is armed and empty, a child git resolves no hooksPath from the host, and the beforeEach enable installed the hook at the repo-local default path. A child given an explicitly hostile GIT_CONFIG_GLOBAL still refuses — that is the guard being correct. * fix(#3901): review fold-ins — isolation hoisted to all three guard suites; changeset Adversarial review: the pin covered suite A only — the B (#3588 B1-B15) and D (real git commit wiring) suites run enable against fresh repos too and failed identically on hostile-global machines; the issue's 18 failures could not have come from A's five tests alone. The pin is extracted to isolateGlobalGitConfig() and armed in all three suites (B9's LOCAL hooksPath test is unaffected — the pin neutralizes only the global scope; D's premise is the hook firing from .git/hooks, which the isolation preserves). * chore(#3901): backfill changeset PR number (4063) * fix(#3901): drop the now-unused before import — max-warnings 0 fails CI --------- Co-authored-by: sim --- .changeset/quick-bears-cheer.md | 5 +++ tests/commands.test.cjs | 63 ++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 .changeset/quick-bears-cheer.md diff --git a/.changeset/quick-bears-cheer.md b/.changeset/quick-bears-cheer.md new file mode 100644 index 000000000..8a5b718e1 --- /dev/null +++ b/.changeset/quick-bears-cheer.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4063 +--- +**Local test runs no longer fail on machines with a global core.hooksPath** — the commit-docs-guard suites refused to install their pre-commit hook in every fresh fixture repo (18 tests read as a guard regression); the suites now pin GIT_CONFIG_GLOBAL to an empty file so children never inherit the host git config (#3901) diff --git a/tests/commands.test.cjs b/tests/commands.test.cjs index 9db3cf35f..98eda61f7 100644 --- a/tests/commands.test.cjs +++ b/tests/commands.test.cjs @@ -9,7 +9,7 @@ * GSD Tools Tests - Commands */ -const { test, describe, beforeEach, afterEach } = require('node:test'); +const { test, describe, after, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); @@ -2897,6 +2897,28 @@ describe('check-commit command', () => { // commit-docs-guard: opt-in pre-commit hook (#3588) // ───────────────────────────────────────────────────────────────────────────── +// #3901: a developer's GLOBAL core.hooksPath (~/.gitconfig) applies to every +// fresh repo — the guard correctly refuses to install a hook git would never +// run, which used to fail the guard suites' beforeEach (18 tests across the +// A/B/D suites) on machines that centralize commit hooks. Pin +// GIT_CONFIG_GLOBAL to an empty file (runGsdTools and the git helpers +// propagate process.env to every child), making the fixtures independent of +// the host's git configuration. A LOCAL repo value cannot isolate this: +// `git config --get` returns any non-empty local value (same refusal), and an +// empty local value makes rev-parse --git-path hooks resolve to `./` — not +// `.git/hooks`. Returns a restore function for the suite's after(). +function isolateGlobalGitConfig() { + const dir = createTempDir('gsd-3901-gitconfig-'); + const prev = process.env.GIT_CONFIG_GLOBAL; + process.env.GIT_CONFIG_GLOBAL = path.join(dir, 'global.gitconfig'); + fs.writeFileSync(process.env.GIT_CONFIG_GLOBAL, ''); + return () => { + if (prev === undefined) delete process.env.GIT_CONFIG_GLOBAL; + else process.env.GIT_CONFIG_GLOBAL = prev; + cleanup(dir); + }; +} + describe('commit-docs-guard hook script (#3588 A1-A5)', () => { const { createTempGitProject, TEST_ENV_BASE } = require('./helpers.cjs'); const { runHook } = require('./helpers/process-seam.cjs'); @@ -2905,6 +2927,10 @@ describe('commit-docs-guard hook script (#3588 A1-A5)', () => { let tmpDir; let hookPath; + // #3901: see isolateGlobalGitConfig — shared by all three guard suites. + const restoreGitConfig = isolateGlobalGitConfig(); + after(restoreGitConfig); + beforeEach(() => { tmpDir = createTempGitProject(); const enableResult = runGsdTools('commit-docs-guard enable --raw', tmpDir); @@ -2912,6 +2938,31 @@ describe('commit-docs-guard hook script (#3588 A1-A5)', () => { hookPath = path.join(tmpDir, '.git', 'hooks', 'pre-commit'); }); + test('#3901: the suite isolates children from the host git config (global core.hooksPath)', () => { + // The developer-machine scenario this suite must survive: a hostile + // ~/.gitconfig with core.hooksPath set. The before() hook pins + // GIT_CONFIG_GLOBAL to an empty file; this pins the seam is actually + // armed and reaching children — a child git sees NO hooksPath from the + // host, so the guard never refuses and the 18 tests never fail. (A child + // given an explicitly hostile GIT_CONFIG_GLOBAL still refuses — that is + // the guard being correct, and it is covered where the refusal is + // asserted.) + assert.ok( + process.env.GIT_CONFIG_GLOBAL && fs.existsSync(process.env.GIT_CONFIG_GLOBAL), + 'the isolation file is armed for this suite', + ); + assert.equal(fs.readFileSync(process.env.GIT_CONFIG_GLOBAL, 'utf-8'), '', + 'the isolation file is empty — children inherit no host config'); + const { spawnSync } = require('node:child_process'); + const probe = spawnSync('git', ['config', '--get', 'core.hooksPath'], { + cwd: tmpDir, + encoding: 'utf-8', + timeout: 15_000, + }); + assert.notEqual(probe.status, 0, `a child git must not see a host core.hooksPath; got: ${probe.stdout}`); + assert.ok(fs.existsSync(hookPath), 'the beforeEach enable installed the hook at the repo-local default path'); + }); + afterEach(() => { cleanup(tmpDir); }); @@ -2960,6 +3011,11 @@ describe('commit-docs-guard enable/disable (#3588 B1-B15)', () => { const { createTempGitProject } = require('./helpers.cjs'); let tmpDir; + // #3901: this suite also runs `enable` against fresh repos — the same + // hostile-global exposure as the A suite (review finding). + const restoreGitConfigB = isolateGlobalGitConfig(); + after(restoreGitConfigB); + afterEach(() => { if (tmpDir) cleanup(tmpDir); tmpDir = undefined; @@ -3162,6 +3218,11 @@ describe('commit-docs-guard real git commit wiring (#3588 D1-D3)', () => { const REPO_ROOT = path.join(__dirname, '..'); let tmpDir; + // #3901: the D suite's premise is the hook firing from .git/hooks/pre-commit + // — a hostile global core.hooksPath broke its beforeEach identically. + const restoreGitConfigD = isolateGlobalGitConfig(); + after(restoreGitConfigD); + beforeEach(() => { tmpDir = createTempGitProject(); const enableResult = runGsdTools('commit-docs-guard enable --raw', tmpDir);