patched 3.1.37

Author: seal-devops

.github/workflows/cygwin-test.yml

@@ -4,7 +4,7 @@ on: [push, pull_request, workflow_dispatch]
 
 jobs:
   build:
-    runs-on: windows-latest
+    runs-on: windows-2022
     strategy:
       fail-fast: false
     env:
@@ -61,4 +61,5 @@ jobs:
     - name: Test with pytest
       run: |
         set +x
+        git config --global --add safe.directory '*'
         /usr/bin/python -m pytest

.github/workflows/lint.yml

@@ -4,11 +4,11 @@ on: [push, pull_request, workflow_dispatch]
 
 jobs:
   lint:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-22.04
 
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-python@v4
+      - uses: MatteoH2O1999/setup-python@v4
         with:
           python-version: "3.x"
       - uses: pre-commit/action@v3.0.0

.github/workflows/pythonpackage.yml

@@ -11,7 +11,7 @@ permissions:
 jobs:
   build:
 
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-22.04
     strategy:
       fail-fast: false
       matrix:
@@ -31,7 +31,7 @@ jobs:
         submodules: recursive
 
     - name: Set up Python ${{ matrix.python-version }}
-      uses: actions/setup-python@v4
+      uses: MatteoH2O1999/setup-python@v4
       with:
         python-version: ${{ matrix.python-version }}
         allow-prereleases: ${{ matrix.experimental }}

.pre-commit-config.yaml

@@ -9,7 +9,7 @@ repos:
             flake8-comprehensions==3.14.0,
             flake8-typing-imports==1.14.0,
           ]
-        exclude: ^doc|^git/ext/
+        exclude: ^test/fixtures/polyglot$|^doc|^git/ext/
 
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v4.4.0

VERSION

@@ -1 +1 @@
-3.1.37
+3.1.37+sp1

git/cmd.py

@@ -43,6 +43,7 @@
     Iterator,
     List,
     Mapping,
+    Optional,
     Sequence,
     TYPE_CHECKING,
     TextIO,
@@ -99,7 +100,7 @@ def handle_process_output(
         Callable[[bytes, "Repo", "DiffIndex"], None],
     ],
     stderr_handler: Union[None, Callable[[AnyStr], None], Callable[[List[AnyStr]], None]],
-    finalizer: Union[None, Callable[[Union[subprocess.Popen, "Git.AutoInterrupt"]], None]] = None,
+    finalizer: Union[None, Callable[[Union[Popen, "Git.AutoInterrupt"]], None]] = None,
     decode_streams: bool = True,
     kill_after_timeout: Union[None, float] = None,
 ) -> None:
