fix: preserve quoting of run commands and forwarded script args - #1464
Open
Bineroflux wants to merge 2 commits into
Open
fix: preserve quoting of run commands and forwarded script args#1464Bineroflux wants to merge 2 commits into
Bineroflux wants to merge 2 commits into
Conversation
Args forwarded via `lefthook run <hook> -- <args...>` were joined into
the script command string without any escaping. An argument containing
spaces arrived at the script split into several arguments, and shell
special characters like parentheses made the wrapper shell fail before
the script even started:
sh: -c: line 1: syntax error near unexpected token `('
Quote each forwarded arg with shellescape, the same way the script path
on the same command line and the files templates in run commands are
already quoted. Args without special characters are left untouched, so
plain flag forwarding keeps working as before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Windows executor built the process command line by hand:
"sh" -c "cmdstr"
without escaping double quotes or backslashes inside cmdstr. Any command
containing a double quote was mangled by the msys argument parsing before
sh even saw it: `echo "hello world"` printed only `hello`, quoted variable
expansions were split into words, and multi-line run commands failed with
"syntax error: unexpected end of file" (evilmartians#1167).
Pass the command string as a regular argument instead and let os/exec
quote it, the same way the unix executor and the skip/only command
executor already do. sh now receives the command verbatim, so run
commands behave the same on Windows as on other platforms:
command before after
echo "hello world" hello hello world
printf '%s' "$VAR" splits on spaces verbatim
multi-line run with quotes syntax error works
args with quotes (') (") mangled verbatim
Fixes evilmartians#1167
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
|
@greptileai review, please |
This was referenced Aug 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1167
Context
Two related quoting bugs mangle command strings on their way to
sh:Forwarded args lose their quoting (all platforms).
lefthook run <hook> -- <args...>joins the args into the script command string without escaping: args with spaces arrive split, andlefthook run my-hook -- "file (1).txt"fails withsh: syntax error near unexpected token `(`.The Windows executor mangles any command containing a double quote (sh syntax error when running on windows #1167). It hand-builds the raw process command line as
"sh" -c "cmdstr"without escaping, so the msys argument parser corrupts the command beforeshsees it:echo "hello world"hellohello worldprintf '<%s>' "$VAR"run:with quotes (#1167)Since even
echo "hello world"mis-executes today, no config can rely on the old behavior — the fix restores parity with other platforms and matches how theskip/onlycheck executor already invokessh.Changes
build_script.go— quote each forwarded arg withshellescape.Quote, like the script path and files templates already are. Args without special characters stay untouched, so-- -Apply-style flag forwarding is unchanged.exec_windows.go— drop the hand-builtSysProcAttr.CmdLine; pass-c cmdstras regular arguments and letos/execquote it, same as the unix executor. Also resolves the "consider quoting" comment in that file.run_script_forwarded_args,run_sh_quotes) that fail onmasterand pass with this change.Verified on Windows 11 (Git for Windows) and Ubuntu:
go test ./..., the full integration suite, and end-to-end runs withshandpowershell -Filescripts receiving filenames with spaces, parentheses, and quotes.🤖 Generated with Claude Code
Confidence Score: 5/5
Both changes are minimal, targeted, and verified by new tests; the Windows path now matches the Unix executor exactly.
The forwarded-arg fix is a straightforward one-liner wrapped in a loop, confined to the already-quoted else-branch. The Windows executor change removes a hand-rolled quoting scheme that was provably broken and replaces it with the same exec.CommandContext call the Unix executor has used all along. Both directions of the fix are covered by a new unit test and two integration tests that reproduce the original bugs. No behaviour is changed on paths that already worked.
No files require special attention.
Reviews (1): Last reviewed commit: "fix: pass the command to sh verbatim on ..." | Re-trigger Greptile