fix(hooks): stop probing a /bin/sh launcher wrapper as if it were python (#3027)

graphify/hooks.py

@@ -83,9 +83,29 @@
             */env\\ *) GRAPHIFY_PYTHON="${_SHEBANG#*/env }" ;;
             *)         GRAPHIFY_PYTHON="$_SHEBANG" ;;
         esac
+        # The launcher may be a /bin/sh wrapper (pipx via distlib's exec trick,
+        # emitted when the install path has spaces, e.g. macOS's
+        # "Application Support/pipx"): its shebang names the SHELL, and probing
+        # the shell with -c "import ..." runs whatever `import` is on PATH -
+        # ImageMagick's screenshot tool, which dumped its usage text on every
+        # commit (#3027). The wrapper's second line execs the real interpreter
+        # (exec' "/path/to/python" "$0" "$@"): take it from there, and never
+        # probe anything whose name does not contain python (python3.12,
+        # python@3.12, cpython, pypy3 all pass; sh, bash, node do not).
+        case "$(basename "$GRAPHIFY_PYTHON" 2>/dev/null)" in
+            *python*|*pypy*) ;;
+            *) GRAPHIFY_PYTHON=$(printf '%s\\n' "$_GFY_HEAD" | sed -n '2s/^...exec. *"\\{0,1\\}\\([^"]*\\)"\\{0,1\\} .*/\\1/p') ;;
+        esac
+        case "$(basename "$GRAPHIFY_PYTHON" 2>/dev/null)" in
+            *python*|*pypy*) ;;
+            *) GRAPHIFY_PYTHON= ;;
+        esac
         # Allowlist: only keep characters valid in a filesystem path to prevent
-        # injection if the shebang contains shell metacharacters.
-        case "$GRAPHIFY_PYTHON" in
+        # injection if the shebang contains shell metacharacters. A space is
+        # not one - $GRAPHIFY_PYTHON is always expanded quoted - and a path
+        # that reaches this branch through a wrapper contains one by
+        # construction, so spaces are folded away before the check.
+        case "$(printf '%s' "$GRAPHIFY_PYTHON" | tr ' ' '_')" in
             *[!a-zA-Z0-9/_.@:\\\\-]*) GRAPHIFY_PYTHON="" ;;
         esac
         if [ -n "$GRAPHIFY_PYTHON" ] && ! "$GRAPHIFY_PYTHON" -c "$_GFY_PROBE" 2>/dev/null; then

tests/test_hook_sh_wrapper_launcher.py

@@ -0,0 +1,97 @@
+"""A /bin/sh launcher wrapper must not be probed as if it were python (#3027).
+
+pipx (via distlib's "exec trick", used when the install path has spaces)
+generates a `graphify` launcher whose shebang is `#!/bin/sh` and whose second
+line execs the real interpreter. The hook parsed that shebang into
+GRAPHIFY_PYTHON and ran `/bin/sh -c "import graphify"` — which is not a
+shell builtin, so it resolved on PATH to ImageMagick's `import` screenshot
+tool and dumped its usage text on every commit. Only stderr was silenced.
+"""
+from __future__ import annotations
+
+import os
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+from graphify.hooks import _PYTHON_DETECT
+
+pytestmark = pytest.mark.skipif(shutil.which("sh") is None, reason="sh required to run the probe chain")
+
+
+def _stub(path: Path, body: str) -> Path:
+    path.write_text(body, encoding="utf-8", newline="\n")
+    path.chmod(0o755)
+    return path
+
+
+def _machine(tmp_path: Path, *, wrapper_exec: str | None):
+    """PATH holds: a /bin/sh `graphify` wrapper, ImageMagick's `import`, and
+    ambient pythons that cannot import graphify. `wrapper_exec` is the
+    interpreter the wrapper execs (None: a wrapper with no exec line)."""
+    stub_bin = tmp_path / "stubbin"
+    stub_bin.mkdir()
+    _stub(stub_bin / "import", '#!/bin/sh\necho "Usage: import [options ...] [ file ]"\nexit 1\n')
+    for name in ("python3", "python"):
+        _stub(stub_bin / name, "#!/bin/sh\nexit 1\n")
+    exec_line = f"'''exec' \"{wrapper_exec}\" \"$0\" \"$@\"\n' '''\n" if wrapper_exec else "echo wrapper\n"
+    _stub(stub_bin / "graphify", "#!/bin/sh\n" + exec_line)
+    home = tmp_path / "home"
+    home.mkdir()
+    return home, stub_bin
+
+
+def _venv_python(tmp_path: Path, ok: bool = True) -> Path:
+    venv = tmp_path / "pipx venvs" / "graphifyy" / "bin"  # a space, as in the wild
+    venv.mkdir(parents=True)
+    return _stub(venv / "python", "#!/bin/sh\nexit 0\n" if ok else "#!/bin/sh\nexit 1\n")
+
+
+def _run(tmp_path: Path, home: Path, stub_bin: Path) -> subprocess.CompletedProcess:
+    script = tmp_path / "detect_run.sh"
+    script.write_text(_PYTHON_DETECT + '\necho "RESOLVED=$GRAPHIFY_PYTHON"\n', encoding="utf-8", newline="\n")
+    env = dict(os.environ)
+    env["HOME"] = str(home)
+    env.pop("UV_TOOL_DIR", None)
+    env["PATH"] = str(stub_bin) + os.pathsep + env["PATH"]
+    return subprocess.run(["sh", script.name], capture_output=True, text=True, cwd=str(tmp_path), env=env)
+
+
+def test_the_wrappers_interpreter_is_used_and_imagemagick_is_never_run(tmp_path):
+    py = _venv_python(tmp_path)
+    home, stub_bin = _machine(tmp_path, wrapper_exec=py.as_posix())
+    res = _run(tmp_path, home, stub_bin)
+    assert res.returncode == 0, res.stderr
+    assert "Usage: import" not in res.stdout + res.stderr, "the shell was probed with -c 'import ...'"
+    assert f"RESOLVED={py.as_posix()}" in res.stdout, res.stdout + res.stderr
+
+
+def test_a_wrapper_with_no_exec_line_falls_through_quietly(tmp_path):
+    home, stub_bin = _machine(tmp_path, wrapper_exec=None)
+    res = _run(tmp_path, home, stub_bin)
+    assert "Usage: import" not in res.stdout + res.stderr
+    assert "RESOLVED=/bin/sh" not in res.stdout
+    assert "RESOLVED=sh" not in res.stdout
+
+
+def test_a_wrapper_whose_interpreter_cannot_import_graphify_is_not_adopted(tmp_path):
+    py = _venv_python(tmp_path, ok=False)
+    home, stub_bin = _machine(tmp_path, wrapper_exec=py.as_posix())
+    res = _run(tmp_path, home, stub_bin)
+    assert f"RESOLVED={py.as_posix()}" not in res.stdout
+    assert "Usage: import" not in res.stdout + res.stderr
+
+
+def test_a_plain_python_shebang_still_resolves_as_before(tmp_path):
+    py = _venv_python(tmp_path)
+    home, stub_bin = _machine(tmp_path, wrapper_exec=None)
+    _stub(stub_bin / "graphify", f"#!{py.as_posix()}\nimport sys\n")
+    res = _run(tmp_path, home, stub_bin)
+    assert f"RESOLVED={py.as_posix()}" in res.stdout, res.stdout + res.stderr
+
+
+def test_only_python_like_interpreters_are_ever_probed():
+    """The emitted script gates the probe on the interpreter's name."""
+    assert "*python*|*pypy*)" in _PYTHON_DETECT