Skip to content

fix(terminal): sanitize binary lifecycle script reads - #79022

Closed
diatche wants to merge 1 commit into
NousResearch:mainfrom
diatche:fix/lifecycle-guard-remote-binary-safe
Closed

fix(terminal): sanitize binary lifecycle script reads#79022
diatche wants to merge 1 commit into
NousResearch:mainfrom
diatche:fix/lifecycle-guard-remote-binary-safe

Conversation

@diatche

@diatche diatche commented Aug 5, 2026

Copy link
Copy Markdown

User-facing problem

When Hermes runs inside the gateway, it scans terminal commands and referenced scripts to prevent tools from restarting or stopping the gateway itself.

If a command referenced an executable binary—for example, when launching Python, Codex, or another CLI—the scanner could accidentally read that binary as script text. Embedded NUL bytes could then reach filesystem path handling and raise:

ValueError: embedded null byte

From the user's perspective, the terminal command never starts. The tool call may remain pending until the gateway's inactivity watchdog times out, making the conversation appear stalled and leaving background work incomplete.

Summary

  • prevent the terminal lifecycle guard callback from feeding NUL bytes back into recursive path scanning
  • sanitize both local file bytes and remote cat output before scanning
  • preserve detection of lifecycle commands hidden in NUL-containing content instead of treating NUL as proof that a file is safe

Root cause and fix

The core lifecycle scanner already treats NUL-containing referenced files as binary leaves. terminal_tool then retried the same file through its environment reader, decoded the content with replacement, and returned embedded NUL characters to the recursive scanner. A resulting token could reach os.open(...) and raise ValueError: embedded null byte.

This PR removes NUL bytes before best-effort scanning, so referenced binaries no longer crash the guard.

Simply skipping NUL-containing content would introduce a lifecycle bypass: shells can execute commands after embedded NUL bytes, even when a file begins with an ELF- or PE-like prefix. The sanitized content therefore remains subject to normal lifecycle scanning, preserving the gateway self-protection.

Tests

  • regression: local executable binary content does not crash or hang the terminal guard
  • regression: remote binary output does not reintroduce embedded NUL bytes
  • regression: a NUL-containing script with a spoofed executable magic prefix remains blocked
  • existing remote unsafe-script scan remains blocked even with banner text

Validation:

85 passed in tests/hermes_cli/test_gateway_restart_loop.py
ruff check: passed
git diff --check: passed

@alt-glitch alt-glitch added type/bug Something isn't working tool/terminal Terminal execution and process management P2 Medium — degraded but workaround exists labels Aug 5, 2026
@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.

@diatche
diatche deleted the fix/lifecycle-guard-remote-binary-safe branch August 7, 2026 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

3 participants