Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions tests/tools/test_file_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -673,3 +673,81 @@ def test_plain_utf8_text_not_flagged(self, tmp_path):
ops = ShellFileOperations(make_real_subprocess_env(str(tmp_path)))
# Proper UTF-8 (including non-ASCII) must still read as text.
assert ops._is_likely_binary("notes.txt", "café résumé\nsecond\n") is False

# ------------------------------------------------------------------
# Regression #76886: a valid UTF-8 file whose 1000-byte sample cuts a
# multibyte character must NOT be flagged binary. `head -c 1000` cuts at
# a byte boundary, the errors="replace" decode then fabricates a U+FFFD
# the file never contained, and the U+FFFD check above used to reject it.
# ------------------------------------------------------------------

def test_sample_cut_multibyte_char_not_flagged_binary(self, tmp_path):
"""Issue repro: 'ç' starting at byte 1000 → lossy sample ends with a
synthetic U+FFFD, but the file is valid UTF-8 from start to finish."""
ops = ShellFileOperations(make_real_subprocess_env(str(tmp_path)))
path = tmp_path / "fails.md"
path.write_bytes(b"a" * 999 + "\u00e7\nx\n".encode("utf-8")) # 'ç' at byte 1000
lossy_sample = "a" * 999 + "\ufffd" # what the terminal env's decode yields
assert ops._is_likely_binary(str(path), lossy_sample) is False

def test_real_non_utf8_byte_at_sample_boundary_still_binary(self, tmp_path):
"""A genuine non-UTF-8 byte landing exactly on the sample boundary
must stay binary — the mojibake round-trip guard still applies."""
ops = ShellFileOperations(make_real_subprocess_env(str(tmp_path)))
path = tmp_path / "latin.md"
path.write_bytes(b"a" * 999 + b"\xe9\n") # lone latin-1 é at byte 1000
lossy_sample = "a" * 999 + "\ufffd"
assert ops._is_likely_binary(str(path), lossy_sample) is True

def _make_lossy_decode_env(self, tmp_path, content: bytes):
"""Mock env mimicking the real terminal backend: stdout decoded with
errors=\"replace\", so a byte-truncated sample fabricates U+FFFD."""
path = tmp_path / "sample.md"
path.write_bytes(content)
env = MagicMock()
env.cwd = str(tmp_path)

def execute(command, **kwargs):
if "od -An -v -tx1" in command:
n = int(command.split("head -c ", 1)[1].split()[0])
data = path.read_bytes()[:n]
return {"output": " " + " ".join(f"{b:02x}" for b in data) + "\n", "returncode": 0}
if command.startswith("wc -c"):
return {"output": f"{path.stat().st_size}\n", "returncode": 0}
if command.startswith("head -c 1000"):
data = path.read_bytes()[:1000]
return {"output": data.decode("utf-8", errors="replace"), "returncode": 0}
if command.startswith("sed -n"):
return {"output": path.read_text(encoding="utf-8"), "returncode": 0}
if command.startswith("cat "):
return {"output": path.read_text(encoding="utf-8"), "returncode": 0}
if command.startswith("wc -l"):
newline_count = path.read_bytes().count(b"\n")
return {"output": f"{newline_count}\n", "returncode": 0}
return {"output": "", "returncode": 0}

env.execute = execute
ops = ShellFileOperations(env)
return ops, path

def test_read_file_utf8_sample_cut_multibyte_char_ok(self, tmp_path):
"""End-to-end: read_file must open a valid UTF-8 file whose 1000-byte
sample boundary cuts a multibyte char (issue's exact fails.md)."""
ops, path = self._make_lossy_decode_env(
tmp_path, b"a" * 999 + "\u00e7\nx\n".encode("utf-8")
)
result = ops.read_file(str(path))
assert result.is_binary is False
assert result.error is None
assert "ç" in result.content

def test_read_file_raw_utf8_sample_cut_multibyte_char_ok(self, tmp_path):
"""End-to-end: read_file_raw has the same sampling path and must also
accept valid UTF-8 whose sample boundary cuts a multibyte char."""
ops, path = self._make_lossy_decode_env(
tmp_path, b"a" * 999 + "\u00e7\nx\n".encode("utf-8")
)
result = ops.read_file_raw(str(path))
assert result.is_binary is False
assert result.error is None
assert "ç" in result.content
56 changes: 54 additions & 2 deletions tools/file_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -893,14 +893,66 @@ def _is_likely_binary(self, path: str, content_sample: str = None) -> bool:
# sample carries the replacement char as binary (read-only) so the
# agent can't corrupt it. Legitimate UTF-8 text effectively never
# contains U+FFFD.
if "\ufffd" in content_sample[:1000]:
sample = content_sample[:1000]
if "\ufffd" in sample and not self._sample_ufffd_is_truncation_artifact(path, sample):
return True
non_printable = sum(1 for c in content_sample[:1000]
non_printable = sum(1 for c in sample
if ord(c) < 32 and c not in '\n\r\t')
return non_printable / min(len(content_sample), 1000) > 0.30

return False

def _sample_raw_bytes(self, path: str, byte_count: int) -> bytes:
"""Read up to ``byte_count`` raw bytes of ``path`` without loss.

The terminal env decodes stdout with errors="replace", which destroys
byte-level information (and fabricates U+FFFD for truncated multibyte
chars). Piping through ``od -An -v -tx1`` emits the raw bytes as plain
ASCII hex, which survives the lossy decode round-trip intact.
"""
cmd = (
f"head -c {byte_count} {self._escape_shell_arg(path)} 2>/dev/null "
"| od -An -v -tx1"
)
result = self._exec(cmd)
if result.exit_code != 0:
return b""
hex_text = _strip_terminal_fence_leaks(result.stdout)
try:
return bytes.fromhex(hex_text)
except ValueError:
return b""

def _sample_ufffd_is_truncation_artifact(self, path: str, sample: str) -> bool:
"""True if the sample's U+FFFD is a decode artifact, not file content.

``head -c 1000`` cuts at a byte boundary, so it can slice a multibyte
UTF-8 char in half; the terminal env's errors="replace" decode then
emits a SYNTHETIC U+FFFD the file never contained. Such an artifact is
always the LAST character of the sample and is the sample's only
U+FFFD (a cut sequence decodes to exactly one). When that pattern
matches, re-read the raw bytes and strict-decode slightly larger
windows: valid UTF-8 decodes cleanly once the boundary crosses onto a
character edge, so the U+FFFD was fabricated; a file that keeps
failing really does contain non-UTF-8 bytes and stays binary.
"""
if sample.count("\ufffd") != 1 or not sample.endswith("\ufffd"):
return False
# A cut UTF-8 char needs at most 3 extra bytes to complete (max
# 4-byte sequence), so any of these windows is enough for a valid
# file; a file ending mid-character (invalid UTF-8 at EOF) never
# decodes and stays binary.
for byte_count in (1000, 1004, 1008, 1016, 1032, 1064):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These fixed endpoints are not character-safe: a valid UTF-8 file can place another multibyte character across each of 1000, 1004, 1008, 1016, 1032, and 1064, making every whole-prefix strict decode fail with unexpected end of data. Use strict incremental decoding of a bounded extension (finalizing only at EOF) so a later boundary cut does not reintroduce the false binary classification.

raw = self._sample_raw_bytes(path, byte_count)
if not raw:
return False
try:
raw.decode("utf-8", errors="strict")
return True
except UnicodeDecodeError:
continue
return False

def _is_image(self, path: str) -> bool:
"""Check if file is an image we can return as base64."""
ext = os.path.splitext(path)[1].lower()
Expand Down
Loading