From ca97d5426149fbe77934b61ef6f107d660da739f Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 30 Aug 2026 22:20:33 -0700 Subject: [PATCH 1/3] Isolate freshness-check fixtures from operator git hooks Scratch repos still need git history so freshness can compare src/ to the version-introducing commit, but git commit ran the operator core.hooksPath and failed a global author allow-list. Fixtures now write history with commit-tree under a test-owned git config, and a regression plants a rejecting commit-msg hook to keep that isolation. --- .../src/freshness-check.test.ts | 167 +++++++++++++++++- 1 file changed, 158 insertions(+), 9 deletions(-) diff --git a/packages/tool-registry-publish/src/freshness-check.test.ts b/packages/tool-registry-publish/src/freshness-check.test.ts index 9b4e6ebab..c7ca782d1 100644 --- a/packages/tool-registry-publish/src/freshness-check.test.ts +++ b/packages/tool-registry-publish/src/freshness-check.test.ts @@ -2,9 +2,11 @@ // loud — the failure mode `publishCorbitsToolsRegistry` cannot catch, // because it skips an already-published name@version rather than // comparing source. Fixtures are scratch git repos so this suite does -// not depend on the worktree's own dirty tool packages. +// not depend on the worktree's own dirty tool packages. History is +// 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 { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { chmod, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { CORBITS_TOOL_PACKAGE_DIRS } from "./registry"; @@ -17,6 +19,7 @@ import { } from "./freshness-check"; const scratchDirs: string[] = []; +let fixtureGitConfig: string | undefined; afterAll(async () => { await Promise.all( @@ -30,25 +33,104 @@ async function scratchDir(prefix: string): Promise { return dir; } -async function git(cwd: string, args: readonly string[]): Promise { +async function fixtureGitEnv(): Promise> { + if (fixtureGitConfig === undefined) { + const dir = await scratchDir("corbits-tools-freshness-gitconfig-"); + const hooks = path.join(dir, "hooks"); + await mkdir(hooks); + fixtureGitConfig = path.join(dir, "config"); + await writeFile( + fixtureGitConfig, + [ + "[user]", + "\tname = Freshness", + "\temail = freshness@test", + "[core]", + `\thooksPath = ${hooks}`, + "[commit]", + "\tgpgsign = false", + "", + ].join("\n"), + ); + } + return { + ...process.env, + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_GLOBAL: fixtureGitConfig, + }; +} + +async function git(cwd: string, args: readonly string[]): Promise { const proc = Bun.spawn( [ "git", "-c", + "core.hooksPath=", + "-c", "user.email=freshness@test", "-c", "user.name=Freshness", + "-c", + "commit.gpgsign=false", ...args, ], - { cwd, stdout: "pipe", stderr: "pipe" }, + { + cwd, + env: await fixtureGitEnv(), + stdout: "pipe", + stderr: "pipe", + }, ); - const [stderr, code] = await Promise.all([ + 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 ${args.join(" ")} failed: ${stderr}`); + } + return stdout.trim(); +} + +async function rawGit( + cwd: string, + args: readonly string[], + env: Record, +): Promise { + const proc = Bun.spawn(["git", ...args], { + cwd, + env, + 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 ${args.join(" ")} failed: ${stderr}`); } + return stdout.trim(); +} + +// `git commit` runs the operator's commit-msg / author hooks. Plumbing +// writes the same history without that surface, so this suite does not +// depend on whoever owns `core.hooksPath` on the machine. +async function commitAll(root: string, message: string): Promise { + await git(root, ["add", "."]); + const tree = await git(root, ["write-tree"]); + let parent: string | undefined; + try { + parent = await git(root, ["rev-parse", "--verify", "HEAD"]); + } catch { + parent = undefined; + } + const sha = + parent === undefined + ? await git(root, ["commit-tree", tree, "-m", message]) + : await git(root, ["commit-tree", tree, "-p", parent, "-m", message]); + await git(root, ["update-ref", "HEAD", sha]); } async function writePkg( @@ -72,11 +154,30 @@ async function committedPackage(src: string): Promise<{ await git(root, ["init", "-b", "main"]); const pkg = path.join(root, "fake-tools"); await writePkg(pkg, "0.0.1", src); - await git(root, ["add", "."]); - await git(root, ["commit", "-m", "initial @corbits/fake-tools@0.0.1"]); + await commitAll(root, "initial @corbits/fake-tools@0.0.1"); return { root, pkg }; } +async function plantRejectingAuthorHook(): Promise { + const work = await scratchDir("corbits-tools-freshness-hooks-"); + const hooksDir = path.join(work, "hooks"); + await mkdir(hooksDir); + const hook = path.join(hooksDir, "commit-msg"); + await writeFile( + hook, + [ + "#!/bin/sh", + 'echo "commit blocked: author must be listed in allowed-emails" >&2', + "exit 1", + "", + ].join("\n"), + ); + await chmod(hook, 0o755); + const globalConfig = path.join(work, "gitconfig"); + await writeFile(globalConfig, `[core]\nhooksPath = ${hooksDir}\n`); + return globalConfig; +} + describe("staleToolPackages", () => { test("src-changed-without-bump is a finding", () => { expect( @@ -218,13 +319,61 @@ describe("checkToolPackageFreshness", () => { test("committed src change after the version-introducing commit is loud", async () => { const { root, pkg } = await committedPackage("export const n = 1;\n"); await writePkg(pkg, "0.0.1", "export const n = 2;\n"); - await git(root, ["add", "."]); - await git(root, ["commit", "-m", "src change, forgot the bump"]); + await commitAll(root, "src change, forgot the bump"); await expect( checkToolPackageFreshness({ packageDirs: [pkg] }), ).rejects.toBeInstanceOf(StaleToolPackageError); }); + + test("fixture history is written even when a global author hook would reject git commit", async () => { + const globalConfig = await plantRejectingAuthorHook(); + const hostileEnv = { + ...process.env, + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_GLOBAL: globalConfig, + }; + + const victim = await scratchDir("corbits-tools-freshness-victim-"); + await rawGit( + victim, + [ + "-c", + "user.email=freshness@test", + "-c", + "user.name=Freshness", + "init", + "-b", + "main", + ], + hostileEnv, + ); + await writeFile(path.join(victim, "README"), "victim\n"); + await rawGit(victim, ["add", "."], hostileEnv); + let unguarded: unknown; + try { + await rawGit( + victim, + [ + "-c", + "user.email=freshness@test", + "-c", + "user.name=Freshness", + "commit", + "-m", + "should be blocked", + ], + hostileEnv, + ); + } catch (err) { + unguarded = err; + } + expect(unguarded).toBeInstanceOf(Error); + expect((unguarded as Error).message).toContain("allowed-emails"); + + const { pkg } = await committedPackage("export const n = 1;\n"); + await checkToolPackageFreshness({ packageDirs: [pkg] }); + }); }); describe("snapshotToolPackages", () => { From 93906c51213e7755de44835430b4569944275c82 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 30 Aug 2026 22:32:57 -0700 Subject: [PATCH 2/3] Prove freshness fixtures survive a rejecting global git hook The regression planted a rejecting commit-msg hook but then wrote fixture history through git() env that never included that hook. Pass the hostile env into committedPackage so the writer is actually under the hook when it succeeds. --- .../src/freshness-check.test.ts | 40 +++++++++++++------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/packages/tool-registry-publish/src/freshness-check.test.ts b/packages/tool-registry-publish/src/freshness-check.test.ts index c7ca782d1..c80a76f2a 100644 --- a/packages/tool-registry-publish/src/freshness-check.test.ts +++ b/packages/tool-registry-publish/src/freshness-check.test.ts @@ -60,7 +60,11 @@ async function fixtureGitEnv(): Promise> { }; } -async function git(cwd: string, args: readonly string[]): Promise { +async function git( + cwd: string, + args: readonly string[], + env?: Record, +): Promise { const proc = Bun.spawn( [ "git", @@ -76,7 +80,7 @@ async function git(cwd: string, args: readonly string[]): Promise { ], { cwd, - env: await fixtureGitEnv(), + env: env ?? (await fixtureGitEnv()), stdout: "pipe", stderr: "pipe", }, @@ -117,20 +121,24 @@ async function rawGit( // `git commit` runs the operator's commit-msg / author hooks. Plumbing // writes the same history without that surface, so this suite does not // depend on whoever owns `core.hooksPath` on the machine. -async function commitAll(root: string, message: string): Promise { - await git(root, ["add", "."]); - const tree = await git(root, ["write-tree"]); +async function commitAll( + root: string, + message: string, + env?: Record, +): Promise { + await git(root, ["add", "."], env); + const tree = await git(root, ["write-tree"], env); let parent: string | undefined; try { - parent = await git(root, ["rev-parse", "--verify", "HEAD"]); + parent = await git(root, ["rev-parse", "--verify", "HEAD"], env); } catch { parent = undefined; } const sha = parent === undefined - ? await git(root, ["commit-tree", tree, "-m", message]) - : await git(root, ["commit-tree", tree, "-p", parent, "-m", message]); - await git(root, ["update-ref", "HEAD", sha]); + ? await git(root, ["commit-tree", tree, "-m", message], env) + : await git(root, ["commit-tree", tree, "-p", parent, "-m", message], env); + await git(root, ["update-ref", "HEAD", sha], env); } async function writePkg( @@ -146,15 +154,18 @@ async function writePkg( await writeFile(path.join(pkg, "src", "index.ts"), src); } -async function committedPackage(src: string): Promise<{ +async function committedPackage( + src: string, + env?: Record, +): Promise<{ root: string; pkg: string; }> { const root = await scratchDir("corbits-tools-freshness-"); - await git(root, ["init", "-b", "main"]); + await git(root, ["init", "-b", "main"], env); const pkg = path.join(root, "fake-tools"); await writePkg(pkg, "0.0.1", src); - await commitAll(root, "initial @corbits/fake-tools@0.0.1"); + await commitAll(root, "initial @corbits/fake-tools@0.0.1", env); return { root, pkg }; } @@ -371,7 +382,10 @@ describe("checkToolPackageFreshness", () => { expect(unguarded).toBeInstanceOf(Error); expect((unguarded as Error).message).toContain("allowed-emails"); - const { pkg } = await committedPackage("export const n = 1;\n"); + const { pkg } = await committedPackage( + "export const n = 1;\n", + hostileEnv, + ); await checkToolPackageFreshness({ packageDirs: [pkg] }); }); }); From 4d8896701b7252c4d29fae2364b5bb79f9097cfd Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 30 Aug 2026 22:39:27 -0700 Subject: [PATCH 3/3] Format the freshness-check fixture test --- .../tool-registry-publish/src/freshness-check.test.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/tool-registry-publish/src/freshness-check.test.ts b/packages/tool-registry-publish/src/freshness-check.test.ts index c80a76f2a..6e17852d7 100644 --- a/packages/tool-registry-publish/src/freshness-check.test.ts +++ b/packages/tool-registry-publish/src/freshness-check.test.ts @@ -137,7 +137,11 @@ async function commitAll( const sha = parent === undefined ? await git(root, ["commit-tree", tree, "-m", message], env) - : await git(root, ["commit-tree", tree, "-p", parent, "-m", message], env); + : await git( + root, + ["commit-tree", tree, "-p", parent, "-m", message], + env, + ); await git(root, ["update-ref", "HEAD", sha], env); } @@ -382,10 +386,7 @@ describe("checkToolPackageFreshness", () => { expect(unguarded).toBeInstanceOf(Error); expect((unguarded as Error).message).toContain("allowed-emails"); - const { pkg } = await committedPackage( - "export const n = 1;\n", - hostileEnv, - ); + const { pkg } = await committedPackage("export const n = 1;\n", hostileEnv); await checkToolPackageFreshness({ packageDirs: [pkg] }); }); });