From c3cb8c058f335fd3da162d688b4c293073b3f82e Mon Sep 17 00:00:00 2001 From: Rishet Mehra Date: Thu, 23 Jul 2026 20:11:01 +0530 Subject: [PATCH 1/2] fix(hooks): accept Windows backslash paths in interpreter allowlist (#2126) The post-commit/post-checkout hook's interpreter-detection allowlist silently rejected valid Windows paths (C:\...\python.exe) on Git-Bash. bash treats a lone backslash inside [...] as an escape that consumes itself, so the emitted glob never matched a real backslash at runtime, even though the pattern looked correct in Python's install-time re dialect. Fix both allowlists (.graphify_python file path and shebang-parsed launcher path) to emit [!a-zA-Z0-9/_.@:\\-], a doubled-backslash form verified against bash and dash: it accepts Windows paths and still rejects ; ` $ injection. The shebang allowlist additionally lacked : and backslash entirely. Install-time _pinned_python() re is left as-is. Add shell-runtime tests that execute the emitted case/esac glob directly against Windows paths and shell metacharacters. --- graphify/hooks.py | 4 ++-- tests/test_hooks.py | 57 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/graphify/hooks.py b/graphify/hooks.py index 77ef39cd3..cbaae1b43 100644 --- a/graphify/hooks.py +++ b/graphify/hooks.py @@ -41,7 +41,7 @@ if [ -f "$_GFY_PYTHON_FILE" ]; then _FROM_FILE=$(cat "$_GFY_PYTHON_FILE" 2>/dev/null | tr -d '[:space:]') case "$_FROM_FILE" in - *[!a-zA-Z0-9/_.@:\\-]*) _FROM_FILE="" ;; # allowlist (covers Windows paths) + *[!a-zA-Z0-9/_.@:\\\\-]*) _FROM_FILE="" ;; # allowlist (covers Windows paths) esac if [ -n "$_FROM_FILE" ] && [ -x "$_FROM_FILE" ] && "$_FROM_FILE" -c "$_GFY_PROBE" 2>/dev/null; then GRAPHIFY_PYTHON="$_FROM_FILE" @@ -79,7 +79,7 @@ # Allowlist: only keep characters valid in a filesystem path to prevent # injection if the shebang contains shell metacharacters. case "$GRAPHIFY_PYTHON" in - *[!a-zA-Z0-9/_.@-]*) GRAPHIFY_PYTHON="" ;; + *[!a-zA-Z0-9/_.@:\\\\-]*) GRAPHIFY_PYTHON="" ;; esac if [ -n "$GRAPHIFY_PYTHON" ] && ! "$GRAPHIFY_PYTHON" -c "$_GFY_PROBE" 2>/dev/null; then GRAPHIFY_PYTHON="" diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 8e95aabbe..c38bd88f9 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -425,6 +425,63 @@ def test_probe_prefers_sibling_python_exe_on_windows_layouts(): assert "/python.exe" in _PYTHON_DETECT +def _extract_case_pattern(marker: str) -> str: + """Pull the `*[!...]*` glob portion of a real case arm out of _PYTHON_DETECT + by a unique anchor, so tests run against the emitted text, not a copy.""" + from graphify.hooks import _PYTHON_DETECT + for line in _PYTHON_DETECT.splitlines(): + if marker in line: + return line.strip().split(")")[0] + raise AssertionError(f"case arm containing {marker!r} not found in _PYTHON_DETECT") + + +def _shell_verdict(pattern: str, candidate: str) -> str: + result = subprocess.run( + ["bash", "-c", f'case "$1" in\n{pattern}) echo REJECTED ;;\n*) echo ACCEPTED ;;\nesac', "_", candidate], + capture_output=True, text=True, + ) + return result.stdout.strip() + + +@pytest.mark.skipif(shutil.which("bash") is None, reason="bash required to exercise emitted glob") +@pytest.mark.parametrize("winpath", [ + r"C:\Users\u\.venv\Scripts\python.exe", + r"C:\Python311\python.exe", +]) +def test_file_path_allowlist_accepts_windows_backslash_path(winpath): + """#2126: the .graphify_python FILE allowlist must accept real Windows paths + at actual shell runtime. Old pattern rejected them due to bash bracket-escape.""" + pattern = _extract_case_pattern('_FROM_FILE=""') + assert _shell_verdict(pattern, winpath) == "ACCEPTED", ( + f"Windows path {winpath!r} rejected by file-path allowlist at shell runtime" + ) + + +@pytest.mark.skipif(shutil.which("bash") is None, reason="bash required to exercise emitted glob") +@pytest.mark.parametrize("shebang_path", [ + r"C:\Users\u\.venv\Scripts\python.exe", +]) +def test_shebang_allowlist_accepts_windows_backslash_path(shebang_path): + """#2126: the shebang-parsed launcher allowlist had no `:` or `\\` at all, so + any Windows-style shebang path was unconditionally emptied. Must ACCEPT now.""" + pattern = _extract_case_pattern('GRAPHIFY_PYTHON="" ;;') + assert _shell_verdict(pattern, shebang_path) == "ACCEPTED", ( + f"Windows shebang path {shebang_path!r} rejected by launcher allowlist" + ) + + +@pytest.mark.skipif(shutil.which("bash") is None, reason="bash required to exercise emitted glob") +@pytest.mark.parametrize("dangerous", ["foo;rm -rf /", "foo`id`", "foo$(id)", "foo$IFS"]) +def test_python_detect_allowlists_still_reject_shell_metacharacters(dangerous): + """Guard against a naive fix (backslash right before `]`) that forms a + `:`-to-`\\` range admitting `;`, backtick, `$`. Both allowlists must reject.""" + for marker in ('_FROM_FILE=""', 'GRAPHIFY_PYTHON="" ;;'): + pattern = _extract_case_pattern(marker) + assert _shell_verdict(pattern, dangerous) == "REJECTED", ( + f"{marker} allowlist wrongly accepted dangerous input {dangerous!r}" + ) + + @pytest.mark.parametrize("name,script", _HOOK_SCRIPTS) def test_hooks_reuse_git_dir_from_env(name, script): """git exports GIT_DIR to hooks, so the rev-parse fallback should only run From e70673382d2ffa6848fcc64699361ff702006613 Mon Sep 17 00:00:00 2001 From: Rishet Mehra Date: Thu, 23 Jul 2026 22:30:48 +0530 Subject: [PATCH 2/2] test(hooks): surface bash failures in _shell_verdict helper Assert returncode == 0 so a malformed case snippet fails fast with stderr instead of silently returning an empty string. Addresses Copilot review feedback on #2133. --- tests/test_hooks.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_hooks.py b/tests/test_hooks.py index c38bd88f9..2f0fc5884 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -440,6 +440,11 @@ def _shell_verdict(pattern: str, candidate: str) -> str: ["bash", "-c", f'case "$1" in\n{pattern}) echo REJECTED ;;\n*) echo ACCEPTED ;;\nesac', "_", candidate], capture_output=True, text=True, ) + # Fail loudly on a malformed case snippet instead of returning "" and + # producing a confusing ACCEPTED/REJECTED mismatch downstream. + assert result.returncode == 0, ( + f"bash exited {result.returncode} for pattern {pattern!r}: {result.stderr.strip()}" + ) return result.stdout.strip()