Skip to content

fix(terminal): tolerate malformed lifecycle script paths - #78430

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

fix(terminal): tolerate malformed lifecycle script paths#78430
randomsnowflake wants to merge 1 commit into
NousResearch:mainfrom
randomsnowflake:fix/lifecycle-guard-nul

Conversation

@randomsnowflake

Copy link
Copy Markdown

Summary

  • treat embedded-NUL script paths as unreadable in the gateway lifecycle guard
  • prevent malformed shell input from crashing terminal before command execution
  • retain the lifecycle protection itself rather than weakening the restart-loop safeguard

Reproduction

A referenced path containing an embedded NUL reaches _read_referenced_script(). Path.resolve() already handles ValueError, but the subsequent os.open() did not, so terminal calls failed with:

ValueError: embedded null byte

Verification

  • uv run pytest -q tests/hermes_cli/test_gateway_restart_loop.py — 83 passed
  • uv run ruff check cron/lifecycle_guard.py tests/hermes_cli/test_gateway_restart_loop.py — passed

@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 4, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #78013: both prevent embedded-NUL lifecycle-guard path tokens from crashing terminal commands. #78013 covers the same os.open escape plus the earlier pathlib/token entry points and broader regressions.

@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.

3 participants