Skip to content

fix: never crash lifecycle guard on NUL-byte paths from binaries - #79038

Closed
etc wants to merge 1 commit into
NousResearch:mainfrom
etc:fix/lifecycle-guard-nul-byte-crash
Closed

fix: never crash lifecycle guard on NUL-byte paths from binaries#79038
etc wants to merge 1 commit into
NousResearch:mainfrom
etc:fix/lifecycle-guard-nul-byte-crash

Conversation

@etc

@etc etc commented Aug 5, 2026

Copy link
Copy Markdown

Summary

cron/lifecycle_guard.py — the gateway lifecycle guard — crashes with ValueError: embedded null byte when a terminal command references a binary executable (any executable whose path contains a slash). Inside a gateway process this takes down every guarded terminal call, e.g. uv pip install ... or python -c ..., with the raw traceback surfaced as the tool error.

Root cause

  1. _iter_referenced_shell_scripts treats any executable containing a / as a script to scan.
  2. _read_referenced_script reads the first chunk and deliberately skips binaries (NUL bytes) by returning (None, False).
  3. _contains_unsafe_gateway_action interprets script_text is None as "file missing" and calls read_remote_script — which re-fetches the file's raw bytes as text (via the cat fallback in _read_script_in_env).
  4. The decoded binary content is tokenized as shell; a token containing a NUL byte becomes a Path, and _read_referenced_script's os.open(path) raises ValueError: embedded null byte — uncaught, because the existing terminal tool: lifecycle_guard crashes on absolute-path executables (ValueError: embedded null byte), blocks all such commands #76762 guard only wrapped Path.resolve().

Fix

  • Safety net: os.open now catches (OSError, ValueError) and returns (None, False) — a guarded path must never crash the guard.
  • Root cause: the binary skip now returns ("", False) instead of (None, False), so the remote-read fallback (keyed on script_text is None) is never triggered for content that was deliberately skipped as non-script.

Verification

  • 86/86 tests pass in tests/hermes_cli/test_gateway_restart_loop.py, including 4 new regression tests:
    • a NUL-byte path is skipped by the read, not crashed on
    • a binary referenced script returns "" (not None)
    • a command referencing a local binary passes the guard and executes normally
    • binary content returned by the remote-read fallback does not crash the guard
  • Live on a gateway process: python -c "print(1)" and uv pip list previously crashed with the ValueError; after the fix they execute normally, while blocking of hermes gateway restart / launchctl submit still holds.

Related: #76762 (a guarded path must never crash the guard), #30719 (original restart-loop defense).

A terminal command referencing a binary executable (any executable whose
path contains a slash, e.g. `uv pip install ...` or `python -c ...`)
crashed the gateway lifecycle guard with `ValueError: embedded null
byte` from os.open. The local scan deliberately skips binaries by their
NUL bytes, but the remote-read fallback then re-fetched the raw bytes as
text; tokenizing machine code produced NUL-byte paths that the guard's
os.open call did not survive. The NousResearch#76762 fix guarded Path.resolve but
missed os.open.

- Catch ValueError in _read_referenced_script's os.open: a guarded path
  must never crash the guard
- Return "" instead of None when a referenced file is a binary, so the
  remote-read fallback is not triggered for content that was
  deliberately skipped as non-script
- Regression tests: NUL-byte path read, binary skip contract, binary
  referenced script passes the guard, binary remote content does not
  crash it

Related: NousResearch#76762, NousResearch#30719
@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 needs-decision Awaiting maintainer decision before any implementation labels Aug 5, 2026
@etc

etc commented Aug 5, 2026

Copy link
Copy Markdown
Author

Note: This PR was created by my Hermes agent using DeepSeek V4 Flash 0731. It fixes a bug that kept slowing it down on various coding tasks.

@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 needs-decision Awaiting maintainer decision before any implementation 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