Prevent git rebase --exec test runs from corrupting the enclosing worktree (gitgitgadget#2157)

Running `git rebase -x 'npm test'` from a worktree wreaked havoc on my
real repository: `core.bare = true` and test `url.*.insteadOf` settings
ended up in the shared `.git/config`, test commits landed on the real
HEAD, and a bogus `refs/notes/gitgitgadget` ref appeared out of nowhere.
For a terrifying moment I thought I had lost actual work.

The root cause turned out to be `git rebase --exec` setting `GIT_DIR` in
the environment. This leaked into the Node.js test processes, causing
`git init <target>` to silently reinitialize the *real* repository
instead of creating a fresh one in the target directory -- git simply
ignores the target argument when `GIT_DIR` is set.

Diagnosing this was a bit trickier than I hoped for. Adding
`GIT_CEILING_DIRECTORIES` alone did not help because `GIT_DIR` takes
precedence over repository discovery, making ceiling directories
irrelevant. A secondary leak hid in `misc-helper.test.ts` which
captured `process.env` at module load time, before `testCreateRepo` had
a chance to clear the offending variables.

This series clears `GIT_DIR`/`GIT_WORK_TREE` early, adds
`GIT_CEILING_DIRECTORIES` as defense-in-depth, introduces a
`validateWorkDir()` safety net that fails loudly if git would operate
outside the test directory, and fixes a pre-existing test that
accidentally depended on the enclosing repo's config. It also tells Jest
to ignore `.test-dir/` so stale files from corrupted runs don't cause
confusing failures on the next run.

Author: dscho

lib/git.ts

@@ -1,4 +1,5 @@
 import { ChildProcess } from "child_process";
+import * as path from "path";
 import { exec as GitProcess } from "dugite";
 
 // For convenience, let's add helpers to call Git:
@@ -16,6 +17,18 @@ export interface IGitOptions {
 export const emptyBlobName = "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391";
 export const emptyTreeName = "4b825dc642cb6eb9a060e54bf8d69288fbee4904";
 
+function validateWorkDir(workDir: string): void {
+    const prefix = process.env.GIT_WORK_DIR_PREFIX;
+    if (!prefix) return;
+    const gitDir = process.env.GIT_DIR;
+    if (gitDir) {
+        throw new Error(`GIT_DIR is set to '${gitDir}'; This should not happen in tests`);
+    }
+    if (!path.resolve(workDir).startsWith(prefix)) {
+        throw new Error(`workDir '${workDir}' is not inside '${prefix}'`);
+    }
+}
+
 function trimTrailingNewline(str: string): string {
     return str.replace(/\r?\n$/, "");
 }
@@ -43,6 +56,44 @@ export function git(args: string[], options?: IGitOptions): Promise<string> {
         args = [`--git-dir=${options.workDir}`, ...args];
         workDir = ".";
     }
+    const prefix = process.env.GIT_WORK_DIR_PREFIX;
+    if (prefix) {
+        let cmdStart = 0;
+
+        // --git-dir=<path> must be the very first argument (prepended by the bare-repo handling above)
+        if (args[0]?.startsWith("--git-dir=")) {
+            validateWorkDir(args[0].substring("--git-dir=".length));
+            cmdStart = 1;
+        }
+
+        if (args[cmdStart] === "init") {
+            // Strictly parse: init [--bare] [--initial-branch <name>] <directory>
+            let i = cmdStart + 1;
+            while (i < args.length) {
+                if (args[i] === "--bare") {
+                    i++;
+                } else if (args[i] === "--initial-branch" || args[i] === "-b") {
+                    if (i + 1 >= args.length) throw new Error(`git init: '${args[i]}' requires a value`);
+                    i += 2;
+                } else if (args[i].startsWith("-")) {
+                    throw new Error(`git init: unexpected option '${args[i]}'`);
+                } else {
+                    break;
+                }
+            }
+            if (i >= args.length) {
+                throw new Error(`git init without an explicit target directory is not allowed in tests`);
+            }
+            if (i + 1 < args.length) {
+                throw new Error(`git init: unexpected trailing argument '${args[i + 1]}'`);
+            }
+            validateWorkDir(args[i]);
+        } else if (cmdStart === 0) {
+            // No --git-dir, not init: validate workDir
+            validateWorkDir(workDir);
+        }
+        // If --git-dir was present and subcommand is not init, it was already validated above
+    }
     if (options && options.trace) {
         process.stderr.write(`Called 'git ${args.join(" ")}' in '${workDir}':\n${new Error().stack}\n`);
     }
@@ -158,7 +209,9 @@ export function git(args: string[], options?: IGitOptions): Promise<string> {
  * @returns { string | undefined } the full SHA-1, or undefined
  */
 export async function revParse(argument: string, workDir?: string): Promise<string | undefined> {
-    const result = await GitProcess(["rev-parse", "--verify", "-q", argument], workDir || ".");
+    const dir = workDir || ".";
+    validateWorkDir(dir);
+    const result = await GitProcess(["rev-parse", "--verify", "-q", argument], dir);
     return result.exitCode ? undefined : trimTrailingNewline(result.stdout);
 }
 
@@ -171,6 +224,7 @@ export async function revParse(argument: string, workDir?: string): Promise<stri
  * @returns number the number of commits in the commit range
  */
 export async function revListCount(rangeArgs: string | string[], workDir = "."): Promise<number> {
+    validateWorkDir(workDir);
     const gitArgs: string[] = ["rev-list", "--count"];
     if (typeof rangeArgs === "string") {
         gitArgs.push(rangeArgs);
@@ -196,7 +250,9 @@ export async function commitExists(commit: string, workDir: string): Promise<boo
 }
 
 export async function gitConfig(key: string, workDir?: string): Promise<string | undefined> {
-    const result = await GitProcess(["config", key], workDir || ".");
+    const dir = workDir || ".";
+    validateWorkDir(dir);
+    const result = await GitProcess(["config", key], dir);
     if (result.exitCode !== 0) {
         return undefined;
     }
@@ -208,12 +264,16 @@ export async function gitConfigForEach(
     callbackfn: (value: string) => void,
     workDir?: string,
 ): Promise<void> {
-    const result = await GitProcess(["config", "--get-all", key], workDir || ".");
+    const dir = workDir || ".";
+    validateWorkDir(dir);
+    const result = await GitProcess(["config", "--get-all", key], dir);
     result.stdout.split(/\r?\n/).map(callbackfn);
 }
 
 export async function gitCommandExists(command: string, workDir?: string): Promise<boolean> {
-    const result = await GitProcess([command, "-h"], workDir || ".");
+    const dir = workDir || ".";
+    validateWorkDir(dir);
+    const result = await GitProcess([command, "-h"], dir);
     return result.exitCode === 129;
 }
 

package.json

@@ -53,7 +53,10 @@
         }
       ]
     },
-    "testRegex": "/tests/.*\\.test\\.(ts|tsx|js)$"
+    "testRegex": "/tests/.*\\.test\\.(ts|tsx|js)$",
+    "testPathIgnorePatterns": [
+      "\\.test-dir/"
+    ]
   },
   "devDependencies": {
     "@stylistic/eslint-plugin": "^5.9.0",

tests/git.test.ts

@@ -18,8 +18,8 @@ const sleep = async (ms: number) => {
     });
 };
 
