Skip to content

Commit 2a3213e

Browse files
fix(check): expand env vars in --rev-range
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>
1 parent a523e55 commit 2a3213e

2 files changed

Lines changed: 97 additions & 2 deletions

File tree

commitizen/commands/check.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import os
34
import re
45
import sys
56
from pathlib import Path
@@ -40,7 +41,11 @@ def __init__(self, config: BaseConfig, arguments: CheckArgs, *args: object) -> N
4041
"""
4142
self.commit_msg_file = arguments.get("commit_msg_file")
4243
self.commit_msg = arguments.get("message")
43-
self.rev_range = arguments.get("rev_range")
44+
rev_range = arguments.get("rev_range")
45+
# Expand env vars so the packaged ``commitizen-branch`` pre-push hook
46+
# (which passes the literal ``$PRE_COMMIT_FROM_REF..$PRE_COMMIT_TO_REF``)
47+
# keeps working after the ``shell=False`` switch in #1941. See #2003.
48+
self.rev_range = os.path.expandvars(rev_range) if rev_range else rev_range
4449
self.allow_abort = bool(
4550
arguments.get("allow_abort", config.settings["allow_abort"])
4651
)

tests/commands/test_check_command.py

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import pytest
77

8-
from commitizen import commands, git
8+
from commitizen import cmd, commands, git
99
from commitizen.cz import registry
1010
from commitizen.cz.base import BaseCommitizen
1111
from commitizen.exceptions import (
@@ -172,6 +172,53 @@ def test_check_a_range_of_git_commits_and_failed(config, mocker: MockFixture):
172172
commands.Check(config=config, arguments={"rev_range": "HEAD~10..master"})()
173173

174174

175+
def test_check_rev_range_expands_env_vars(
176+
config, success_mock: MockType, mocker: MockFixture, monkeypatch: pytest.MonkeyPatch
177+
):
178+
"""The ``commitizen-branch`` pre-push hook passes the literal string
179+
``$PRE_COMMIT_FROM_REF..$PRE_COMMIT_TO_REF`` and relies on env-var
180+
expansion. Regression test for
181+
https://github.com/commitizen-tools/commitizen/issues/2003.
182+
"""
183+
monkeypatch.setenv("PRE_COMMIT_FROM_REF", "abc123")
184+
monkeypatch.setenv("PRE_COMMIT_TO_REF", "def456")
185+
get_commits = mocker.patch(
186+
"commitizen.git.get_commits",
187+
return_value=_build_fake_git_commits(COMMIT_LOG),
188+
)
189+
190+
commands.Check(
191+
config=config,
192+
arguments={"rev_range": "$PRE_COMMIT_FROM_REF..$PRE_COMMIT_TO_REF"},
193+
)()
194+
195+
success_mock.assert_called_once()
196+
get_commits.assert_called_once_with(None, "abc123..def456")
197+
198+
199+
def test_check_rev_range_leaves_unset_env_vars_literal(
200+
config, mocker: MockFixture, monkeypatch: pytest.MonkeyPatch
201+
):
202+
"""Unset env-var references should pass through unchanged so git can
203+
surface a clear ``ambiguous argument`` error instead of being silently
204+
rewritten to an empty range."""
205+
monkeypatch.delenv("PRE_COMMIT_FROM_REF", raising=False)
206+
monkeypatch.delenv("PRE_COMMIT_TO_REF", raising=False)
207+
get_commits = mocker.patch(
208+
"commitizen.git.get_commits",
209+
return_value=_build_fake_git_commits(COMMIT_LOG),
210+
)
211+
212+
commands.Check(
213+
config=config,
214+
arguments={"rev_range": "$PRE_COMMIT_FROM_REF..$PRE_COMMIT_TO_REF"},
215+
)()
216+
217+
get_commits.assert_called_once_with(
218+
None, "$PRE_COMMIT_FROM_REF..$PRE_COMMIT_TO_REF"
219+
)
220+
221+
175222
def test_check_command_with_invalid_argument(config):
176223
with pytest.raises(
177224
InvalidCommandArgumentError,
@@ -193,6 +240,49 @@ def test_check_command_with_empty_range(config: BaseConfig, util: UtilFixture):
193240
commands.Check(config=config, arguments={"rev_range": "master..master"})()
194241

195242

243+
@pytest.mark.usefixtures("tmp_commitizen_project")
244+
def test_check_rev_range_pre_commit_branch_hook_regression(
245+
config: BaseConfig,
246+
util: UtilFixture,
247+
capsys: pytest.CaptureFixture[str],
248+
monkeypatch: pytest.MonkeyPatch,
249+
):
250+
"""End-to-end regression test for the packaged ``commitizen-branch``
251+
pre-push hook.
252+
253+
The hook in ``.pre-commit-hooks.yaml`` runs::
254+
255+
cz check --rev-range "$PRE_COMMIT_FROM_REF..$PRE_COMMIT_TO_REF"
256+
257+
``pre-commit`` exports those refs as environment variables but does
258+
*not* expand them in argv, so the literal string reaches ``cz check``.
259+
Before this fix, ``Check`` forwarded that literal to ``git log`` via
260+
``shell=False`` (PR #1941, CWE-78 hardening) and git aborted with
261+
``fatal: ambiguous argument '$PRE_COMMIT_FROM_REF..$PRE_COMMIT_TO_REF'``.
262+
263+
This test exercises the real subprocess path -- no mocks on
264+
``git.get_commits`` -- to guard against any future regression that
265+
bypasses env-var expansion in the rev-range argument.
266+
267+
See https://github.com/commitizen-tools/commitizen/issues/2003.
268+
"""
269+
util.create_file_and_commit("feat: initial")
270+
util.create_file_and_commit("fix: second commit")
271+
272+
from_ref = cmd.run(["git", "rev-parse", "HEAD~1"]).out.strip()
273+
to_ref = cmd.run(["git", "rev-parse", "HEAD"]).out.strip()
274+
monkeypatch.setenv("PRE_COMMIT_FROM_REF", from_ref)
275+
monkeypatch.setenv("PRE_COMMIT_TO_REF", to_ref)
276+
277+
commands.Check(
278+
config=config,
279+
arguments={"rev_range": "$PRE_COMMIT_FROM_REF..$PRE_COMMIT_TO_REF"},
280+
)()
281+
282+
captured = capsys.readouterr()
283+
assert "Commit validation: successful!" in captured.out
284+
285+
196286
def test_check_a_range_of_failed_git_commits(config, mocker: MockFixture):
197287
ill_formatted_commits_msgs = [
198288
"First commit does not follow rule",

0 commit comments

Comments
 (0)