From 2321bcb5b25ef57ce98c78a4d5ad7d7b5c39c33d Mon Sep 17 00:00:00 2001 From: sashankh Date: Fri, 21 Aug 2026 02:54:18 +0530 Subject: [PATCH] test(hooks): probe for a bash that runs, not one that exists (#2915) `shutil.which("bash") is None` is not an availability check on Windows. Every Windows 10/11 install ships `C:\Windows\System32\bash.exe` - the WSL launcher - and System32 precedes Git Bash on PATH, so `which` always answers non-None. With no WSL distribution provisioned the launcher prints a UTF-16 notice and exits 1 without running the script, so the four allowlist tests failed on `bash exited 1` rather than skipping, while the working `C:\Program Files\Git\bin\bash.exe` further down PATH went unused. Walk every bash on PATH in order and probe each the same way the harness invokes it - a script file, run from the working directory - then take the first that answers. A probe that asked an easier question than the test (`bash --version`, say) would reintroduce exactly the mismatch #2641 fixed by baking the payload into a file. The interpreter is now passed as an absolute path; only argv[1:] goes through the MSYS re-parse that mangles backslashes, so the script argument stays a bare filename as before. On Windows without WSL this turns 2 failures into 4 tests that actually run against Git Bash; on Linux the first PATH hit still wins on the first probe. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/test_hooks.py | 105 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 98 insertions(+), 7 deletions(-) diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 3d8bcfc2ac..bb77d5158c 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -2,6 +2,8 @@ import os import shutil import subprocess +import sys +import tempfile from types import SimpleNamespace from pathlib import Path import pytest @@ -465,6 +467,93 @@ def _sh_single_quote(value: str) -> str: return "'" + value.replace("'", "'\\''") + "'" +def _iter_bash_on_path(): + """Yield every ``bash`` on PATH, in PATH order, deduplicated by real path. + + ``shutil.which`` only ever reports the first hit, which is the one entry we + specifically cannot trust on Windows. + """ + seen = set() + for entry in os.environ.get("PATH", "").split(os.pathsep): + if not entry: + continue + found = shutil.which("bash", path=entry) + if found is None: + continue + try: + key = os.path.normcase(os.path.realpath(found)) + except OSError: # pragma: no cover - defensive + key = os.path.normcase(found) + if key not in seen: + seen.add(key) + yield found + + +def _resolve_bash(): + """Return a bash that can actually run a script file, or ``None``. + + ``shutil.which("bash") is not None`` is not a usable guard on Windows. + Every Windows 10/11 install ships ``C:\\Windows\\System32\\bash.exe`` — the + WSL launcher — and it sits on PATH *ahead* of Git Bash. With no WSL + distribution installed it prints a UTF-16 notice and exits 1, so the + allowlist tests below died on ``bash exited 1`` instead of skipping, on a + machine that had a perfectly usable bash further down PATH. + + Probe each candidate exactly the way the harness invokes it — a script + file, run from the working directory — and take the first that works. That + keeps this guard honest for the same reason #2641 baked the payload into a + file: a probe that asks a different question than the test proves nothing. + """ + for exe in _iter_bash_on_path(): + with tempfile.TemporaryDirectory() as probe_dir: + probe = Path(probe_dir) / "probe.sh" + probe.write_text("printf ok\n", encoding="utf-8", newline="\n") + try: + result = subprocess.run( + [exe, probe.name], + capture_output=True, text=True, cwd=probe_dir, timeout=60, + ) + except (OSError, subprocess.SubprocessError): # pragma: no cover - env specific + continue + if result.returncode == 0 and result.stdout.strip() == "ok": + return exe + return None + + +_BASH = _resolve_bash() + +_requires_bash = pytest.mark.skipif( + _BASH is None, + reason="no bash on PATH can run a script file (on Windows System32\\bash.exe is the WSL " + "launcher, which fails when no distribution is installed)", +) + + +def test_resolve_bash_rejects_an_interpreter_that_cannot_run_the_probe(monkeypatch): + """Presence on PATH is not proof of a usable bash. + + ``C:\\Windows\\System32\\bash.exe`` exists on every Windows 10/11 install and + precedes Git Bash on PATH, but it is the WSL launcher: with no distribution + installed it exits 1 without running the script. A ``shutil.which`` guard + accepts it, and the allowlist tests below then die on ``bash exited 1`` + instead of skipping. ``sys.executable`` stands in for such a launcher here — + it exists and runs, but cannot execute a shell script. + """ + monkeypatch.setattr( + sys.modules[__name__], "_iter_bash_on_path", lambda: iter([sys.executable]) + ) + assert _resolve_bash() is None + + +@_requires_bash +def test_resolve_bash_skips_a_broken_candidate_and_keeps_looking(monkeypatch): + """A broken first hit must not mask a working bash further down PATH.""" + monkeypatch.setattr( + sys.modules[__name__], "_iter_bash_on_path", lambda: iter([sys.executable, _BASH]) + ) + assert _resolve_bash() == _BASH + + def _shell_verdict(pattern: str, candidate: str, tmp_path) -> str: """Run one `case` arm against ``candidate`` in a real bash, and report which branch matched. @@ -493,9 +582,11 @@ def _shell_verdict(pattern: str, candidate: str, tmp_path) -> str: newline="\n", ) # Invoke by bare filename from cwd: a Windows absolute path in argv would - # hit the same backslash mangling the script contents just avoided. + # hit the same backslash mangling the script contents just avoided. The + # interpreter itself is an absolute path because argv[0] is consumed by + # CreateProcess, not by the MSYS re-parse that mangles argv[1:]. result = subprocess.run( - ["bash", script.name], + [_BASH, script.name], capture_output=True, text=True, cwd=str(tmp_path), ) # Fail loudly on a malformed case snippet instead of returning "" and @@ -525,7 +616,7 @@ def _assert_harness_can_reject(pattern: str, tmp_path) -> None: ) -@pytest.mark.skipif(shutil.which("bash") is None, reason="bash required to exercise emitted glob") +@_requires_bash @pytest.mark.parametrize("winpath", [ r"C:\Users\u\.venv\Scripts\python.exe", r"C:\Python311\python.exe", @@ -540,7 +631,7 @@ def test_file_path_allowlist_accepts_windows_backslash_path(winpath, tmp_path): ) -@pytest.mark.skipif(shutil.which("bash") is None, reason="bash required to exercise emitted glob") +@_requires_bash @pytest.mark.parametrize("shebang_path", [ r"C:\Users\u\.venv\Scripts\python.exe", ]) @@ -554,7 +645,7 @@ def test_shebang_allowlist_accepts_windows_backslash_path(shebang_path, tmp_path ) -@pytest.mark.skipif(shutil.which("bash") is None, reason="bash required to exercise emitted glob") +@_requires_bash @pytest.mark.parametrize("dangerous", ["foo;rm -rf /", "foo`id`", "foo$(id)", "foo$IFS"]) def test_python_detect_allowlists_still_reject_shell_metacharacters(dangerous, tmp_path): """Guard against a naive fix (backslash right before `]`) that forms a @@ -566,7 +657,7 @@ def test_python_detect_allowlists_still_reject_shell_metacharacters(dangerous, t ) -@pytest.mark.skipif(shutil.which("bash") is None, reason="bash required to exercise emitted glob") +@_requires_bash @pytest.mark.parametrize("payload", [ r"C:\Users\u\.venv\Scripts\python.exe", # backslashes get stripped by argv "foo$IFS", # expands to "foo" — an ALLOWED value @@ -587,7 +678,7 @@ def test_shell_verdict_delivers_the_payload_bash_unmodified(payload, tmp_path): encoding="utf-8", newline="\n", ) result = subprocess.run( - ["bash", script.name], capture_output=True, text=True, cwd=str(tmp_path), + [_BASH, script.name], capture_output=True, text=True, cwd=str(tmp_path), ) assert result.returncode == 0, result.stderr assert result.stdout == payload, (