fix(guard): don't cat binaries via lifecycle-guard remote fallback

[yang-fireworkhq]

Bug Description

Running a terminal command that references an existing binary by its absolute path (e.g. --version on a large native CLI installed under a full path) makes the terminal-tool lifecycle guard hang for ~30 minutes, then fail with:

ValueError: embedded null byte

Concrete symptoms: a terminal tool call that should return in <1s instead hangs the whole agent turn for 20–30 minutes and eventually errors out. Two occurrences of this exact signature were observed in production logs (1769s and 1944s tool-call durations), both triggered by commands that referenced a 275MB binary by full path.

Root Cause

tools/terminal_tool.py runs contains_gateway_lifecycle_command_or_referenced_script(command, ..., read_remote_script=_read_script_in_env) before executing every command. The referenced-script walk (cron/lifecycle_guard.py) yields absolute executable paths from the command, then calls _read_referenced_script on each.

_read_referenced_script deliberately skips binaries — it reads the first chunk, sees a NUL byte, and returns (None, False) ("nothing to scan", #76762). The bug: the caller can't distinguish that from "path missing", so when read_remote_script is supplied it falls back to:

env.execute(f"cat {shlex.quote(script_path)}")

For a 275MB ELF binary this returns the entire decoded machine code, which the walk then recursively tokenizes and scans: ~800k junk segments, each doing Path()/os.open() work, recursing into any real paths embedded in the binary — a ~30 minute hang that finally crashes when a NUL-laden junk path reaches os.open (ValueError: embedded null byte). The existing #76762 fix only covered the resolve() site, not this os.open path through the remote fallback.

Fix

cron/lifecycle_guard.py

• Only invoke the remote fallback when the local path is genuinely absent (resolved.exists() and is_file()). A NUL-skipped binary is "nothing to scan", not "missing" — so existing binaries are never cat-ed.
• Wrap _read_referenced_script in try/except (OSError, ValueError) so a NUL-laden junk path can never crash the guard, mirroring the existing resolve() guard.

tools/terminal_tool.py (defense in depth)

• _read_script_in_env: cap env.execute output at 1MB and return None when the first 4KB contains a NUL byte, so a genuinely remote large binary (SSH/Modal/Daytona) can't trigger the same explosion.

Remote-backend behavior is preserved: a genuinely missing path still goes through read_remote_script.

How to Verify

1. python -m pytest tests/hermes_cli/test_gateway_restart_loop.py -o 'addopts=' -q → 84 passed
2. Regression test test_existing_binary_path_does_not_trigger_remote_fallback fails on main (the fallback spy is invoked with the binary path) and passes with this fix.
3. Manual: contains_gateway_lifecycle_command_or_referenced_script("<path-to-large-binary> --version", cwd="/", read_remote_script=<spy>) returns in milliseconds with zero spy calls on the fixed code; the pre-fix code calls the spy with the binary path.

Test Plan

• Added regression tests (existing binary path → no remote fallback; missing path → fallback still fires)
• Existing tests still pass (84/84 in the guard test module)
• Manual verification of the fix (see above)

Risk Assessment

Low — the change only narrows when the remote cat fallback fires (existing local file → never) and hardens two crash sites. The genuinely-missing-path case (the only one remote backends rely on) is covered by a dedicated test.

[kshitijk4poor]

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.