Skip to content

Commit ad8190b

Browse files
committed
Wrap docstrings and comments in _safer_popen_windows
It gained an indentation level in dc95a76, so its docstrings and comments were no longer wrapped to 88 columns as most docstrings and comments have been (absent a reason not to) since gitpython-developers#1850. This wraps it, but some parts may benefit from some other adjustments.
1 parent 465ab56 commit ad8190b

File tree

1 file changed

+34
-27
lines changed

1 file changed

+34
-27
lines changed

Diff for: git/cmd.py

+34-27
Original file line numberDiff line numberDiff line change
@@ -232,47 +232,54 @@ def _safer_popen_windows(
232232
env: Optional[Mapping[str, str]] = None,
233233
**kwargs: Any,
234234
) -> Popen:
235-
"""Call :class:`subprocess.Popen` on Windows but don't include a CWD in the search.
236-
237-
This avoids an untrusted search path condition where a file like ``git.exe`` in a
238-
malicious repository would be run when GitPython operates on the repository. The
239-
process using GitPython may have an untrusted repository's working tree as its
240-
current working directory. Some operations may temporarily change to that directory
241-
before running a subprocess. In addition, while by default GitPython does not run
242-
external commands with a shell, it can be made to do so, in which case the CWD of
243-
the subprocess, which GitPython usually sets to a repository working tree, can
244-
itself be searched automatically by the shell. This wrapper covers all those cases.
235+
"""Call :class:`subprocess.Popen` on Windows but don't include a CWD in the
236+
search.
237+
238+
This avoids an untrusted search path condition where a file like ``git.exe`` in
239+
a malicious repository would be run when GitPython operates on the repository.
240+
The process using GitPython may have an untrusted repository's working tree as
241+
its current working directory. Some operations may temporarily change to that
242+
directory before running a subprocess. In addition, while by default GitPython
243+
does not run external commands with a shell, it can be made to do so, in which
244+
case the CWD of the subprocess, which GitPython usually sets to a repository
245+
working tree, can itself be searched automatically by the shell. This wrapper
246+
covers all those cases.
245247
246248
:note:
247-
This currently works by setting the :envvar:`NoDefaultCurrentDirectoryInExePath`
248-
environment variable during subprocess creation. It also takes care of passing
249-
Windows-specific process creation flags, but that is unrelated to path search.
249+
This currently works by setting the
250+
:envvar:`NoDefaultCurrentDirectoryInExePath` environment variable during
251+
subprocess creation. It also takes care of passing Windows-specific process
252+
creation flags, but that is unrelated to path search.
250253
251254
:note:
252255
The current implementation contains a race condition on :attr:`os.environ`.
253-
GitPython isn't thread-safe, but a program using it on one thread should ideally
254-
be able to mutate :attr:`os.environ` on another, without unpredictable results.
255-
See comments in https://github.com/gitpython-developers/GitPython/pull/1650.
256+
GitPython isn't thread-safe, but a program using it on one thread should
257+
ideally be able to mutate :attr:`os.environ` on another, without
258+
unpredictable results. See comments in:
259+
https://github.com/gitpython-developers/GitPython/pull/1650
256260
"""
257-
# CREATE_NEW_PROCESS_GROUP is needed for some ways of killing it afterwards. See:
261+
# CREATE_NEW_PROCESS_GROUP is needed for some ways of killing it afterwards.
258262
# https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
259263
# https://docs.python.org/3/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP
260264
creationflags = subprocess.CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP
261265

262-
# When using a shell, the shell is the direct subprocess, so the variable must be
263-
# set in its environment, to affect its search behavior. (The "1" can be any value.)
266+
# When using a shell, the shell is the direct subprocess, so the variable must
267+
# be set in its environment, to affect its search behavior. (The "1" can be any
268+
# value.)
264269
if shell:
265-
# The original may be immutable or reused by the caller. Make changes in a copy.
270+
# The original may be immutable or reused by the caller. Make changes in a
271+
# copy.
266272
env = {} if env is None else dict(env)
267273
env["NoDefaultCurrentDirectoryInExePath"] = "1"
268274

269-
# When not using a shell, the current process does the search in a CreateProcessW
270-
# API call, so the variable must be set in our environment. With a shell, this is
271-
# unnecessary, in versions where https://github.com/python/cpython/issues/101283 is
272-
# patched. If that is unpatched, then in the rare case the ComSpec environment
273-
# variable is unset, the search for the shell itself is unsafe. Setting
274-
# NoDefaultCurrentDirectoryInExePath in all cases, as is done here, is simpler and
275-
# protects against that. (As above, the "1" can be any value.)
275+
# When not using a shell, the current process does the search in a
276+
# CreateProcessW API call, so the variable must be set in our environment. With
277+
# a shell, this is unnecessary, in versions where
278+
# https://github.com/python/cpython/issues/101283 is patched. If that is
279+
# unpatched, then in the rare case the ComSpec environment variable is unset,
280+
# the search for the shell itself is unsafe. Setting
281+
# NoDefaultCurrentDirectoryInExePath in all cases, as is done here, is simpler
282+
# and protects against that. (As above, the "1" can be any value.)
276283
with patch_env("NoDefaultCurrentDirectoryInExePath", "1"):
277284
return Popen(
278285
command,

0 commit comments

Comments
 (0)