From dfea1a99d6ea205848809b21e539393a4714e5c8 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Wed, 12 Aug 2026 09:24:03 +0700 Subject: [PATCH] fix(tests): scrub GIT_* env vars leaking from the pre-commit hook into git-fixture tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit server/lib/update-check.js's execGit() and two test helpers (lanes-cli.test.js, update-check.test.js) shelled out to git with an explicit `cwd` but no `env` override. A parent git hook process (this repo's own .husky/pre-commit, which runs `npm run test:server`) sets GIT_DIR/GIT_INDEX_FILE in its own environment; those leak to every child process and take precedence over `cwd` for repo discovery, so every git command these tests ran against their throwaway tmp repos was silently redirected at the real repo running the hook instead — reproduced firsthand as four foreign "init"/"fixture" commits overwriting a worktree branch mid pre-commit run. Fixes it the same way server/lib/worktree.js already documented and did for its own git calls: strip the GIT_* vars before exec. --- server/__tests__/lanes-cli.test.js | 18 ++++++++++++++++- server/__tests__/update-check.test.js | 28 +++++++++++++++++++++++++-- server/lib/update-check.js | 22 ++++++++++++++++++++- 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/server/__tests__/lanes-cli.test.js b/server/__tests__/lanes-cli.test.js index 844be18..f3fa85b 100644 --- a/server/__tests__/lanes-cli.test.js +++ b/server/__tests__/lanes-cli.test.js @@ -45,9 +45,25 @@ const CLI = path.join(__dirname, "..", "..", "bin", "ccam.js"); let server; let BASE; +// Strip GIT_* vars a parent git hook (e.g. the pre-commit hook running this +// very suite) sets in its own environment — those leak to every child +// process and override an explicit `cwd`, so without this a git command +// meant for this test's throwaway tmp repo silently operates on the real +// repo running the hook instead. +const GIT_ENV = { ...process.env }; +delete GIT_ENV.GIT_DIR; +delete GIT_ENV.GIT_WORK_TREE; +delete GIT_ENV.GIT_INDEX_FILE; +delete GIT_ENV.GIT_COMMON_DIR; +delete GIT_ENV.GIT_OBJECT_DIRECTORY; +delete GIT_ENV.GIT_ALTERNATE_OBJECT_DIRECTORIES; +delete GIT_ENV.GIT_PREFIX; +delete GIT_ENV.GIT_NAMESPACE; +delete GIT_ENV.GIT_CONFIG_PARAMETERS; + function git(args, cwd) { return new Promise((resolve, reject) => { - const child = spawn("git", args, { cwd }); + const child = spawn("git", args, { cwd, env: GIT_ENV }); let stderr = ""; child.stderr.on("data", (chunk) => (stderr += chunk)); child.on("error", reject); diff --git a/server/__tests__/update-check.test.js b/server/__tests__/update-check.test.js index 844d89a..7efd394 100644 --- a/server/__tests__/update-check.test.js +++ b/server/__tests__/update-check.test.js @@ -14,11 +14,28 @@ const { execFileSync } = require("child_process"); const { getUpdatesStatus } = require("../lib/update-check"); +// Strip GIT_* vars a parent git hook (e.g. the pre-commit hook running this +// very suite) sets in its own environment — those leak to every child +// process and override an explicit `cwd`, so without this a git command +// meant for this test's throwaway tmp repo silently operates on the real +// repo running the hook instead. +const GIT_ENV = { ...process.env }; +delete GIT_ENV.GIT_DIR; +delete GIT_ENV.GIT_WORK_TREE; +delete GIT_ENV.GIT_INDEX_FILE; +delete GIT_ENV.GIT_COMMON_DIR; +delete GIT_ENV.GIT_OBJECT_DIRECTORY; +delete GIT_ENV.GIT_ALTERNATE_OBJECT_DIRECTORIES; +delete GIT_ENV.GIT_PREFIX; +delete GIT_ENV.GIT_NAMESPACE; +delete GIT_ENV.GIT_CONFIG_PARAMETERS; + function git(cwd, args) { return execFileSync("git", args, { cwd, stdio: ["ignore", "pipe", "pipe"], encoding: "utf8", + env: GIT_ENV, }).trim(); } @@ -29,6 +46,7 @@ function makeBareRemote(parent, name) { // i.e. far older than --initial-branch. execFileSync("git", ["-c", "init.defaultBranch=master", "init", "--bare", repo], { stdio: "ignore", + env: GIT_ENV, }); return repo; } @@ -36,7 +54,10 @@ function makeBareRemote(parent, name) { function makeWorkingRepo(parent, dir, originUrl) { const repo = path.join(parent, dir); fs.mkdirSync(repo, { recursive: true }); - execFileSync("git", ["-c", "init.defaultBranch=master", "init", repo], { stdio: "ignore" }); + execFileSync("git", ["-c", "init.defaultBranch=master", "init", repo], { + stdio: "ignore", + env: GIT_ENV, + }); fs.writeFileSync(path.join(repo, "README.md"), "fixture\n"); git(repo, ["-c", "user.email=t@t", "-c", "user.name=t", "add", "."]); git(repo, ["-c", "user.email=t@t", "-c", "user.name=t", "commit", "-m", "init"]); @@ -146,7 +167,10 @@ describe("getUpdatesStatus — no remotes configured", () => { it("returns a soft no-remotes payload", async () => { const repo = path.join(tmpDir, "noremote"); fs.mkdirSync(repo, { recursive: true }); - execFileSync("git", ["-c", "init.defaultBranch=master", "init", repo], { stdio: "ignore" }); + execFileSync("git", ["-c", "init.defaultBranch=master", "init", repo], { + stdio: "ignore", + env: GIT_ENV, + }); fs.writeFileSync(path.join(repo, "README.md"), "lonely\n"); git(repo, ["-c", "user.email=t@t", "-c", "user.name=t", "add", "."]); git(repo, ["-c", "user.email=t@t", "-c", "user.name=t", "commit", "-m", "init"]); diff --git a/server/lib/update-check.js b/server/lib/update-check.js index dd7448b..37612ac 100644 --- a/server/lib/update-check.js +++ b/server/lib/update-check.js @@ -19,13 +19,33 @@ const DEFAULT_ROOT = path.join(__dirname, "..", ".."); // never make the update checker report commits from somebody else's repo. const REMOTE_PRIORITY = ["origin"]; +// Scrub git hook environment variables (GIT_DIR, GIT_INDEX_FILE, etc.) that +// leak from a parent git hook process — e.g. this repo's own pre-commit +// hook, which runs `npm run test:server` and therefore this module too. +// Without this, every git call below silently targets the OUTER repo (the +// hook's) instead of `cwd`, since GIT_DIR takes precedence over cwd-based +// discovery. Same scrub `server/lib/worktree.js` already applies. +const GIT_ENV = { ...process.env }; +delete GIT_ENV.GIT_DIR; +delete GIT_ENV.GIT_WORK_TREE; +delete GIT_ENV.GIT_INDEX_FILE; +delete GIT_ENV.GIT_COMMON_DIR; +delete GIT_ENV.GIT_OBJECT_DIRECTORY; +delete GIT_ENV.GIT_ALTERNATE_OBJECT_DIRECTORIES; +delete GIT_ENV.GIT_PREFIX; +delete GIT_ENV.GIT_NAMESPACE; +delete GIT_ENV.GIT_CONFIG_PARAMETERS; +for (const name of Object.keys(GIT_ENV)) { + if (/^GIT_CONFIG_(COUNT|KEY_\d+|VALUE_\d+|GLOBAL|SYSTEM)$/.test(name)) delete GIT_ENV[name]; +} + function execGit(cwd, args, opts = {}) { const timeout = opts.timeout ?? 120_000; return new Promise((resolve, reject) => { execFile( "git", args, - { cwd, timeout, maxBuffer: 2_000_000, encoding: "utf8" }, + { cwd, timeout, maxBuffer: 2_000_000, encoding: "utf8", env: GIT_ENV }, (err, stdout) => { if (err) reject(err); else resolve(String(stdout).trim());