fix: reject symlinked hook directories before mutation

src/hooks.ts

@@ -1,22 +1,33 @@
 import { spawnSync } from "node:child_process";
-import { chmodSync, lstatSync } from "node:fs";
+import { closeSync, constants, fchmodSync, fstatSync, openSync } from "node:fs";
 import { join } from "node:path";
 import { gitEnvironmentForRepository } from "./lib/gitProcess.ts";
+import { workspaceLstat } from "./lib/workspaceFs.ts";
 
 export function installHooks(repoRoot: string): string {
   const hook = join(repoRoot, ".githooks", "pre-commit");
-  const stat = lstatSync(hook);
+  const stat = workspaceLstat(repoRoot, ".githooks/pre-commit");
+  if (!stat) throw new Error(".githooks/pre-commit is missing");
   if (!stat.isFile() || stat.isSymbolicLink()) {
     throw new Error(".githooks/pre-commit must be a regular file");
   }
-  const result = spawnSync("git", ["config", "core.hooksPath", ".githooks"], {
-    cwd: repoRoot,
-    encoding: "utf8",
-    env: gitEnvironmentForRepository(),
-  });
-  if (result.status !== 0) {
-    throw new Error(result.stderr.trim() || "could not configure core.hooksPath");
+  const descriptor = openSync(hook, constants.O_RDONLY | constants.O_NOFOLLOW);
+  try {
+    const opened = fstatSync(descriptor);
+    if (!opened.isFile() || opened.dev !== stat.dev || opened.ino !== stat.ino) {
+      throw new Error(".githooks/pre-commit changed while installing hooks");
+    }
+    const result = spawnSync("git", ["config", "core.hooksPath", ".githooks"], {
+      cwd: repoRoot,
+      encoding: "utf8",
+      env: gitEnvironmentForRepository(),
+    });
+    if (result.status !== 0) {
+      throw new Error(result.stderr.trim() || "could not configure core.hooksPath");
+    }
+    fchmodSync(descriptor, opened.mode | 0o111);
+  } finally {
+    closeSync(descriptor);
   }
-  chmodSync(hook, stat.mode | 0o111);
   return `Git hooks use ${repoRoot}/.githooks`;
 }

test/hooks.test.ts

@@ -1,6 +1,15 @@
 import assert from "node:assert/strict";
 import { execFileSync, spawnSync } from "node:child_process";
-import { chmodSync, mkdirSync, mkdtempSync, realpathSync, writeFileSync } from "node:fs";
+import {
+  chmodSync,
+  mkdirSync,
+  mkdtempSync,
+  realpathSync,
+  rmSync,
+  statSync,
+  symlinkSync,
+  writeFileSync,
+} from "node:fs";
 import { tmpdir } from "node:os";
 import { dirname, join } from "node:path";
 import { fileURLToPath } from "node:url";
@@ -10,6 +19,63 @@ import { gitEnvironmentForRepository } from "../src/lib/gitProcess.ts";
 const root = join(dirname(fileURLToPath(import.meta.url)), "..");
 const cli = join(root, "src", "cli.ts");
 
+for (const missing of ["directory", "file"]) {
+  test(`hooks install reports a missing hook ${missing} without changing config`, () => {
+    const fixture = mkdtempSync(join(tmpdir(), "workspace-hooks-"));
+    try {
+      if (missing === "file") mkdirSync(join(fixture, ".githooks"));
+      git(fixture, "init", "-q");
+      git(fixture, "config", "core.hooksPath", "existing-hooks");
+      const configBefore = git(fixture, "config", "--local", "--list");
+      const install = spawnSync(process.execPath, [cli, "hooks", "install"], {
+        cwd: fixture,
+        encoding: "utf8",
+      });
+      assert.equal(install.status, 1, install.stderr);
+      assert.match(install.stderr, /\.githooks\/pre-commit is missing/);
+      assert.equal(git(fixture, "config", "--local", "--list"), configBefore);
+    } finally {
+      rmSync(fixture, { recursive: true, force: true });
+    }
+  });
+}
+
+for (const link of ["parent", "hook"]) {
+  for (const existingHooksPath of [undefined, "existing-hooks"]) {
+    test(`hooks install rejects a symlinked ${link} with hooksPath ${existingHooksPath ?? "unset"}`, () => {
+      const temporaryRoot = mkdtempSync(join(tmpdir(), "workspace-hooks-"));
+      try {
+        const fixture = join(temporaryRoot, "workspace");
+        const external = join(temporaryRoot, "external");
+        mkdirSync(fixture);
+        mkdirSync(external);
+        const externalHook = join(external, "pre-commit");
+        writeFileSync(externalHook, "#!/bin/sh\nexit 0\n");
+        chmodSync(externalHook, 0o600);
+        if (link === "parent") {
+          symlinkSync(external, join(fixture, ".githooks"));
+        } else {
+          mkdirSync(join(fixture, ".githooks"));
+          symlinkSync(externalHook, join(fixture, ".githooks", "pre-commit"));
+        }
+        git(fixture, "init", "-q");
+        if (existingHooksPath) git(fixture, "config", "core.hooksPath", existingHooksPath);
+        const configBefore = git(fixture, "config", "--local", "--list");
+        const install = spawnSync(process.execPath, [cli, "hooks", "install"], {
+          cwd: fixture,
+          encoding: "utf8",
+        });
+        assert.equal(statSync(externalHook).mode & 0o777, 0o600);
+        assert.equal(git(fixture, "config", "--local", "--list"), configBefore);
+        assert.equal(install.status, 1, install.stderr);
+        assert.match(install.stderr, link === "parent" ? /symbolic-link parent/ : /regular file/);
+      } finally {
+        rmSync(temporaryRoot, { recursive: true, force: true });
+      }
+    });
+  }
+}
+
 function git(cwd: string, ...args: string[]): string {
   return execFileSync("git", args, {
     cwd,