@@ -207,6 +208,68 @@ def pump_stream(
         return None
 
 
+def _safer_popen_windows(
+    command: Union[str, Sequence[Any]],
+    *,
+    shell: bool = False,
+    env: Optional[Mapping[str, str]] = None,
+    **kwargs: Any,
+) -> Popen:
+    """Call :class:`subprocess.Popen` on Windows but don't include a CWD in the search.
+
+    This avoids an untrusted search path condition where a file like ``git.exe`` in a
+    malicious repository would be run when GitPython operates on the repository. The
+    process using GitPython may have an untrusted repository's working tree as its
+    current working directory. Some operations may temporarily change to that directory
+    before running a subprocess. In addition, while by default GitPython does not run
+    external commands with a shell, it can be made to do so, in which case the CWD of
+    the subprocess, which GitPython usually sets to a repository working tree, can
+    itself be searched automatically by the shell. This wrapper covers all those cases.
+
+    :note: This currently works by setting the ``NoDefaultCurrentDirectoryInExePath``
+        environment variable during subprocess creation. It also takes care of passing
+        Windows-specific process creation flags, but that is unrelated to path search.
+
+    :note: The current implementation contains a race condition on :attr:`os.environ`.
+        GitPython isn't thread-safe, but a program using it on one thread should ideally
+        be able to mutate :attr:`os.environ` on another, without unpredictable results.
+        See comments in https://github.com/gitpython-developers/GitPython/pull/1650.
+    """
+    # CREATE_NEW_PROCESS_GROUP is needed for some ways of killing it afterwards. See:
+    # https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
+    # https://docs.python.org/3/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP
+    creationflags = subprocess.CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP
+
+    # When using a shell, the shell is the direct subprocess, so the variable must be
+    # set in its environment, to affect its search behavior. (The "1" can be any value.)
+    if shell:
+        safer_env = {} if env is None else dict(env)
+        safer_env["NoDefaultCurrentDirectoryInExePath"] = "1"
+    else:
+        safer_env = env
+
+    # When not using a shell, the current process does the search in a CreateProcessW
+    # API call, so the variable must be set in our environment. With a shell, this is
+    # unnecessary, in versions where https://github.com/python/cpython/issues/101283 is
+    # patched. If not, in the rare case the ComSpec environment variable is unset, the
+    # shell is searched for unsafely. Setting NoDefaultCurrentDirectoryInExePath in all
+    # cases, as here, is simpler and protects against that. (The "1" can be any value.)
+    with patch_env("NoDefaultCurrentDirectoryInExePath", "1"):
+        return Popen(
+            command,
+            shell=shell,
+            env=safer_env,
+            creationflags=creationflags,
+            **kwargs,
+        )
+
+
+if os.name == "nt":
+    safer_popen = _safer_popen_windows
+else:
+    safer_popen = Popen
+
+
 def dashify(string: str) -> str:
     return string.replace("_", "-")
 
@@ -225,16 +288,6 @@ def dict_to_slots_and__excluded_are_none(self: object, d: Mapping[str, Any], exc
 ## -- End Utilities -- @}
 
 
-# value of Windows process creation flag taken from MSDN
-CREATE_NO_WINDOW = 0x08000000
-
-## CREATE_NEW_PROCESS_GROUP is needed to allow killing it afterwards,
-# see https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
-PROC_CREATIONFLAGS = (
-    CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP if is_win else 0  # type: ignore[attr-defined]
-)  # mypy error if not windows
-
-
 class Git(LazyMixin):
 
     """
@@ -963,11 +1016,8 @@ def execute(
                     redacted_command,
                     '"kill_after_timeout" feature is not supported on Windows.',
                 )
-            # Only search PATH, not CWD. This must be in the *caller* environment. The "1" can be any value.
-            maybe_patch_caller_env = patch_env("NoDefaultCurrentDirectoryInExePath", "1")
         else:
             cmd_not_found_exception = FileNotFoundError  # NOQA # exists, flake8 unknown @UndefinedVariable
-            maybe_patch_caller_env = contextlib.nullcontext()
         # end handle
 
         stdout_sink = PIPE if with_stdout else getattr(subprocess, "DEVNULL", None) or open(os.devnull, "wb")
@@ -983,21 +1033,18 @@ def execute(
             istream_ok,
         )
         try:
-            with maybe_patch_caller_env:
-                proc = Popen(
-                    command,
-                    env=env,
-                    cwd=cwd,
-                    bufsize=-1,
-                    stdin=istream or DEVNULL,
-                    stderr=PIPE,
-                    stdout=stdout_sink,
-                    shell=shell is not None and shell or self.USE_SHELL,
-                    close_fds=is_posix,  # unsupported on windows
-                    universal_newlines=universal_newlines,
-                    creationflags=PROC_CREATIONFLAGS,
-                    **subprocess_kwargs,
-                )
+            proc = safer_popen(
+                command,
+                env=env,
+                cwd=cwd,
+                bufsize=-1,
+                stdin=(istream or DEVNULL),
+                stderr=PIPE,
+                stdout=stdout_sink,
+                shell=shell,
+                universal_newlines=universal_newlines,
+                **subprocess_kwargs,
+            )
         except cmd_not_found_exception as err:
             raise GitCommandNotFound(redacted_command, err) from err
         else:

git/index/fun.py

@@ -16,7 +16,7 @@
 )
 import subprocess
 
-from git.cmd import PROC_CREATIONFLAGS, handle_process_output
+from git.cmd import handle_process_output, safer_popen
 from git.compat import (
     defenc,
     force_text,
@@ -102,14 +102,14 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None:
             relative_hp = Path(hp).relative_to(index.repo.working_dir).as_posix()
             cmd = ["bash.exe", relative_hp]
 
-        process = subprocess.Popen(
+        # process = subprocess.Popen(
+        process = safer_popen(
             cmd + list(args),
             env=env,
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
             cwd=index.repo.working_dir,
             close_fds=is_posix,
-            creationflags=PROC_CREATIONFLAGS,
         )
     except Exception as ex:
         raise HookExecutionError(hp, ex) from ex

git/util.py

@@ -291,6 +291,17 @@ def _get_exe_extensions() -> Sequence[str]:
 
 
 def py_where(program: str, path: Optional[PathLike] = None) -> List[str]:
+    """Perform a path search to assist :func:`is_cygwin_git`.
+
+    This is not robust for general use. It is an implementation detail of
+    :func:`is_cygwin_git`. When a search following all shell rules is needed,
+    :func:`shutil.which` can be used instead.
+
+    :note: Neither this function nor :func:`shutil.which` will predict the effect of an
+        executable search on a native Windows system due to a :class:`subprocess.Popen`
+        call without ``shell=True``, because shell and non-shell executable search on
+        Windows differ considerably.
+    """
     # From: http://stackoverflow.com/a/377028/548792
     winprog_exts = _get_exe_extensions()
 

test/fixtures/polyglot

@@ -0,0 +1,8 @@
+#!/usr/bin/env sh
+# Valid script in both Bash and Python, but with different behavior.
+""":"
+echo 'Ran intended hook.' >output.txt
+exit
+" """
+from pathlib import Path
+Path('payload.txt').write_text('Ran impostor hook!', encoding='utf-8')

test/lib/helper.py

@@ -13,6 +13,7 @@
 import textwrap
 import time
 import unittest
+import venv
 
 from git.compat import is_win
 from git.util import rmtree, cwd
@@ -38,6 +39,7 @@
     "with_rw_repo",
     "with_rw_and_rw_remote_repo",
     "TestBase",
+    "VirtualEnvironment",
     "TestCase",
     "SkipTest",
     "skipIf",
@@ -89,12 +91,12 @@ def with_rw_directory(func):
     test succeeds, but leave it otherwise to aid additional debugging"""
 
     @wraps(func)
-    def wrapper(self):
+    def wrapper(self, *args, **kwargs):
         path = tempfile.mktemp(prefix=func.__name__)
         os.mkdir(path)
         keep = False
         try:
-            return func(self, path)
+            return func(self, path, *args, **kwargs)
         except Exception:
             log.info(
                 "Test %s.%s failed, output is at %r\n",
@@ -401,3 +403,46 @@ def _make_file(self, rela_path, data, repo=None):
         with open(abs_path, "w") as fp:
             fp.write(data)
         return abs_path
+
+
+class VirtualEnvironment:
+    """A newly created Python virtual environment for use in a test."""
+
+    __slots__ = ("_env_dir",)
+
+    def __init__(self, env_dir, *, with_pip):
+        if os.name == "nt":
+            self._env_dir = osp.realpath(env_dir)
+            venv.create(self.env_dir, symlinks=False, with_pip=with_pip)
+        else:
+            self._env_dir = env_dir
+            venv.create(self.env_dir, symlinks=True, with_pip=with_pip)
+
+    @property
+    def env_dir(self):
+        """The top-level directory of the environment."""
+        return self._env_dir
+
+    @property
+    def python(self):
+        """Path to the Python executable in the environment."""
+        return self._executable("python")
+
+    @property
+    def pip(self):
+        """Path to the pip executable in the environment, or RuntimeError if absent."""
+        return self._executable("pip")
+
+    @property
+    def sources(self):
+        """Path to a src directory in the environment, which may not exist yet."""
+        return os.path.join(self.env_dir, "src")
+
+    def _executable(self, basename):
+        if os.name == "nt":
+            path = osp.join(self.env_dir, "Scripts", basename + ".exe")
+        else:
+            path = osp.join(self.env_dir, "bin", basename)
+        if osp.isfile(path) or osp.islink(path):
+            return path
+        raise RuntimeError(f"no regular file or symlink {path!r}")