test: Windows path portability in tests; llm emits POSIX source_file to the model (#2620)

This commit is contained in:
rajashidattapy
2026-08-12 20:56:28 +01:00
committed by safishamsi
parent e4bfd2ad1a
commit 1aab181d15
5 changed files with 93 additions and 21 deletions
+3 -1
View File
@@ -29,7 +29,9 @@ def _cpp_preprocess(path: Path) -> bytes:
try:
# Pass an absolute path so a corpus file named like "-I/etc/x.F90" cannot
# be parsed by cpp as an option (cpp does not accept a "--" end-of-options
# terminator). An absolute path always begins with "/".
# terminator). What matters is that an absolute path cannot begin with
# "-" — not that it begins with "/", which only holds on POSIX (a Windows
# absolute path starts with a drive letter, and is equally safe).
result = subprocess.run(
["cpp", "-w", "-P", "-nostdinc", "-I", "/dev/null", str(path.resolve())],
capture_output=True,
+12 -4
View File
@@ -581,9 +581,13 @@ def _read_files(units: "list[Path | FileSlice]", root: Path) -> str:
print(f"[graphify] skipping {p}: symlink target outside corpus root", file=sys.stderr)
continue
try:
rel = str(p.relative_to(root))
# as_posix, not str: `rel` is handed to the model as the literal
# source_file to emit, so a native backslash spelling on Windows
# lands in the graph and splits one file across two source_file
# forms (#683 / #2259).
rel = p.relative_to(root).as_posix()
except ValueError:
rel = str(p)
rel = Path(p).as_posix()
try:
if isinstance(u, FileSlice):
content = read_slice_text(u)
@@ -813,9 +817,13 @@ def _build_image_refs(image_files: list[Path], root: Path, *, read_bytes: bool =
print(f"[graphify] skipping image {p}: symlink target outside corpus root", file=sys.stderr)
continue
try:
rel = str(p.relative_to(root))
# as_posix, not str: `rel` is handed to the model as the literal
# source_file to emit, so a native backslash spelling on Windows
# lands in the graph and splits one file across two source_file
# forms (#683 / #2259).
rel = p.relative_to(root).as_posix()
except ValueError:
rel = str(p)
rel = Path(p).as_posix()
media = _IMAGE_MEDIA_TYPES.get(p.suffix.lower(), "image/png")
raw: bytes | None = None
if read_bytes:
+44 -5
View File
@@ -4,13 +4,15 @@ A corpus file is attacker-named; cpp does not accept a "--" end-of-options
terminator, so _cpp_preprocess passes an absolute path which can never be parsed
as a cpp option.
"""
import os
from pathlib import Path
import pytest
from graphify import extract
def test_cpp_preprocess_passes_absolute_path(tmp_path, monkeypatch):
f = tmp_path / "weird.F90"
f.write_text("program x\nend program x\n")
def _capture_cpp_argv(monkeypatch):
captured = {}
def fake_run(argv, **kwargs):
@@ -24,9 +26,46 @@ def test_cpp_preprocess_passes_absolute_path(tmp_path, monkeypatch):
monkeypatch.setattr("shutil.which", lambda name: "/usr/bin/cpp")
monkeypatch.setattr("subprocess.run", fake_run)
return captured
def test_cpp_preprocess_passes_absolute_path(tmp_path, monkeypatch):
f = tmp_path / "weird.F90"
f.write_text("program x\nend program x\n")
captured = _capture_cpp_argv(monkeypatch)
out = extract._cpp_preprocess(f)
assert out == b"preprocessed"
last_arg = captured["argv"][-1]
assert last_arg.startswith("/"), f"path arg must be absolute, got {last_arg!r}"
# Test the property, not the spelling: a leading "/" is only what "absolute"
# looks like on POSIX, so the literal check failed on a perfectly correct
# Windows path (C:\...\weird.F90). os.path.isabs answers for the host.
assert os.path.isabs(last_arg), f"path arg must be absolute, got {last_arg!r}"
assert not last_arg.startswith("-"), "path arg must never look like an option"
@pytest.mark.parametrize("hostile_name", ["-Ietc.F90", "-include.F90"])
def test_cpp_preprocess_absolutises_a_relative_attacker_named_file(
tmp_path, monkeypatch, hostile_name
):
"""The guard only does work when the incoming path is RELATIVE.
The test above hands in an already-absolute path, so it passes whether or
not `_cpp_preprocess` resolves anything removing the `.resolve()` does not
make it fail. This is the case the hardening actually exists for: a corpus
file whose own name is a cpp option, reached by a relative path.
"""
(tmp_path / hostile_name).write_text("program x\nend program x\n")
monkeypatch.chdir(tmp_path)
captured = _capture_cpp_argv(monkeypatch)
assert extract._cpp_preprocess(Path(hostile_name)) == b"preprocessed"
last_arg = captured["argv"][-1]
assert os.path.isabs(last_arg), (
f"relative path was passed through unresolved: {last_arg!r}"
)
assert not last_arg.startswith("-"), (
f"cpp would parse {last_arg!r} as an option, not a filename"
)
+28 -7
View File
@@ -7,6 +7,18 @@ from graphify import detect as detect_mod
FIXTURES = Path(__file__).parent / "fixtures"
def as_posix_list(paths) -> list[str]:
"""Normalize detect() output to forward slashes before matching on it.
detect() returns native absolute paths, so a literal like
``"vendor/sub/important.py"`` never matches on Windows. That breaks positive
assertions outright, and worse makes NEGATIVE ones
(``not any(... in ...)``) pass unconditionally, so the property the test
exists to guard is never actually checked.
"""
return [Path(p).as_posix() for p in paths]
def test_classify_python():
assert classify_file(Path("foo.py")) == FileType.CODE
@@ -445,9 +457,9 @@ def test_gitignore_nested_negation_overrides_broader_root_rule(tmp_path):
(sub / "other.py").write_text("c = 1")
result = detect(tmp_path)
code = result["files"]["code"]
code = as_posix_list(result["files"]["code"])
# nested `!important.py` re-includes it despite the root `*.py` exclude...
assert any("vendor/sub/important.py" in f for f in code)
assert any(f.endswith("vendor/sub/important.py") for f in code)
# ...while the root-excluded and non-re-included files stay out
assert not any(f.endswith("root.py") for f in code)
assert not any(f.endswith("other.py") for f in code)
@@ -467,8 +479,8 @@ def test_nested_ignore_overrides_git_info_exclude_and_root(tmp_path):
(tmp_path / "drop.py").write_text("y = 1") # only info/exclude -> excluded
result = detect(tmp_path)
code = result["files"]["code"]
assert any("a/b/keep.py" in f for f in code), "nested ! must beat root + info/exclude"
code = as_posix_list(result["files"]["code"])
assert any(f.endswith("a/b/keep.py") for f in code), "nested ! must beat root + info/exclude"
assert not any(f.endswith("drop.py") for f in code)
@@ -1016,9 +1028,18 @@ def test_path_pattern_single_star_does_not_cross_segment(tmp_path):
for pattern in ("/src/*.py", "src/*.py"):
(tmp_path / ".graphifyignore").write_text(f"{pattern}\n")
result = detect(tmp_path)
files = [path for paths in result["files"].values() for path in paths]
assert not any(path.endswith("src/main.py") for path in files)
assert any(path.endswith("src/app/main.py") for path in files)
files = as_posix_list(
path for paths in result["files"].values() for path in paths
)
# This negative is the actual subject of the test — that `*` did NOT
# cross a separator. Without the posix normalization it matched nothing
# on Windows and passed no matter what the matcher did.
assert not any(path.endswith("src/main.py") for path in files), (
f"`{pattern}` failed to exclude the direct child: {files}"
)
assert any(path.endswith("src/app/main.py") for path in files), (
f"`{pattern}` crossed a path segment and excluded the nested file: {files}"
)
def test_directory_only_negation_does_not_reinclude_file(tmp_path):
+6 -4
View File
@@ -1683,10 +1683,12 @@ def test_queue_and_drain_pending_round_trip(tmp_path):
pending_file = out / _PENDING_FILENAME
assert pending_file.exists()
# Each path written on its own line.
assert pending_file.read_text(encoding="utf-8").splitlines() == [
"a.py", "sub/b.py", "c.md",
]
# Each path written on its own line. Compared as Paths, not as strings: the
# documented contract is "one path per line" (see _queue_pending), not a
# separator convention, and os.fspath emits the native one — so a literal
# "sub/b.py" fails on Windows without any real defect.
lines = pending_file.read_text(encoding="utf-8").splitlines()
assert [Path(line) for line in lines] == paths
drained = _drain_pending(out)
assert drained == paths