From c9d990aac91499d7a6234bc91f0494666d7768ed Mon Sep 17 00:00:00 2001 From: BigSimmo Date: Sat, 22 Aug 2026 02:41:11 +0800 Subject: [PATCH 1/2] fix(hooks): recognise a relative core.hooksPath so the push guard self-disables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `push-format-guard.sh` documents that it "runs ONLY when the git hook is absent or not wired to this repo's .githooks directory" and otherwise "exits in milliseconds having done nothing". It did not. The wiring test was a suffix glob: case "$normalised" in */.githooks) `core.hooksPath` is absolute OR relative to the top of the working tree, and this repo's own `npm install` writes the bare relative form `.githooks`. That value has no `/` before the name, so it never matched, and a correctly wired checkout fell through to `npx --no-install prettier --check .` on EVERY push. Measured on the Windows workstation on 2026-08-22: a `git push origin HEAD` payload ran 103,570 ms and was still going when a 100 s timeout killed it. After this change the same payload exits silently in 1,380 ms. Resolve the value to a single form and compare it for equality with `$repo_root/.githooks` instead. That also closes a second, quieter hole: the old suffix match accepted ANY path ending in `/.githooks`, including a different checkout's, whose pre-push hook does not guard this push at all. The guard is strictly tighter than before — it now fires in a case where it previously stayed silent. The `-x "$repo_root/.githooks/pre-push"` requirement is unchanged, so wired-but- missing and wired-but-not-executable still fall through to the Prettier check. Adds tests/push-format-guard.test.ts — this hook previously had no coverage at all. Nine cases pin both directions: the three legitimate spellings of a wired `core.hooksPath` stay silent, and unset / missing pre-push / non-executable pre-push / foreign .githooks all still deny. The fixture rigs `npx` to report unformatted files so "the guard ran" is observable as a deny decision rather than as silence, which the hook also produces when it self-disables — a test asserting only empty stdout would pass with the guard deleted. Like the sibling hook contract, the suite is skipIf(win32): Windows' `bash.exe` is a WSL launcher, and `core.fileMode=false` on the ReFS Dev Drive makes the not-executable case unrepresentable there. It is pinned on Linux CI. Co-Authored-By: Claude Opus 5 --- .claude/hooks/push-format-guard.sh | 23 +++- tests/push-format-guard.test.ts | 178 +++++++++++++++++++++++++++++ 2 files changed, 196 insertions(+), 5 deletions(-) create mode 100644 tests/push-format-guard.test.ts diff --git a/.claude/hooks/push-format-guard.sh b/.claude/hooks/push-format-guard.sh index 8a395f8e6..ff81b71b7 100755 --- a/.claude/hooks/push-format-guard.sh +++ b/.claude/hooks/push-format-guard.sh @@ -92,13 +92,26 @@ if [ -n "$hooks_path" ]; then # Normalise Windows backslashes so a native `git config` value compares equal # to the POSIX path this script sees. normalised="$(printf '%s' "$hooks_path" | tr '\\' '/')" + repo_root_n="$(printf '%s' "$repo_root" | tr '\\' '/')" + repo_root_n="${repo_root_n%/}" + # `core.hooksPath` is absolute OR relative to the top of the working tree, and + # git treats `.githooks`, `./.githooks` and the absolute spelling as the same + # directory. Resolve to one form before comparing. A `*/.githooks` suffix glob + # cannot do that: it needs a `/` before the name, so it silently missed the + # bare relative value this repo's own `npm install` writes — leaving the guard + # running a full-repository Prettier check on every push in a checkout that was + # in fact correctly wired (measured at >100 s per push on the Windows + # workstation, 2026-08-22). case "$normalised" in - */.githooks) - if [ -x "$repo_root/.githooks/pre-push" ]; then - exit 0 - fi - ;; + /* | ?:/*) resolved="$normalised" ;; + *) resolved="$repo_root_n/${normalised#./}" ;; esac + # Exact equality, not a suffix match: another repository's `.githooks` also + # ends in `/.githooks`, and its pre-push hook would not guard THIS push. + if [ "${resolved%/}" = "$repo_root_n/.githooks" ] \ + && [ -x "$repo_root/.githooks/pre-push" ]; then + exit 0 + fi fi # --- run the repository-wide check, never a per-file one --------------------- diff --git a/tests/push-format-guard.test.ts b/tests/push-format-guard.test.ts new file mode 100644 index 000000000..a5112c032 --- /dev/null +++ b/tests/push-format-guard.test.ts @@ -0,0 +1,178 @@ +import { execFileSync, spawnSync } from "node:child_process"; +import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; + +const hook = join(process.cwd(), ".claude/hooks/push-format-guard.sh"); +const scratchRoots: string[] = []; + +afterEach(() => { + for (const root of scratchRoots.splice(0)) { + rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } +}); + +/** + * A repo whose Prettier check is rigged to FAIL, so "the guard ran" is + * observable as a deny decision rather than as silence. Silence is ambiguous + * on its own: the hook also exits 0 and prints nothing when it self-disables, + * when npx is missing, and when node_modules/prettier is absent — so a test + * asserting only `stdout === ""` would pass even if the guard had been deleted. + */ +function riggedRepo(options?: { prePush?: "executable" | "not-executable" | "absent" }): { + root: string; + binDir: string; +} { + const root = mkdtempSync(join(tmpdir(), "push-format-guard-")); + scratchRoots.push(root); + execFileSync("git", ["init", "-q"], { cwd: root }); + + // The hook refuses to run the check unless a real prettier install looks + // present; the directory alone satisfies that gate. + mkdirSync(join(root, "node_modules/prettier"), { recursive: true }); + + const prePush = options?.prePush ?? "executable"; + if (prePush !== "absent") { + mkdirSync(join(root, ".githooks"), { recursive: true }); + const path = join(root, ".githooks/pre-push"); + writeFileSync(path, "#!/usr/bin/env bash\nexit 0\n"); + chmodSync(path, prePush === "executable" ? 0o755 : 0o644); + } else { + mkdirSync(join(root, ".githooks"), { recursive: true }); + } + + // Shim npx so the repository-wide Prettier check reports unformatted files + // without needing a real Prettier in the fixture. + const binDir = mkdtempSync(join(tmpdir(), "push-format-guard-bin-")); + scratchRoots.push(binDir); + const npx = join(binDir, "npx"); + writeFileSync(npx, '#!/usr/bin/env bash\necho "[warn] bad.js"\nexit 1\n'); + chmodSync(npx, 0o755); + + return { root, binDir }; +} + +function setHooksPath(root: string, value: string | null): void { + if (value === null) { + spawnSync("git", ["config", "--unset", "core.hooksPath"], { cwd: root }); + return; + } + execFileSync("git", ["config", "core.hooksPath", value], { cwd: root }); +} + +function runHook( + root: string, + binDir: string, + command = "git push origin HEAD", +): { status: number | null; stdout: string; denied: boolean } { + const result = spawnSync("bash", [hook], { + cwd: root, + input: JSON.stringify({ + session_id: "sess", + tool_name: "Bash", + tool_input: { command }, + cwd: root, + }), + encoding: "utf8", + env: { + ...process.env, + CLAUDE_PROJECT_DIR: root, + PATH: `${binDir}:${process.env.PATH ?? ""}`, + }, + }); + const stdout = result.stdout ?? ""; + return { + status: result.status, + stdout, + denied: stdout.includes('"permissionDecision":"deny"'), + }; +} + +// The hook is a Bash contract exercised on Linux CI. Windows' `bash.exe` is a +// WSL launcher: it cannot execute the native absolute paths this fixture gives +// it, and `core.fileMode=false` on the ReFS Dev Drive makes the +// not-executable case unrepresentable there. That is neither the hook's +// runtime nor meaningful Windows coverage, so avoid false local reds. +describe.skipIf(process.platform === "win32")("push-format-guard", () => { + describe("self-disables when this repo's pre-push hook is genuinely wired", () => { + // `core.hooksPath` is absolute OR relative to the top of the working tree. + // `npm install` in this repo writes the bare relative form, which a + // `*/.githooks` suffix glob cannot match — so the guard ran a full + // repository Prettier check on every push in a correctly wired checkout + // (>100 s per push, measured 2026-08-22). All three spellings name the + // same directory and must behave identically. + for (const spelling of [".githooks", "./.githooks"]) { + it(`stays silent for the relative spelling ${JSON.stringify(spelling)}`, () => { + const { root, binDir } = riggedRepo(); + setHooksPath(root, spelling); + const out = runHook(root, binDir); + expect(out.denied).toBe(false); + expect(out.stdout).toBe(""); + expect(out.status).toBe(0); + }); + } + + it("stays silent for the absolute spelling", () => { + const { root, binDir } = riggedRepo(); + setHooksPath(root, join(root, ".githooks")); + const out = runHook(root, binDir); + expect(out.denied).toBe(false); + expect(out.stdout).toBe(""); + }); + }); + + describe("still fires in the gap case it exists for", () => { + it("denies an unformatted push when core.hooksPath is unset", () => { + const { root, binDir } = riggedRepo(); + setHooksPath(root, null); + const out = runHook(root, binDir); + expect(out.denied).toBe(true); + expect(out.status).toBe(0); + }); + + it("denies when core.hooksPath is wired but pre-push is missing", () => { + const { root, binDir } = riggedRepo({ prePush: "absent" }); + setHooksPath(root, ".githooks"); + expect(runHook(root, binDir).denied).toBe(true); + }); + + it("denies when core.hooksPath is wired but pre-push is not executable", () => { + const { root, binDir } = riggedRepo({ prePush: "not-executable" }); + setHooksPath(root, ".githooks"); + expect(runHook(root, binDir).denied).toBe(true); + }); + + it("denies when core.hooksPath points at a DIFFERENT repository's .githooks", () => { + // A suffix match on `*/.githooks` accepted any path ending in that name, + // including another checkout's — whose pre-push hook does not guard this + // push at all. The comparison is exact for that reason. + const { root, binDir } = riggedRepo(); + const foreign = mkdtempSync(join(tmpdir(), "push-format-guard-foreign-")); + scratchRoots.push(foreign); + mkdirSync(join(foreign, ".githooks"), { recursive: true }); + const path = join(foreign, ".githooks/pre-push"); + writeFileSync(path, "#!/usr/bin/env bash\nexit 0\n"); + chmodSync(path, 0o755); + setHooksPath(root, join(foreign, ".githooks")); + expect(runHook(root, binDir).denied).toBe(true); + }); + }); + + describe("scope", () => { + it("ignores a command that is not a push, even with the guard armed", () => { + const { root, binDir } = riggedRepo(); + setHooksPath(root, null); + const out = runHook(root, binDir, "echo hello"); + expect(out.denied).toBe(false); + expect(out.stdout).toBe(""); + }); + + it("honours the documented CLAUDE_ALLOW_UNFORMATTED_PUSH=1 prefix", () => { + const { root, binDir } = riggedRepo(); + setHooksPath(root, null); + const out = runHook(root, binDir, "CLAUDE_ALLOW_UNFORMATTED_PUSH=1 git push origin HEAD"); + expect(out.denied).toBe(false); + }); + }); +}); From 687b166d01388ea192ac07a0d937892e941e7f21 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 19:41:56 +0000 Subject: [PATCH 2/2] fix(hooks): compare Windows drive-letter hook paths case-insensitively `core.hooksPath` on the Windows workstation can carry different casing from `CLAUDE_PROJECT_DIR` (`d:/Database/.githooks` vs `D:/Database`) while naming the same wired directory. Bash `=` is case-sensitive, so the exact-equality comparison introduced by this branch failed to self-disable there and ran the full-repository Prettier check on every push - the regression this PR set out to remove. Fold case only for the unambiguous `X:/...` drive-letter spelling. The MSYS `/c/...` form is byte-identical to a real POSIX path, where case IS significant, so folding it could silently self-disable the guard against a foreign hooks directory. Adds Linux-runnable coverage: the fixture creates a literal `D:` directory inside the scratch root so the drive-letter string the hook compares is exact while every `$repo_root/...` lookup still resolves to real files. Three case-variant spellings must stay silent, a case-variant path naming a different directory must still deny, and POSIX paths must stay case-sensitive. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ApB8NBygQn9cosxCQ8omk9 --- .claude/hooks/push-format-guard.sh | 20 ++++++++- tests/push-format-guard.test.ts | 72 +++++++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/.claude/hooks/push-format-guard.sh b/.claude/hooks/push-format-guard.sh index ff81b71b7..4d69756c2 100755 --- a/.claude/hooks/push-format-guard.sh +++ b/.claude/hooks/push-format-guard.sh @@ -106,9 +106,27 @@ if [ -n "$hooks_path" ]; then /* | ?:/*) resolved="$normalised" ;; *) resolved="$repo_root_n/${normalised#./}" ;; esac + resolved="${resolved%/}" + expected="$repo_root_n/.githooks" + # Windows drive-letter paths are case-insensitive, and Bash `=` is not. Git + # wires `d:/Database/.githooks` and `D:/Database/.githooks` to the same + # directory, so comparing the raw bytes puts the primary (Windows ReFS Dev + # Drive) workstation straight back into the >100 s full-repository Prettier + # run this whole check exists to avoid. Fold case only for the unambiguous + # `X:/...` spelling: the MSYS `/c/...` form is byte-identical to a real POSIX + # path, where case IS significant, and folding that could silently + # self-disable the guard against a FOREIGN hooks directory — the unsafe + # direction. Testing `resolved` alone is enough: a relative `core.hooksPath` + # has already been joined onto `repo_root_n`, so it inherits that form. + case "$resolved" in + ?:/*) + resolved="$(printf '%s' "$resolved" | tr '[:upper:]' '[:lower:]')" + expected="$(printf '%s' "$expected" | tr '[:upper:]' '[:lower:]')" + ;; + esac # Exact equality, not a suffix match: another repository's `.githooks` also # ends in `/.githooks`, and its pre-push hook would not guard THIS push. - if [ "${resolved%/}" = "$repo_root_n/.githooks" ] \ + if [ "$resolved" = "$expected" ] \ && [ -x "$repo_root/.githooks/pre-push" ]; then exit 0 fi diff --git a/tests/push-format-guard.test.ts b/tests/push-format-guard.test.ts index a5112c032..8ee589831 100644 --- a/tests/push-format-guard.test.ts +++ b/tests/push-format-guard.test.ts @@ -61,10 +61,49 @@ function setHooksPath(root: string, value: string | null): void { execFileSync("git", ["config", "core.hooksPath", value], { cwd: root }); } +/** + * A rigged repo reachable through a Windows-style drive-letter path, built so + * the case-sensitivity contract is testable on Linux — where the real suite + * runs, since Windows is skipped below. + * + * The trick is that `D:/Database` is not absolute to a POSIX shell: it is a + * relative path. Creating a literal `D:` directory inside the scratch root and + * running the hook with that root as its cwd makes every `$repo_root/...` + * lookup in the hook resolve to real files, while the string the hook compares + * is byte-for-byte the drive-letter spelling Git reports on Windows. + */ +function riggedWindowsStyleRepo(hooksPath: string): { + base: string; + binDir: string; + projectDir: string; +} { + const base = mkdtempSync(join(tmpdir(), "push-format-guard-win-")); + scratchRoots.push(base); + const projectDir = "D:/Database"; + const repo = join(base, "D:", "Database"); + mkdirSync(repo, { recursive: true }); + execFileSync("git", ["init", "-q"], { cwd: repo }); + mkdirSync(join(repo, "node_modules/prettier"), { recursive: true }); + mkdirSync(join(repo, ".githooks"), { recursive: true }); + const prePush = join(repo, ".githooks/pre-push"); + writeFileSync(prePush, "#!/usr/bin/env bash\nexit 0\n"); + chmodSync(prePush, 0o755); + execFileSync("git", ["config", "core.hooksPath", hooksPath], { cwd: repo }); + + const binDir = mkdtempSync(join(tmpdir(), "push-format-guard-bin-")); + scratchRoots.push(binDir); + const npx = join(binDir, "npx"); + writeFileSync(npx, '#!/usr/bin/env bash\necho "[warn] bad.js"\nexit 1\n'); + chmodSync(npx, 0o755); + + return { base, binDir, projectDir }; +} + function runHook( root: string, binDir: string, command = "git push origin HEAD", + projectDir: string = root, ): { status: number | null; stdout: string; denied: boolean } { const result = spawnSync("bash", [hook], { cwd: root, @@ -77,7 +116,7 @@ function runHook( encoding: "utf8", env: { ...process.env, - CLAUDE_PROJECT_DIR: root, + CLAUDE_PROJECT_DIR: projectDir, PATH: `${binDir}:${process.env.PATH ?? ""}`, }, }); @@ -120,6 +159,21 @@ describe.skipIf(process.platform === "win32")("push-format-guard", () => { expect(out.denied).toBe(false); expect(out.stdout).toBe(""); }); + + // Windows drive-letter paths are case-insensitive; Bash `=` is not. Git + // reports whatever casing `npm install` happened to write, so a checkout + // Claude Code knows as `D:/Database` can carry `core.hooksPath` of + // `d:/database/.githooks` — the same wired directory. Comparing raw bytes + // reintroduced the >100 s full-repository Prettier run on every push. + for (const spelling of ["d:/Database/.githooks", "D:/database/.githooks", "d:/database/.GITHOOKS"]) { + it(`stays silent for the case-variant absolute spelling ${JSON.stringify(spelling)}`, () => { + const { base, binDir, projectDir } = riggedWindowsStyleRepo(spelling); + const out = runHook(base, binDir, "git push origin HEAD", projectDir); + expect(out.denied).toBe(false); + expect(out.stdout).toBe(""); + expect(out.status).toBe(0); + }); + } }); describe("still fires in the gap case it exists for", () => { @@ -157,6 +211,22 @@ describe.skipIf(process.platform === "win32")("push-format-guard", () => { setHooksPath(root, join(foreign, ".githooks")); expect(runHook(root, binDir).denied).toBe(true); }); + + it("denies a case-variant Windows path naming a DIFFERENT directory", () => { + // Case folding must not decay into a loose match: only the casing may + // differ, never the directory itself. + const { base, binDir, projectDir } = riggedWindowsStyleRepo("d:/other-repo/.githooks"); + expect(runHook(base, binDir, "git push origin HEAD", projectDir).denied).toBe(true); + }); + + it("keeps POSIX paths case-sensitive", () => { + // `/c/...`-style and ordinary POSIX paths are NOT folded: on Linux a + // casing difference is a genuinely different directory, and folding it + // would silently self-disable the guard against a foreign hooks dir. + const { root, binDir } = riggedRepo(); + setHooksPath(root, `${join(root, ".githooks").toUpperCase()}`); + expect(runHook(root, binDir).denied).toBe(true); + }); }); describe("scope", () => {