fix(tests): scrub GIT_* env vars leaking from the pre-commit hook into git-fixture tests
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.
This commit is contained in:
@@ -45,9 +45,25 @@ const CLI = path.join(__dirname, "..", "..", "bin", "ccam.js");
|
|||||||
let server;
|
let server;
|
||||||
let BASE;
|
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) {
|
function git(args, cwd) {
|
||||||
return new Promise((resolve, reject) => {
|
return new Promise((resolve, reject) => {
|
||||||
const child = spawn("git", args, { cwd });
|
const child = spawn("git", args, { cwd, env: GIT_ENV });
|
||||||
let stderr = "";
|
let stderr = "";
|
||||||
child.stderr.on("data", (chunk) => (stderr += chunk));
|
child.stderr.on("data", (chunk) => (stderr += chunk));
|
||||||
child.on("error", reject);
|
child.on("error", reject);
|
||||||
|
|||||||
@@ -14,11 +14,28 @@ const { execFileSync } = require("child_process");
|
|||||||
|
|
||||||
const { getUpdatesStatus } = require("../lib/update-check");
|
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) {
|
function git(cwd, args) {
|
||||||
return execFileSync("git", args, {
|
return execFileSync("git", args, {
|
||||||
cwd,
|
cwd,
|
||||||
stdio: ["ignore", "pipe", "pipe"],
|
stdio: ["ignore", "pipe", "pipe"],
|
||||||
encoding: "utf8",
|
encoding: "utf8",
|
||||||
|
env: GIT_ENV,
|
||||||
}).trim();
|
}).trim();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -29,6 +46,7 @@ function makeBareRemote(parent, name) {
|
|||||||
// i.e. far older than --initial-branch.
|
// i.e. far older than --initial-branch.
|
||||||
execFileSync("git", ["-c", "init.defaultBranch=master", "init", "--bare", repo], {
|
execFileSync("git", ["-c", "init.defaultBranch=master", "init", "--bare", repo], {
|
||||||
stdio: "ignore",
|
stdio: "ignore",
|
||||||
|
env: GIT_ENV,
|
||||||
});
|
});
|
||||||
return repo;
|
return repo;
|
||||||
}
|
}
|
||||||
@@ -36,7 +54,10 @@ function makeBareRemote(parent, name) {
|
|||||||
function makeWorkingRepo(parent, dir, originUrl) {
|
function makeWorkingRepo(parent, dir, originUrl) {
|
||||||
const repo = path.join(parent, dir);
|
const repo = path.join(parent, dir);
|
||||||
fs.mkdirSync(repo, { recursive: true });
|
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");
|
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", "add", "."]);
|
||||||
git(repo, ["-c", "user.email=t@t", "-c", "user.name=t", "commit", "-m", "init"]);
|
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 () => {
|
it("returns a soft no-remotes payload", async () => {
|
||||||
const repo = path.join(tmpDir, "noremote");
|
const repo = path.join(tmpDir, "noremote");
|
||||||
fs.mkdirSync(repo, { recursive: true });
|
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");
|
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", "add", "."]);
|
||||||
git(repo, ["-c", "user.email=t@t", "-c", "user.name=t", "commit", "-m", "init"]);
|
git(repo, ["-c", "user.email=t@t", "-c", "user.name=t", "commit", "-m", "init"]);
|
||||||
|
|||||||
@@ -19,13 +19,33 @@ const DEFAULT_ROOT = path.join(__dirname, "..", "..");
|
|||||||
// never make the update checker report commits from somebody else's repo.
|
// never make the update checker report commits from somebody else's repo.
|
||||||
const REMOTE_PRIORITY = ["origin"];
|
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 = {}) {
|
function execGit(cwd, args, opts = {}) {
|
||||||
const timeout = opts.timeout ?? 120_000;
|
const timeout = opts.timeout ?? 120_000;
|
||||||
return new Promise((resolve, reject) => {
|
return new Promise((resolve, reject) => {
|
||||||
execFile(
|
execFile(
|
||||||
"git",
|
"git",
|
||||||
args,
|
args,
|
||||||
{ cwd, timeout, maxBuffer: 2_000_000, encoding: "utf8" },
|
{ cwd, timeout, maxBuffer: 2_000_000, encoding: "utf8", env: GIT_ENV },
|
||||||
(err, stdout) => {
|
(err, stdout) => {
|
||||||
if (err) reject(err);
|
if (err) reject(err);
|
||||||
else resolve(String(stdout).trim());
|
else resolve(String(stdout).trim());
|
||||||
|
|||||||
Reference in New Issue
Block a user