Skip to content

fix(hooks): resolve the uv tool venv when pin and launcher probes miss (#2852) - #2929

Open
santhiprakash wants to merge 1 commit into
Graphify-Labs:v8from
santhiprakash:fix/hooks-uv-tool-interpreter-probe-2852
Open

fix(hooks): resolve the uv tool venv when pin and launcher probes miss (#2852)#2929
santhiprakash wants to merge 1 commit into
Graphify-Labs:v8from
santhiprakash:fix/hooks-uv-tool-interpreter-probe-2852

Conversation

@santhiprakash

Copy link
Copy Markdown

Problem

uv tool install is the README's recommended install method, but the post-commit hook cannot find that install when the install-time pin is dead, and then dies with no rebuild (#2852). On a machine where 30 repos plus ~/.git-templates carried the hook, graphs drifted 127h/175h stale with the hook "installed" and commits succeeding.

Resolution chain under uv tool install with a dead _PINNED (git-template hooks, a pin rejected by the allowlist, or a venv relocated by an upgrade):

  1. .graphify_python — absent in repos that never built a graph.
  2. launcher on PATH — a binary trampoline on Windows: command -v graphify (Git-Bash) returns it without the .exe suffix, so the *.exe guard misses and there is no shebang to parse at all. On POSIX the shim's shebang does resolve, which is why this hides on Linux/macOS.
  3. last resort python3/python — no ambient python can import graphify from an isolated uv venv → warning + exit 0, commit succeeds, nothing rebuilt.

Two gaps in graphify/hooks.py _PYTHON_DETECT (embedded in both the post-commit and post-checkout scripts):

Fix

  • uv tool-env scan, tried after the launcher probe and before the python3/python last resort: iterate ${UV_TOOL_DIR:-}, $HOME/.local/share/uv/tools, $HOME/AppData/Roaming/uv/tools and probe */bin/python and */Scripts/python.exe with the existing _GFY_PROBE. A tool env is adopted only if its python passes the probe, so a co-installed tool without graphify never satisfies it (glob order is not trusted). This is the direction suggested in the issue, generalized to not hardcode the dist name.
  • #! gate on the shebang read: the launcher's head bytes are only parsed as a shebang when the file actually starts with #! — a trampoline (or any non-script launcher) is rejected structurally instead of by the allowlist catching the parsed garbage.

The loud could not locate a Python with graphify installed stderr warning (restored on v8 after 0.9.46's bare exit 0) is kept as the terminal failure — it stays out of this change's way and the scan makes it far rarer.

Test

  • test_uv_tool_env_rescues_hook_when_pin_and_launcher_failpost-commit hook silently exits 0 under uv tool install: never rebuilds, no error #2852's machine reproduced on POSIX (dead pin, no .graphify_python, binary trampoline launcher on PATH, ambient pythons that cannot import graphify): the emitted _PYTHON_DETECT run under real sh must resolve the uv venv python, walking past a sibling tool env whose python lacks graphify. Fails on pre-fix code.
  • test_uv_tool_env_honors_uv_tool_dir_and_windows_layoutUV_TOOL_DIR override + the Windows Scripts\python.exe layout. Fails pre-fix.
  • test_shebang_parse_requires_leading_hash_bang — a non-script launcher whose first line merely names a working python must not hijack resolution (the misparse that used to feed trampoline bytes into the chain). Fails pre-fix.
  • test_uv_tool_env_without_graphify_still_fails_loudly — the scan never adopts a venv without graphify; the chain still ends in the loud warning, never a silent adopt.
  • Existing suite intact: tests/test_hooks.py 96 passed; uv run --frozen pytest tests/ -q matches the pre-PR baseline (the test_skillgen.py audit-coverage and test_ollama_retry_cap.py failures reproduce on a clean origin/v8 tree in this checkout and are untouched by this diff; CI's fetch-depth: 0 covers the former).

Sandbox end-to-end against a real uv tool install of the local tree: with the pin blanked and the launcher replaced by a binary blob, pre-fix commit → warning, log untouched; post-fix commit → hook resolves ~/.local/share/uv/tools/graphifyy/bin/python and the detached rebuild runs ([graphify hook] 1 file(s) changed - rebuilding graph...).

Related: #2629 (pipx python -E shebang arg) is the same probe chain but a different parse failure and is not addressed here. Builds on the #2126/#2166 allowlist lineage in this block.

Graphify-Labs#2852)

uv tool install puts graphify in an isolated venv no ambient python can
import, and on Windows its launcher on PATH is a binary trampoline with no
shebang, so with the pin dead every probe missed and the hook no-op'd with
only a warning. Scan uv tool envs (UV_TOOL_DIR + default locations, POSIX
bin/python and Windows Scripts/python.exe layouts) before the python3/python
last resort, and gate the shebang read on a leading '#!' so launcher bytes
never reach the parse.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.


Graphify review — findings

Adds a uv-tool-env probe to the hook interpreter-resolution chain in graphify/hooks.py: after the pin/shebang/ambient probes miss, it scans UV_TOOL_DIR and the default POSIX/Windows uv tool locations and adopts the first bin/python/Scripts/python.exe that passes the graphify import probe. Also gates the launcher shebang parse on a leading #!, so binary trampolines (uv installs on Windows reached without .exe) no longer feed bytes into the parse. Covers both with new test_hooks cases that run the emitted _PYTHON_DETECT under real sh against a simulated broken uv machine (#2852).

Worth a look

  • UV tool scan executes arbitrary candidates from environment-controlled directorygraphify/hooks.py:109 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 343 functions depend on the 189 functions this change touches.

Health — this change adds coupling hotspots:

  • new: dispatch_command() — 2 callers, 119 callees
  • new: install() — 33 callers, 7 callees
  • new: dispatch_install_cli() — 2 callers, 31 callees
  • new: status() — 8 callers, 6 callees
  • new: uninstall() — 9 callers, 5 callees
  • new: uninstall_all() — 2 callers, 13 callees
  • new: test_poisoned_manifest_is_healed() — 0 callers, 6 callees

Verification — 343 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 196 function(s) in the blast radius were not formally verified this run

· 7 more finding(s) on lines outside this diff (see the check run).

@santhiprakash

Copy link
Copy Markdown
Author

Re: the advisory on the uv tool-env scan executing candidates from an environment-controlled directory — the scan's trust scope matches the probes that already run in _PYTHON_DETECT: probe 3 executes the python.exe beside the PATH-resolved graphify launcher and the interpreter parsed from that launcher's shebang, and the last-resort probe executes python3/python straight off PATH. UV_TOOL_DIR is no more privileged than PATH, and the default scan roots are under /home/ubuntu/.local/share/uv/tools, where an attacker with write access could already replace the PATH-visible launcher that probe 3 executes. No new trust boundary is crossed; adoption additionally requires passing the graphify import probe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant