From 3da2cba80bf92754fc9816d60a2c38baac9d0ec3 Mon Sep 17 00:00:00 2001 From: Altay Date: Sat, 5 Sep 2026 14:31:10 +0300 Subject: [PATCH 1/2] fix: reject symlinked hook directories before mutation --- src/hooks.ts | 32 ++++++++++++++++++++----------- test/hooks.test.ts | 47 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/src/hooks.ts b/src/hooks.ts index 20f9430..c1afd21 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -1,22 +1,32 @@ 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); - if (!stat.isFile() || stat.isSymbolicLink()) { + const stat = workspaceLstat(repoRoot, ".githooks/pre-commit"); + 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`; } diff --git a/test/hooks.test.ts b/test/hooks.test.ts index aedf7c1..16d8f64 100644 --- a/test/hooks.test.ts +++ b/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,42 @@ import { gitEnvironmentForRepository } from "../src/lib/gitProcess.ts"; const root = join(dirname(fileURLToPath(import.meta.url)), ".."); const cli = join(root, "src", "cli.ts"); +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, From 46705b68e01338fc1511fdad40b677c1eeffec80 Mon Sep 17 00:00:00 2001 From: Altay Date: Sat, 5 Sep 2026 14:38:34 +0300 Subject: [PATCH 2/2] fix: distinguish missing hooks from invalid hook types --- src/hooks.ts | 3 ++- test/hooks.test.ts | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/hooks.ts b/src/hooks.ts index c1afd21..60a74a7 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -7,7 +7,8 @@ import { workspaceLstat } from "./lib/workspaceFs.ts"; export function installHooks(repoRoot: string): string { const hook = join(repoRoot, ".githooks", "pre-commit"); const stat = workspaceLstat(repoRoot, ".githooks/pre-commit"); - if (!stat?.isFile() || stat.isSymbolicLink()) { + 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 descriptor = openSync(hook, constants.O_RDONLY | constants.O_NOFOLLOW); diff --git a/test/hooks.test.ts b/test/hooks.test.ts index 16d8f64..eab7171 100644 --- a/test/hooks.test.ts +++ b/test/hooks.test.ts @@ -19,6 +19,27 @@ 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"}`, () => {