Isolate git fixture tests in temp repos (CL-7372)

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

@@ -1,47 +1,19 @@
 import { afterAll, expect, test } from "bun:test";
-import { mkdirSync, mkdtempSync, rmSync } from "node:fs";
-import { tmpdir } from "node:os";
+import { mkdirSync } from "node:fs";
 import path from "node:path";
+import {
+  cleanupGitFixtures,
+  gitFixture,
+  scratchGitFixtureDir,
+} from "../test/git-fixture";
 import { createBootAssetWiring } from "./asset-service-factory";
 import { HubDataDirInsideGitWorkTreeError } from "./hub-data-dir-git-safety";
 
-const scratch: string[] = [];
-
-afterAll(() => {
-  for (const dir of scratch) rmSync(dir, { recursive: true, force: true });
-});
-
-function scratchDir(prefix: string): string {
-  const dir = mkdtempSync(path.join(tmpdir(), prefix));
-  scratch.push(dir);
-  return dir;
-}
-
-function git(cwd: string, args: readonly string[]): void {
-  const env = { ...process.env };
-  delete env["GIT_DIR"];
-  delete env["GIT_WORK_TREE"];
-  delete env["GIT_INDEX_FILE"];
-  env["GIT_AUTHOR_NAME"] = "safety-test";
-  env["GIT_AUTHOR_EMAIL"] = "safety@test";
-  env["GIT_COMMITTER_NAME"] = "safety-test";
-  env["GIT_COMMITTER_EMAIL"] = "safety@test";
-  const result = Bun.spawnSync(["git", "-c", "core.hooksPath=", ...args], {
-    cwd,
-    stdout: "pipe",
-    stderr: "pipe",
-    env,
-  });
-  if (result.exitCode !== 0) {
-    throw new Error(
-      `git ${args.join(" ")} failed: ${result.stderr.toString()}`,
-    );
-  }
-}
+afterAll(cleanupGitFixtures);
 
 test("createBootAssetWiring refuses a data dir inside an existing git work tree", async () => {
-  const workTree = scratchDir("boot-asset-inside-");
-  git(workTree, ["init", "-b", "main"]);
+  const workTree = scratchGitFixtureDir("boot-asset-inside-");
+  gitFixture(workTree, ["init", "-b", "main"]);
   const dataDir = path.join(workTree, ".data", "hub");
   mkdirSync(dataDir, { recursive: true });
   await expect(
@@ -50,8 +22,8 @@ test("createBootAssetWiring refuses a data dir inside an existing git work tree"
 });
 
 test("createBootAssetWiring opt-in skips the work-tree refusal", async () => {
-  const workTree = scratchDir("boot-asset-opt-in-");
-  git(workTree, ["init", "-b", "main"]);
+  const workTree = scratchGitFixtureDir("boot-asset-opt-in-");
+  gitFixture(workTree, ["init", "-b", "main"]);
   const dataDir = path.join(workTree, ".data", "hub");
   mkdirSync(dataDir, { recursive: true });
   try {

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

@@ -1,65 +1,31 @@
 import { afterAll, expect, test } from "bun:test";
-import {
-  mkdirSync,
-  mkdtempSync,
-  realpathSync,
-  rmSync,
-  writeFileSync,
-} from "node:fs";
-import { tmpdir } from "node:os";
+import { mkdirSync, realpathSync, writeFileSync } from "node:fs";
 import path from "node:path";
+import {
+  cleanupGitFixtures,
+  gitFixture,
+  scratchGitFixtureDir,
+} from "../test/git-fixture";
 import {
   assertHubDataDirGitSafety,
   enclosingGitWorkTree,
   HubDataDirInsideGitWorkTreeError,
 } from "./hub-data-dir-git-safety";
 
-const scratch: string[] = [];
-
-afterAll(() => {
-  for (const dir of scratch) rmSync(dir, { recursive: true, force: true });
-});
-
-function scratchDir(prefix: string): string {
-  const dir = mkdtempSync(path.join(tmpdir(), prefix));
-  scratch.push(dir);
-  return dir;
-}
-
-function git(cwd: string, args: readonly string[]): void {
-  const env = { ...process.env };
-  delete env["GIT_DIR"];
-  delete env["GIT_WORK_TREE"];
-  delete env["GIT_INDEX_FILE"];
-  env["GIT_AUTHOR_NAME"] = "safety-test";
-  env["GIT_AUTHOR_EMAIL"] = "safety@test";
-  env["GIT_COMMITTER_NAME"] = "safety-test";
-  env["GIT_COMMITTER_EMAIL"] = "safety@test";
-  const result = Bun.spawnSync(["git", "-c", "core.hooksPath=", ...args], {
-    cwd,
-    stdout: "pipe",
-    stderr: "pipe",
-    env,
-  });
-  if (result.exitCode !== 0) {
-    throw new Error(
-      `git ${args.join(" ")} failed: ${result.stderr.toString()}`,
-    );
-  }
-}
+afterAll(cleanupGitFixtures);
 
 test("a directory with no enclosing git work tree is allowed", () => {
-  const dir = scratchDir("hub-data-outside-");
+  const dir = scratchGitFixtureDir("hub-data-outside-");
   expect(enclosingGitWorkTree(dir)).toBeNull();
   expect(() => assertHubDataDirGitSafety(dir)).not.toThrow();
 });
 
 test("a directory inside an existing git work tree fails loud", () => {
-  const workTree = scratchDir("hub-data-inside-wt-");
-  git(workTree, ["init", "-b", "main"]);
+  const workTree = scratchGitFixtureDir("hub-data-inside-wt-");
+  gitFixture(workTree, ["init", "-b", "main"]);
   writeFileSync(path.join(workTree, "keep.txt"), "keep\n");
-  git(workTree, ["add", "keep.txt"]);
-  git(workTree, ["commit", "-m", "keep"]);
+  gitFixture(workTree, ["add", "keep.txt"]);
+  gitFixture(workTree, ["commit", "-m", "keep"]);
   const dataDir = path.join(workTree, ".data", "hub");
   mkdirSync(dataDir, { recursive: true });
 
@@ -81,8 +47,8 @@ test("a directory inside an existing git work tree fails loud", () => {
 });
 
 test("opt-in allows initializing inside an existing git work tree", () => {
-  const workTree = scratchDir("hub-data-opt-in-");
-  git(workTree, ["init", "-b", "main"]);
+  const workTree = scratchGitFixtureDir("hub-data-opt-in-");
+  gitFixture(workTree, ["init", "-b", "main"]);
   const dataDir = path.join(workTree, ".data", "hub");
   expect(() =>
     assertHubDataDirGitSafety(dataDir, { allowInsideWorkTree: true }),

apps/hub/test/git-fixture.ts

@@ -0,0 +1,89 @@
+// Shared git-fixture helper for apps/hub's own tests (CL-7372):
+// `asset-service-factory.test.ts` and `hub-data-dir-git-safety.test.ts`
+// each duplicated an mkdtemp-scratch-dir-plus-`git()` helper before this —
+// this is the one shared copy. Every fixture repo lives in its own
+// mkdtemp scratch directory outside this repository's own work tree, and
+// every git invocation runs with GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE
+// unset so it can never pick up an inherited pointer at real history.
+import { mkdirSync, mkdtempSync, realpathSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import path from "node:path";
+
+const scratch: string[] = [];
+let repositoryRoot: string | undefined;
+
+function findRepositoryRoot(): string {
+  if (repositoryRoot === undefined) {
+    const result = Bun.spawnSync(["git", "rev-parse", "--show-toplevel"], {
+      cwd: process.cwd(),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    if (result.exitCode !== 0) {
+      throw new Error(
+        `git rev-parse --show-toplevel failed: ${result.stderr.toString()}`,
+      );
+    }
+    repositoryRoot = realpathSync(result.stdout.toString().trim());
+  }
+  return repositoryRoot;
+}
+
+/** Throws if `dir` sits inside this repository's own work tree — a git
+ * fixture must live in an mkdtemp scratch dir outside any real work tree,
+ * never inside the checkout a stray commit could actually corrupt. */
+function assertOutsideRepository(dir: string): void {
+  const resolved = realpathSync(dir);
+  const root = findRepositoryRoot();
+  if (resolved === root || resolved.startsWith(`${root}${path.sep}`)) {
+    throw new Error(
+      `git fixture directory ${resolved} is inside the repository root ` +
+        `${root} — fixtures must live in an mkdtemp scratch dir outside ` +
+        `any work tree.`,
+    );
+  }
+}
+
+/** Creates a fresh mkdtemp scratch directory for a git fixture and tracks
+ * it for `cleanupGitFixtures`. */
+export function scratchGitFixtureDir(prefix: string): string {
+  const dir = mkdtempSync(path.join(tmpdir(), prefix));
+  scratch.push(dir);
+  return dir;
+}
+
+/** Runs `git` synchronously against `cwd`, guarded against ever touching
+ * this repository's own history: GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE are
+ * stripped from the child's env, and `cwd` is asserted outside the
+ * repository root before the process ever spawns. */
+export function gitFixture(cwd: string, args: readonly string[]): void {
+  mkdirSync(cwd, { recursive: true });
+  assertOutsideRepository(cwd);
+  const env = { ...process.env };
+  delete env["GIT_DIR"];
+  delete env["GIT_WORK_TREE"];
+  delete env["GIT_INDEX_FILE"];
+  env["GIT_AUTHOR_NAME"] = "safety-test";
+  env["GIT_AUTHOR_EMAIL"] = "safety@test";
+  env["GIT_COMMITTER_NAME"] = "safety-test";
+  env["GIT_COMMITTER_EMAIL"] = "safety@test";
+  const result = Bun.spawnSync(["git", "-c", "core.hooksPath=", ...args], {
+    cwd,
+    stdout: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  if (result.exitCode !== 0) {
+    throw new Error(
+      `git ${args.join(" ")} failed: ${result.stderr.toString()}`,
+    );
+  }
+}
+
+/** Removes every scratch directory this module created. Call from the
+ * test file's own `afterAll`. */
+export function cleanupGitFixtures(): void {
+  for (const dir of scratch.splice(0)) {
+    rmSync(dir, { recursive: true, force: true });
+  }
+}

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

@@ -12,16 +12,60 @@ import {
   mkdir,
   mkdtemp,
   readFile,
+  realpath,
   rm,
   writeFile,
 } from "node:fs/promises";
 import { tmpdir } from "node:os";
-import { join } from "node:path";
+import { join, sep } from "node:path";
 import { installDisposableHubDataDir } from "../../../test/disposable-hub-data-dir";
 import { createGitWorkflowPusher } from "./workflow-push";
 
 installDisposableHubDataDir();
 
+let repositoryRoot: string | undefined;
+
+async function findRepositoryRoot(): Promise<string> {
+  if (repositoryRoot === undefined) {
+    const proc = Bun.spawn(["git", "rev-parse", "--show-toplevel"], {
+      cwd: process.cwd(),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, code] = await Promise.all([
+      new Response(proc.stdout).text(),
+      new Response(proc.stderr).text(),
+      proc.exited,
+    ]);
+    if (code !== 0) {
+      throw new Error(`git rev-parse --show-toplevel failed: ${stderr}`);
+    }
+    repositoryRoot = await realpath(stdout.trim());
+  }
+  return repositoryRoot;
+}
+
+// Every fixture repo this suite writes must live outside the repository's
+// own work tree (CL-7372) — a git-fixture test must never be able to
+// touch real history, however indirectly.
+async function assertOutsideRepository(work: string): Promise<void> {
+  const resolved = await realpath(work);
+  const root = await findRepositoryRoot();
+  if (resolved === root || resolved.startsWith(`${root}${sep}`)) {
+    throw new Error(
+      `git fixture directory ${resolved} is inside the repository root ` +
+        `${root} — fixtures must live in an mkdtemp scratch dir outside ` +
+        `any work tree.`,
+    );
+  }
+}
+
+async function scratchWorkDir(prefix: string): Promise<string> {
+  const dir = await mkdtemp(join(tmpdir(), prefix));
+  await assertOutsideRepository(dir);
+  return dir;
+}
+
 async function git(args: string[], cwd: string): Promise<string> {
   const env = { ...process.env };
   delete env["GIT_DIR"];
@@ -46,7 +90,7 @@ async function git(args: string[], cwd: string): Promise<string> {
 
 describe("createGitWorkflowPusher", () => {
   test("force-repoints a seed-owned asset whose main has diverged, rather than failing the whole seed", async () => {
-    const work = await mkdtemp(join(tmpdir(), "workflow-push-test-"));
+    const work = await scratchWorkDir("workflow-push-test-");
     try {
       const remoteDir = join(work, "remote.git");
       // A bare repo whose default branch ("trunk") is not "main" and
@@ -105,7 +149,7 @@ describe("createGitWorkflowPusher", () => {
   });
 
   test("reports unchanged, without pushing, when the seed-owned content already matches", async () => {
-    const work = await mkdtemp(join(tmpdir(), "workflow-push-test-"));
+    const work = await scratchWorkDir("workflow-push-test-");
     try {
       const remoteDir = join(work, "remote.git");
       await git(["init", "--bare", "--initial-branch=main", remoteDir], work);
@@ -134,7 +178,7 @@ describe("createGitWorkflowPusher", () => {
   });
 
   test("commits the seed tree even when the operator's inherited hooksPath would reject seed@workbench.localhost", async () => {
-    const work = await mkdtemp(join(tmpdir(), "workflow-push-hooks-"));
+    const work = await scratchWorkDir("workflow-push-hooks-");
     const previousGlobal = process.env.GIT_CONFIG_GLOBAL;
     try {
       const hooksDir = join(work, "hooks");
@@ -186,7 +230,7 @@ describe("createGitWorkflowPusher", () => {
   });
 
   test("does not commit onto an inherited GIT_DIR pointing at another work tree", async () => {
-    const work = await mkdtemp(join(tmpdir(), "workflow-push-git-dir-"));
+    const work = await scratchWorkDir("workflow-push-git-dir-");
     const previousGitDir = process.env.GIT_DIR;
     const previousWorkTree = process.env.GIT_WORK_TREE;
     try {