mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-27 00:36:39 +00:00
fix(hooks): a driveless rooted path is not cwd-relative on Windows (#2795)
The out-of-project read guard treated any non-absolute path as cwd-relative, but on Windows a rooted-but-driveless path (`\foo\bar`) is not absolute yet resolves against the current drive root, outside the project. Classify with a platform-correct predicate (`not root and not drive` under the host path flavour) so the containment check is reached. On POSIX the predicate reduces to `not is_absolute()`, so no behaviour changes there.
This commit is contained in:
abhay-codes07
committed by
safishamsi
safishamsi
parent
26092ce5b1
commit
e4b85d3b50
+29
-2
@@ -12,7 +12,7 @@ import re
|
||||
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
|
||||
|
||||
@@ -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() == ""
|
||||
Reference in New Issue
Block a user