npm postinstall runs `lefthook install -f`, bypassing the core.hooksPath guard added in #1292

[shbernal]

Summary

#1292 made lefthook install refuse to write into a configured core.hooksPath, and the warning it prints is good. But postinstall.js runs:

spawnSync(getExePath(), ["install", "-f"], {
  cwd: process.env.INIT_CWD || process.cwd(),
  stdio: "inherit",
});

-f is precisely the flag that bypasses that refusal. So for anyone installing through npm, the guard added in #1292 never fires: a plain npm install overwrites the hooks in the configured hooks directory, without the user ever typing lefthook install.

Tested with lefthook 2.1.10 (current latest) on Windows 11.

Why this is different from a user typing --force

The refusal exists because writing into a shared core.hooksPath destroys hooks belonging to other tools and other repositories. --force is a deliberate, interactive override of that. A postinstall is neither deliberate nor interactive, and it can fire from a directory the user never chose:

• npm runs prepare for git dependencies, which means it installs that dependency's devDependencies into a temp clone under <npm-cache>/_cacache/tmp/git-cloneXXXXXX.
• If lefthook is one of those devDependencies, its postinstall runs with INIT_CWD inside that temp clone.
• The temp clone is a git repo, so the user's global core.hooksPath applies to it.
• lefthook then force-installs into the user's global hooks directory.

The result on my machine: installing an unrelated package, in an unrelated project, rewrote three hooks shared by every repository on the box. It replaced a hand-written commit-msg guard and a delegating shim that other repositories' hook managers rely on, leaving them silently inert. That is the same loss #1248 described and that #1292 was meant to prevent, reached by a path the guard does not cover.

I can tell the install ran from inside the temp clone because the generated wrapper still names it (see the last section).

Reproduction

Self-contained, and it does not touch your real git config:

#!/bin/sh
set -u
SB=$(mktemp -d)
LEFTHOOK=${LEFTHOOK_BIN:-lefthook}

mkdir -p "$SB/globalhooks" "$SB/repo"
printf '#!/bin/sh\necho MARKER-ORIGINAL\n' > "$SB/globalhooks/pre-commit"
chmod +x "$SB/globalhooks/pre-commit"

printf '[core]\n\thooksPath = %s\n[user]\n\tname = t\n\temail = t@t\n' \
  "$SB/globalhooks" > "$SB/gitconfig"
: > "$SB/gitconfig-system"
export GIT_CONFIG_GLOBAL="$SB/gitconfig"
export GIT_CONFIG_SYSTEM="$SB/gitconfig-system"

cd "$SB/repo"
git init -q .
printf 'pre-commit:\n  jobs:\n    - name: demo\n      run: echo demo\n' > lefthook.yml

echo "--- install (no -f) ---"
"$LEFTHOOK" install; echo "exit=$?"

echo "--- install -f  (what postinstall runs) ---"
"$LEFTHOOK" install -f; echo "exit=$?"
ls "$SB/globalhooks"

Observed on 2.1.10:

• no -f: exit 1, the feat: unset core.hookspath in lefthook install #1292 warning prints, nothing is written. Correct.
• -f: exit 0, Renamed .../pre-commit to .../pre-commit.old, and the marker hook is replaced by the lefthook wrapper.

To be fair to the backup logic: I checked whether a second install -f overwrites pre-commit.old with lefthook's own wrapper, and it does not. The original survives. So this is not cumulative data loss, just the one silent replacement.

LEFTHOOK=0 does not prevent it

const isEnabled = (value) => value && value !== "0" && value !== "false";
if (isEnabled(process.env.CI) && !isEnabled(process.env.LEFTHOOK)) {
  return
}

The early return requires CI to be truthy. On a developer machine CI is unset, so the first operand is false and the install proceeds regardless of LEFTHOOK. I verified that LEFTHOOK=0 lefthook install -f still clobbers the hook. So there is no environment-variable escape hatch for a developer who wants the package but not the forced install; the only lever is the package manager's own script blocking.

Suggested fix

Drop -f from the postinstall. If core.hooksPath is set, let the guard fire, print its message, and let the postinstall exit 0 without installing. Anyone who genuinely wants hooks in a custom path can run lefthook install --force once, deliberately, which is what the flag is for.

A narrower variant, if dropping it outright is too disruptive: keep -f only when the resolved hooks path is inside the repository being installed, and fall back to the guard when it is not. That preserves the "overwrite my own stale .git/hooks files" case, which I assume is why -f is there, while never reaching outside the project.

This also seems aligned with #1439: if the postinstall is going to be blocked by default under npm's RFC #868 anyway, it should not be the thing that silently overrides a safety check in the meantime.

Secondary: the wrapper hardcodes the install-time binary path

The generated wrapper embeds the absolute path of the binary as it was at install time. On my machine the global pre-commit still contains:

elif C:/Users/<user>/AppData/Local/npm-cache/_cacache/tmp/git-cloneVv6mhJ/node_modules/lefthook-windows-x64/bin/lefthook.exe -h >/dev/null 2>&1
then
  C:/Users/<user>/AppData/Local/npm-cache/_cacache/tmp/git-cloneVv6mhJ/node_modules/lefthook-windows-x64/bin/lefthook.exe "$@"

That is npm's temp clone directory, deleted right after the install. The branch is dead weight in a hook that every repository on the machine executes, and it points at a path under a cache directory that later installs write to. Resolving the binary relative to the repo at run time, which the following branches already do, seems strictly better than emitting an absolute path that is known to be temporary.

[hamodywe]

Confirmed on current main, and the flag itself is where the two concerns got welded together — cmd/install.go documents --force as "overwrite .old files and proceed even if core.hooksPath is set". The postinstall wants the first half and silently gets the second.

Reproduced with a binary built from main: repo with core.hooksPath pointing at a directory holding a hand-written pre-commit.

$ lefthook install          # what a plain install does
core.hooksPath is set locally to '.../hooks-global'
Custom hooks paths are not supported by default.
exit=1                      # hook untouched

$ lefthook install -f       # what postinstall runs
exit=0
$ head -1 .../hooks-global/pre-commit
#!/bin/sh                   # now the lefthook wrapper; original moved to pre-commit.old

On your main suggestion — dropping -f outright looks free, which I didn't expect until I read cleanHook. force is only consulted there when a foreign hook and its .old backup both already exist. An existing lefthook hook is matched by isLefthookFile and removed with no force needed, and that is the case a postinstall actually hits on upgrade. Checked against a real repo: second plain lefthook install reports sync hooks: ✔️(pre-commit) and creates no .old. internal/command/install_test.go already pins it as "with existing lefthook hooks" with force unset.

So the narrower variant you offered as a fallback isn't needed — the only case that changes is foreign-hook-plus-existing-.old, which now stops with the existing error instead of silently overwriting the backup, and --force remains there for whoever means it.

PR coming with that plus a docs note. I've left the second point (the wrapper embedding the install-time absolute path) alone — it's a separate concern and deserves its own issue rather than riding along.