| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 63fd1e7 commit 0add016
13 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -4,7 +4,7 @@ on: [push, pull_request, workflow_dispatch] | |||
| 4 | 4 | ||
| 5 | 5 | jobs: | |
| 6 | 6 | build: | |
| 7 | - runs-on: windows-latest | ||
| 7 | + runs-on: windows-2022 | ||
| 8 | 8 | strategy: | |
| 9 | 9 | fail-fast: false | |
| 10 | 10 | env: | |
@@ -61,4 +61,5 @@ jobs: | |||
| 61 | 61 | - name: Test with pytest | |
| 62 | 62 | run: | | |
| 63 | 63 | set +x | |
| 64 | + git config --global --add safe.directory '*' | ||
| 64 | 65 | /usr/bin/python -m pytest | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -4,11 +4,11 @@ on: [push, pull_request, workflow_dispatch] | |||
| 4 | 4 | ||
| 5 | 5 | jobs: | |
| 6 | 6 | lint: | |
| 7 | - runs-on: ubuntu-latest | ||
| 7 | + runs-on: ubuntu-22.04 | ||
| 8 | 8 | ||
| 9 | 9 | steps: | |
| 10 | 10 | - uses: actions/checkout@v4 | |
| 11 | - - uses: actions/setup-python@v4 | ||
| 11 | + - uses: MatteoH2O1999/setup-python@v4 | ||
| 12 | 12 | with: | |
| 13 | 13 | python-version: "3.x" | |
| 14 | 14 | - uses: pre-commit/action@v3.0.0 | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -11,7 +11,7 @@ permissions: | |||
| 11 | 11 | jobs: | |
| 12 | 12 | build: | |
| 13 | 13 | ||
| 14 | - runs-on: ubuntu-latest | ||
| 14 | + runs-on: ubuntu-22.04 | ||
| 15 | 15 | strategy: | |
| 16 | 16 | fail-fast: false | |
| 17 | 17 | matrix: | |
@@ -31,7 +31,7 @@ jobs: | |||
| 31 | 31 | submodules: recursive | |
| 32 | 32 | ||
| 33 | 33 | - name: Set up Python ${{ matrix.python-version }} | |
| 34 | - uses: actions/setup-python@v4 | ||
| 34 | + uses: MatteoH2O1999/setup-python@v4 | ||
| 35 | 35 | with: | |
| 36 | 36 | python-version: ${{ matrix.python-version }} | |
| 37 | 37 | allow-prereleases: ${{ matrix.experimental }} | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -9,7 +9,7 @@ repos: | |||
| 9 | 9 | flake8-comprehensions==3.14.0, | |
| 10 | 10 | flake8-typing-imports==1.14.0, | |
| 11 | 11 | ] | |
| 12 | - exclude: ^doc|^git/ext/ | ||
| 12 | + exclude: ^test/fixtures/polyglot$|^doc|^git/ext/ | ||
| 13 | 13 | ||
| 14 | 14 | - repo: https://github.com/pre-commit/pre-commit-hooks | |
| 15 | 15 | rev: v4.4.0 | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1 +1 @@ | |||
| 1 | - 3.1.37 | ||
| 1 | + 3.1.37+sp1 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -43,6 +43,7 @@ | |||
| 43 | 43 | Iterator, | |
| 44 | 44 | List, | |
| 45 | 45 | Mapping, | |
| 46 | + Optional, | ||
| 46 | 47 | Sequence, | |
| 47 | 48 | TYPE_CHECKING, | |
| 48 | 49 | TextIO, | |
@@ -99,7 +100,7 @@ def handle_process_output( | |||
| 99 | 100 | Callable[[bytes, "Repo", "DiffIndex"], None], | |
| 100 | 101 | ], | |
| 101 | 102 | stderr_handler: Union[None, Callable[[AnyStr], None], Callable[[List[AnyStr]], None]], | |
| 102 | - finalizer: Union[None, Callable[[Union[subprocess.Popen, "Git.AutoInterrupt"]], None]] = None, | ||
| 103 | + finalizer: Union[None, Callable[[Union[Popen, "Git.AutoInterrupt"]], None]] = None, | ||
| 103 | 104 | decode_streams: bool = True, | |
| 104 | 105 | kill_after_timeout: Union[None, float] = None, | |
| 105 | 106 | ) -> None: | |
@@ -207,6 +208,68 @@ def pump_stream( | |||
| 207 | 208 | return None | |
| 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,16 +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 | - # value of Windows process creation flag taken from MSDN | ||
| 229 | - CREATE_NO_WINDOW = 0x08000000 | ||
| 230 | - | ||
| 231 | - ## CREATE_NEW_PROCESS_GROUP is needed to allow killing it afterwards, | ||
| 232 | - # see https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal | ||
| 233 | - PROC_CREATIONFLAGS = ( | ||
| 234 | - CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP if is_win else 0 # type: ignore[attr-defined] | ||
| 235 | - ) # mypy error if not windows | ||
| 236 | - | ||
| 237 | - | ||
| 238 | 291 | class Git(LazyMixin): | |
| 239 | 292 | ||
| 240 | 293 | """ | |
@@ -963,11 +1016,8 @@ def execute( | |||
| 963 | 1016 | redacted_command, | |
| 964 | 1017 | '"kill_after_timeout" feature is not supported on Windows.', | |
| 965 | 1018 | ) | |
| 966 | - # Only search PATH, not CWD. This must be in the *caller* environment. The "1" can be any value. | ||
| 967 | - maybe_patch_caller_env = patch_env("NoDefaultCurrentDirectoryInExePath", "1") | ||
| 968 | 1019 | else: | |
| 969 | 1020 | cmd_not_found_exception = FileNotFoundError # NOQA # exists, flake8 unknown @UndefinedVariable | |
| 970 | - maybe_patch_caller_env = contextlib.nullcontext() | ||
| 971 | 1021 | # end handle | |
| 972 | 1022 | ||
| 973 | 1023 | stdout_sink = PIPE if with_stdout else getattr(subprocess, "DEVNULL", None) or open(os.devnull, "wb") | |
@@ -983,21 +1033,18 @@ def execute( | |||
| 983 | 1033 | istream_ok, | |
| 984 | 1034 | ) | |
| 985 | 1035 | try: | |
| 986 | - with maybe_patch_caller_env: | ||
| 987 | - proc = Popen( | ||
| 988 | - command, | ||
| 989 | - env=env, | ||
| 990 | - cwd=cwd, | ||
| 991 | - bufsize=-1, | ||
| 992 | - stdin=istream or DEVNULL, | ||
| 993 | - stderr=PIPE, | ||
| 994 | - stdout=stdout_sink, | ||
| 995 | - shell=shell is not None and shell or self.USE_SHELL, | ||
| 996 | - close_fds=is_posix, # unsupported on windows | ||
| 997 | - universal_newlines=universal_newlines, | ||
| 998 | - creationflags=PROC_CREATIONFLAGS, | ||
| 999 | - **subprocess_kwargs, | ||
| 1000 | - ) | ||
| 1036 | + proc = safer_popen( | ||
| 1037 | + command, | ||
| 1038 | + env=env, | ||
| 1039 | + cwd=cwd, | ||
| 1040 | + bufsize=-1, | ||
| 1041 | + stdin=(istream or DEVNULL), | ||
| 1042 | + stderr=PIPE, | ||
| 1043 | + stdout=stdout_sink, | ||
| 1044 | + shell=shell, | ||
| 1045 | + universal_newlines=universal_newlines, | ||
| 1046 | + **subprocess_kwargs, | ||
| 1047 | + ) | ||
| 1001 | 1048 | except cmd_not_found_exception as err: | |
| 1002 | 1049 | raise GitCommandNotFound(redacted_command, err) from err | |
| 1003 | 1050 | else: | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -16,7 +16,7 @@ | |||
| 16 | 16 | ) | |
| 17 | 17 | import subprocess | |
| 18 | 18 | ||
| 19 | - from git.cmd import PROC_CREATIONFLAGS, handle_process_output | ||
| 19 | + from git.cmd import handle_process_output, safer_popen | ||
| 20 | 20 | from git.compat import ( | |
| 21 | 21 | defenc, | |
| 22 | 22 | force_text, | |
@@ -102,14 +102,14 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: | |||
| 102 | 102 | relative_hp = Path(hp).relative_to(index.repo.working_dir).as_posix() | |
| 103 | 103 | cmd = ["bash.exe", relative_hp] | |
| 104 | 104 | ||
| 105 | - process = subprocess.Popen( | ||
| 105 | + # process = subprocess.Popen( | ||
| 106 | + process = safer_popen( | ||
| 106 | 107 | cmd + list(args), | |
| 107 | 108 | env=env, | |
| 108 | 109 | stdout=subprocess.PIPE, | |
| 109 | 110 | stderr=subprocess.PIPE, | |
| 110 | 111 | cwd=index.repo.working_dir, | |
| 111 | 112 | close_fds=is_posix, | |
| 112 | - creationflags=PROC_CREATIONFLAGS, | ||
| 113 | 113 | ) | |
| 114 | 114 | except Exception as ex: | |
| 115 | 115 | raise HookExecutionError(hp, ex) from ex | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -291,6 +291,17 @@ def _get_exe_extensions() -> Sequence[str]: | |||
| 291 | 291 | ||
| 292 | 292 | ||
| 293 | 293 | def py_where(program: str, path: Optional[PathLike] = None) -> List[str]: | |
| 294 | + """Perform a path search to assist :func:`is_cygwin_git`. | ||
| 295 | + | ||
| 296 | + This is not robust for general use. It is an implementation detail of | ||
| 297 | + :func:`is_cygwin_git`. When a search following all shell rules is needed, | ||
| 298 | + :func:`shutil.which` can be used instead. | ||
| 299 | + | ||
| 300 | + :note: Neither this function nor :func:`shutil.which` will predict the effect of an | ||
| 301 | + executable search on a native Windows system due to a :class:`subprocess.Popen` | ||
| 302 | + call without ``shell=True``, because shell and non-shell executable search on | ||
| 303 | + Windows differ considerably. | ||
| 304 | + """ | ||
| 294 | 305 | # From: http://stackoverflow.com/a/377028/548792 | |
| 295 | 306 | winprog_exts = _get_exe_extensions() | |
| 296 | 307 | ||
| 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 | |
|---|---|---|---|
@@ -13,6 +13,7 @@ | |||
| 13 | 13 | import textwrap | |
| 14 | 14 | import time | |
| 15 | 15 | import unittest | |
| 16 | + import venv | ||
| 16 | 17 | ||
| 17 | 18 | from git.compat import is_win | |
| 18 | 19 | from git.util import rmtree, cwd | |
@@ -38,6 +39,7 @@ | |||
| 38 | 39 | "with_rw_repo", | |
| 39 | 40 | "with_rw_and_rw_remote_repo", | |
| 40 | 41 | "TestBase", | |
| 42 | + "VirtualEnvironment", | ||
| 41 | 43 | "TestCase", | |
| 42 | 44 | "SkipTest", | |
| 43 | 45 | "skipIf", | |
@@ -89,12 +91,12 @@ def with_rw_directory(func): | |||
| 89 | 91 | test succeeds, but leave it otherwise to aid additional debugging""" | |
| 90 | 92 | ||
| 91 | 93 | @wraps(func) | |
| 92 | - def wrapper(self): | ||
| 94 | + def wrapper(self, *args, **kwargs): | ||
| 93 | 95 | path = tempfile.mktemp(prefix=func.__name__) | |
| 94 | 96 | os.mkdir(path) | |
| 95 | 97 | keep = False | |
| 96 | 98 | try: | |
| 97 | - return func(self, path) | ||
| 99 | + return func(self, path, *args, **kwargs) | ||
| 98 | 100 | except Exception: | |
| 99 | 101 | log.info( | |
| 100 | 102 | "Test %s.%s failed, output is at %r\n", | |
@@ -401,3 +403,46 @@ def _make_file(self, rela_path, data, repo=None): | |||
| 401 | 403 | with open(abs_path, "w") as fp: | |
| 402 | 404 | fp.write(data) | |
| 403 | 405 | return abs_path | |
| 406 | + | ||
| 407 | + | ||
| 408 | + class VirtualEnvironment: | ||
| 409 | + """A newly created Python virtual environment for use in a test.""" | ||
| 410 | + | ||
| 411 | + __slots__ = ("_env_dir",) | ||
| 412 | + | ||
| 413 | + def __init__(self, env_dir, *, with_pip): | ||
| 414 | + if os.name == "nt": | ||
| 415 | + self._env_dir = osp.realpath(env_dir) | ||
| 416 | + venv.create(self.env_dir, symlinks=False, with_pip=with_pip) | ||
| 417 | + else: | ||
| 418 | + self._env_dir = env_dir | ||
| 419 | + venv.create(self.env_dir, symlinks=True, with_pip=with_pip) | ||
| 420 | + | ||
| 421 | + @property | ||
| 422 | + def env_dir(self): | ||
| 423 | + """The top-level directory of the environment.""" | ||
| 424 | + return self._env_dir | ||
| 425 | + | ||
| 426 | + @property | ||
| 427 | + def python(self): | ||
| 428 | + """Path to the Python executable in the environment.""" | ||
| 429 | + return self._executable("python") | ||
| 430 | + | ||
| 431 | + @property | ||
| 432 | + def pip(self): | ||
| 433 | + """Path to the pip executable in the environment, or RuntimeError if absent.""" | ||
| 434 | + return self._executable("pip") | ||
| 435 | + | ||
| 436 | + @property | ||
| 437 | + def sources(self): | ||
| 438 | + """Path to a src directory in the environment, which may not exist yet.""" | ||
| 439 | + return os.path.join(self.env_dir, "src") | ||
| 440 | + | ||
| 441 | + def _executable(self, basename): | ||
| 442 | + if os.name == "nt": | ||
| 443 | + path = osp.join(self.env_dir, "Scripts", basename + ".exe") | ||
| 444 | + else: | ||
| 445 | + path = osp.join(self.env_dir, "bin", basename) | ||
| 446 | + if osp.isfile(path) or osp.islink(path): | ||
| 447 | + return path | ||
| 448 | + raise RuntimeError(f"no regular file or symlink {path!r}") | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments