diff --git a/tests/v1/test_git_utils.py b/tests/v1/test_git_utils.py new file mode 100644 index 0000000000..092ac7a8f0 --- /dev/null +++ b/tests/v1/test_git_utils.py @@ -0,0 +1,65 @@ +"""Focused coverage for the filename-handling gaps in #2196: ignore entries +must reach git as literal pathspecs, and non-UTF-8 filenames must fail loudly +instead of producing a wrong patch.""" + +from types import SimpleNamespace + +import pytest + +from verifiers.v1.runtimes.base import ProgramResult +from verifiers.v1.utils.git import capture_patch, snapshot_untracked + + +class ScriptedRuntime: + """Duck-typed runtime: records run() argv and plays back scripted results.""" + + def __init__(self, results: list[ProgramResult], read_data: bytes = b""): + self.calls: list[list[str]] = [] + self._results = list(results) + self._read_data = read_data + + async def run(self, cmd: list[str], env: dict) -> ProgramResult: + self.calls.append(list(cmd)) + return self._results.pop(0) + + async def read(self, path: str) -> bytes: + return self._read_data + + +@pytest.mark.asyncio +async def test_capture_patch_passes_ignore_entries_as_literal_pathspecs(): + # A file named `*.py` must not glob-unstage every Python file the agent + # touched, and a leading `:` must not be parsed as pathspec magic. + ok = ProgramResult(exit_code=0, stdout="", stderr="") + runtime = ScriptedRuntime([ok, ok], read_data=b"") + trace = SimpleNamespace(info={}) + + await capture_patch(trace, runtime, ignore=["*.py", "weird[name].txt", ":odd"]) + + diff_argv = runtime.calls[0] + assert diff_argv[4:] == [ + ":(literal)*.py", + ":(literal)weird[name].txt", + ":(literal):odd", + ] + + +@pytest.mark.asyncio +async def test_snapshot_untracked_splits_nul_delimited_names(): + runtime = ScriptedRuntime( + [ProgramResult(exit_code=0, stdout="a.txt\0dir/b bin\0", stderr="")] + ) + assert await snapshot_untracked(runtime) == ["a.txt", "dir/b bin"] + + +@pytest.mark.asyncio +async def test_snapshot_untracked_rejects_lossily_decoded_names(): + # Runtimes decode stdout with errors="replace": a non-UTF-8 filename + # arrives as U+FFFD and can never match the real file again. Returning it + # would leave the file out of the ignore set and credit the agent with an + # image file — fail before a wrong patch can be produced. + runtime = ScriptedRuntime( + [ProgramResult(exit_code=0, stdout="caf�.bin\0", stderr="")] + ) + with pytest.raises(ValueError, match="not\\s+valid UTF-8"): + await snapshot_untracked(runtime) diff --git a/verifiers/v1/utils/git.py b/verifiers/v1/utils/git.py index 0a29b835b3..7864b7680b 100644 --- a/verifiers/v1/utils/git.py +++ b/verifiers/v1/utils/git.py @@ -100,7 +100,21 @@ async def snapshot_untracked(runtime: Runtime, env: dict | None = None) -> list[ ) if result.exit_code != 0: return [] - return [path for path in (result.stdout or "").split("\0") if path] + stdout = result.stdout or "" + # Runtimes decode stdout with errors="replace", so a filename that isn't + # valid UTF-8 reaches us with U+FFFD substituted — it can never match the + # real file again, and capture_patch would credit the agent with an image + # file it didn't create. A wrong patch is worse than a loud setup failure. + # (A filename legitimately containing U+FFFD trips this too; that is the + # price of the runtimes' lossy text contract.) + if "�" in stdout: + raise ValueError( + "snapshot_untracked: `git ls-files` returned a filename that is not " + "valid UTF-8; it cannot round-trip through the runtime's text " + "decoding, so the pre-agent untracked set would be wrong. Rename " + "the offending file in the image or exclude it via .gitignore." + ) + return [path for path in stdout.split("\0") if path] async def capture_patch( @@ -142,9 +156,14 @@ async def capture_patch( nonce = uuid.uuid4().hex full, capped = f"{_FULL}_{nonce}", f"{_CAPPED}_{nonce}" cmd = _DIFF.format(full=full, capped=capped, cap=PATCH_CAP_BYTES + 1) + # `git reset -- ` treats arguments as pathspecs, so a file named + # `*.py` would unstage every Python file the agent touched. `:(literal)` + # makes git match the name byte-for-byte (and also neutralizes names that + # start with `:`, which git would otherwise parse as pathspec magic). + ignore_pathspecs = [f":(literal){path}" for path in ignore or []] try: result = await runtime.run( - ["sh", "-c", cmd, "vf-capture-patch", *(ignore or [])], + ["sh", "-c", cmd, "vf-capture-patch", *ignore_pathspecs], {**(env or {}), "VF_DIFF_BASE": base_commit or "HEAD"}, ) if result.exit_code != 0: