Skip to content

Commit 8bebd8b

Browse files
EliahKaganclaude
andcommitted
Rewrite Git.execute() command parameter docstring
gitpython-developers#2146 identifies two issues with gitpython-developers#2144's :param command: rewrite, which this fixes by reworking the block as four paragraphs: 1. Description: a sequence of program arguments is recommended; a string is also accepted, but its meaning is strongly platform-dependent. 2. Platform-specific string handling. By default, a shell is not used. On Unix-like systems the whole string is the program name (so multi-word strings raise GitCommandNotFound); on Windows the OS parses the string itself, so multi-word strings can work but are not portable. This corrects gitpython-developers#2144's claim that with shell=False the string "is passed as a single executable name to subprocess.Popen" -- accurate on Unix-like systems, but on Windows subprocess.Popen forwards the string to CreateProcessW and Windows command-line parsing produces argv. 3. shell=True / Git.USE_SHELL warning. The shell interprets metacharacters such as ;, |, &, $(...) (on POSIX), and ^ (on Windows) as syntax; untrusted text in the command can then execute arbitrary OS commands. Cross-references USE_SHELL for the long-form discussion (which has the attack examples and history). 4. shlex.split caveat. Producing a sequence automatically via shlex.split is far safer than shell=True (so gitpython-developers#2144's harm-reduction note is preserved), but qualified: shlex.split parses POSIX shell syntax on all systems and the result is still unsafe for anything but fixed, fully trusted strings. Calls out the f-string-then-shlex.split anti-pattern explicitly -- whitespace or quoting in an untrusted interpolated value can still inject arguments. For input derived in any way from untrusted data, build the argument sequence yourself, while ensuring each argument is fully sanitized. Add a related paragraph to :param shell: noting that, on Unix-like systems, shlex.split may serve in rare cases as a transitional step when migrating existing string-command shell=True calls, with extreme care, and cross-referencing :param command: for the risks. Removes gitpython-developers#2144's hedged "possible security implications" wording and its implicit recommendation of shlex.split as a general string-to-argv splitter, in favor of named mechanisms and a narrow, qualified mention. Documentation only; behavior is unchanged. For gitpython-developers#2146. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7b83f7a commit 8bebd8b

1 file changed

Lines changed: 27 additions & 10 deletions

File tree

‎git/cmd.py‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,16 +1131,28 @@ 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. A sequence of program arguments is recommended.
1135+
A string is also accepted, but its meaning is strongly platform-dependent.
1136+
1137+
By default, a shell is not used. On Unix-like systems, a string is the
1138+
whole program name (so ``"git log -n 1"`` raises
1139+
:class:`GitCommandNotFound`). On Windows, the OS parses the string itself,
1140+
so multi-word strings can work but are not portable.
1141+
1142+
Avoid ``shell=True`` (and :attr:`Git.USE_SHELL`): this runs the command in
1143+
a shell, which is generally unsafe. The shell interprets metacharacters such
1144+
as ``;``, ``|``, ``&``, ``$(...)`` (on POSIX), and ``^`` (on Windows), as
1145+
syntax. Any untrusted text in the command can then execute arbitrary OS
1146+
commands. See :attr:`Git.USE_SHELL`.
1147+
1148+
Producing a sequence automatically by :func:`shlex.split` and passing it
1149+
as the command is far safer than ``shell=True``. But :func:`shlex.split`
1150+
parses POSIX shell syntax on all systems, and the result is still unsafe
1151+
for anything but *fixed, fully trusted* strings. Do not use it on strings
1152+
built by interpolating values: whitespace or quoting in an untrusted value
1153+
can still inject arguments. For input derived in any way from untrusted
1154+
data, build the argument sequence yourself, while ensuring each argument
1155+
is fully sanitized.
11441156
11451157
:param istream:
11461158
Standard input filehandle passed to :class:`subprocess.Popen`.
@@ -1208,6 +1220,11 @@ def execute(
12081220
needed (nor useful) to work around any known operating system specific
12091221
issues.
12101222
1223+
On Unix-like systems, when migrating away from passing string commands with
1224+
``shell=True``, :func:`shlex.split` may serve as a transitional step in rare
1225+
cases, but this should be undertaken with extreme care. See the `command`
1226+
parameter above on the risks.
1227+
12111228
:param env:
12121229
A dictionary of environment variables to be passed to
12131230
:class:`subprocess.Popen`.

0 commit comments

Comments
 (0)