mirror of
https://github.com/safishamsi/graphify.git
synced 2026-09-23 05:55:54 +00:00
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.
This commit is contained in:
+2
-2
@@ -41,7 +41,7 @@ if [ -z "$GRAPHIFY_PYTHON" ]; then
|
||||
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 @@ if [ -z "$GRAPHIFY_PYTHON" ]; then
|
||||
# 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=""
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user