Skip to content

Commit c0a57d7

Browse files
TheGreatAxiosTheGreatAxios
committed
Isolate git fixture tests in temp repos (CL-7372)
1 parent 740a669 commit c0a57d7

5 files changed

Lines changed: 218 additions & 95 deletions

File tree

‎apps/hub/src/asset-service-factory.test.ts‎

Lines changed: 11 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,19 @@
11
import { afterAll, expect, test } from "bun:test";
2-
import { mkdirSync, mkdtempSync, rmSync } from "node:fs";
3-
import { tmpdir } from "node:os";
2+
import { mkdirSync } from "node:fs";
43
import path from "node:path";
4+
import {
5+
cleanupGitFixtures,
6+
gitFixture,
7+
scratchGitFixtureDir,
8+
} from "../test/git-fixture";
59
import { createBootAssetWiring } from "./asset-service-factory";
610
import { HubDataDirInsideGitWorkTreeError } from "./hub-data-dir-git-safety";
711

8-
const scratch: string[] = [];
9-
10-
afterAll(() => {
11-
for (const dir of scratch) rmSync(dir, { recursive: true, force: true });
12-
});
13-
14-
function scratchDir(prefix: string): string {
15-
const dir = mkdtempSync(path.join(tmpdir(), prefix));
16-
scratch.push(dir);
17-
return dir;
18-
}
19-
20-
function git(cwd: string, args: readonly string[]): void {
21-
const env = { ...process.env };
22-
delete env["GIT_DIR"];
23-
delete env["GIT_WORK_TREE"];
24-
delete env["GIT_INDEX_FILE"];
25-
env["GIT_AUTHOR_NAME"] = "safety-test";
26-
env["GIT_AUTHOR_EMAIL"] = "safety@test";
27-
env["GIT_COMMITTER_NAME"] = "safety-test";
28-
env["GIT_COMMITTER_EMAIL"] = "safety@test";
29-
const result = Bun.spawnSync(["git", "-c", "core.hooksPath=", ...args], {
30-
cwd,
31-
stdout: "pipe",
32-
stderr: "pipe",
33-
env,
34-
});
35-
if (result.exitCode !== 0) {
36-
throw new Error(
37-
`git ${args.join(" ")} failed: ${result.stderr.toString()}`,
38-
);
39-
}
40-
}
12+
afterAll(cleanupGitFixtures);
4113

