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 <[email protected]>

Author: bearomorphism

commitizen/cmd.py

@@ -9,7 +9,7 @@
 from commitizen.exceptions import CharacterSetDecodeError
 
 if TYPE_CHECKING:
-    from collections.abc import Mapping
+    from collections.abc import Mapping, Sequence
 
 
 class Command(NamedTuple):
@@ -35,12 +35,21 @@ def _try_decode(bytes_: bytes) -> str:
         raise CharacterSetDecodeError() from e
 
 
-def run(cmd: str, env: Mapping[str, str] | None = None) -> Command:
+def run(cmd: str | Sequence[str], env: Mapping[str, str] | None = None) -> Command:
+    """Run a command and return its output.
+
+    When *cmd* is a list/sequence of strings the command is executed directly
+    (shell=False) which avoids shell-injection vulnerabilities (CWE-78).
+    When *cmd* is a plain string it is executed via the shell for backward
+    compatibility with callers that rely on shell features.
+    """
     if env is not None:
         env = {**os.environ, **env}
+
+    use_shell = isinstance(cmd, str)
     process = subprocess.Popen(
         cmd,
-        shell=True,
+        shell=use_shell,
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
         stdin=subprocess.PIPE,

commitizen/git.py

@@ -164,17 +164,17 @@ def tag(
     tag: str, annotated: bool = False, signed: bool = False, msg: str | None = None
 ) -> cmd.Command:
     if not annotated and not signed:
-        return cmd.run(f"git tag {tag}")
+        return cmd.run(["git", "tag", tag])
 
     # according to https://git-scm.com/book/en/v2/Git-Basics-Tagging,
     # we're not able to create lightweight tag with message.
     # by adding message, we make it a annotated tags
     option = "-s" if signed else "-a"  # The else case is for annotated tags
-    return cmd.run(f'git tag {option} {tag} -m "{msg or tag}"')
+    return cmd.run(["git", "tag", option, tag, "-m", msg or tag])
 
 
 def add(*args: str) -> cmd.Command:
-    return cmd.run(f"git add {' '.join(args)}")
+    return cmd.run(["git", "add", *args])
 
 
 def commit(
@@ -186,20 +186,18 @@ def commit(
     f.write(message.encode("utf-8"))
     f.close()
 
-    command = _create_commit_cmd_string(args, committer_date, f.name)
-    c = cmd.run(command)
-    os.unlink(f.name)
-    return c
+    cmd_args = ["git", "commit"]
+    if args:
+        cmd_args.extend(args.split())
+    cmd_args.extend(["-F", f.name])
 
+    env: dict[str, str] | None = None
+    if committer_date:
+        env = {"GIT_COMMITTER_DATE": committer_date}
 
-def _create_commit_cmd_string(args: str, committer_date: str | None, name: str) -> str:
-    command = f'git commit {args} -F "{name}"'
-    if not committer_date:
-        return command
-    if os.name != "nt":
-        return f"GIT_COMMITTER_DATE={committer_date} {command}"
-    # Using `cmd /v /c "{command}"` sets environment variables only for that command
-    return f'cmd /v /c "set GIT_COMMITTER_DATE={committer_date}&& {command}"'
+    c = cmd.run(cmd_args, env=env)
+    os.unlink(f.name)
+    return c
 
 
 def get_commits(
@@ -226,7 +224,10 @@ def get_filenames_in_commit(git_reference: str = "") -> list[str]:
 
     :returns: file names committed in the last commit by default or inside the passed git reference
     """
-    c = cmd.run(f"git show --name-only --pretty=format: {git_reference}")
+    cmd_args = ["git", "show", "--name-only", "--pretty=format:"]
+    if git_reference:
+        cmd_args.append(git_reference)
+    c = cmd.run(cmd_args)
     if c.return_code == 0:
         return c.out.strip().split("\n")
     raise GitCommandError(c.err)
@@ -237,15 +238,17 @@ def get_tags(
 ) -> list[GitTag]:
     inner_delimiter = "---inner_delimiter---"
     formatter = (
-        f'"%(refname:strip=2){inner_delimiter}'
+        f"%(refname:strip=2){inner_delimiter}"
         f"%(objectname){inner_delimiter}"
         f"%(creatordate:format:{dateformat}){inner_delimiter}"
-        f'%(object)"'
+        f"%(object)"
     )
-    extra = "--merged" if reachable_only else ""
+    cmd_args = ["git", "tag", f"--format={formatter}", "--sort=-creatordate"]
+    if reachable_only:
+        cmd_args.append("--merged")
     # Force the default language for parsing
     env = {"LC_ALL": "C", "LANG": "C", "LANGUAGE": "C"}
-    c = cmd.run(f"git tag --format={formatter} --sort=-creatordate {extra}", env=env)
+    c = cmd.run(cmd_args, env=env)
     if c.return_code != 0:
         if reachable_only and c.err == "fatal: malformed object name HEAD\n":
             # this can happen if there are no commits in the repo yet
@@ -262,12 +265,12 @@ def get_tags(
 
 
 def tag_exist(tag: str) -> bool:
-    c = cmd.run(f"git tag --list {tag}")
+    c = cmd.run(["git", "tag", "--list", tag])
     return tag in c.out
 
 
 def is_signed_tag(tag: str) -> bool:
-    return cmd.run(f"git tag -v {tag}").return_code == 0
+    return cmd.run(["git", "tag", "-v", tag]).return_code == 0
 
 
 def get_latest_tag_name() -> str | None:
@@ -278,7 +281,7 @@ def get_latest_tag_name() -> str | None:
 
 
 def get_tag_message(tag: str) -> str | None:
-    c = cmd.run(f"git tag -l --format='%(contents:subject)' {tag}")
+    c = cmd.run(["git", "tag", "-l", "--format=%(contents:subject)", tag])
     if c.err:
         return None
     return c.out.strip()
@@ -326,9 +329,19 @@ def _get_log_as_str_list(start: str | None, end: str, args: str) -> list[str]:
     delimiter = "----------commit-delimiter----------"
     log_format: str = "%H%n%P%n%s%n%an%n%ae%n%b"
     command_range = f"{start}..{end}" if start else end
-    command = f"git -c log.showSignature=False log --pretty={log_format}{delimiter} {args} {command_range}"
+    cmd_args = [
+        "git",
+        "-c",
+        "log.showSignature=False",
+        "log",
+        f"--pretty={log_format}{delimiter}",
+    ]
+    if args:
+        cmd_args.extend(args.split())
+    if command_range:
+        cmd_args.append(command_range)
 
-    c = cmd.run(command)
+    c = cmd.run(cmd_args)
     if c.return_code != 0:
         raise GitCommandError(c.err)
     return c.out.split(f"{delimiter}\n")

tests/test_git.py

@@ -2,7 +2,6 @@
 
 import inspect
 import os
-import platform
 from typing import TYPE_CHECKING
 
 import pytest
@@ -346,25 +345,23 @@ def test_create_tag_with_message(util: UtilFixture):
     tag_message = "test message"
     util.create_tag(tag_name, tag_message)
     assert git.get_latest_tag_name() == tag_name
-    assert git.get_tag_message(tag_name) == (
-        tag_message if platform.system() != "Windows" else f"'{tag_message}'"
-    )
+    assert git.get_tag_message(tag_name) == tag_message
 
 
 @pytest.mark.parametrize(
     ("file_path", "expected_cmd"),
     [
         (
             "/tmp/temp file",
-            'git commit --signoff -F "/tmp/temp file"',
+            ["git", "commit", "--signoff", "-F", "/tmp/temp file"],
         ),
         (
             "/tmp dir/temp file",
-            'git commit --signoff -F "/tmp dir/temp file"',
+            ["git", "commit", "--signoff", "-F", "/tmp dir/temp file"],
         ),
         (
             "/tmp/tempfile",
-            'git commit --signoff -F "/tmp/tempfile"',
+            ["git", "commit", "--signoff", "-F", "/tmp/tempfile"],
         ),
     ],
     ids=[
@@ -374,7 +371,7 @@ def test_create_tag_with_message(util: UtilFixture):
     ],
 )
 def test_commit_with_spaces_in_path(
-    mocker: MockFixture, file_path: str, expected_cmd: str, util: UtilFixture
+    mocker: MockFixture, file_path: str, expected_cmd: list[str], util: UtilFixture
 ):
     mock_run = util.mock_cmd()
     mock_unlink = mocker.patch("os.unlink")
@@ -383,7 +380,7 @@ def test_commit_with_spaces_in_path(
 
     git.commit("feat: new feature", "--signoff")
 
-    mock_run.assert_called_once_with(expected_cmd)
+    mock_run.assert_called_once_with(expected_cmd, env=None)
     mock_unlink.assert_called_once_with(file_path)
 
 
@@ -474,29 +471,39 @@ def test_git_commit_from_rev_and_commit(linebreak):
 
 
 @pytest.mark.parametrize(
-    ("os_name", "committer_date", "expected_cmd"),
+    "committer_date",
     [
-        (
-            "nt",
-            "2024-03-20",
-            'cmd /v /c "set GIT_COMMITTER_DATE=2024-03-20&& git commit  -F "temp.txt""',
-        ),
-        (
-            "posix",
-            "2024-03-20",
-            'GIT_COMMITTER_DATE=2024-03-20 git commit  -F "temp.txt"',
-        ),
-        ("nt", None, 'git commit  -F "temp.txt"'),
-        ("posix", None, 'git commit  -F "temp.txt"'),
+        "2024-03-20",
+        None,
     ],
 )
-def test_create_commit_cmd_string(
-    mocker: MockFixture, os_name: str, committer_date: str, expected_cmd: str
-):
-    """Test the OS-specific behavior of _create_commit_cmd_string"""
-    mocker.patch("os.name", os_name)
-    result = git._create_commit_cmd_string("", committer_date, "temp.txt")
-    assert result == expected_cmd
+def test_commit_uses_list_args_and_env(mocker: MockFixture, committer_date: str | None):
+    """Test that git.commit uses list-based subprocess (no shell injection)."""
+    mock_run = mocker.patch("commitizen.cmd.run")
+    mock_run.return_value = cmd.Command("", "", b"", b"", 0)
+
+    # We need to mock NamedTemporaryFile to control the file name
+    mock_file = mocker.MagicMock()
+    mock_file.name = "temp.txt"
+    mock_file.__enter__ = mocker.MagicMock(return_value=mock_file)
+    mock_file.__exit__ = mocker.MagicMock(return_value=False)
+    mocker.patch("commitizen.git.NamedTemporaryFile", return_value=mock_file)
+    mocker.patch("os.unlink")
+
+    git.commit("test message", args="-a --no-verify", committer_date=committer_date)
+
+    call_args = mock_run.call_args
+    # First positional arg should be a list (no shell injection)
+    cmd_list = call_args[0][0]
+    assert isinstance(cmd_list, list)
+    assert cmd_list == ["git", "commit", "-a", "--no-verify", "-F", "temp.txt"]
+
+    if committer_date:
+        env = call_args[1]["env"]
+        assert env == {"GIT_COMMITTER_DATE": committer_date}
+    else:
+        env = call_args[1]["env"]
+        assert env is None
 
 
 def test_get_default_branch_success(util: UtilFixture):