Extract shared logic for using Popen safely on Windows · gitpython-developers/GitPython@c551e91

@@ -33,6 +33,7 @@

3333

IO,

3434

Iterator,

3535

List,

36+

Mapping,

3637

Optional,

3738

Pattern,

3839

Sequence,

@@ -327,6 +328,17 @@ def _get_exe_extensions() -> Sequence[str]:

327328328329329330

def 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

331343

winprog_exts = _get_exe_extensions()

332344

@@ -524,6 +536,67 @@ def remove_password_if_present(cmdline: Sequence[str]) -> List[str]:

524536

return 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