Skip to content

fix(file-ops): base64-encode binary sample to avoid UTF-8 truncation false positive - #80864

Closed
8696 wants to merge 1 commit into
NousResearch:mainfrom
8696:fix/binary-detection-utf8-truncation
Closed

fix(file-ops): base64-encode binary sample to avoid UTF-8 truncation false positive#80864
8696 wants to merge 1 commit into
NousResearch:mainfrom
8696:fix/binary-detection-utf8-truncation

Conversation

@8696

@8696 8696 commented Aug 7, 2026

Copy link
Copy Markdown

Problem

_is_likely_binary samples the first 1000 bytes via head -c 1000. On files with high-density multi-byte UTF-8 (e.g. CJK text), the byte-boundary cut splits a character mid-sequence. The terminal env decodes stdout with errors="replace", turning the orphaned lead byte into U+FFFD. The old detector treated any U+FFFD in the sample as proof of a non-UTF-8 file and returned is_binary=True — blocking legitimate text files from being read.

This is reproducible: a ~3.8 KB UTF-8 Chinese text file whose 1000th byte falls on a 3-byte character lead byte (0xe8) is consistently misidentified as binary.

Root Cause

The sampling layer (head -c at byte level) and the detection layer (_is_likely_binary at character level) are connected by errors="replace" decoding, which destroys the distinction between truncation-induced U+FFFD and genuine non-UTF-8 bytes.

head -c 1000 <path>          # byte-level truncation, may split multi-byte char
  → Popen(errors="replace")  # orphaned lead byte → U+FFFD
    → _is_likely_binary()    # sees U+FFFD → "binary!" (false positive)

Fix

  • Pipe the sample through base64 so raw bytes survive the terminal env lossless. base64.b64decode() on the Python side recovers exact bytes.
  • Rewrite _is_likely_binary to accept bytes instead of str:
    1. Try strict UTF-8 decode, peeling 1–3 trailing bytes to handle truncation at a multi-byte boundary.
    2. If strict decode fails after peeling → genuinely non-UTF-8 → binary.
    3. If strict decode succeeds → fall back to the non-printable ratio heuristic for NUL-heavy content.

Scope

  • No new dependencies: base64 is Python stdlib; the base64 shell command is coreutils (same package as head already in use).
  • No backend changes: no changes to any environment backend (local/ssh/docker/modal/etc.) or the execute()/_run_bash()/_wait_for_process() pipeline.
  • 1 file changed in source (tools/file_operations.py), 1 test file updated to match the new bytes signature and | base64 sample command.

Verification

68 passed, 3 skipped in 1.05s

The previously misidentified file now correctly reads as text:

USER.md sample size: 1000 bytes
Verdict: TEXT (not binary)

…false positive

`_is_likely_binary` samples the first 1000 bytes via `head -c 1000`.
On files with high-density multi-byte UTF-8 (e.g. CJK text), the byte
boundary cut splits a character mid-sequence. The terminal env decodes
stdout with `errors="replace"`, turning the orphaned lead byte into
U+FFFD. The old detector treated any U+FFFD in the sample as proof of
a non-UTF-8 file and returned `is_binary=True` -- blocking legitimate
text files from being read.

Root cause: the sampling layer (`head -c` at byte level) and the
detection layer (`_is_likely_binary` at character level) are connected
by `errors="replace"` decoding, which destroys the distinction between
truncation-induced U+FFFD and genuine non-UTF-8 bytes.

Fix:
- Pipe the sample through `base64` so raw bytes survive the terminal
  env lossless. b64decode on the Python side recovers exact bytes.
- Rewrite `_is_likely_binary` to accept `bytes` instead of `str`:
  try strict UTF-8 decode, peeling 1-3 trailing bytes to handle
  truncation at a multi-byte boundary. If strict decode fails after
  peeling, the file is genuinely non-UTF-8 -> binary. If it succeeds,
  fall back to the non-printable ratio heuristic for NUL-heavy content.
- No new dependencies (`base64` is stdlib; the `base64` shell command
  is coreutils, same package as `head` already in use).
- No changes to any environment backend (local/ssh/docker/modal/etc.)
  or the `execute()`/`_run_bash()`/`_wait_for_process()` pipeline.

Tests updated to match the new `bytes` signature and the `| base64`
sample command.
@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets tool/file File tools (read, write, patch, search) P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Aug 7, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #80440: both preserve the raw head sample through base64 and classify it at the byte layer to avoid the CJK UTF-8 boundary false positive. #80440 is the earlier, more defensive implementation.

@teknium1

teknium1 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Closing — superseded by #81961 (salvaged from #80440). Same base64-sample idea, and you had it early; #80440 won on a stricter boundary contract + NUL rigor. Thanks for the well-aimed fix.

@teknium1 teknium1 closed this Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists tool/file File tools (read, write, patch, search) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants