diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dce3eb38ae7..6e236444e20 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -74,7 +74,11 @@ bun run setup:hooks This installs the `post-merge` hook, which rebuilds the packaged dashboard when a merge changes its source. It also removes the unmodified, retired repository -pre-push hook from Git's resolved hooks directory, including linked worktrees -and `core.hooksPath` setups. Custom pre-push hooks are preserved. Validation no -longer runs automatically on every push; existing contributors should rerun the -setup command once to migrate their hooks. +pre-push hook from Git's resolved hooks directory, including linked worktrees. +Custom pre-push hooks are preserved. For safety, setup refuses to touch hooks +whenever `core.hooksPath` is configured, whether it points at a repository-local, +global, or shared directory. +If an earlier setup already installed these hooks into that shared directory, +remove them there by hand; the refusal leaves them untouched. +Validation no longer runs automatically on every push; existing contributors +should rerun the setup command once to migrate their hooks. diff --git a/scripts/setup-hooks.ts b/scripts/setup-hooks.ts index 57bc8c0262a..68cd46714fb 100644 --- a/scripts/setup-hooks.ts +++ b/scripts/setup-hooks.ts @@ -16,8 +16,28 @@ import { join, resolve } from "node:path"; const repoRoot = resolve(import.meta.dirname, ".."); -// Resolve the real hooks dir via git so linked worktrees (`.git` file), core.hooksPath, -// and non-default git dirs all work. Hard-coding /.git/hooks breaks those setups. +// A configured hooksPath may be shared by unrelated repositories. Installing our +// cwd-dependent hooks there would replace shared policy hooks and run another +// repository's package scripts. Keep installation scoped to this repository. +try { + execFileSync("git", ["config", "--get", "core.hooksPath"], { + cwd: repoRoot, + encoding: "utf8", + // A wedged git config read must not stall the setup run: bound the call and treat a + // timeout like any other inspection failure below. + timeout: 10_000, + }); + console.error("setup-hooks: refusing to install because core.hooksPath is configured."); + process.exit(1); +} catch (error) { + if (typeof error === "object" && error !== null && "status" in error && error.status !== 1) { + console.error("setup-hooks: could not inspect core.hooksPath."); + process.exit(1); + } +} + +// Resolve Git's repository-local hooks dir so linked worktrees and non-default +// git dirs work without assuming that /.git is a directory. let hooksDir: string; try { hooksDir = execFileSync( diff --git a/tests/ci-workflows/setup-hooks.test.ts b/tests/ci-workflows/setup-hooks.test.ts index 838fc087f8e..f585462f1a4 100644 --- a/tests/ci-workflows/setup-hooks.test.ts +++ b/tests/ci-workflows/setup-hooks.test.ts @@ -114,7 +114,7 @@ describe("local hook setup", () => { expect(readFileSync(join(hooks(root), "pre-push"), "utf8")).toBe(custom); }); - test("uses a configured hooks directory without touching the default one", () => { + test("refuses a configured hooksPath that may be shared by other repositories", () => { const root = fixture(); const original = hooks(root); writeFileSync(join(original, "pre-push"), legacyHook); @@ -122,9 +122,11 @@ describe("local hook setup", () => { mkdirSync(customDir); writeFileSync(join(customDir, "pre-push"), legacyHook); git(root, "config", "core.hooksPath", customDir); - setup(root); - expect(existsSync(join(customDir, "pre-push"))).toBe(false); - expect(existsSync(join(customDir, "post-merge"))).toBe(true); + expect(() => setup(root)).toThrow(/core.hooksPath is configured/); + // Nothing is written anywhere: the shared directory keeps its own hook and the + // repository-local one is untouched too. + expect(readFileSync(join(customDir, "pre-push"), "utf8")).toBe(legacyHook); + expect(existsSync(join(customDir, "post-merge"))).toBe(false); expect(readFileSync(join(original, "pre-push"), "utf8")).toBe(legacyHook); });