diff --git a/graphify/extractors/fortran.py b/graphify/extractors/fortran.py index 58ffedfc..a3c96248 100644 --- a/graphify/extractors/fortran.py +++ b/graphify/extractors/fortran.py @@ -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, diff --git a/graphify/llm.py b/graphify/llm.py index c499678c..46a4785d 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -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: diff --git a/tests/test_cpp_preprocess.py b/tests/test_cpp_preprocess.py index 75ca8de5..e130b642 100644 --- a/tests/test_cpp_preprocess.py +++ b/tests/test_cpp_preprocess.py @@ -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" + ) diff --git a/tests/test_detect.py b/tests/test_detect.py index e9676a10..a17b182f 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -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): diff --git a/tests/test_watch.py b/tests/test_watch.py index 00572dde..c5785e79 100644 --- a/tests/test_watch.py +++ b/tests/test_watch.py @@ -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