From ee35087aea428b12415aef2c5112078936d925c7 Mon Sep 17 00:00:00 2001 From: nothariharan Date: Tue, 29 Sep 2026 13:23:31 +0530 Subject: [PATCH] fix(hooks): refuse a core.hooksPath that leaves the repository hook install followed git's hooks path even when core.hooksPath resolved outside the checkout, then created that directory and wrote post-commit there. The bytes are Graphify's own hook, not repository-chosen code, but the write still escapes the repo. Keep in-repo paths such as .husky, and git's own hooks directory (including a linked worktree's shared .git/hooks). Anything else falls back to that default. Fixes #3869 Co-Authored-By: Grok 4.7 --- graphify/hooks.py | 75 +++++++++++++++++++++++++++++++++++++++++++-- tests/test_hooks.py | 64 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 2 deletions(-) diff --git a/graphify/hooks.py b/graphify/hooks.py index 3aa372ec1..abc869c19 100644 --- a/graphify/hooks.py +++ b/graphify/hooks.py @@ -576,6 +576,66 @@ def _reject_windows_path(value: str, source: str) -> None: ) +def _is_within(child: Path, parent: Path) -> bool: + """True if `child` is `parent` or a path underneath it.""" + try: + child.resolve().relative_to(parent.resolve()) + except (ValueError, OSError): + return False + return True + + +def _rev_parse_path(root: Path, flag: str) -> Path | None: + """Resolve one ``git rev-parse`` path flag against ``root``. + + ``-c core.hooksPath=`` is not used: an empty value makes ``--git-path hooks`` + print ``./`` instead of the real hooks directory. + """ + import subprocess as _sp + try: + res = _sp.run( + ["git", "-C", str(root), "rev-parse", flag], + capture_output=True, text=True, + ) + except (OSError, FileNotFoundError): + return None + if res.returncode != 0: + return None + raw = res.stdout.strip() + if not raw or any(c in raw for c in ("\n", "\r", "\x00")): + return None + path = Path(raw) + if not path.is_absolute(): + path = root / path + return path.resolve() + + +def _builtin_hooks_dir(root: Path) -> Path | None: + """Git's own hooks directory, ignoring core.hooksPath. + + ``--git-path hooks`` follows core.hooksPath, so the default is derived from + the common git dir. A linked worktree's hooks live in the main repo's + ``.git/hooks``, which is outside the worktree root. + """ + for flag in ("--git-common-dir", "--git-dir"): + git_dir = _rev_parse_path(root, flag) + if git_dir is not None: + return (git_dir / "hooks").resolve() + return None + + +def _hooks_path_allowed(root: Path, candidate: Path) -> bool: + """Allow in-repo hook dirs (Husky) and git's own hooks dir only. + + A core.hooksPath that resolves anywhere else is repository-controlled + local config. Honoring it makes `hook install` write outside the repo (#3869). + """ + if _is_within(candidate, root): + return True + builtin = _builtin_hooks_dir(root) + return builtin is not None and candidate.resolve() == builtin.resolve() + + def _hooks_dir(root: Path) -> Path: """Return the git hooks directory, respecting core.hooksPath if set (e.g. Husky). @@ -616,8 +676,19 @@ def _hooks_dir(root: Path) -> Path: if raw and not any(c in raw for c in ("\n", "\r", "\x00")): _reject_windows_path(raw, "git rev-parse --git-path hooks") d = (root / raw).resolve() - d.mkdir(parents=True, exist_ok=True) - return d + if _hooks_path_allowed(root, d): + d.mkdir(parents=True, exist_ok=True) + return d + print( + f"[graphify hooks] refusing hooks path {d}: it is outside " + f"{root.resolve()}. Installing into the default git hooks " + f"directory instead (#3869).", + file=sys.stderr, + ) + default = _builtin_hooks_dir(root) + if default is not None: + default.mkdir(parents=True, exist_ok=True) + return default except (OSError, FileNotFoundError): pass d = root / ".git" / "hooks" diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 0910b6816..a4f04bcce 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -589,6 +589,70 @@ def test_posix_custom_hookspath_still_works(tmp_path): assert (repo / ".husky" / "post-commit").exists() +def test_hookspath_outside_repo_falls_back_to_default(tmp_path, capsys): + """#3869: an absolute core.hooksPath outside the repo must not be created. + + The setting lives in local git config (a zip of .git can carry it; a clone + does not). hook install used to mkdir that path and write post-commit there. + """ + repo = _make_git_repo(tmp_path / "repo") + outside = tmp_path / "outside" + _set_hookspath(repo, str(outside)) + msg = install(repo) + err = capsys.readouterr().err + assert "refusing hooks path" in err + assert not outside.exists() + assert (repo / ".git" / "hooks" / "post-commit").exists() + assert (repo / ".git" / "hooks" / "post-checkout").exists() + assert ".git" in msg and "hooks" in msg + + +def test_hookspath_relative_escape_falls_back_to_default(tmp_path, capsys): + """#3869: a relative core.hooksPath that resolves outside the repo is refused.""" + repo = _make_git_repo(tmp_path / "repo") + _set_hookspath(repo, "../escaped") + _hooks_dir(repo) + err = capsys.readouterr().err + assert "refusing hooks path" in err + assert not (tmp_path / "escaped").exists() + assert _hooks_dir(repo) == (repo / ".git" / "hooks").resolve() + + +def test_linked_worktree_keeps_its_default_hooks_dir(tmp_path): + """A linked worktree's hooks live under the common git dir, outside the + worktree root. That default must still be used when core.hooksPath is unset.""" + main = tmp_path / "main" + main.mkdir() + subprocess.run(["git", "init", str(main)], check=True, capture_output=True) + subprocess.run(["git", "-C", str(main), "config", "user.email", "t@example.com"], + check=True, capture_output=True) + subprocess.run(["git", "-C", str(main), "config", "user.name", "t"], + check=True, capture_output=True) + (main / "f").write_text("x\n", encoding="utf-8") + subprocess.run(["git", "-C", str(main), "add", "f"], check=True, capture_output=True) + subprocess.run(["git", "-C", str(main), "commit", "-m", "x"], + check=True, capture_output=True) + wt = tmp_path / "wt" + subprocess.run(["git", "-C", str(main), "worktree", "add", "-q", str(wt), "-b", "wt"], + check=True, capture_output=True) + expected = subprocess.run( + ["git", "-C", str(wt), "rev-parse", "--git-path", "hooks"], + check=True, capture_output=True, text=True, + ).stdout.strip() + got = _hooks_dir(wt) + assert got == (wt / expected).resolve() + assert _is_within_repo_git(got, main) + assert not (wt / ".git" / "hooks").exists() + + +def _is_within_repo_git(hooks: Path, main: Path) -> bool: + try: + hooks.resolve().relative_to((main / ".git").resolve()) + except ValueError: + return False + return True + + def test_default_hooks_dir_unaffected(tmp_path): """No core.hooksPath -> normal .git/hooks install, no rejection.""" repo = _make_git_repo(tmp_path)