fix: use split() instead of resolve(None) for builtin hook argument parsing#1415
Merged
j178 merged 2 commits intoj178:masterfrom Jan 19, 2026
Merged
Conversation
4222aed to
1e6806a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1415 +/- ##
=======================================
Coverage 90.66% 90.66%
=======================================
Files 82 82
Lines 16698 16698
=======================================
Hits 15140 15140
Misses 1558 1558 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📦 Cargo Bloat ComparisonBinary size change: +0.00% (22.5 MiB → 22.5 MiB) Expand for cargo-bloat outputHead Branch ResultsBase Branch Results |
1e6806a to
a7622ed
Compare
…arsing Builtin hooks were calling `hook.entry.resolve(None)` to get arguments for clap parsing. After commit 85448ac, `resolve(None)` falls back to system PATH, which can find system-wide binaries (e.g., from `pip install pre-commit-hooks`), parse their shebangs, and corrupt argument parsing. The fix changes these 6 builtin hooks to use `split()` instead, which just tokenizes the entry string without PATH resolution - exactly what's needed for argument parsing. Fixes j178#1412
a6a56f3 to
e2484e1
Compare
192ed89 to
0e1a1ce
Compare
Adds a test that verifies builtin hooks work correctly even when system-wide binaries with matching names exist on PATH (e.g., from `pip install pre-commit-hooks`). The test creates a fake `trailing-whitespace-fixer` binary with a shebang in a temp directory, adds it to PATH, and verifies the builtin trailing-whitespace hook still works correctly.
0e1a1ce to
d5b7373
Compare
Owner
|
Thanks! |
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
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.
Builtin hooks were using
resolve(None)to parse entry arguments, which searches PATH and parses shebangs. Whenpre-commit-hooksis pip-installed, binaries liketrailing-whitespace-fixerexist on PATH with shebangs—this corrupted argument parsing.The fix uses
split()instead, which just parses the entry string into arguments without PATH resolution. Builtin hooks don't execute external binaries, so this is the correct approach.Test plan
Fixes #1412