Skip to content

Commit d51f9ae

Browse files
committed
tests: add a safety net to prevent git from operating on the wrong repo
The GIT_CEILING_DIRECTORIES fix in the preceding commit should be sufficient to prevent test-spawned git processes from discovering the enclosing repository. However, when that protection was put to the test by running `rm -rf node_modules/ build/ && npm ci && npm run build && npm test` from a worktree, the test commits still ended up on the real worktree's HEAD for reasons that could not be fully diagnosed. Since the consequences of this happening are severe (the primary worktree was marked as `core.bare = true` and test commits, notes refs, and fake upstream refs all landed on the real repository), an additional layer of defense is warranted. Introduce a `validateWorkDir()` function in `lib/git.ts` that is called from every function that spawns a git process (`git()`, `revParse()`, `revListCount()`, `gitConfig()`, `gitConfigForEach()`, `gitCommandExists()`). When the `GIT_WORK_DIR_PREFIX` environment variable is set, the function verifies that the resolved `workDir` falls inside the expected directory, and throws otherwise. For `git init <path>` and `git --git-dir=<path>` invocations, the target path from the arguments is validated instead of the cwd, since those commands legitimately operate on a path specified as an argument rather than through the `workDir` option. In `testCreateRepo()`, the guard is suspended during the setup phase (which needs to run `git init` and `git config --global` without a test repo `workDir`), then activated by setting `GIT_WORK_DIR_PREFIX` to the `.test-dir/` path before returning. This ensures that all subsequent `git()` calls in the test process must use a `workDir` inside the test directory, or fail loudly. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
1 parent 0fda24f commit d51f9ae

2 files changed

Lines changed: 71 additions & 4 deletions

File tree

lib/git.ts

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { ChildProcess } from "child_process";
2+
import * as path from "path";
23
import { exec as GitProcess } from "dugite";
34

