Move safer_popen from git.util to git.cmd · gitpython-developers/GitPython@3eb7c2a (original) (raw)
`@@ -29,8 +29,8 @@
`
29
29
`cygpath,
`
30
30
`expand_path,
`
31
31
`is_cygwin_git,
`
``
32
`+
patch_env,
`
32
33
`remove_password_if_present,
`
33
``
`-
safer_popen,
`
34
34
`stream_copy,
`
35
35
`)
`
36
36
``
`@@ -46,6 +46,7 @@
`
46
46
`Iterator,
`
47
47
`List,
`
48
48
`Mapping,
`
``
49
`+
Optional,
`
49
50
`Sequence,
`
50
51
`TYPE_CHECKING,
`
51
52
`TextIO,
`
`@@ -102,7 +103,7 @@ def handle_process_output(
`
102
103
`Callable[[bytes, "Repo", "DiffIndex"], None],
`
103
104
` ],
`
104
105
`stderr_handler: Union[None, Callable[[AnyStr], None], Callable[[List[AnyStr]], None]],
`
105
``
`-
finalizer: Union[None, Callable[[Union[subprocess.Popen, "Git.AutoInterrupt"]], None]] = None,
`
``
106
`+
finalizer: Union[None, Callable[[Union[Popen, "Git.AutoInterrupt"]], None]] = None,
`
106
107
`decode_streams: bool = True,
`
107
108
`kill_after_timeout: Union[None, float] = None,
`
108
109
`) -> None:
`
`@@ -207,6 +208,68 @@ def pump_stream(
`
207
208
`finalizer(process)
`
208
209
``
209
210
``
``
211
`+
def _safer_popen_windows(
`
``
212
`+
command: Union[str, Sequence[Any]],
`
``
213
`+
*,
`
``
214
`+
shell: bool = False,
`
``
215
`+
env: Optional[Mapping[str, str]] = None,
`
``
216
`+
**kwargs: Any,
`
``
217
`+
) -> Popen:
`
``
218
`` +
"""Call :class:subprocess.Popen on Windows but don't include a CWD in the search.
``
``
219
+
``
220
This avoids an untrusted search path condition where a file like ``git.exe`` in a
``
221
`+
malicious repository would be run when GitPython operates on the repository. The
`
``
222
`+
process using GitPython may have an untrusted repository's working tree as its
`
``
223
`+
current working directory. Some operations may temporarily change to that directory
`
``
224
`+
before running a subprocess. In addition, while by default GitPython does not run
`
``
225
`+
external commands with a shell, it can be made to do so, in which case the CWD of
`
``
226
`+
the subprocess, which GitPython usually sets to a repository working tree, can
`
``
227
`+
itself be searched automatically by the shell. This wrapper covers all those cases.
`
``
228
+
``
229
:note: This currently works by setting the ``NoDefaultCurrentDirectoryInExePath``
``
230
`+
environment variable during subprocess creation. It also takes care of passing
`
``
231
`+
Windows-specific process creation flags, but that is unrelated to path search.
`
``
232
+
``
233
`` +
:note: The current implementation contains a race condition on :attr:os.environ.
``
``
234
`+
GitPython isn't thread-safe, but a program using it on one thread should ideally
`
``
235
`` +
be able to mutate :attr:os.environ on another, without unpredictable results.
``
``
236
`+
See comments in https://github.com/gitpython-developers/GitPython/pull/1650.
`
``
237
`+
"""
`
``
238
`+
CREATE_NEW_PROCESS_GROUP is needed for some ways of killing it afterwards. See:
`
``
239
`+
https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
`
``
240
`+
https://docs.python.org/3/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP
`
``
241
`+
creationflags = subprocess.CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP
`
``
242
+
``
243
`+
When using a shell, the shell is the direct subprocess, so the variable must be
`
``
244
`+
set in its environment, to affect its search behavior. (The "1" can be any value.)
`
``
245
`+
if shell:
`
``
246
`+
safer_env = {} if env is None else dict(env)
`
``
247
`+
safer_env["NoDefaultCurrentDirectoryInExePath"] = "1"
`
``
248
`+
else:
`
``
249
`+
safer_env = env
`
``
250
+
``
251
`+
When not using a shell, the current process does the search in a CreateProcessW
`
``
252
`+
API call, so the variable must be set in our environment. With a shell, this is
`
``
253
`+
unnecessary, in versions where https://github.com/python/cpython/issues/101283 is
`
``
254
`+
patched. If not, in the rare case the ComSpec environment variable is unset, the
`
``
255
`+
shell is searched for unsafely. Setting NoDefaultCurrentDirectoryInExePath in all
`
``
256
`+
cases, as here, is simpler and protects against that. (The "1" can be any value.)
`
``
257
`+
with patch_env("NoDefaultCurrentDirectoryInExePath", "1"):
`
``
258
`+
return Popen(
`
``
259
`+
command,
`
``
260
`+
shell=shell,
`
``
261
`+
env=safer_env,
`
``
262
`+
creationflags=creationflags,
`
``
263
`+
**kwargs,
`
``
264
`+
)
`
``
265
+
``
266
+
``
267
`+
if os.name == "nt":
`
``
268
`+
safer_popen = _safer_popen_windows
`
``
269
`+
else:
`
``
270
`+
safer_popen = Popen
`
``
271
+
``
272
+
210
273
`def dashify(string: str) -> str:
`
211
274
`return string.replace("_", "-")
`
212
275
``