From 51f12c407c28350d7d5f88a9cd242b26a89e9ee6 Mon Sep 17 00:00:00 2001 From: "Fuad Al Fajri (lightnet19)" Date: Wed, 5 Aug 2026 13:40:42 +0700 Subject: [PATCH] fix: terminal tool crashes with 'embedded null byte' when command references a binary via absolute path The gateway lifecycle guard (cron/lifecycle_guard.py) scans command tokens for referenced shell scripts. _read_referenced_script already skips binaries (NUL byte in first chunk -> 'nothing to scan', #76762), but tools/terminal_tool.py passes a remote-read fallback (_read_script_in_env) as read_remote_script=. When the local read reports 'nothing to scan', the guard calls the fallback, which decoded the ELF bytes with errors='replace'. NUL (U+0000) is valid UTF-8, so it survives into the returned text. The guard then recursed into that NUL-laden text as if it were a shell script; tokenization produced paths with embedded NUL bytes and os.open raised ValueError: embedded null byte, failing every terminal call that references a binary by absolute path (e.g. venv python). Fix the whole bug class: - tools/terminal_tool.py: _read_script_in_env now mirrors _read_referenced_script and returns None when the file chunk contains a NUL byte (binary == nothing to scan); also guards the remote cat output for NUL. - cron/lifecycle_guard.py: _read_referenced_script tolerates ValueError from os.open just as it already tolerates it from Path.resolve, so a NUL-bearing path token can never crash the guard. Adds two regression tests: - test_binary_read_via_remote_callback_does_not_crash_guard (reproduces the gateway path: read_remote_script decodes a binary with errors='replace') - test_nul_bearing_script_path_does_not_crash_guard (defense-in-depth) Full suite: 84 passed. Runtime verified after the gateway was restarted: venv/bin/python -c 'print(1)' previously crashed, now runs clean. --- cron/lifecycle_guard.py | 5 +- tests/hermes_cli/test_gateway_restart_loop.py | 52 +++++++++++++++++++ tools/terminal_tool.py | 18 ++++++- 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/cron/lifecycle_guard.py b/cron/lifecycle_guard.py index 6c7a5eaad062f..0ac337198eccd 100644 --- a/cron/lifecycle_guard.py +++ b/cron/lifecycle_guard.py @@ -258,7 +258,10 @@ def _read_referenced_script(path: Path) -> tuple[Optional[str], bool]: flags = os.O_RDONLY | getattr(os, "O_NONBLOCK", 0) try: descriptor = os.open(path, flags) - except OSError: + except (OSError, ValueError): + # OSError: unreadable/missing paths. ValueError: embedded NUL byte — + # a NUL-bearing path token must never crash the guard (#76762). + # Both mean "no script to scan", not "unsafe". return None, False try: metadata = os.fstat(descriptor) diff --git a/tests/hermes_cli/test_gateway_restart_loop.py b/tests/hermes_cli/test_gateway_restart_loop.py index bd90e99130101..d205794217c12 100644 --- a/tests/hermes_cli/test_gateway_restart_loop.py +++ b/tests/hermes_cli/test_gateway_restart_loop.py @@ -8,7 +8,10 @@ import json import os +import sys from argparse import Namespace +from pathlib import Path +from typing import Optional import pytest @@ -695,6 +698,55 @@ def test_absolute_path_binary_does_not_crash_guard(self): ) assert result is False + def test_binary_read_via_remote_callback_does_not_crash_guard(self, tmp_path): + """#76762 regression: a read_remote_script fallback that decodes a + binary's bytes must not crash the guard's recursion with + ValueError: embedded null byte. + + The gateway passes terminal_tool's _read_script_in_env here; before + the fix it read a binary's bytes and decoded them with + errors="replace" — NUL (U+0000) is valid UTF-8, so it survived into + the returned text. The recursion then re-tokenized NUL-laden machine + code into paths and os.open crashed on the embedded NUL. A small + synthetic binary keeps the walk deterministic and fast. + """ + from cron.lifecycle_guard import ( + contains_gateway_lifecycle_command_or_referenced_script, + ) + + binary = tmp_path / "tiny.bin" + binary.write_bytes(b"\x7fELF\x02\x01\x01\x00\x00\x00print(1)\x00") + + def binary_remote_read(script_path: str) -> Optional[str]: + # Reproduce the pre-fix _read_script_in_env behavior. NUL bytes + # survive errors="replace" decoding. Missing/unreadable/NUL paths + # yield nothing rather than raising. + try: + data = Path(script_path).read_bytes() + except (OSError, ValueError): + return None + return data.decode("utf-8", errors="replace") + + result = contains_gateway_lifecycle_command_or_referenced_script( + f'{binary} -c "print(1)"', + read_remote_script=binary_remote_read, + ) + assert result is False + + def test_nul_bearing_script_path_does_not_crash_guard(self): + """#76762 defense-in-depth: a command token containing an embedded NUL + byte must never crash _read_referenced_script's os.open with + ValueError — the guard tolerates ValueError from os.open just as it + already tolerates it from Path.resolve.""" + from cron.lifecycle_guard import ( + contains_gateway_lifecycle_command_or_referenced_script, + ) + + result = contains_gateway_lifecycle_command_or_referenced_script( + "/usr/bin/python3\x00evil -c 'print(1)'" + ) + assert result is False + def test_shell_script_reference_walk_still_works(self, tmp_path): """The referenced-script walk still applies to real shell scripts: a .sh script that itself invokes a lifecycle command is caught.""" diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index d929947f41e07..ee88079cb22e9 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -2545,6 +2545,17 @@ def _read_script_in_env(script_path: str) -> Optional[str]: if stat.S_ISREG(metadata.st_mode) and metadata.st_size <= 1024 * 1024: data = local_path.read_bytes() if len(data) <= 1024 * 1024: + # Skip binaries: a NUL byte in the chunk marks + # machine code, not a shell script. Decoding it + # with errors="replace" keeps U+0000 intact (NUL + # is valid UTF-8), and the guard's recursion + # would re-tokenize that text into NUL-bearing + # paths that crash os.open with + # `ValueError: embedded null byte` (#76762). + # Mirror _read_referenced_script: binary == + # "nothing to scan". + if b"\x00" in data: + return None return data.decode("utf-8", errors="replace") except Exception: pass @@ -2552,7 +2563,12 @@ def _read_script_in_env(script_path: str) -> Optional[str]: try: result = env.execute(f"cat {shlex.quote(script_path)}") if result.get("returncode", -1) == 0: - return result.get("output", "") + output = result.get("output", "") + # Same NUL guard for remote reads: a binary dumped by + # `cat` must not feed NUL-laden text into the guard. + if "\x00" in output: + return None + return output except Exception: pass return None