fix(check): expand env vars in --rev-range#2005
Conversation
The packaged commitizen-branch pre-push hook in .pre-commit-hooks.yaml passes the literal string \..\ as an argv element and relied on shell expansion. After #1941 switched git execution to shell=False (CWE-78 hardening), git received the literal string and aborted with atal: ambiguous argument, breaking every commitizen release after v4.15.0 for users of that hook. Expand env vars explicitly on the --rev-range argument via os.path.expandvars so the hook keeps working without reintroducing shell execution. Unset variables are left literal so git surfaces a clear error instead of being silently rewritten to an empty range. Closes #2003 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔍 Commitizen bump previewMerging this PR will produce the following bump: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2005 +/- ##
=======================================
Coverage 98.24% 98.24%
=======================================
Files 61 61
Lines 2785 2787 +2
=======================================
+ Hits 2736 2738 +2
Misses 49 49 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
This PR fixes the cz check --rev-range path used by the packaged commitizen-branch pre-push hook by expanding environment variables in the revision range before invoking git, preserving the pre-shell=False behavior without reintroducing shell execution.
Changes:
- Adds
os.path.expandvarshandling forCheck.rev_range. - Adds unit coverage for expanded and unset environment variables.
- Adds an end-to-end regression test using a real git repo for the pre-push hook scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
commitizen/commands/check.py |
Expands environment variables in --rev-range before git commit lookup. |
tests/commands/test_check_command.py |
Adds regression coverage for env-var expansion and the packaged pre-push hook flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The new tests will fail without the code change in check.py. Now it's ready for review. |
Description
Fix regression where the packaged
commitizen-branchpre-push hook stopped working after v4.15.0.The hook passes the literal
..as a single argv element. Before #1941 the surroundingshell=Trueexpanded the env vars; after #1941 (CWE-78 hardening) git receives the literal string and aborts withfatal: ambiguous argument.Fix: expand env vars on the
--rev-rangeargument viaos.path.expandvarsinCheck.__init__. Unset vars are left literal so git can surface a clear error rather than silently rewriting the range to empty.Closes #2003. Credit to @j178 for the root-cause analysis.
Checklist
Was generative AI tooling used to co-author this PR?
Generated-by: GitHub Copilot CLI following the guidelines
Code Changes
uv run poe alllocally to ensure this change passes linter check and testsExpected Behavior
cz check --rev-range "$VAR1..$VAR2"resolves env vars before invokinggit log, matching the pre-#1941 contract that the packaged hook relied on.Steps to Test This Pull Request
Before:
fatal: ambiguous argument(exit 23). After:Commit validation: successful!.New automated coverage in
tests/commands/test_check_command.py: two unit tests plustest_check_rev_range_pre_commit_branch_hook_regression-- an E2E test against a real git repo that reproduces the exact issue error when the fix is reverted.