Skip to content

Commit b58f63a

Browse files
fix(security): prevent command injection via shell=True (CWE-78)
Replace shell=True with list-based subprocess calls for all git.py functions that interpolate user-controlled values (tag names, messages, file paths, git references). This prevents shell injection attacks where malicious values in pyproject.toml could execute arbitrary commands during CI/CD runs of 'cz bump'. Changes: - cmd.run() now accepts str | Sequence[str]; lists use shell=False - git.tag() uses list args (fixes primary attack vector) - git.add() uses list args - git.commit() uses list args + env= for GIT_COMMITTER_DATE - git.tag_exist/is_signed_tag/get_tag_message use list args - git.get_filenames_in_commit() uses list args - git.get_tags() uses list args - git._get_log_as_str_list() uses list args Closes #1918 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 35ffe03 commit b58f63a

10 files changed

Lines changed: 168 additions & 116 deletions

File tree

commitizen/cmd.py

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from commitizen.exceptions import CharacterSetDecodeError
1010

1111
if TYPE_CHECKING:
12-
from collections.abc import Mapping
12+
from collections.abc import Mapping, Sequence
1313

1414

1515
class Command(NamedTuple):
@@ -35,12 +35,18 @@ def _try_decode(bytes_: bytes) -> str:
3535
raise CharacterSetDecodeError() from e
3636

3737

38-
def run(cmd: str, env: Mapping[str, str] | None = None) -> Command:
38+
def _popen(
39+
cmd: str | Sequence[str],
40+
*,
41+
shell: bool,
42+
env: Mapping[str, str] | None = None,
43+
) -> Command:
3944
if env is not None:
4045
env = {**os.environ, **env}
46+
4147
process = subprocess.Popen(
4248
cmd,
43-
shell=True,
49+
shell=shell,
4450
stdout=subprocess.PIPE,
4551
stderr=subprocess.PIPE,
4652
stdin=subprocess.PIPE,
@@ -55,3 +61,22 @@ def run(cmd: str, env: Mapping[str, str] | None = None) -> Command:
5561
stderr,
5662
return_code,
5763
)
64+
65+
66+
def run(cmd: Sequence[str], env: Mapping[str, str] | None = None) -> Command:
67+
"""Run a command safely without shell interpretation (shell=False).
68+
69+
Arguments are passed directly to the OS, preventing shell-injection
70+
vulnerabilities (CWE-78).
71+
"""
72+
return _popen(cmd, shell=False, env=env)
73+
74+
75+
def run_shell(cmd: str, env: Mapping[str, str] | None = None) -> Command:
76+
"""Run a command string via the system shell (shell=True).
77+
78+
Only use this for cases that intentionally require shell features
79+
(e.g., user-defined hooks with pipes/redirects). Never pass
80+
untrusted/user-controlled values into *cmd*.
81+
"""
82+
return _popen(cmd, shell=True, env=env)

commitizen/commands/init.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,12 @@ def __call__(self) -> None:
130130
"pre-commit is not installed in current environment."
131131
)
132132

