fix(cron): lifecycle guard crashes with 'embedded null byte' instead of failing open — kills terminal tool (#78256) - #78448
Conversation
…ousResearch#78256) os.open() raises ValueError — not OSError — when a path contains an embedded NUL byte, and _read_referenced_script caught only OSError. A recursive scan that produced such a token (reporter: any 'python -m pip' command from inside the gateway on v0.20.0) crashed the entire guard, taking the terminal tool call down with an unhandled traceback instead of the fail-open skip NousResearch#76762 established for exactly this byte in scanned contents. Treat NUL-in-path identically to an unreadable file: nothing to scan. Regressions: unit (NUL path → (None, False)), end-to-end (guard survives 'source /tmp/e\x00vil.sh'), and the real-lifecycle-script block still fires.
Duplicate of #77898: both apply the same os.open ValueError catch for embedded-NUL referenced paths with the same fail-open behavior and regression coverage. |
|
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 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. |
|
Boundary fix over per-callsite is clearly right — the four-incidents-one-frame-apart history makes the case by itself. The adversarial never-raises sweep is the shape these guards should all converge on. Thanks for the close-out note. |
Summary
Any terminal command whose recursive script-scan produces a path token with an embedded NUL byte crashes the entire lifecycle guard — and the terminal tool call around it — with an unhandled
ValueError: embedded null byte(#78256; reporter hits it on everypython -m pip …from inside a v0.20.0 gateway). One-line-class fix restoring the fail-open contract #76762 established.Root cause
_read_referenced_script()(cron/lifecycle_guard.py:260) catches onlyOSErroraroundos.open()— but CPython raisesValueError(notOSError) for a NUL byte in the path itself:#76762 handled NUL bytes in scanned contents (binary detection) and in
Path.resolve— this is the third member of the same class, one call earlier. Reproducible on current main without any environment specifics:Changes
cron/lifecycle_guard.py:os.opengets an explicitexcept ValueError(documented — NUL-in-path is "a path the OS cannot even represent", same semantics as unreadable); the fstat/read block broadened to(OSError, ValueError).tests/hermes_cli/test_gateway_restart_loop.py:TestNulByteReferencedPathFailsOpen— unit (NUL path →(None, False)), end-to-end (guard survives the NUL-carrying command), and a positive control (real lifecycle script still blocked).Validation
_read_referenced_script(Path("x\x00y"))ValueErrorescapes(None, False)source /tmp/e\x00vil.shFalse(nothing to scan)hermes gateway restartscripts/run_tests.sh tests/hermes_cli/test_gateway_restart_loop.pyScope notes
Fail-open here is the guard's documented design (#76762): a token the OS can't even represent as a path is by definition not a readable shell script — there is nothing to scan, and a crashed guard blocks every command, which is strictly worse than skipping one unscannable token. The reporter's exact NUL source (their
piplauncher shape) isn't needed to fix the class: any NUL-carrying token now takes the same skip path as unreadable files.Fixes #78256.