Skip to content

Commit 4fe9966

Browse files
EliahKaganclaude
andcommitted
Refactor Git.execute() command docstring after gitpython-developers#2144
Rework :param command: as four parts: 1. Brief parameter description, with a recommendation to pass a sequence and a platform-dependent note on string handling: on POSIX a string is the program name, on Windows the OS splits it into argv. Corrects gitpython-developers#2144's claim that with shell=False the 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. shell=True (or Git.USE_SHELL) explanation: it sends the command to the platform shell rather than executing it directly, and the shell interprets ;, |, &, $(...), etc. as syntax. With untrusted text in the command -- paths, branch names, URLs, etc. -- this is arbitrary OS command execution. Cross-references Git.USE_SHELL for the long-form discussion. 3. shlex.split explanation: runs no shell, so the command-injection risk does not apply, but its POSIX shell rules on Windows match neither the shell=False OS argv parsing nor the shell=True cmd.exe parsing. Untrusted whitespace or quoting can therefore shift token boundaries, injecting extra arguments into git's option parser. 4. Asymmetric conclusion: build the sequence form directly; shell=True is the more dangerous route (arbitrary command execution), but no automatic-splitting route is safe with untrusted input. Replaces gitpython-developers#2144's hedged "possible security implications" wording with named mechanisms; preserves the asymmetry between command injection (shell=True) and argument injection (shlex.split on Windows). No worked examples (verbosity); the existing USE_SHELL docstring carries the full attack discussion. Documentation only; behavior is unchanged. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7b83f7a commit 4fe9966

1 file changed

Lines changed: 24 additions & 10 deletions

File tree

git/cmd.py

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,16 +1131,30 @@ 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 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`) sends the command to
1142+
the platform shell rather than executing it directly. The shell
1143+
interprets characters such as ``;``, ``|``, ``&``, and
1144+
``$(...)`` as syntax, so any untrusted text in the command
1145+
(paths, branch names, URLs, etc.) can execute arbitrary OS
1146+
commands. See :attr:`Git.USE_SHELL` for further detail.
1147+
1148+
:func:`shlex.split` runs no shell, so command injection is
1149+
not a concern, but its POSIX shell rules on Windows match
1150+
neither the OS argv parsing under ``shell=False`` nor the
1151+
``cmd.exe`` parsing under ``shell=True``. Untrusted
1152+
whitespace or quoting can shift token boundaries, injecting
1153+
extra arguments into git's option parser.
1154+
1155+
Build the sequence form directly. With untrusted input,
1156+
``shell=True`` is the more dangerous (arbitrary command
1157+
execution); no automatic-splitting route is safe.
11441158
11451159
:param istream:
11461160
Standard input filehandle passed to :class:`subprocess.Popen`.

0 commit comments

Comments
 (0)