-test("finds core.bare", async () => {
-    expect(await gitConfig("core.bare")).toMatch(/true|false/);
+test("reads config", async () => {
+    expect(await gitConfig("TEST.TS.case0")).toMatch("test.case0 value");
 });
 
 test("serialization", async () => {

tests/misc-helper.test.ts

@@ -69,6 +69,8 @@ const helperEnv = {
     GIT_COMMITTER_NAME: "J Doe",
     GIT_COMMITTER_EMAIL: "jdoe@example.com",
     ...process.env,
+    GIT_DIR: undefined,
+    GIT_WORK_TREE: undefined,
 };
 
 async function getNote(reg: RegExp, workDir: string): Promise<string> {

tests/test-lib.ts

@@ -136,6 +136,21 @@ export async function testCreateRepo(name: string, suffix?: string): Promise<Tes
     }
     tmp = await realpath(tmp);
 
+    // Prevent git from discovering or operating on the enclosing repository.
+    // GIT_DIR/GIT_WORK_TREE are set e.g. by `git rebase --exec` in worktrees.
+    delete process.env.GIT_DIR;
+    delete process.env.GIT_WORK_TREE;
+
+    // Prevent git from discovering the enclosing repository
+    const ceilings = [];
+    for (let dir = tmp, parent; (parent = path.dirname(dir)) !== dir; dir = parent) {
+        ceilings.push(parent);
+    }
+    process.env.GIT_CEILING_DIRECTORIES = ceilings.join(path.delimiter);
+
+    // Suspend the workDir guard during repo setup (git init, git config --global)
+    delete process.env.GIT_WORK_DIR_PREFIX;
+
     // eslint-disable-next-line security/detect-unsafe-regex
     const match = name.match(/^(.*[\\/])?(.*?)(\.test)?\.ts$/);
     if (match) {
@@ -179,6 +194,10 @@ export async function testCreateRepo(name: string, suffix?: string): Promise<Tes
         workDir: dir,
     };
     await git(["commit-tree", "-m", "Test commit", "4b825dc642cb6eb9a060e54bf8d69288fbee4904"], opts);
+
+    // From now on, ensure that all git() calls use a workDir inside tmp
+    process.env.GIT_WORK_DIR_PREFIX = tmp;
+
     const gitOpts: ITestCommitOptions = { workDir: dir };
 
     return new TestRepo(gitOpts);