4214
test("createBootAssetWiring refuses a data dir inside an existing git work tree", async () => {
43-
const workTree = scratchDir("boot-asset-inside-");
44-
git(workTree, ["init", "-b", "main"]);
15+
const workTree = scratchGitFixtureDir("boot-asset-inside-");
16+
gitFixture(workTree, ["init", "-b", "main"]);
4517
const dataDir = path.join(workTree, ".data", "hub");
4618
mkdirSync(dataDir, { recursive: true });
4719
await expect(
@@ -50,8 +22,8 @@ test("createBootAssetWiring refuses a data dir inside an existing git work tree"
5022
});
5123

5224
test("createBootAssetWiring opt-in skips the work-tree refusal", async () => {
53-
const workTree = scratchDir("boot-asset-opt-in-");
54-
git(workTree, ["init", "-b", "main"]);
25+
const workTree = scratchGitFixtureDir("boot-asset-opt-in-");
26+
gitFixture(workTree, ["init", "-b", "main"]);
5527
const dataDir = path.join(workTree, ".data", "hub");
5628
mkdirSync(dataDir, { recursive: true });
5729
try {

‎apps/hub/src/hub-data-dir-git-safety.test.ts‎

Lines changed: 14 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,31 @@
11
import { afterAll, expect, test } from "bun:test";
2-
import {
3-
mkdirSync,
4-
mkdtempSync,
5-
realpathSync,
6-
rmSync,
7-
writeFileSync,
8-
} from "node:fs";
9-
import { tmpdir } from "node:os";
2+
import { mkdirSync, realpathSync, writeFileSync } from "node:fs";
103
import path from "node:path";
4+
import {
5+
cleanupGitFixtures,
6+
gitFixture,
7+
scratchGitFixtureDir,
8+
} from "../test/git-fixture";
119
import {
1210
assertHubDataDirGitSafety,
1311
enclosingGitWorkTree,
1412
HubDataDirInsideGitWorkTreeError,
1513
} from "./hub-data-dir-git-safety";
1614

17-
const scratch: string[] = [];
18-
19-
afterAll(() => {
20-
for (const dir of scratch) rmSync(dir, { recursive: true, force: true });
21-
});
22-
23-
function scratchDir(prefix: string): string {
24-
const dir = mkdtempSync(path.join(tmpdir(), prefix));
25-
scratch.push(dir);
26-
return dir;
27-
}
28-
29-
function git(cwd: string, args: readonly string[]): void {
30-
const env = { ...process.env };
31-
delete env["GIT_DIR"];
32-
delete env["GIT_WORK_TREE"];
33-
delete env["GIT_INDEX_FILE"];
34-
env["GIT_AUTHOR_NAME"] = "safety-test";
35-
env["GIT_AUTHOR_EMAIL"] = "safety@test";
36-
env["GIT_COMMITTER_NAME"] = "safety-test";
37-
env["GIT_COMMITTER_EMAIL"] = "safety@test";
38-
const result = Bun.spawnSync(["git", "-c", "core.hooksPath=", ...args], {
39-
cwd,
40-
stdout: "pipe",
41-
stderr: "pipe",
42-
env,
43-
});
44-
if (result.exitCode !== 0) {
45-
throw new Error(
46-
`git ${args.join(" ")} failed: ${result.stderr.toString()}`,
47-
);
48-
}
49-
}
15+
afterAll(cleanupGitFixtures);
5016

5117
test("a directory with no enclosing git work tree is allowed", () => {
52-
const dir = scratchDir("hub-data-outside-");
18+
const dir = scratchGitFixtureDir("hub-data-outside-");
5319
expect(enclosingGitWorkTree(dir)).toBeNull();
5420
expect(() => assertHubDataDirGitSafety(dir)).not.toThrow();
5521
});
5622

5723
test("a directory inside an existing git work tree fails loud", () => {
58-
const workTree = scratchDir("hub-data-inside-wt-");
59-
git(workTree, ["init", "-b", "main"]);
24+
const workTree = scratchGitFixtureDir("hub-data-inside-wt-");
25+
gitFixture(workTree, ["init", "-b", "main"]);
6026
writeFileSync(path.join(workTree, "keep.txt"), "keep\n");
61-
git(workTree, ["add", "keep.txt"]);
62-
git(workTree, ["commit", "-m", "keep"]);
27+
gitFixture(workTree, ["add", "keep.txt"]);
28+
gitFixture(workTree, ["commit", "-m", "keep"]);
6329
const dataDir = path.join(workTree, ".data", "hub");
6430
mkdirSync(dataDir, { recursive: true });
6531

@@ -81,8 +47,8 @@ test("a directory inside an existing git work tree fails loud", () => {
8147
});
8248

8349
test("opt-in allows initializing inside an existing git work tree", () => {
84-
const workTree = scratchDir("hub-data-opt-in-");
85-
git(workTree, ["init", "-b", "main"]);
50+
const workTree = scratchGitFixtureDir("hub-data-opt-in-");
51+
gitFixture(workTree, ["init", "-b", "main"]);
8652
const dataDir = path.join(workTree, ".data", "hub");
8753
expect(() =>
8854
assertHubDataDirGitSafety(dataDir, { allowInsideWorkTree: true }),

‎apps/hub/test/git-fixture.ts‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Shared git-fixture helper for apps/hub's own tests (CL-7372):
2+
// `asset-service-factory.test.ts` and `hub-data-dir-git-safety.test.ts`
3+
// each duplicated an mkdtemp-scratch-dir-plus-`git()` helper before this —
4+
// this is the one shared copy. Every fixture repo lives in its own
5+
// mkdtemp scratch directory outside this repository's own work tree, and
6+
// every git invocation runs with GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE
7+
// unset so it can never pick up an inherited pointer at real history.
8+
import { mkdirSync, mkdtempSync, realpathSync, rmSync } from "node:fs";
9+
import { tmpdir } from "node:os";
10+
import path from "node:path";
11+
12+
const scratch: string[] = [];
13+
let repositoryRoot: string | undefined;
14+
15+
function findRepositoryRoot(): string {
16+
if (repositoryRoot === undefined) {
17+
const result = Bun.spawnSync(["git", "rev-parse", "--show-toplevel"], {
18+
cwd: process.cwd(),
19+
stdout: "pipe",
20+
stderr: "pipe",
21+
});
22+
if (result.exitCode !== 0) {
23+
throw new Error(
24+
`git rev-parse --show-toplevel failed: ${result.stderr.toString()}`,
25+
);
26+
}
27+
repositoryRoot = realpathSync(result.stdout.toString().trim());
28+
}
29+
return repositoryRoot;
30+
}
31+
32+
/** Throws if `dir` sits inside this repository's own work tree — a git
33+
* fixture must live in an mkdtemp scratch dir outside any real work tree,
34+
* never inside the checkout a stray commit could actually corrupt. */
35+
function assertOutsideRepository(dir: string): void {
36+
const resolved = realpathSync(dir);
37+
const root = findRepositoryRoot();
38+
if (resolved === root || resolved.startsWith(`${root}${path.sep}`)) {
39+
throw new Error(
40+
`git fixture directory ${resolved} is inside the repository root ` +
41+
`${root} — fixtures must live in an mkdtemp scratch dir outside ` +
42+
`any work tree.`,
43+
);
44+
}
45+
}
46+
47+
/** Creates a fresh mkdtemp scratch directory for a git fixture and tracks
48+
* it for `cleanupGitFixtures`. */
49+
export function scratchGitFixtureDir(prefix: string): string {
50+
const dir = mkdtempSync(path.join(tmpdir(), prefix));
51+
scratch.push(dir);
52+
return dir;
53+
}
54+
55+
/** Runs `git` synchronously against `cwd`, guarded against ever touching
56+
* this repository's own history: GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE are
57+
* stripped from the child's env, and `cwd` is asserted outside the
58+
* repository root before the process ever spawns. */
59+
export function gitFixture(cwd: string, args: readonly string[]): void {
60+
mkdirSync(cwd, { recursive: true });
61+
assertOutsideRepository(cwd);
62+
const env = { ...process.env };
63+
delete env["GIT_DIR"];
64+
delete env["GIT_WORK_TREE"];
65+
delete env["GIT_INDEX_FILE"];
66+
env["GIT_AUTHOR_NAME"] = "safety-test";
67+
env["GIT_AUTHOR_EMAIL"] = "safety@test";
68+
env["GIT_COMMITTER_NAME"] = "safety-test";
69+
env["GIT_COMMITTER_EMAIL"] = "safety@test";
70+
const result = Bun.spawnSync(["git", "-c", "core.hooksPath=", ...args], {
71+
cwd,
72+
stdout: "pipe",
73+
stderr: "pipe",
74+
env,
75+
});
76+
if (result.exitCode !== 0) {
77+
throw new Error(
78+
`git ${args.join(" ")} failed: ${result.stderr.toString()}`,
79+
);
80+
}
81+
}
82+
83+
/** Removes every scratch directory this module created. Call from the
84+
* test file's own `afterAll`. */
85+
export function cleanupGitFixtures(): void {
86+
for (const dir of scratch.splice(0)) {
87+
rmSync(dir, { recursive: true, force: true });
88+
}
89+
}

‎packages/hub-client/src/workflow-push.test.ts‎

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,60 @@ import {
1212
mkdir,
1313
mkdtemp,
1414
readFile,
15+
realpath,
1516
rm,
1617
writeFile,
1718
} from "node:fs/promises";
1819
import { tmpdir } from "node:os";
19-
import { join } from "node:path";
20+
import { join, sep } from "node:path";
2021
import { installDisposableHubDataDir } from "../../../test/disposable-hub-data-dir";
2122
import { createGitWorkflowPusher } from "./workflow-push";
2223

2324
installDisposableHubDataDir();
2425

26+
let repositoryRoot: string | undefined;
27+
28+
async function findRepositoryRoot(): Promise<string> {
29+
if (repositoryRoot === undefined) {
30+
const proc = Bun.spawn(["git", "rev-parse", "--show-toplevel"], {
31+
cwd: process.cwd(),
32+
stdout: "pipe",
33+
stderr: "pipe",
34+
});
35+
const [stdout, stderr, code] = await Promise.all([
36+
new Response(proc.stdout).text(),
37+
new Response(proc.stderr).text(),
38+
proc.exited,
39+
]);
40+
if (code !== 0) {
41+
throw new Error(`git rev-parse --show-toplevel failed: ${stderr}`);
42+
}
43+
repositoryRoot = await realpath(stdout.trim());
44+
}
45+
return repositoryRoot;
46+
}
47+
48+
// Every fixture repo this suite writes must live outside the repository's
49+
// own work tree (CL-7372) — a git-fixture test must never be able to
50+
// touch real history, however indirectly.
51+
async function assertOutsideRepository(work: string): Promise<void> {
52+
const resolved = await realpath(work);
53+
const root = await findRepositoryRoot();
54+
if (resolved === root || resolved.startsWith(`${root}${sep}`)) {
55+
throw new Error(
56+
`git fixture directory ${resolved} is inside the repository root ` +
57+
`${root} — fixtures must live in an mkdtemp scratch dir outside ` +
58+
`any work tree.`,
59+
);
60+
}
61+
}
62+
63+
async function scratchWorkDir(prefix: string): Promise<string> {
64+
const dir = await mkdtemp(join(tmpdir(), prefix));
65+
await assertOutsideRepository(dir);
66+
return dir;
67+
}
68+
2569
async function git(args: string[], cwd: string): Promise<string> {
2670
const env = { ...process.env };
2771
delete env["GIT_DIR"];
@@ -46,7 +90,7 @@ async function git(args: string[], cwd: string): Promise<string> {
4690

4791
describe("createGitWorkflowPusher", () => {
4892
test("force-repoints a seed-owned asset whose main has diverged, rather than failing the whole seed", async () => {
49-
const work = await mkdtemp(join(tmpdir(), "workflow-push-test-"));
93+
const work = await scratchWorkDir("workflow-push-test-");
5094
try {
5195
const remoteDir = join(work, "remote.git");
5296
// A bare repo whose default branch ("trunk") is not "main" and
@@ -105,7 +149,7 @@ describe("createGitWorkflowPusher", () => {
105149
});
106150

107151
test("reports unchanged, without pushing, when the seed-owned content already matches", async () => {
108-
const work = await mkdtemp(join(tmpdir(), "workflow-push-test-"));
152+
const work = await scratchWorkDir("workflow-push-test-");
109153
try {
110154
const remoteDir = join(work, "remote.git");
111155
await git(["init", "--bare", "--initial-branch=main", remoteDir], work);
@@ -134,7 +178,7 @@ describe("createGitWorkflowPusher", () => {
134178
});
135179

136180
test("commits the seed tree even when the operator's inherited hooksPath would reject seed@workbench.localhost", async () => {
137-
const work = await mkdtemp(join(tmpdir(), "workflow-push-hooks-"));
181+
const work = await scratchWorkDir("workflow-push-hooks-");
138182
const previousGlobal = process.env.GIT_CONFIG_GLOBAL;
139183
try {
140184
const hooksDir = join(work, "hooks");
@@ -186,7 +230,7 @@ describe("createGitWorkflowPusher", () => {
186230
});
187231

188232
test("does not commit onto an inherited GIT_DIR pointing at another work tree", async () => {
189-
const work = await mkdtemp(join(tmpdir(), "workflow-push-git-dir-"));
233+
const work = await scratchWorkDir("workflow-push-git-dir-");
190234
const previousGitDir = process.env.GIT_DIR;
191235
const previousWorkTree = process.env.GIT_WORK_TREE;
192236
try {

0 commit comments

Comments
 (0)