45
// For convenience, let's add helpers to call Git:
@@ -16,6 +17,18 @@ export interface IGitOptions {
1617
export const emptyBlobName = "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391";
1718
export const emptyTreeName = "4b825dc642cb6eb9a060e54bf8d69288fbee4904";
1819

20+
function validateWorkDir(workDir: string): void {
21+
const prefix = process.env.GIT_WORK_DIR_PREFIX;
22+
if (!prefix) return;
23+
const gitDir = process.env.GIT_DIR;
24+
if (gitDir) {
25+
throw new Error(`GIT_DIR is set to '${gitDir}'; This should not happen in tests`);
26+
}
27+
if (!path.resolve(workDir).startsWith(prefix)) {
28+
throw new Error(`workDir '${workDir}' is not inside '${prefix}'`);
29+
}
30+
}
31+
1932
function trimTrailingNewline(str: string): string {
2033
return str.replace(/\r?\n$/, "");
2134
}
@@ -43,6 +56,44 @@ export function git(args: string[], options?: IGitOptions): Promise<string> {
4356
args = [`--git-dir=${options.workDir}`, ...args];
4457
workDir = ".";
4558
}
59+
const prefix = process.env.GIT_WORK_DIR_PREFIX;
60+
if (prefix) {
61+
let cmdStart = 0;
62+
63+
// --git-dir=<path> must be the very first argument (prepended by the bare-repo handling above)
64+
if (args[0]?.startsWith("--git-dir=")) {
65+
validateWorkDir(args[0].substring("--git-dir=".length));
66+
cmdStart = 1;
67+
}
68+
69+
if (args[cmdStart] === "init") {
70+
// Strictly parse: init [--bare] [--initial-branch <name>] <directory>
71+
let i = cmdStart + 1;
72+
while (i < args.length) {
73+
if (args[i] === "--bare") {
74+
i++;
75+
} else if (args[i] === "--initial-branch" || args[i] === "-b") {
76+
if (i + 1 >= args.length) throw new Error(`git init: '${args[i]}' requires a value`);
77+
i += 2;
78+
} else if (args[i].startsWith("-")) {
79+
throw new Error(`git init: unexpected option '${args[i]}'`);
80+
} else {
81+
break;
82+
}
83+
}
84+
if (i >= args.length) {
85+
throw new Error(`git init without an explicit target directory is not allowed in tests`);
86+
}
87+
if (i + 1 < args.length) {
88+
throw new Error(`git init: unexpected trailing argument '${args[i + 1]}'`);
89+
}
90+
validateWorkDir(args[i]);
91+
} else if (cmdStart === 0) {
92+
// No --git-dir, not init: validate workDir
93+
validateWorkDir(workDir);
94+
}
95+
// If --git-dir was present and subcommand is not init, it was already validated above
96+
}
4697
if (options && options.trace) {
4798
process.stderr.write(`Called 'git ${args.join(" ")}' in '${workDir}':\n${new Error().stack}\n`);
4899
}
@@ -158,7 +209,9 @@ export function git(args: string[], options?: IGitOptions): Promise<string> {
158209
* @returns { string | undefined } the full SHA-1, or undefined
159210
*/
160211
export async function revParse(argument: string, workDir?: string): Promise<string | undefined> {
161-
const result = await GitProcess(["rev-parse", "--verify", "-q", argument], workDir || ".");
212+
const dir = workDir || ".";
213+
validateWorkDir(dir);
214+
const result = await GitProcess(["rev-parse", "--verify", "-q", argument], dir);
162215
return result.exitCode ? undefined : trimTrailingNewline(result.stdout);
163216
}
164217

@@ -171,6 +224,7 @@ export async function revParse(argument: string, workDir?: string): Promise<stri
171224
* @returns number the number of commits in the commit range
172225
*/
173226
export async function revListCount(rangeArgs: string | string[], workDir = "."): Promise<number> {
227+
validateWorkDir(workDir);
174228
const gitArgs: string[] = ["rev-list", "--count"];
175229
if (typeof rangeArgs === "string") {
176230
gitArgs.push(rangeArgs);
@@ -196,7 +250,9 @@ export async function commitExists(commit: string, workDir: string): Promise<boo
196250
}
197251

198252
export async function gitConfig(key: string, workDir?: string): Promise<string | undefined> {
199-
const result = await GitProcess(["config", key], workDir || ".");
253+
const dir = workDir || ".";
254+
validateWorkDir(dir);
255+
const result = await GitProcess(["config", key], dir);
200256
if (result.exitCode !== 0) {
201257
return undefined;
202258
}
@@ -208,12 +264,16 @@ export async function gitConfigForEach(
208264
callbackfn: (value: string) => void,
209265
workDir?: string,
210266
): Promise<void> {
211-
const result = await GitProcess(["config", "--get-all", key], workDir || ".");
267+
const dir = workDir || ".";
268+
validateWorkDir(dir);
269+
const result = await GitProcess(["config", "--get-all", key], dir);
212270
result.stdout.split(/\r?\n/).map(callbackfn);
213271
}
214272

215273
export async function gitCommandExists(command: string, workDir?: string): Promise<boolean> {
216-
const result = await GitProcess([command, "-h"], workDir || ".");
274+
const dir = workDir || ".";
275+
validateWorkDir(dir);
276+
const result = await GitProcess([command, "-h"], dir);
217277
return result.exitCode === 129;
218278
}
219279

tests/test-lib.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,9 @@ export async function testCreateRepo(name: string, suffix?: string): Promise<Tes
148148
}
149149
process.env.GIT_CEILING_DIRECTORIES = ceilings.join(path.delimiter);
150150

151+
// Suspend the workDir guard during repo setup (git init, git config --global)
152+
delete process.env.GIT_WORK_DIR_PREFIX;
153+
151154
// eslint-disable-next-line security/detect-unsafe-regex
152155
const match = name.match(/^(.*[\\/])?(.*?)(\.test)?\.ts$/);
153156
if (match) {
@@ -191,6 +194,10 @@ export async function testCreateRepo(name: string, suffix?: string): Promise<Tes
191194
workDir: dir,
192195
};
193196
await git(["commit-tree", "-m", "Test commit", "4b825dc642cb6eb9a060e54bf8d69288fbee4904"], opts);
197+
198+
// From now on, ensure that all git() calls use a workDir inside tmp
199+
process.env.GIT_WORK_DIR_PREFIX = tmp;
200+
194201
const gitOpts: ITestCommitOptions = { workDir: dir };
195202

196203
return new TestRepo(gitOpts);

0 commit comments

Comments
 (0)