| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
7 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -35,7 +35,7 @@ | |||
| 35 | 35 | is_win, | |
| 36 | 36 | ) | |
| 37 | 37 | from git.exc import CommandError, UnsafeOptionError, UnsafeProtocolError | |
| 38 | - from git.util import is_cygwin_git, cygpath, expand_path | ||
| 38 | + from git.util import is_cygwin_git, cygpath, expand_path, patch_env | ||
| 39 | 39 | ||
| 40 | 40 | from .exc import ( | |
| 41 | 41 | GitCommandError, | |
@@ -132,7 +132,7 @@ def pump_stream(cmdline, name, stream, is_decode, handler): | |||
| 132 | 132 | return finalizer(process) | |
| 133 | 133 | ||
| 134 | 134 | ||
| 135 | - def _safer_popen_windows(command, shell, env=None, **kwargs): | ||
| 135 | + def _safer_popen_windows(command, shell=False, env=None, **kwargs): | ||
| 136 | 136 | """Call :class:`subprocess.Popen` on Windows but don't include a CWD in the search. | |
| 137 | 137 | This avoids an untrusted search path condition where a file like ``git.exe`` in a | |
| 138 | 138 | malicious repository would be run when GitPython operates on the repository. The | |
@@ -154,15 +154,13 @@ def _safer_popen_windows(command, shell, env=None, **kwargs): | |||
| 154 | 154 | # https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal | |
| 155 | 155 | # https://docs.python.org/3/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP | |
| 156 | 156 | creationflags = subprocess.CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP | |
| 157 | - | ||
| 158 | 157 | # When using a shell, the shell is the direct subprocess, so the variable must be | |
| 159 | 158 | # set in its environment, to affect its search behavior. (The "1" can be any value.) | |
| 160 | 159 | if shell: | |
| 161 | 160 | safer_env = {} if env is None else dict(env) | |
| 162 | 161 | safer_env["NoDefaultCurrentDirectoryInExePath"] = "1" | |
| 163 | 162 | else: | |
| 164 | 163 | safer_env = env | |
| 165 | - | ||
| 166 | 164 | # When not using a shell, the current process does the search in a CreateProcessW | |
| 167 | 165 | # API call, so the variable must be set in our environment. With a shell, this is | |
| 168 | 166 | # unnecessary, in versions where https://github.com/python/cpython/issues/101283 is | |
@@ -185,6 +183,12 @@ def _safer_popen_windows(command, shell, env=None, **kwargs): | |||
| 185 | 183 | safer_popen = Popen | |
| 186 | 184 | ||
| 187 | 185 | ||
| 186 | + if os.name == "nt": | ||
| 187 | + safer_popen = _safer_popen_windows | ||
| 188 | + else: | ||
| 189 | + safer_popen = Popen | ||
| 190 | + | ||
| 191 | + | ||
| 188 | 192 | def dashify(string): | |
| 189 | 193 | return string.replace('_', '-') | |
| 190 | 194 | ||
@@ -801,15 +805,11 @@ def execute(self, command, | |||
| 801 | 805 | cmd_not_found_exception = OSError | |
| 802 | 806 | if kill_after_timeout: | |
| 803 | 807 | raise GitCommandError(command, '"kill_after_timeout" feature is not supported on Windows.') | |
| 804 | - | ||
| 805 | - # Only search PATH, not CWD. This must be in the *caller* environment. The "1" can be any value. | ||
| 806 | - patch_caller_env = unittest.mock.patch.dict(os.environ, {"NoDefaultCurrentDirectoryInExePath": "1"}) | ||
| 807 | 808 | else: | |
| 808 | 809 | if sys.version_info[0] > 2: | |
| 809 | 810 | cmd_not_found_exception = FileNotFoundError # NOQA # exists, flake8 unknown @UndefinedVariable | |
| 810 | 811 | else: | |
| 811 | 812 | cmd_not_found_exception = OSError | |
| 812 | - patch_caller_env = nullcontext() | ||
| 813 | 813 | # end handle | |
| 814 | 814 | ||
| 815 | 815 | stdout_sink = (PIPE | |
@@ -829,7 +829,7 @@ def execute(self, command, | |||
| 829 | 829 | stdin=istream, | |
| 830 | 830 | stderr=PIPE, | |
| 831 | 831 | stdout=stdout_sink, | |
| 832 | - shell=shell is not None and shell or self.USE_SHELL, | ||
| 832 | + shell=shell, | ||
| 833 | 833 | universal_newlines=universal_newlines, | |
| 834 | 834 | **subprocess_kwargs | |
| 835 | 835 | ) | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -74,11 +74,11 @@ def run_commit_hook(name, index, *args): | |||
| 74 | 74 | env["GIT_EDITOR"] = ":" | |
| 75 | 75 | try: | |
| 76 | 76 | cmd = safer_popen([hp] + list(args), | |
| 77 | - env=env, | ||
| 78 | - stdout=subprocess.PIPE, | ||
| 79 | - stderr=subprocess.PIPE, | ||
| 80 | - cwd=index.repo.working_dir, | ||
| 81 | - close_fds=is_posix) | ||
| 77 | + env=env, | ||
| 78 | + stdout=subprocess.PIPE, | ||
| 79 | + stderr=subprocess.PIPE, | ||
| 80 | + cwd=index.repo.working_dir, | ||
| 81 | + close_fds=is_posix) | ||
| 82 | 82 | except Exception as ex: | |
| 83 | 83 | raise HookExecutionError(hp, ex) | |
| 84 | 84 | else: | |
| 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 | |
|---|---|---|---|
@@ -6,7 +6,8 @@ | |||
| 6 | 6 | # the BSD License: http://www.opensource.org/licenses/bsd-license.php | |
| 7 | 7 | from __future__ import print_function | |
| 8 | 8 | ||
| 9 | - import contextlib | ||
| 9 | + import contextlib2 as contextlib | ||
| 10 | + import ddt | ||
| 10 | 11 | import os | |
| 11 | 12 | import shutil | |
| 12 | 13 | import subprocess | |
@@ -31,10 +32,11 @@ | |||
| 31 | 32 | assert_equal, | |
| 32 | 33 | assert_true, | |
| 33 | 34 | assert_match, | |
| 34 | - fixture_path | ||
| 35 | + fixture_path, | ||
| 36 | + with_rw_directory, | ||
| 35 | 37 | ) | |
| 36 | - from git.test.lib import with_rw_directory | ||
| 37 | - from git.util import finalize_process | ||
| 38 | + | ||
| 39 | + from git.util import finalize_process, cwd | ||
| 38 | 40 | ||
| 39 | 41 | import os.path as osp | |
| 40 | 42 | ||
@@ -51,6 +53,7 @@ | |||
| 51 | 53 | ||
| 52 | 54 | from git.compat import is_win | |
| 53 | 55 | ||
| 56 | + | ||
| 54 | 57 | @contextlib.contextmanager | |
| 55 | 58 | def _chdir(new_dir): | |
| 56 | 59 | """Context manager to temporarily change directory. Not reentrant.""" | |
@@ -77,7 +80,7 @@ def _patch_out_env(name): | |||
| 77 | 80 | os.environ[name] = old_value | |
| 78 | 81 | ||
| 79 | 82 | ||
| 80 | - | ||
| 83 | + @ddt.ddt | ||
| 81 | 84 | class TestGit(TestBase): | |
| 82 | 85 | ||
| 83 | 86 | @classmethod | |
@@ -137,23 +140,46 @@ def test_it_transforms_kwargs_into_git_command_arguments(self): | |||
| 137 | 140 | def test_it_executes_git_to_shell_and_returns_result(self): | |
| 138 | 141 | assert_match(r'^git version [\d\.]{2}.*$', self.git.execute(["git", "version"])) | |
| 139 | 142 | ||
| 140 | - def test_it_executes_git_not_from_cwd(self): | ||
| 141 | - with TemporaryDirectory() as tmpdir: | ||
| 142 | - if is_win: | ||
| 143 | - # Copy an actual binary executable that is not git. | ||
| 144 | - other_exe_path = os.path.join(os.getenv("WINDIR"), "system32", "hostname.exe") | ||
| 145 | - impostor_path = os.path.join(tmpdir, "git.exe") | ||
| 146 | - shutil.copy(other_exe_path, impostor_path) | ||
| 147 | - else: | ||
| 148 | - # Create a shell script that doesn't do anything. | ||
| 149 | - impostor_path = os.path.join(tmpdir, "git") | ||
| 150 | - with open(impostor_path, mode="w") as file: | ||
| 151 | - print("#!/bin/sh", file=file) | ||
| 152 | - os.chmod(impostor_path, 0o755) | ||
| 153 | - | ||
| 154 | - with _chdir(tmpdir): | ||
| 155 | - # six.assertRegex(self.git.execute(["git", "version"]).encode("UTF-8"), r"^git version\b") | ||
| 156 | - self.assertRegexpMatches(self.git.execute(["git", "version"]), r"^git version\b") | ||
| 143 | + @ddt.data( | ||
| 144 | + # chdir_to_repo, shell, command, use_shell_impostor | ||
| 145 | + (False, False, ["git", "version"], False), | ||
| 146 | + (False, True, "git version", False), | ||
| 147 | + (False, True, "git version", True), | ||
| 148 | + (True, False, ["git", "version"], False), | ||
| 149 | + (True, True, "git version", False), | ||
| 150 | + (True, True, "git version", True), | ||
| 151 | + ) | ||
| 152 | + def test_it_executes_git_not_from_cwd(self, case): | ||
| 153 | + self.internal_it_executes_git_not_from_cwd(case) | ||
| 154 | + | ||
| 155 | + @with_rw_directory | ||
| 156 | + def internal_it_executes_git_not_from_cwd(self, rw_dir, case): | ||
| 157 | + chdir_to_repo, shell, command, use_shell_impostor = case | ||
| 158 | + repo = Repo.init(rw_dir) | ||
| 159 | + if os.name == "nt": | ||
| 160 | + # Copy an actual binary executable that is not git. (On Windows, running | ||
| 161 | + # "hostname" only displays the hostname, it never tries to change it.) | ||
| 162 | + other_exe_path = Path(os.environ["SystemRoot"], "system32", "hostname.exe") | ||
| 163 | + impostor_path = Path(rw_dir, "git.exe") | ||
| 164 | + shutil.copy(other_exe_path, str(impostor_path)) | ||
| 165 | + else: | ||
| 166 | + # Create a shell script that doesn't do anything. | ||
| 167 | + impostor_path = Path(rw_dir, "git") | ||
| 168 | + impostor_path.write_text(u"#!/bin/sh\n", encoding="utf-8") | ||
| 169 | + os.chmod(str(impostor_path), 0o755) | ||
| 170 | + if use_shell_impostor: | ||
| 171 | + shell_name = "cmd.exe" if os.name == "nt" else "sh" | ||
| 172 | + shutil.copy(str(impostor_path), str(Path(rw_dir, shell_name))) | ||
| 173 | + with contextlib.ExitStack() as stack: | ||
| 174 | + if chdir_to_repo: | ||
| 175 | + stack.enter_context(cwd(rw_dir)) | ||
| 176 | + if use_shell_impostor: | ||
| 177 | + stack.enter_context(_patch_out_env("ComSpec")) | ||
| 178 | + # Run the command without raising an exception on failure, as the exception | ||
| 179 | + # message is currently misleading when the command is a string rather than a | ||
| 180 | + # sequence of strings (it really runs "git", but then wrongly reports "g"). | ||
| 181 | + output = repo.git.execute(command, with_exceptions=False, shell=shell) | ||
| 182 | + self.assertRegexpMatches(output, r"^git version\b") | ||
| 157 | 183 | ||
| 158 | 184 | def test_it_accepts_stdin(self): | |
| 159 | 185 | filename = fixture_path("cat_file_blob") | |
@@ -325,7 +351,7 @@ def test_environment(self, rw_dir): | |||
| 325 | 351 | self.assertIn('FOO', str(err)) | |
| 326 | 352 | ||
| 327 | 353 | def test_handle_process_output(self): | |
| 328 | - from git.cmd import handle_process_output | ||
| 354 | + from git.cmd import handle_process_output, safer_popen | ||
| 329 | 355 | ||
| 330 | 356 | line_count = 5002 | |
| 331 | 357 | count = [None, 0, 0] | |
@@ -337,13 +363,11 @@ def counter_stderr(line): | |||
| 337 | 363 | count[2] += 1 | |
| 338 | 364 | ||
| 339 | 365 | cmdline = [sys.executable, fixture_path('cat_file.py'), str(fixture_path('issue-301_stderr'))] | |
| 340 | - proc = subprocess.Popen(cmdline, | ||
| 341 | - stdin=None, | ||
| 342 | - stdout=subprocess.PIPE, | ||
| 343 | - stderr=subprocess.PIPE, | ||
| 344 | - shell=False, | ||
| 345 | - # creationflags=cmd.PROC_CREATIONFLAGS, | ||
| 346 | - ) | ||
| 366 | + proc = safer_popen(cmdline, | ||
| 367 | + stdin=None, | ||
| 368 | + stdout=subprocess.PIPE, | ||
| 369 | + stderr=subprocess.PIPE, | ||
| 370 | + shell=False) | ||
| 347 | 371 | ||
| 348 | 372 | handle_process_output(proc, counter_stdout, counter_stderr, finalize_process) | |
| 349 | 373 | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments