Skip to content

Commit a42ea0a

Browse files
committed
Cover absent/no-distro bash.exe in hooks "not from cwd" test
See comments for details on the test's new implementation and what it achieves.
1 parent 7751436 commit a42ea0a

File tree

3 files changed

+49
-24
lines changed

3 files changed

+49
-24
lines changed

.pre-commit-config.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ repos:
2929
hooks:
3030
- id: shellcheck
3131
args: [--color]
32-
exclude: ^git/ext/
32+
exclude: ^test/fixtures/polyglot$|^git/ext/
3333

3434
- repo: https://github.com/pre-commit/pre-commit-hooks
3535
rev: v4.4.0

test/fixtures/polyglot

+8
Original file line numberDiff line numberDiff 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')

test/test_index.py

+40-23
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,14 @@
4141
from git.objects import Blob
4242
from git.util import Actor, cwd, hex_to_bin, rmtree
4343
from gitdb.base import IStream
44-
from test.lib import TestBase, fixture, fixture_path, with_rw_directory, with_rw_repo
44+
from test.lib import (
45+
TestBase,
46+
VirtualEnvironment,
47+
fixture,
48+
fixture_path,
49+
with_rw_directory,
50+
with_rw_repo,
51+
)
4552

4653
HOOKS_SHEBANG = "#!/usr/bin/env sh\n"
4754

@@ -1016,36 +1023,46 @@ def test_run_commit_hook(self, rw_repo):
10161023
output = Path(rw_repo.git_dir, "output.txt").read_text(encoding="utf-8")
10171024
self.assertEqual(output, "ran fake hook\n")
10181025

1019-
# FIXME: Figure out a way to make this test also work with Absent and WslNoDistro.
1020-
@pytest.mark.xfail(
1021-
type(_win_bash_status) is WinBashStatus.WslNoDistro,
1022-
reason="Currently uses the bash.exe of WSL, even with no WSL distro installed",
1023-
raises=HookExecutionError,
1024-
)
10251026
@ddt.data((False,), (True,))
10261027
@with_rw_directory
10271028
def test_hook_uses_shell_not_from_cwd(self, rw_dir, case):
10281029
(chdir_to_repo,) = case
10291030

1031+
shell_name = "bash.exe" if os.name == "nt" else "sh"
1032+
maybe_chdir = cwd(rw_dir) if chdir_to_repo else contextlib.nullcontext()
10301033
repo = Repo.init(rw_dir)
1031-
_make_hook(repo.git_dir, "fake-hook", "echo 'ran fake hook' >output.txt")
10321034

1033-
if os.name == "nt":
1034-
# Copy an actual binary that is not bash.
1035-
other_exe_path = Path(os.environ["SystemRoot"], "system32", "hostname.exe")
1036-
impostor_path = Path(rw_dir, "bash.exe")
1037-
shutil.copy(other_exe_path, impostor_path)
1035+
# We need an impostor shell that works on Windows and that can be distinguished
1036+
# from the real bash.exe. But even if the real bash.exe is absent or unusable,
1037+
# we should verify that the impostor is not run. So the impostor needs a clear
1038+
# side effect (unlike in TestGit.test_it_executes_git_not_from_cwd). Popen on
1039+
# Windows uses CreateProcessW, which disregards PATHEXT; the impostor may need
1040+
# to be a binary executable to ensure the vulnerability is found if present. No
1041+
# compiler need exist, shipping a binary in the test suite may target the wrong
1042+
# architecture, and generating one in a bespoke way may cause virus scanners to
1043+
# give a false positive. So we use a Bash/Python polyglot for the hook and use
1044+
# the Python interpreter itself as the bash.exe impostor. But an interpreter
1045+
# from a venv may not run outside of it, and a global interpreter won't run from
1046+
# a different location if it was installed from the Microsoft Store. So we make
1047+
# a new venv in rw_dir and use its interpreter.
1048+
venv = VirtualEnvironment(rw_dir, with_pip=False)
1049+
shutil.copy(venv.python, Path(rw_dir, shell_name))
1050+
shutil.copy(fixture_path("polyglot"), hook_path("polyglot", repo.git_dir))
1051+
payload = Path(rw_dir, "payload.txt")
1052+
1053+
if type(_win_bash_status) in {WinBashStatus.Absent, WinBashStatus.WslNoDistro}:
1054+
# The real shell can't run, but the impostor should still not be used.
1055+
with self.assertRaises(HookExecutionError):
1056+
with maybe_chdir:
1057+
run_commit_hook("polyglot", repo.index)
1058+
self.assertFalse(payload.exists())
10381059
else:
1039-
# Create a shell script that doesn't do anything.
1040-
impostor_path = Path(rw_dir, "sh")
1041-
impostor_path.write_text("#!/bin/sh\n", encoding="utf-8")
1042-
os.chmod(impostor_path, 0o755)
1043-
1044-
with cwd(rw_dir) if chdir_to_repo else contextlib.nullcontext():
1045-
run_commit_hook("fake-hook", repo.index)
1046-
1047-
output = Path(rw_dir, "output.txt").read_text(encoding="utf-8")
1048-
self.assertEqual(output, "ran fake hook\n")
1060+
# The real shell should run, and not the impostor.
1061+
with maybe_chdir:
1062+
run_commit_hook("polyglot", repo.index)
1063+
self.assertFalse(payload.exists())
1064+
output = Path(rw_dir, "output.txt").read_text(encoding="utf-8")
1065+
self.assertEqual(output, "Ran intended hook.\n")
10491066

10501067
@pytest.mark.xfail(
10511068
type(_win_bash_status) is WinBashStatus.Absent,

0 commit comments

Comments
 (0)