Skip to content

Commit f13df09

Browse files
EliahKaganclaude
andcommitted
Refactor Git.execute() command docstring after gitpython-developers#2144
Rework :param command: as three parts: 1. Brief description: parameter type, sequence recommended, brief platform-dependent string note. Corrects gitpython-developers#2144's claim that with shell=False a string is "passed as a single executable name to subprocess.Popen" -- accurate on POSIX, but on Windows subprocess.Popen forwards the string to CreateProcessW, which tokenizes via Windows command-line parsing. 2. Asymmetric security paragraphs: * shell=True (or Git.USE_SHELL) runs the command through the shell, which interprets metacharacters anywhere in it; with untrusted input that is OS command injection. Cross-references USE_SHELL and the shell parameter for detail. * shlex.split runs no shell, but tokenizes by POSIX shell rules. On Windows those rules differ from both shell=False's OS argv parsing and shell=True's cmd.exe parsing, so untrusted whitespace or quoting can shift token boundaries and inject extra arguments into git's own option parser. 3. Conclusion: neither automatic-splitting approach is safe with untrusted input; build the sequence form directly, one value per argv slot. Replaces gitpython-developers#2144's hedged "possible security implications" wording with named mechanisms and keeps the asymmetry between command injection (shell=True, catastrophic) and argument injection (shlex.split on Windows, milder) visible. No worked examples to keep the docstring compact; the existing USE_SHELL and shell- parameter docstrings give the full picture for shell=True. Documentation only; behavior is unchanged. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7b83f7a commit f13df09

1 file changed

Lines changed: 25 additions & 10 deletions

File tree

git/cmd.py

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,16 +1131,31 @@ def execute(
11311131
information (stdout).
11321132
11331133
:param command:
1134-
The command to execute. A sequence of program arguments is the
1135-
recommended form when `shell` is ``False`` (the default), e.g.
1136-
``["git", "log", "-n", "1"]``.
1137-
1138-
A string is accepted, but with `shell` set to ``False`` it is passed
1139-
as a single executable name to :class:`subprocess.Popen`. For example,
1140-
``"git log -n 1"`` looks for an executable literally named
1141-
``git log -n 1`` and will fail with :class:`GitCommandNotFound`. To
1142-
split a command string into argv tokens, pass ``shlex.split(...)`` as
1143-
a sequence or set `shell` to ``True`` (see the warning below).
1134+
The command argument list to execute.
1135+
It should be a sequence of program arguments, or a string.
1136+
Passing a sequence of program arguments is recommended. With a
1137+
string, parsing is delegated to :class:`subprocess.Popen` and
1138+
is platform-dependent: on POSIX the entire string is taken as
1139+
the program name, while on Windows the OS splits it into argv.
1140+
1141+
``shell=True`` (or :attr:`Git.USE_SHELL`) runs the command
1142+
through the platform shell rather than executing it directly.
1143+
The shell interprets metacharacters such as ``;``, ``|``,
1144+
``&``, and ``$(...)`` anywhere in the command, so untrusted
1145+
input — paths, branch names, URLs, etc. — is then OS command
1146+
injection. See :attr:`Git.USE_SHELL` and the ``shell``
1147+
parameter below for further detail.
1148+
1149+
:func:`shlex.split` does not invoke a shell, but tokenizes by
1150+
POSIX shell rules. On Windows those rules differ from both
1151+
the Windows command-line parsing used under ``shell=False``
1152+
and the ``cmd.exe`` parsing used under ``shell=True``; its
1153+
tokens may match neither, so untrusted whitespace or quoting
1154+
can shift token boundaries and inject extra arguments that
1155+
git's own option parser may act on.
1156+
1157+
Neither approach is safe with untrusted input. Build the
1158+
sequence form directly, one value per argv slot.
11441159
11451160
:param istream:
11461161
Standard input filehandle passed to :class:`subprocess.Popen`.

0 commit comments

Comments
 (0)