Skip to content

fix(gateway): bound lifecycle scans of binary paths - #79375

Closed
poctek wants to merge 1 commit into
NousResearch:mainfrom
poctek:fix/lifecycle-guard-binary-fallback
Closed

fix(gateway): bound lifecycle scans of binary paths#79375
poctek wants to merge 1 commit into
NousResearch:mainfrom
poctek:fix/lifecycle-guard-binary-fallback

Conversation

@poctek

@poctek poctek commented Aug 5, 2026

Copy link
Copy Markdown

Summary

  • distinguish missing referenced scripts from local binaries before remote fallback
  • skip NUL-bearing local and remote binary content instead of recursively tokenizing it
  • handle NUL-bearing candidate paths without raising ValueError or calling the remote reader
  • cap remote script reads at 1 MiB + 1 byte and fail closed on oversized text
  • preserve remote scanning for scripts that are genuinely absent on the host

Why this is still needed after #77703

#77703 prevents the eventual ValueError and skips NUL-bearing output, but the remote fallback still uses an unbounded cat. For a local executable larger than 1 MiB, _read_script_in_env() skips its bounded local branch and then cats the same file through the environment. The NUL check only runs after the entire binary has been captured.

This PR fixes that remaining memory/hang path by distinguishing missing from present but binary, and by bounding genuinely remote reads. In production, a 273 MB Rust debug ELF drove the gateway to multi-GB memory use and stalled it before the executable ran.

Root cause

_read_referenced_script() returned None both when a local path was missing and when it had positively identified a NUL-bearing binary. The caller interpreted both states as remotely missing and invoked read_remote_script. Path-based executables could therefore be read again and fed into the lifecycle regex/path walk.

The tri-state result now permits remote fallback only when os.open() could not open the path. Once a local path has opened, binary detection or a later read failure cannot reclassify it as remotely missing.

Tests

  • uv run --extra dev pytest -q tests/hermes_cli/test_gateway_restart_loop.py tests/tools/test_terminal_tool.py tests/cron — 493 passed
  • uv run --extra dev ruff check cron/lifecycle_guard.py tools/terminal_tool.py tests/hermes_cli/test_gateway_restart_loop.py tests/tools/test_terminal_tool.py — passed
  • python -m py_compile for all modified Python files — passed
  • real 273 MB target/debug/solana-deploy lifecycle-guard harness — allowed in 0.004s with no remote callback

One pre-existing cron test warning remains: _send_to_platform coroutine was never awaited.

Fixes #78800
Related: #76762, #77703, #78942

@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 sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation labels Aug 5, 2026
Distinguish missing paths from local binaries before remote fallback, reject unscannable NUL path candidates without crashing, and cap remote script reads before recursive lifecycle scanning.\n\nAdds regression coverage for local and remote binaries, oversized remote content, missing remote scripts, and post-open read failures.\n\nFixes NousResearch#78800.
@poctek
poctek force-pushed the fix/lifecycle-guard-binary-fallback branch from fd01eba to 3e297e4 Compare August 5, 2026 19:41
@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 P2 Medium — degraded but workaround exists sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation 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.

Gateway lifecycle guard crashes with "ValueError: embedded null byte" on commands referencing binary paths

3 participants