hooks: keep the rebuild root inside the repo (#3265)

[ayushcodes10]

Summary

Fixes #3265. .graphify_root lives inside the directory Team setup documents committing, so its contents are checkout controlled — but the generated post-commit / post-checkout hooks read that file and pass its value straight to _rebuild_code with no bound. A value planted there by a malicious fork or PR (an absolute path outside the repository) would steer the rebuild, and therefore what it reads and what it writes back into that same committed directory, wherever the checkout names, not the repository the hook was installed into.

Both rebuild bodies (_REBUILD_BODY_COMMIT and _REBUILD_BODY_CHECKOUT in graphify/hooks.py) now only adopt the saved root when it resolves inside the working tree the hook is actually running from (_root.resolve() == cwd or a descendant of it); anything else falls back to the repo top, the previous default when the marker was absent, and prints a note saying so.

This does not narrow the legitimate case: a subdirectory scoped root (#1173, e.g. graphify update backend/) is still a descendant of the repo root the hook runs from, so it's still honoured — covered by a new test.

This PR only addresses the .graphify_root half of the report. The reporter's earlier .graphify_python follow-up explicitly asks for a maintainer preference among three directions (drop the working tree probe, relocate it outside the repo, or document it in .gitignore), so I've left that to a maintainer call rather than guessing.

Test plan

• Added 2 regression tests (parametrized over both hook bodies) in tests/test_hooks.py: one confirms a .graphify_root pointing outside the repo is rejected and falls back to the repo top, one confirms a subdirectory scoped root (post-commit hook overwrites .graphify_root, silently expanding a scoped build to the whole repo #1173) is still honoured. Both execute the actual shipped snippet (extracted from the body text via regex) rather than a hand copy, so they can't drift from the real hook source.
• Manually reproduced: wrote .graphify_root pointing at a directory outside the repo, confirmed the recovered root falls back to the repo top; wrote a subdirectory value, confirmed it's still honoured.
• Full suite: python3 -m pytest -q — 5424 passed, only the pre-existing unrelated failures (test_ollama_retry_cap.py missing openai in this env, one flaky timing assertion in test_ts_import_type_arguments.py).
• python3 -m tools.skillgen --check — OK.
• Existing hook tests all pass unchanged, including the shell quote safety and prefix consistency checks the new code had to satisfy.

🤖 Generated with Claude Code

https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh

[graphify-labs]

Graphify reviewed this change.

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

---

Graphify review — findings

Constrains the .graphify_root value recovered by the generated post-commit and post-checkout hooks to paths inside the working tree the hook runs from, so a checkout-controlled root planted in the committed graphify-out/ can no longer redirect where the rebuild reads and writes; out-of-repo values are ignored with a printed notice and the hook falls back to .. Subdirectory-scoped roots inside the repo are still honoured. Adds parametrized tests that execute the shipped resolution snippet extracted straight from the rebuild bodies, covering both the rejected out-of-repo case and the preserved in-repo scoped case.

Worth a look

• Symlink-loop .graphify_root can crash generated hooks — graphify/hooks.py:194 · Escalate · medium
  • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
• Checkout hook validated path is reopened after symlink race — graphify/hooks.py:272 · 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 — 396 functions depend on the 211 functions this change touches.

Health — this change adds coupling hotspots:

• new: install() — 37 callers, 7 callees
• new: dispatch_command() — 2 callers, 124 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 — 396 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: 226 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

21 of 268 test file(s) selected (8%) via static blast radius.

• tests/test_affected_cli.py — impact
• tests/test_agents_platform.py — impact
• tests/test_codebuddy.py — impact
• tests/test_devin.py — impact
• tests/test_explain_cli.py — impact
• tests/test_extract_cli.py — impact
• tests/test_global_add_tag_inference.py — impact
• tests/test_god_nodes_cli.py — impact
• tests/test_hollow_chunks_arm_shrink_guard.py — impact
• tests/test_hook_chain_survives_skip.py — impact
• tests/test_hooks.py — impact, changed-test
• tests/test_incomplete_build_guard.py — impact
• tests/test_install.py — impact
• tests/test_install_references.py — impact
• tests/test_merge_chunks_validation.py — impact
• tests/test_multigraph_diagnostics.py — impact
• tests/test_no_dedup_flag.py — impact
• tests/test_path_cli.py — impact
• tests/test_query_cli.py — impact
• tests/test_query_induced_edges.py — impact
• tests/test_unverified_semantic_shrink.py — impact

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

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

[ayushcodes10]

Thanks for the review.

"Symlink-loop .graphify_root can crash generated hooks" — confirmed and fixed. Reproduced directly: Path.resolve() raises RuntimeError for a symlink loop, not OSError, so the guard's except OSError let it propagate. It doesn't produce a raw traceback (the outer except Exception in the hook body catches it and reports "Rebuild failed"), but that's still wrong — a bad marker should fall back to the repo top like every other invalid value, not abort the rebuild. Fixed in the latest commit by also catching RuntimeError, with a regression test that builds an actual two-symlink loop and confirms the fallback now fires.

"Checkout hook validated path is reopened after symlink race" — looked at this, leaving it as is. This is a TOCTOU between the resolve() check and _rebuild_code's actual use of _root a few lines later. I don't think it changes the threat model this PR is closing: the scenario #3265 describes (and what this guard defends against) is a value committed to the repo steering a later, unattended run of the hook. A TOCTOU exploit needs an attacker with live, precisely-timed write access to the working tree during the hook's execution window on the victim's own machine — at that point they already have a much more direct path to doing damage than racing a path check (they could edit the source files being rebuilt, or the hook script itself). Happy to revisit if there's a concrete scenario where that access exists without also implying worse compromise already, but I don't think a file-descriptor-based rewrite of _rebuild_code's read/write path is proportionate to add here.

[graphify-labs]

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

Constrains the .graphify_root recovered from a checkout-committed file to paths inside the working tree the hook runs from: an absolute or out-of-repo value is now ignored (with a printed notice) in favor of the repo root, while a same-repo subdirectory scope is still honored. Falls back to the repo top when Path.resolve() can't classify the candidate, catching both OSError and the RuntimeError a symlink loop raises. Adds parametrized post-commit/post-checkout tests that execute the shipped resolution snippet itself for the out-of-repo, in-repo-scoped, and symlink-loop cases.

Worth a look

• Out-of-repo .graphify_root is no longer honored by installed hooks — graphify/hooks.py:195 · 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 — 398 functions depend on the 213 functions this change touches.

Health — this change adds coupling hotspots:

• new: install() — 37 callers, 7 callees
• new: dispatch_command() — 2 callers, 124 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 — 398 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: 228 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

21 of 268 test file(s) selected (8%) via static blast radius.

• tests/test_affected_cli.py — impact
• tests/test_agents_platform.py — impact
• tests/test_codebuddy.py — impact
• tests/test_devin.py — impact
• tests/test_explain_cli.py — impact
• tests/test_extract_cli.py — impact
• tests/test_global_add_tag_inference.py — impact
• tests/test_god_nodes_cli.py — impact
• tests/test_hollow_chunks_arm_shrink_guard.py — impact
• tests/test_hook_chain_survives_skip.py — impact
• tests/test_hooks.py — impact, changed-test
• tests/test_incomplete_build_guard.py — impact
• tests/test_install.py — impact
• tests/test_install_references.py — impact
• tests/test_merge_chunks_validation.py — impact
• tests/test_multigraph_diagnostics.py — impact
• tests/test_no_dedup_flag.py — impact
• tests/test_path_cli.py — impact
• tests/test_query_cli.py — impact
• tests/test_query_induced_edges.py — impact
• tests/test_unverified_semantic_shrink.py — impact

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

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

[ayushcodes10]

Re "Out-of-repo .graphify_root is no longer honored by installed hooks" — that's the intended fix, not a regression. #3265 is exactly about an out-of-repo .graphify_root value being honored when it shouldn't be (a checkout-controlled file steering the rebuild outside the repository); restricting it to in-repo paths is the whole point of this PR. An already-installed hook picks up the new behavior the next time graphify hook install regenerates it, same as any other hook-body fix.

[safishamsi]

Shipped in v0.9.59 — landed on v8 via cherry-pick with your authorship preserved. Thanks @ayushcodes10 — the git hook keeps its rebuild root inside the repository, ignoring a committed .graphify_root that points outside. Release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.59