diff --git a/apps/hub/src/asset-service-factory.test.ts b/apps/hub/src/asset-service-factory.test.ts index 97a2f2ea3..0cdef5dbf 100644 --- a/apps/hub/src/asset-service-factory.test.ts +++ b/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 { diff --git a/apps/hub/src/hub-data-dir-git-safety.test.ts b/apps/hub/src/hub-data-dir-git-safety.test.ts index 904ddadd3..1e5a60af0 100644 --- a/apps/hub/src/hub-data-dir-git-safety.test.ts +++ b/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 }), diff --git a/apps/hub/test/git-fixture.ts b/apps/hub/test/git-fixture.ts new file mode 100644 index 000000000..48c917517 --- /dev/null +++ b/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 }); + } +} diff --git a/packages/hub-client/src/workflow-push.test.ts b/packages/hub-client/src/workflow-push.test.ts index d6db1f282..fe62ce00c 100644 --- a/packages/hub-client/src/workflow-push.test.ts +++ b/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 { + 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 { + 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 { + const dir = await mkdtemp(join(tmpdir(), prefix)); + await assertOutsideRepository(dir); + return dir; +} + async function git(args: string[], cwd: string): Promise { const env = { ...process.env }; delete env["GIT_DIR"]; @@ -46,7 +90,7 @@ async function git(args: string[], cwd: string): Promise { 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 { diff --git a/packages/tool-registry-publish/src/freshness-check.test.ts b/packages/tool-registry-publish/src/freshness-check.test.ts index 6e17852d7..4e5faf0af 100644 --- a/packages/tool-registry-publish/src/freshness-check.test.ts +++ b/packages/tool-registry-publish/src/freshness-check.test.ts @@ -6,7 +6,14 @@ // written with plumbing (`commit-tree`) under a test-owned git config // so a developer `core.hooksPath` cannot fail the suite. import { afterAll, describe, expect, test } from "bun:test"; -import { chmod, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { + chmod, + mkdir, + mkdtemp, + realpath, + rm, + writeFile, +} from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { CORBITS_TOOL_PACKAGE_DIRS } from "./registry"; @@ -20,6 +27,7 @@ import { const scratchDirs: string[] = []; let fixtureGitConfig: string | undefined; +let repositoryRoot: string | undefined; afterAll(async () => { await Promise.all( @@ -27,8 +35,44 @@ afterAll(async () => { ); }); +async function findRepositoryRoot(): Promise { + 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(dir: string): Promise { + const resolved = await realpath(dir); + const root = await 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.`, + ); + } +} + async function scratchDir(prefix: string): Promise { const dir = await mkdtemp(path.join(tmpdir(), prefix)); + await assertOutsideRepository(dir); scratchDirs.push(dir); return dir; } @@ -53,8 +97,12 @@ async function fixtureGitEnv(): Promise> { ].join("\n"), ); } + const env = { ...process.env }; + delete env["GIT_DIR"]; + delete env["GIT_WORK_TREE"]; + delete env["GIT_INDEX_FILE"]; return { - ...process.env, + ...env, GIT_CONFIG_NOSYSTEM: "1", GIT_CONFIG_GLOBAL: fixtureGitConfig, }; @@ -343,8 +391,12 @@ describe("checkToolPackageFreshness", () => { test("fixture history is written even when a global author hook would reject git commit", async () => { const globalConfig = await plantRejectingAuthorHook(); + const baseEnv = { ...process.env }; + delete baseEnv["GIT_DIR"]; + delete baseEnv["GIT_WORK_TREE"]; + delete baseEnv["GIT_INDEX_FILE"]; const hostileEnv = { - ...process.env, + ...baseEnv, GIT_CONFIG_NOSYSTEM: "1", GIT_CONFIG_GLOBAL: globalConfig, };