From 58392251b3d3579aa2ce74719a54c1c4093cab19 Mon Sep 17 00:00:00 2001 From: abhay-codes07 Date: Sun, 16 Aug 2026 23:19:40 +0530 Subject: [PATCH] fix(hooks): a driveless rooted path is not cwd-relative on Windows (#1840) The read guard's out-of-project check short-circuited on `not Path(v).is_absolute()` with the comment "relative -> anchored at cwd == in project". That premise does not hold on Windows for a rooted path carrying no drive, which is the form POSIX-shaped hosts, WSL and Git Bash send: Path("/somewhere/else/x.py").is_absolute() -> False Path("/somewhere/else/x.py").resolve() -> C:\somewhere\else\x.py Windows anchors it at the current DRIVE root, not at cwd, so it lands outside the project unless the project sits at the drive root. Reading it as cwd-relative made the guard declare it in-project and emit the read nudge -- and in strict mode the once-per-session deny -- for files the graph never indexed. `C:x.py` is the mirror case: drive-relative, anchored at that drive's current directory rather than ours. The question the guard actually asks is "is this resolved against cwd?", whose answer is "no root and no drive", not "not absolute". These remain the host's own rules -- the path is about to be resolved against this filesystem, so paths.is_absolute_any_platform, which is for stored portable paths, is deliberately not used. On POSIX `root` is set exactly when a path is absolute and `drive` is always empty, so behaviour there is unchanged, and a test pins that equivalence. Fixes tests/test_hook_strict.py::test_out_of_project_read_silenced, which has been failing on Windows. --- graphify/cli.py | 31 ++++- tests/test_hook_out_of_project_paths.py | 169 ++++++++++++++++++++++++ 2 files changed, 198 insertions(+), 2 deletions(-) create mode 100644 tests/test_hook_out_of_project_paths.py diff --git a/graphify/cli.py b/graphify/cli.py index 95adad4b9c..03363254c6 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -12,7 +12,7 @@ import sys import time from graphify.paths import GRAPHIFY_OUT as _GRAPHIFY_OUT -from pathlib import Path +from pathlib import Path, PurePosixPath, PureWindowsPath _SEARCH_NUDGE = json.dumps({ @@ -663,7 +663,7 @@ def _run_hook_guard(kind: str, strict: bool = False) -> None: in_project = False for v in explicit: p = Path(v) - if not p.is_absolute(): + if _is_cwd_relative(v): in_project = True # relative -> anchored at cwd == in project break try: @@ -710,6 +710,33 @@ def _run_hook_guard(kind: str, strict: bool = False) -> None: pass +def _is_cwd_relative(value: str) -> bool: + r"""Whether *value* is anchored at the current working directory. + + The hook's out-of-project guard needs "is this path resolved against cwd?", + and ``Path.is_absolute()`` is the wrong question for it on Windows. A + driveless rooted path like ``/tmp/x.py`` — the form POSIX-shaped hosts, WSL + and Git Bash send — is NOT absolute there (no drive letter), but it is not + cwd-relative either: Windows anchors it at the current DRIVE root, so + ``Path("/somewhere/else/x.py").resolve()`` is ``C:\somewhere\else\x.py``, + which is outside the project unless the project sits at ``C:\``. Reading it + as cwd-relative made the guard declare it in-project and emit the read nudge + (and, in strict mode, the once-per-session deny) for files the graph has + nothing to say about. + + ``C:x.py`` is the same trap from the other side: drive-relative, anchored at + that drive's current directory rather than cwd. + + So the test is "no root and no drive", not "not absolute". These stay the + host's own rules — the path is about to be resolved against this filesystem, + so ``paths.is_absolute_any_platform`` (for stored, portable paths) is + deliberately not used. On POSIX ``root`` is set exactly when the path is + absolute and ``drive`` is always empty, so this is unchanged there. + """ + pure = PureWindowsPath(value) if os.name == "nt" else PurePosixPath(value) + return not pure.root and not pure.drive + + def _target_is_indexed(file_path: str, root: "Path") -> bool: """Guard the strict deny: only block a read of a file the graph actually indexes. Reads manifest.json (cheap, capped); on any doubt (missing/corrupt/oversized diff --git a/tests/test_hook_out_of_project_paths.py b/tests/test_hook_out_of_project_paths.py new file mode 100644 index 0000000000..b025a29b14 --- /dev/null +++ b/tests/test_hook_out_of_project_paths.py @@ -0,0 +1,169 @@ +r"""The read hook's out-of-project guard must not treat a rooted-but-driveless +path as cwd-relative. + +`_run_hook_guard` short-circuits on `not Path(v).is_absolute()` with the comment +"relative -> anchored at cwd == in project". On Windows that premise is false for +a path like `/somewhere/else/x.py`: it has no drive, so `is_absolute()` is False, +but Windows anchors it at the current DRIVE root, not at cwd — +`Path("/somewhere/else/x.py").resolve()` is `C:\somewhere\else\x.py`. The guard +therefore declared out-of-project files in-project and emitted the read nudge, +and in strict mode the once-per-session deny, for files the graph never indexed. +`tests/test_hook_strict.py::test_out_of_project_read_silenced` has been failing +on Windows for exactly this reason. + +`C:x.py` is the mirror case: drive-relative, anchored at that drive's current +directory rather than cwd. + +The classification tests below drive `_is_cwd_relative` with `os.name` forced, so +the Windows semantics are exercised on POSIX CI too (`PureWindowsPath` works on +any host). The end-to-end tests need a real Windows `Path` flavour to show the +difference, so those are gated. +""" +import io +import json +import os +import sys +import time + +import pytest + +import graphify.cli as cli +from graphify.cli import _is_cwd_relative + + +def _fake_os_name(monkeypatch, name): + """Force the flavour `_is_cwd_relative` selects, so both platforms' rules can + be checked from either host.""" + monkeypatch.setattr(cli.os, "name", name) + + +# --------------------------------------------------------------------------- +# Classification — real teeth on POSIX CI as well as Windows +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize( + "value", + [ + "/somewhere/else/x.py", # the reported case: rooted, no drive + "/tmp/scratch.py", # what a WSL / Git Bash / POSIX-shaped host sends + "\\somewhere\\else\\x.py", # same path, backslashes + "C:x.py", # drive-relative: anchored at C:'s cwd, not ours + "C:/proj/a.py", # fully qualified + "C:\\proj\\a.py", + "\\\\server\\share\\a.py", # UNC + ], +) +def test_windows_non_cwd_relative_forms(monkeypatch, value): + _fake_os_name(monkeypatch, "nt") + assert _is_cwd_relative(value) is False, value + + +@pytest.mark.parametrize( + "value", + ["src/a.py", "a.py", "./rel.py", "..\\up.py", "sub\\dir\\a.py", "dir/../a.py"], +) +def test_windows_cwd_relative_forms(monkeypatch, value): + _fake_os_name(monkeypatch, "nt") + assert _is_cwd_relative(value) is True, value + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + ("/somewhere/else/x.py", False), + ("/tmp/scratch.py", False), + ("src/a.py", True), + ("a.py", True), + ("./rel.py", True), + # No drives on POSIX: "C:x.py" is an ordinary relative filename there, and + # a backslash is a legal character in a POSIX filename, not a separator. + ("C:x.py", True), + ("C:/proj/a.py", True), + ("\\somewhere\\else\\x.py", True), + ], +) +def test_posix_rules_are_unchanged(monkeypatch, value, expected): + """On POSIX `root` is set exactly when the path is absolute and `drive` is + always empty, so the guard's behaviour there is identical to the old + `not is_absolute()` test. Pinned so the fix stays Windows-only.""" + _fake_os_name(monkeypatch, "posix") + assert _is_cwd_relative(value) is expected, value + + +def test_matches_is_absolute_on_every_posix_input(monkeypatch): + """The property the above table samples: on POSIX, `_is_cwd_relative` is + exactly `not Path(v).is_absolute()`.""" + from pathlib import PurePosixPath + _fake_os_name(monkeypatch, "posix") + for v in ["/a/b", "a/b", "", ".", "..", "/", "//x", "C:x", "\\x", "/a/../b"]: + assert _is_cwd_relative(v) is (not PurePosixPath(v).is_absolute()), v + + +def test_empty_path_is_treated_as_cwd_relative(monkeypatch): + # Callers filter empties out before the loop; pinned so the helper cannot + # raise if that ever changes. + for name in ("nt", "posix"): + _fake_os_name(monkeypatch, name) + assert _is_cwd_relative("") is True + + +# --------------------------------------------------------------------------- +# End-to-end through the guard +# --------------------------------------------------------------------------- + +def _project(tmp_path): + src = tmp_path / "src" + src.mkdir() + f = src / "mod.py" + f.write_text("def x():\n return 1\n", encoding="utf-8") + out = tmp_path / "graphify-out" + out.mkdir() + (out / "manifest.json").write_text( + json.dumps({"src/mod.py": {"mtime": 1}}), encoding="utf-8") + time.sleep(0.02) + (out / "graph.json").write_text('{"nodes":[],"links":[]}', encoding="utf-8") + return f + + +def _invoke(tmp_path, monkeypatch, file_path, *, strict=False): + monkeypatch.chdir(tmp_path) + payload = {"session_id": "s1", "tool_name": "Read", + "tool_input": {"file_path": str(file_path)}} + + class _Stdin: + buffer = io.BytesIO(json.dumps(payload).encode()) + monkeypatch.setattr(sys, "stdin", _Stdin()) + buf = io.StringIO() + monkeypatch.setattr(sys, "stdout", buf) + cli._run_hook_guard("read", strict=strict) + return buf.getvalue() + + +@pytest.mark.skipif(os.name != "nt", + reason="needs a Windows Path flavour: on POSIX these strings are " + "already absolute, so the guard was never wrong about them") +@pytest.mark.parametrize("outside", ["/somewhere/else/x.py", "\\somewhere\\else\\x.py"]) +@pytest.mark.parametrize("strict", [False, True]) +def test_driveless_rooted_path_outside_the_project_is_silent(tmp_path, monkeypatch, outside, strict): + _project(tmp_path) + assert _invoke(tmp_path, monkeypatch, outside, strict=strict).strip() == "" + + +def test_in_project_relative_path_still_nudges(tmp_path, monkeypatch): + """The guard must keep firing for the paths it exists to catch.""" + _project(tmp_path) + assert "MANDATORY" in _invoke(tmp_path, monkeypatch, "src/mod.py") + + +def test_in_project_absolute_path_still_nudges(tmp_path, monkeypatch): + f = _project(tmp_path) + assert "MANDATORY" in _invoke(tmp_path, monkeypatch, f) + + +def test_absolute_path_outside_the_project_is_still_silent(tmp_path, monkeypatch): + """Unchanged behaviour, kept as the control for the cases above.""" + _project(tmp_path) + other = tmp_path.parent / "elsewhere_project" / "z.py" + other.parent.mkdir(parents=True, exist_ok=True) + other.write_text("x = 1\n", encoding="utf-8") + assert _invoke(tmp_path, monkeypatch, other).strip() == ""