Skip to content

Generated hook shim interpolates paths unquoted: a space in the path silently disables all hooks #1488

Description

@emilepatry

Description

internal/templates/hook.tmpl interpolates three path variables unquoted. When any of them contains a space, the generated hook word-splits it, every branch of call_lefthook() fails, and the hook silently no-ops — printing Can't find lefthook in PATH while the commit still succeeds with exit 0.

The failing branch is not stale or missing. The binary exists, is executable, and runs fine when the same path is quoted. It is purely a quoting bug.

The three unquoted interpolations, all in internal/templates/hook.tmpl (still present at v2.1.10):

[ -f {{.Rc}} ] && . {{.Rc}}                          {{/* .Rc — unquoted twice */}}

elif test -n "{{ .LefthookPath }}"                   {{/* test IS quoted... */}}
then
  {{ .LefthookPath }} "$@"                           {{/* ...but the call is not */}}

elif {{ .LefthookPathCurrent }} -h >/dev/null 2>&1   {{/* both unquoted */}}
then
  {{ .LefthookPathCurrent }} "$@"

Note the inconsistency inside this same file: every $dir/node_modules/... candidate below is correctly quoted ("$dir/node_modules/lefthook/bin/index.js"), so the fix matches the surrounding style.

This appears to be a regression of #403 ("Space in path cause lefthook to fail", closed COMPLETED in 2022), which reported the identical symptom at v1.2.6.

Why it is worse than it looks

Because the terminal branch echo "Can't find lefthook in PATH" exits 0 (absent assert_lefthook_installed), the result is not a visible failure — it is silent, permanent skipping of every hook. Formatting and guards never run locally and the problem only surfaces in CI, if at all.

On macOS this is easy to hit without doing anything unusual: ~/Library/Mobile Documents/... (iCloud Drive), ~/Google Drive/My Drive/..., and any project folder with a space in its name.

lefthook.yml

pre-commit:
  commands:
    hello:
      run: echo HOOK_ACTUALLY_RAN

Commands to reproduce

export LEFTHOOK_VERBOSE=true

# a path containing a space, and a lefthook binary living inside it
mkdir -p "/tmp/my project/bin"
cp "$(which lefthook)" "/tmp/my project/bin/lefthook"
cd "/tmp/my project"
git init -q . && git config user.email t@t.t && git config user.name t
printf 'pre-commit:\n  commands:\n    hello:\n      run: echo HOOK_ACTUALLY_RAN\n' > lefthook.yml

# install USING that binary, so its path is baked into the hook
"/tmp/my project/bin/lefthook" install

# the generated shim contains the unquoted path:
grep -n "my project" .git/hooks/pre-commit

echo x > f.txt && git add f.txt && git commit -m test

Actual:

Can't find lefthook in PATH
[main (root-commit) eb1b24a] test

The hello command never runs. The commit succeeds.

The generated lines (verbatim):

  elif /tmp/my project/bin/lefthook -h >/dev/null 2>&1
  then
    /tmp/my project/bin/lefthook "$@"

sh splits that into /tmp/my + project/bin/lefthook, so the elif can never be true.

Control — identical setup, no space in the path:

┃  hello ❯
HOOK_ACTUALLY_RAN
✔️ hello (0.02 seconds)

The space is the only variable.

Lefthook version

Reproduced empirically on 2.1.9 75f99ff325a731c296b572292dd1c855cd970b93 (macOS 26.5.2, arm64, git 2.50.1, /bin/sh).

I did not rebuild v2.1.10 to re-run the repro, but I read internal/templates/hook.tmpl at tag v2.1.10 and all three interpolations above are still unquoted there, so I expect it to reproduce unchanged on latest.

Possible solution

Quote the three interpolations:

[ -f "{{.Rc}}" ] && . "{{.Rc}}"

elif test -n "{{ .LefthookPath }}"
then
  "{{ .LefthookPath }}" "$@"

elif "{{ .LefthookPathCurrent }}" -h >/dev/null 2>&1
then
  "{{ .LefthookPathCurrent }}" "$@"

That is sufficient for spaces. A double quote in a path would still break it, so escaping the value at render time would be more robust if you want to close the general case.

Relationship to #1398

#1398 reports the same LefthookPathCurrent branch failing, but attributes it to the baked absolute path going stale (pointing into a deleted git worktree) and proposes deleting the branch outright.

This report is a distinct and independent defect in the same lines: the branch fails even when the baked path is perfectly valid and the target exists, purely from word-splitting. That means

  • it reproduces in a plain single-checkout repo with no worktrees involved (as above), and
  • for any repo under a path containing a space, that branch has never worked at all and the shim has always been silently relying on the $dir/... fallbacks.

The two are complementary: quoting fixes the general case, and #1398's proposal would independently remove one of the three affected interpolations. Happy to reference whichever direction you prefer.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions