Skip to content

fix(check): expand env vars in --rev-range#2005

Open
bearomorphism wants to merge 1 commit into
masterfrom
fix/2003-rev-range-env-expansion
Open

fix(check): expand env vars in --rev-range#2005
bearomorphism wants to merge 1 commit into
masterfrom
fix/2003-rev-range-env-expansion

Conversation

@bearomorphism
Copy link
Copy Markdown
Collaborator

@bearomorphism bearomorphism commented May 30, 2026

Description

Fix regression where the packaged commitizen-branch pre-push hook stopped working after v4.15.0.

The hook passes the literal .. as a single argv element. Before #1941 the surrounding shell=True expanded the env vars; after #1941 (CWE-78 hardening) git receives the literal string and aborts with fatal: ambiguous argument.

Fix: expand env vars on the --rev-range argument via os.path.expandvars in Check.__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?

  • Yes (please specify the tool below)

Generated-by: GitHub Copilot CLI following the guidelines

Code Changes

  • Add test cases to all the changes you introduce
  • Run uv run poe all locally to ensure this change passes linter check and tests
  • Manually test the changes
  • Update the documentation for the changes (N/A -- no user-facing CLI change)

Expected Behavior

cz check --rev-range "$VAR1..$VAR2" resolves env vars before invoking git log, matching the pre-#1941 contract that the packaged hook relied on.

Steps to Test This Pull Request

git init repro && cd repro
git commit --allow-empty -m "feat: first"
git commit --allow-empty -m "fix: second"
export PRE_COMMIT_FROM_REF=$(git rev-parse HEAD~1)
export PRE_COMMIT_TO_REF=$(git rev-parse HEAD)
cz check --rev-range "$PRE_COMMIT_FROM_REF..$PRE_COMMIT_TO_REF"

Before: fatal: ambiguous argument (exit 23). After: Commit validation: successful!.

New automated coverage in tests/commands/test_check_command.py: two unit tests plus test_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.

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>
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Commitizen bump preview

Merging this PR will produce the following bump:

bump: version 4.16.2 → 4.16.3
tag to create: v4.16.3
increment detected: PATCH

@codecov
Copy link
Copy Markdown

codecov Bot commented May 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.24%. Comparing base (c3f6797) to head (2a3213e).
⚠️ Report is 3 commits behind head on master.
✅ All tests successful. No failed tests found.

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.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.expandvars handling for Check.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.

@bearomorphism
Copy link
Copy Markdown
Collaborator Author

The new tests will fail without the code change in check.py. Now it's ready for review.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pre-commit hook for commitizen-branch broken since 4.15.1

2 participants