133-
cmd_str = "pre-commit install " + " ".join(
134-
f"--hook-type {ty}" for ty in hook_types
135-
)
136-
c = cmd.run(cmd_str)
133+
cmd_args = ["pre-commit", "install"]
134+
for ty in hook_types:
135+
cmd_args.extend(["--hook-type", ty])
136+
c = cmd.run(cmd_args)
137137
if c.return_code != 0:
138+
cmd_str = " ".join(cmd_args)
138139
raise InitFailedError(
139140
"Failed to install pre-commit hook.\n"
140141
f"Error running {cmd_str}."

commitizen/git.py

Lines changed: 46 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class EOLType(Enum):
1919

2020
@classmethod
2121
def for_open(cls) -> str:
22-
c = cmd.run("git config core.eol")
22+
c = cmd.run(["git", "config", "core.eol"])
2323
eol = c.out.strip().upper()
2424
return cls._char_for_open()[cls._safe_cast(eol)]
2525

@@ -164,17 +164,17 @@ def tag(
164164
tag: str, annotated: bool = False, signed: bool = False, msg: str | None = None
165165
) -> cmd.Command:
166166
if not annotated and not signed:
167-
return cmd.run(f"git tag {tag}")
167+
return cmd.run(["git", "tag", tag])
168168

169169
# according to https://git-scm.com/book/en/v2/Git-Basics-Tagging,
170170
# we're not able to create lightweight tag with message.
171171
# by adding message, we make it a annotated tags
172172
option = "-s" if signed else "-a" # The else case is for annotated tags
173-
return cmd.run(f'git tag {option} {tag} -m "{msg or tag}"')
173+
return cmd.run(["git", "tag", option, tag, "-m", msg or tag])
174174

175175

176176
def add(*args: str) -> cmd.Command:
177-
return cmd.run(f"git add {' '.join(args)}")
177+
return cmd.run(["git", "add", *args])
178178

179179

180180
def commit(
@@ -186,20 +186,18 @@ def commit(
186186
f.write(message.encode("utf-8"))
187187
f.close()
188188

189-
command = _create_commit_cmd_string(args, committer_date, f.name)
190-
c = cmd.run(command)
191-
os.unlink(f.name)
192-
return c
189+
cmd_args = ["git", "commit"]
190+
if args:
191+
cmd_args.extend(args.split())
192+
cmd_args.extend(["-F", f.name])
193193

194+
env: dict[str, str] | None = None
195+
if committer_date:
196+
env = {"GIT_COMMITTER_DATE": committer_date}
194197

195-
def _create_commit_cmd_string(args: str, committer_date: str | None, name: str) -> str:
196-
command = f'git commit {args} -F "{name}"'
197-
if not committer_date:
198-
return command
199-
if os.name != "nt":
200-
return f"GIT_COMMITTER_DATE={committer_date} {command}"
201-
# Using `cmd /v /c "{command}"` sets environment variables only for that command
202-
return f'cmd /v /c "set GIT_COMMITTER_DATE={committer_date}&& {command}"'
198+
c = cmd.run(cmd_args, env=env)
199+
os.unlink(f.name)
200+
return c
203201

204202

205203
def get_commits(
@@ -226,7 +224,10 @@ def get_filenames_in_commit(git_reference: str = "") -> list[str]:
226224
227225
:returns: file names committed in the last commit by default or inside the passed git reference
228226
"""
229-
c = cmd.run(f"git show --name-only --pretty=format: {git_reference}")
227+
cmd_args = ["git", "show", "--name-only", "--pretty=format:"]
228+
if git_reference:
229+
cmd_args.append(git_reference)
230+
c = cmd.run(cmd_args)
230231
if c.return_code == 0:
231232
return c.out.strip().split("\n")
232233
raise GitCommandError(c.err)
@@ -237,15 +238,17 @@ def get_tags(
237238
) -> list[GitTag]:
238239
inner_delimiter = "---inner_delimiter---"
239240
formatter = (
240-
f'"%(refname:strip=2){inner_delimiter}'
241+
f"%(refname:strip=2){inner_delimiter}"
241242
f"%(objectname){inner_delimiter}"
242243
f"%(creatordate:format:{dateformat}){inner_delimiter}"
243-
f'%(object)"'
244+
f"%(object)"
244245
)
245-
extra = "--merged" if reachable_only else ""
246+
cmd_args = ["git", "tag", f"--format={formatter}", "--sort=-creatordate"]
247+
if reachable_only:
248+
cmd_args.append("--merged")
246249
# Force the default language for parsing
247250
env = {"LC_ALL": "C", "LANG": "C", "LANGUAGE": "C"}
248-
c = cmd.run(f"git tag --format={formatter} --sort=-creatordate {extra}", env=env)
251+
c = cmd.run(cmd_args, env=env)
249252
if c.return_code != 0:
250253
if reachable_only and c.err == "fatal: malformed object name HEAD\n":
251254
# this can happen if there are no commits in the repo yet
@@ -262,55 +265,55 @@ def get_tags(
262265

263266

264267
def tag_exist(tag: str) -> bool:
265-
c = cmd.run(f"git tag --list {tag}")
268+
c = cmd.run(["git", "tag", "--list", tag])
266269
return tag in c.out
267270

268271

269272
def is_signed_tag(tag: str) -> bool:
270-
return cmd.run(f"git tag -v {tag}").return_code == 0
273+
return cmd.run(["git", "tag", "-v", tag]).return_code == 0
271274

272275

273276
def get_latest_tag_name() -> str | None:
274-
c = cmd.run("git describe --abbrev=0 --tags")
277+
c = cmd.run(["git", "describe", "--abbrev=0", "--tags"])
275278
if c.err:
276279
return None
277280
return c.out.strip()
278281

279282

280283
def get_tag_message(tag: str) -> str | None:
281-
c = cmd.run(f"git tag -l --format='%(contents:subject)' {tag}")
284+
c = cmd.run(["git", "tag", "-l", "--format=%(contents:subject)", tag])
282285
if c.err:
283286
return None
284287
return c.out.strip()
285288

286289

287290
def get_tag_names() -> list[str]:
288-
c = cmd.run("git tag --list")
291+
c = cmd.run(["git", "tag", "--list"])
289292
if c.err:
290293
return []
291294
return [tag for raw in c.out.split("\n") if (tag := raw.strip())]
292295

293296

294297
def find_git_project_root() -> Path | None:
295-
c = cmd.run("git rev-parse --show-toplevel")
298+
c = cmd.run(["git", "rev-parse", "--show-toplevel"])
296299
if c.err:
297300
return None
298301
return Path(c.out.strip())
299302

300303

301304
def is_staging_clean() -> bool:
302305
"""Check if staging is clean."""
303-
c = cmd.run("git diff --no-ext-diff --cached --name-only")
306+
c = cmd.run(["git", "diff", "--no-ext-diff", "--cached", "--name-only"])
304307
return not bool(c.out)
305308

306309

307310
def is_git_project() -> bool:
308-
c = cmd.run("git rev-parse --is-inside-work-tree")
311+
c = cmd.run(["git", "rev-parse", "--is-inside-work-tree"])
309312
return c.out.strip() == "true"
310313

311314

312315
def get_core_editor() -> str | None:
313-
c = cmd.run("git var GIT_EDITOR")
316+
c = cmd.run(["git", "var", "GIT_EDITOR"])
314317
if c.out:
315318
return c.out.strip()
316319
return None
@@ -326,16 +329,26 @@ def _get_log_as_str_list(start: str | None, end: str, args: str) -> list[str]:
326329
delimiter = "----------commit-delimiter----------"
327330
log_format: str = "%H%n%P%n%s%n%an%n%ae%n%b"
328331
command_range = f"{start}..{end}" if start else end
329-
command = f"git -c log.showSignature=False log --pretty={log_format}{delimiter} {args} {command_range}"
332+
cmd_args = [
333+
"git",
334+
"-c",
335+
"log.showSignature=False",
336+
"log",
337+
f"--pretty={log_format}{delimiter}",
338+
]
339+
if args:
340+
cmd_args.extend(args.split())
341+
if command_range:
342+
cmd_args.append(command_range)
330343

331-
c = cmd.run(command)
344+
c = cmd.run(cmd_args)
332345
if c.return_code != 0:
333346
raise GitCommandError(c.err)
334347
return c.out.split(f"{delimiter}\n")
335348

336349

337350
def get_default_branch() -> str:
338-
c = cmd.run("git symbolic-ref refs/remotes/origin/HEAD")
351+
c = cmd.run(["git", "symbolic-ref", "refs/remotes/origin/HEAD"])
339352
if c.return_code != 0:
340353
raise GitCommandError(c.err)
341354
return c.out.strip()

commitizen/hooks.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def run(hooks: str | list[str], _env_prefix: str = "CZ_", **env: object) -> None
1717
for hook in hooks:
1818
out.info(f"Running hook '{hook}'")
1919

20-
c = cmd.run(hook, env=_format_env(_env_prefix, env))
20+
c = cmd.run_shell(hook, env=_format_env(_env_prefix, env))
2121

2222
if c.out:
2323
out.write(c.out)

tests/commands/test_bump_command.py

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import inspect
44
import re
5+
from pathlib import Path
56
from textwrap import dedent
67
from typing import TYPE_CHECKING
78
from unittest.mock import call
@@ -28,8 +29,6 @@
2829
)
2930

3031
if TYPE_CHECKING:
31-
from pathlib import Path
32-
3332
from pytest_mock import MockFixture
3433
from pytest_regressions.file_regression import FileRegressionFixture
3534

@@ -64,7 +63,9 @@ def test_bump_minor_increment(commit_msg: str, util: UtilFixture):
6463
assert git.tag_exist("0.2.0") is True
6564
assert (
6665
"commit:refs/tags/0.2.0"
67-
in cmd.run('git for-each-ref refs/tags --format "%(objecttype):%(refname)"').out
66+
in cmd.run(
67+
["git", "for-each-ref", "refs/tags", "--format", "%(objecttype):%(refname)"]
68+
).out
6869
)
6970

7071

@@ -76,7 +77,9 @@ def test_bump_minor_increment_annotated(commit_msg: str, util: UtilFixture):
7677
assert git.tag_exist("0.2.0") is True
7778
assert (
7879
"tag:refs/tags/0.2.0"
79-
in cmd.run('git for-each-ref refs/tags --format "%(objecttype):%(refname)"').out
80+
in cmd.run(
81+
["git", "for-each-ref", "refs/tags", "--format", "%(objecttype):%(refname)"]
82+
).out
8083
)
8184

8285
assert git.is_signed_tag("0.2.0") is False
@@ -90,7 +93,9 @@ def test_bump_minor_increment_signed(commit_msg: str, util: UtilFixture):
9093
assert git.tag_exist("0.2.0") is True
9194
assert (
9295
"tag:refs/tags/0.2.0"
93-
in cmd.run('git for-each-ref refs/tags --format "%(objecttype):%(refname)"').out
96+
in cmd.run(
97+
["git", "for-each-ref", "refs/tags", "--format", "%(objecttype):%(refname)"]
98+
).out
9499
)
95100

96101
assert git.is_signed_tag("0.2.0") is True
@@ -107,7 +112,9 @@ def test_bump_minor_increment_annotated_config_file(
107112
assert git.tag_exist("0.2.0") is True
108113
assert (
109114
"tag:refs/tags/0.2.0"
110-
in cmd.run('git for-each-ref refs/tags --format "%(objecttype):%(refname)"').out
115+
in cmd.run(
116+
["git", "for-each-ref", "refs/tags", "--format", "%(objecttype):%(refname)"]
117+
).out
111118
)
112119

113120
assert git.is_signed_tag("0.2.0") is False
@@ -125,7 +132,9 @@ def test_bump_minor_increment_signed_config_file(
125132
assert git.tag_exist("0.2.0") is True
126133
assert (
127134
"tag:refs/tags/0.2.0"
128-
in cmd.run('git for-each-ref refs/tags --format "%(objecttype):%(refname)"').out
135+
in cmd.run(
136+
["git", "for-each-ref", "refs/tags", "--format", "%(objecttype):%(refname)"]
137+
).out
129138
)
130139

131140
assert git.is_signed_tag("0.2.0") is True
@@ -283,10 +292,10 @@ def test_bump_command_prerelease_exact_mode(util: UtilFixture):
283292
@pytest.mark.usefixtures("tmp_commitizen_project")
284293
def test_bump_on_git_with_hooks_no_verify_disabled(util: UtilFixture):
285294
"""Bump commit without --no-verify"""
286-
cmd.run("mkdir .git/hooks")
295+
Path(".git/hooks").mkdir(parents=True, exist_ok=True)
287296
with open(".git/hooks/pre-commit", "w", encoding="utf-8") as f:
288297
f.write('#!/usr/bin/env bash\necho "0.1.0"')
289-
cmd.run("chmod +x .git/hooks/pre-commit")
298+
Path(".git/hooks/pre-commit").chmod(0o755)
290299

291300
# MINOR
292301
util.create_file_and_commit("feat: new file")
@@ -296,10 +305,10 @@ def test_bump_on_git_with_hooks_no_verify_disabled(util: UtilFixture):
296305

297306
@pytest.mark.usefixtures("tmp_commitizen_project")
298307
def test_bump_tag_exists_raises_exception(util: UtilFixture):
299-
cmd.run("mkdir .git/hooks")
308+
Path(".git/hooks").mkdir(parents=True, exist_ok=True)
300309
with open(".git/hooks/post-commit", "w", encoding="utf-8") as f:
301310
f.write("#!/usr/bin/env bash\nexit 9")
302-
cmd.run("chmod +x .git/hooks/post-commit")
311+
Path(".git/hooks/post-commit").chmod(0o755)
303312

304313
# MINOR
305314
util.create_file_and_commit("feat: new file")
@@ -312,10 +321,10 @@ def test_bump_tag_exists_raises_exception(util: UtilFixture):
312321

313322
@pytest.mark.usefixtures("tmp_commitizen_project")
314323
def test_bump_on_git_with_hooks_no_verify_enabled(util: UtilFixture):
315-
cmd.run("mkdir .git/hooks")
324+
Path(".git/hooks").mkdir(parents=True, exist_ok=True)
316325
with open(".git/hooks/pre-commit", "w", encoding="utf-8") as f:
317326
f.write('#!/usr/bin/env bash\necho "0.1.0"')
318-
cmd.run("chmod +x .git/hooks/pre-commit")
327+
Path(".git/hooks/pre-commit").chmod(0o755)
319328

320329
# MINOR
321330
util.create_file_and_commit("feat: new file")

0 commit comments

Comments
 (0)