Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion cron/lifecycle_guard.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
52 changes: 52 additions & 0 deletions tests/hermes_cli/test_gateway_restart_loop.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@

import json
import os
import sys
from argparse import Namespace
from pathlib import Path
from typing import Optional

import pytest

Expand Down Expand Up @@ -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."""
Expand Down
18 changes: 17 additions & 1 deletion tools/terminal_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -2545,14 +2545,30 @@ 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
# Remote / sandboxed backend: read via the environment's shell.
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
Expand Down