Skip to content

fix(terminal): guard against NUL bytes in referenced script scanning - #79390

Closed
argokaz wants to merge 1 commit into
NousResearch:mainfrom
argokaz:fix/null-byte-lifecycle-guard
Closed

fix(terminal): guard against NUL bytes in referenced script scanning#79390
argokaz wants to merge 1 commit into
NousResearch:mainfrom
argokaz:fix/null-byte-lifecycle-guard

Conversation

@argokaz

@argokaz argokaz commented Aug 5, 2026

Copy link
Copy Markdown

Summary

Fixes the "embedded null byte" crash in the terminal tool's lifecycle guard. When a terminal command references a binary (e.g. venv/bin/python), the guard's fallback script reader (_read_script_in_env in tools/terminal_tool.py) read the file as text without filtering binaries. The ELF got decoded as text, machine code was tokenized into junk paths, and the recursion hit os.open(path) with an embedded NUL → ValueError: embedded null byte on every terminal-tool call.

The primary reader (_read_referenced_script in cron/lifecycle_guard.py) already returns None for files containing NUL bytes (#76762) — this PR mirrors that behavior in the fallback path, which was missed.

Changes

Two fix sites in _read_script_in_env:

  1. Local read path — return None when the bytes contain a NUL byte (if b"\x00" in data: return None), mirroring _read_referenced_script.
  2. cat fallback path — the local read is skipped for files >1MB (st_size <= 1024*1024 guard), so a large binary falls through to env.execute("cat ...") whose output carries live NUL bytes (0x00 is valid UTF-8; errors="replace" does NOT strip it). Added if "\x00" in output: return None before returning.

Repro

Any terminal command referencing a binary path crashes the guard:

~/.hermes/hermes-agent/venv/bin/python -m pip install foo
# ValueError: embedded null byte — guard crashes before the command runs

Workarounds existed (run via a separate .py file, avoid $VAR paths in scripts, split compound commands) but the root cause is the unfiltered binary read.

Test plan

  • Reproduced the crash on current main with venv/bin/python-referencing commands
  • Applied both fixes; the same commands now pass the guard and execute normally
  • Verified no behavior change for legit script scanning (text files still scanned)

Closes #77988

The lifecycle guard's fallback script reader (_read_script_in_env in
tools/terminal_tool.py) read files as text WITHOUT filtering binaries,
while the primary reader (_read_referenced_script in
cron/lifecycle_guard.py) correctly returns None for files containing
NUL bytes (NousResearch#76762).

When a terminal command references a binary (e.g. venv/bin/python), the
fallback decoded the ELF as text, tokenized machine code into junk
paths, and the recursion hit os.open(path) with an embedded NUL,
crashing with "embedded null byte" on every terminal-tool call.

Two fix sites needed:
1. LOCAL read path: return None when the bytes contain a NUL byte
   (mirrors _read_referenced_script).
2. cat FALLBACK path: the local read is skipped for files >1MB
   (st_size <= 1024*1024 guard), so a large binary falls through to
   env.execute("cat ...") whose output carries live NUL bytes —
   return None when the output contains a NUL byte.

Fixes the crash class reported in NousResearch#77988.
@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management tool/terminal Terminal execution and process management P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Aug 5, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #79022: both patches add the same local and remote NUL-binary guards in tools/terminal_tool.py before recursive lifecycle scanning.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Closing as superseded by #80258, which fixes this whole bug class architecturally rather than per-callsite: path candidates are sanitized once at the ingestion boundary (NUL/empty/unexpandable tokens rejected before any OS call), text from any read_remote_script callback is sanitized at the recursion boundary (NUL = binary = nothing to scan; >1 MiB = fail closed), the remote fallback read is bounded at the source (head -c, so oversized binaries never cross the wire), and the public guard is total by construction — an unexpected walk failure logs and falls back to the direct-scan verdict instead of breaking every terminal command.

Your report and fix targeted a real member of this class — thank you. The per-callsite patches kept leaving sibling frames exposed (#76762#77703#77780#78256 each crashed one frame away from the previous fix), which is why we went with the boundary fix instead of merging the fragments individually. #80258 carries regression tests for the NUL-path, binary-callback, oversized-read, unset-HOME, and walk-crash cases plus an adversarial never-raises sweep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lifecycle_guard: unhandled ValueError ("embedded null byte") crashes every terminal-tool call once a NUL byte appears in a command token

3 participants