| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -29,7 +29,7 @@ repos: | |||
| 29 | 29 | hooks: | |
| 30 | 30 | - id: shellcheck | |
| 31 | 31 | args: [--color] | |
| 32 | - exclude: ^git/ext/ | ||
| 32 | + exclude: ^test/fixtures/polyglot$|^git/ext/ | ||
| 33 | 33 | ||
| 34 | 34 | - repo: https://github.com/pre-commit/pre-commit-hooks | |
| 35 | 35 | rev: v4.4.0 | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -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 | ||
@@ -225,14 +288,6 @@ def dict_to_slots_and__excluded_are_none(self: object, d: Mapping[str, Any], exc | |||
| 225 | 288 | ## -- End Utilities -- @} | |
| 226 | 289 | ||
| 227 | 290 | ||
| 228 | - if os.name == "nt": | ||
| 229 | - # CREATE_NEW_PROCESS_GROUP is needed to allow killing it afterwards. See: | ||
| 230 | - # https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal | ||
| 231 | - PROC_CREATIONFLAGS = subprocess.CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP | ||
| 232 | - else: | ||
| 233 | - PROC_CREATIONFLAGS = 0 | ||
| 234 | - | ||
| 235 | - | ||
| 236 | 291 | class Git(LazyMixin): | |
| 237 | 292 | """The Git class manages communication with the Git binary. | |
| 238 | 293 | ||
@@ -992,11 +1047,8 @@ def execute( | |||
| 992 | 1047 | redacted_command, | |
| 993 | 1048 | '"kill_after_timeout" feature is not supported on Windows.', | |
| 994 | 1049 | ) | |
| 995 | - # Only search PATH, not CWD. This must be in the *caller* environment. The "1" can be any value. | ||
| 996 | - maybe_patch_caller_env = patch_env("NoDefaultCurrentDirectoryInExePath", "1") | ||
| 997 | 1050 | else: | |
| 998 | 1051 | cmd_not_found_exception = FileNotFoundError | |
| 999 | - maybe_patch_caller_env = contextlib.nullcontext() | ||
| 1000 | 1052 | # END handle | |
| 1001 | 1053 | ||
| 1002 | 1054 | stdout_sink = PIPE if with_stdout else getattr(subprocess, "DEVNULL", None) or open(os.devnull, "wb") | |
@@ -1011,20 +1063,18 @@ def execute( | |||
| 1011 | 1063 | universal_newlines, | |
| 1012 | 1064 | ) | |
| 1013 | 1065 | try: | |
| 1014 | - with maybe_patch_caller_env: | ||
| 1015 | - proc = Popen( | ||
| 1016 | - command, | ||
| 1017 | - env=env, | ||
| 1018 | - cwd=cwd, | ||
| 1019 | - bufsize=-1, | ||
| 1020 | - stdin=(istream or DEVNULL), | ||
| 1021 | - stderr=PIPE, | ||
| 1022 | - stdout=stdout_sink, | ||
| 1023 | - shell=shell, | ||
| 1024 | - universal_newlines=universal_newlines, | ||
| 1025 | - creationflags=PROC_CREATIONFLAGS, | ||
| 1026 | - **subprocess_kwargs, | ||
| 1027 | - ) | ||
| 1066 | + proc = safer_popen( | ||
| 1067 | + command, | ||
| 1068 | + env=env, | ||
| 1069 | + cwd=cwd, | ||
| 1070 | + bufsize=-1, | ||
| 1071 | + stdin=(istream or DEVNULL), | ||
| 1072 | + stderr=PIPE, | ||
| 1073 | + stdout=stdout_sink, | ||
| 1074 | + shell=shell, | ||
| 1075 | + universal_newlines=universal_newlines, | ||
| 1076 | + **subprocess_kwargs, | ||
| 1077 | + ) | ||
| 1028 | 1078 | except cmd_not_found_exception as err: | |
| 1029 | 1079 | raise GitCommandNotFound(redacted_command, err) from err | |
| 1030 | 1080 | else: | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -18,7 +18,7 @@ | |||
| 18 | 18 | ) | |
| 19 | 19 | import subprocess | |
| 20 | 20 | ||
| 21 | - from git.cmd import PROC_CREATIONFLAGS, handle_process_output | ||
| 21 | + from git.cmd import handle_process_output, safer_popen | ||
| 22 | 22 | from git.compat import defenc, force_bytes, force_text, safe_decode | |
| 23 | 23 | from git.exc import HookExecutionError, UnmergedEntriesError | |
| 24 | 24 | from git.objects.fun import ( | |
@@ -98,13 +98,12 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: | |||
| 98 | 98 | relative_hp = Path(hp).relative_to(index.repo.working_dir).as_posix() | |
| 99 | 99 | cmd = ["bash.exe", relative_hp] | |
| 100 | 100 | ||
| 101 | - process = subprocess.Popen( | ||
| 101 | + process = safer_popen( | ||
| 102 | 102 | cmd + list(args), | |
| 103 | 103 | env=env, | |
| 104 | 104 | stdout=subprocess.PIPE, | |
| 105 | 105 | stderr=subprocess.PIPE, | |
| 106 | 106 | cwd=index.repo.working_dir, | |
| 107 | - creationflags=PROC_CREATIONFLAGS, | ||
| 108 | 107 | ) | |
| 109 | 108 | except Exception as ex: | |
| 110 | 109 | raise HookExecutionError(hp, ex) from ex | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -327,6 +327,17 @@ def _get_exe_extensions() -> Sequence[str]: | |||
| 327 | 327 | ||
| 328 | 328 | ||
| 329 | 329 | def py_where(program: str, path: Optional[PathLike] = None) -> List[str]: | |
| 330 | + """Perform a path search to assist :func:`is_cygwin_git`. | ||
| 331 | + | ||
| 332 | + This is not robust for general use. It is an implementation detail of | ||
| 333 | + :func:`is_cygwin_git`. When a search following all shell rules is needed, | ||
| 334 | + :func:`shutil.which` can be used instead. | ||
| 335 | + | ||
| 336 | + :note: Neither this function nor :func:`shutil.which` will predict the effect of an | ||
| 337 | + executable search on a native Windows system due to a :class:`subprocess.Popen` | ||
| 338 | + call without ``shell=True``, because shell and non-shell executable search on | ||
| 339 | + Windows differ considerably. | ||
| 340 | + """ | ||
| 330 | 341 | # From: http://stackoverflow.com/a/377028/548792 | |
| 331 | 342 | winprog_exts = _get_exe_extensions() | |
| 332 | 343 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,8 @@ | |||
| 1 | + #!/usr/bin/env sh | ||
| 2 | + # Valid script in both Bash and Python, but with different behavior. | ||
| 3 | + """:" | ||
| 4 | + echo 'Ran intended hook.' >output.txt | ||
| 5 | + exit | ||
| 6 | + " """ | ||
| 7 | + from pathlib import Path | ||
| 8 | + Path('payload.txt').write_text('Ran impostor hook!', encoding='utf-8') | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -14,6 +14,7 @@ | |||
| 14 | 14 | import textwrap | |
| 15 | 15 | import time | |
| 16 | 16 | import unittest | |
| 17 | + import venv | ||
| 17 | 18 | ||
| 18 | 19 | import gitdb | |
| 19 | 20 | ||
@@ -36,6 +37,7 @@ | |||
| 36 | 37 | "with_rw_repo", | |
| 37 | 38 | "with_rw_and_rw_remote_repo", | |
| 38 | 39 | "TestBase", | |
| 40 | + "VirtualEnvironment", | ||
| 39 | 41 | "TestCase", | |
| 40 | 42 | "SkipTest", | |
| 41 | 43 | "skipIf", | |
@@ -88,11 +90,11 @@ def with_rw_directory(func): | |||
| 88 | 90 | test succeeds, but leave it otherwise to aid additional debugging.""" | |
| 89 | 91 | ||
| 90 | 92 | @wraps(func) | |
| 91 | - def wrapper(self): | ||
| 93 | + def wrapper(self, *args, **kwargs): | ||
| 92 | 94 | path = tempfile.mkdtemp(prefix=func.__name__) | |
| 93 | 95 | keep = False | |
| 94 | 96 | try: | |
| 95 | - return func(self, path) | ||
| 97 | + return func(self, path, *args, **kwargs) | ||
| 96 | 98 | except Exception: | |
| 97 | 99 | log.info( | |
| 98 | 100 | "Test %s.%s failed, output is at %r\n", | |
@@ -390,3 +392,46 @@ def _make_file(self, rela_path, data, repo=None): | |||
| 390 | 392 | with open(abs_path, "w") as fp: | |
| 391 | 393 | fp.write(data) | |
| 392 | 394 | return abs_path | |
| 395 | + | ||
| 396 | + | ||
| 397 | + class VirtualEnvironment: | ||
| 398 | + """A newly created Python virtual environment for use in a test.""" | ||
| 399 | + | ||
| 400 | + __slots__ = ("_env_dir",) | ||
| 401 | + | ||
| 402 | + def __init__(self, env_dir, *, with_pip): | ||
| 403 | + if os.name == "nt": | ||
| 404 | + self._env_dir = osp.realpath(env_dir) | ||
| 405 | + venv.create(self.env_dir, symlinks=False, with_pip=with_pip) | ||
| 406 | + else: | ||
| 407 | + self._env_dir = env_dir | ||
| 408 | + venv.create(self.env_dir, symlinks=True, with_pip=with_pip) | ||
| 409 | + | ||
| 410 | + @property | ||
| 411 | + def env_dir(self): | ||
| 412 | + """The top-level directory of the environment.""" | ||
| 413 | + return self._env_dir | ||
| 414 | + | ||
| 415 | + @property | ||
| 416 | + def python(self): | ||
| 417 | + """Path to the Python executable in the environment.""" | ||
| 418 | + return self._executable("python") | ||
| 419 | + | ||
| 420 | + @property | ||
| 421 | + def pip(self): | ||
| 422 | + """Path to the pip executable in the environment, or RuntimeError if absent.""" | ||
| 423 | + return self._executable("pip") | ||
| 424 | + | ||
| 425 | + @property | ||
| 426 | + def sources(self): | ||
| 427 | + """Path to a src directory in the environment, which may not exist yet.""" | ||
| 428 | + return os.path.join(self.env_dir, "src") | ||
| 429 | + | ||
| 430 | + def _executable(self, basename): | ||
| 431 | + if os.name == "nt": | ||
| 432 | + path = osp.join(self.env_dir, "Scripts", basename + ".exe") | ||
| 433 | + else: | ||
| 434 | + path = osp.join(self.env_dir, "bin", basename) | ||
| 435 | + if osp.isfile(path) or osp.islink(path): | ||
| 436 | + return path | ||
| 437 | + raise RuntimeError(f"no regular file or symlink {path!r}") | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments