Extract shared logic for using Popen safely on Windows · gitpython-developers/GitPython@c551e91
@@ -33,6 +33,7 @@
3333IO,
3434Iterator,
3535List,
36+Mapping,
3637Optional,
3738Pattern,
3839Sequence,
@@ -327,6 +328,17 @@ def _get_exe_extensions() -> Sequence[str]:
327328328329329330def py_where(program: str, path: Optional[PathLike] = None) -> List[str]:
331+"""Perform a path search to assist :func:`is_cygwin_git`.
332+333+ This is not robust for general use. It is an implementation detail of
334+ :func:`is_cygwin_git`. When a search following all shell rules is needed,
335+ :func:`shutil.which` can be used instead.
336+337+ :note: Neither this function nor :func:`shutil.which` will predict the effect of an
338+ executable search on a native Windows system due to a :class:`subprocess.Popen`
339+ call without ``shell=True``, because shell and non-shell executable search on
340+ Windows differ considerably.
341+ """
330342# From: http://stackoverflow.com/a/377028/548792
331343winprog_exts = _get_exe_extensions()
332344@@ -524,6 +536,67 @@ def remove_password_if_present(cmdline: Sequence[str]) -> List[str]:
524536return new_cmdline
525537526538539+def _safer_popen_windows(
540+command: Union[str, Sequence[Any]],
541+*,
542+shell: bool = False,
543+env: Optional[Mapping[str, str]] = None,
544+**kwargs: Any,
545+) -> subprocess.Popen:
546+"""Call :class:`subprocess.Popen` on Windows but don't include a CWD in the search.
547+548+ This avoids an untrusted search path condition where a file like ``git.exe`` in a
549+ malicious repository would be run when GitPython operates on the repository. The
550+ process using GitPython may have an untrusted repository's working tree as its
551+ current working directory. Some operations may temporarily change to that directory
552+ before running a subprocess. In addition, while by default GitPython does not run
553+ external commands with a shell, it can be made to do so, in which case the CWD of
554+ the subprocess, which GitPython usually sets to a repository working tree, can
555+ itself be searched automatically by the shell. This wrapper covers all those cases.
556+557+ :note: This currently works by setting the ``NoDefaultCurrentDirectoryInExePath``
558+ environment variable during subprocess creation. It also takes care of passing
559+ Windows-specific process creation flags, but that is unrelated to path search.
560+561+ :note: The current implementation contains a race condition on :attr:`os.environ`.
562+ GitPython isn't thread-safe, but a program using it on one thread should ideally
563+ be able to mutate :attr:`os.environ` on another, without unpredictable results.
564+ See comments in https://github.com/gitpython-developers/GitPython/pull/1650.
565+ """
566+# CREATE_NEW_PROCESS_GROUP is needed for some ways of killing it afterwards. See:
567+# https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
568+# https://docs.python.org/3/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP
569+creationflags = subprocess.CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP
570+571+# When using a shell, the shell is the direct subprocess, so the variable must be
572+# set in its environment, to affect its search behavior. (The "1" can be any value.)
573+if shell:
574+safer_env = {} if env is None else dict(env)
575+safer_env["NoDefaultCurrentDirectoryInExePath"] = "1"
576+else:
577+safer_env = env
578+579+# When not using a shell, the current process does the search in a CreateProcessW
580+# API call, so the variable must be set in our environment. With a shell, this is
581+# unnecessary, in versions where https://github.com/python/cpython/issues/101283 is
582+# patched. If not, in the rare case the ComSpec environment variable is unset, the
583+# shell is searched for unsafely. Setting NoDefaultCurrentDirectoryInExePath in all
584+# cases, as here, is simpler and protects against that. (The "1" can be any value.)
585+with patch_env("NoDefaultCurrentDirectoryInExePath", "1"):
586+return subprocess.Popen(
587+command,
588+shell=shell,
589+env=safer_env,
590+creationflags=creationflags,
591+**kwargs,
592+ )
593+594+595+if os.name == "nt":
596+safer_popen = _safer_popen_windows
597+else:
598+safer_popen = subprocess.Popen
599+527600# } END utilities
528601529602# { Classes