Skip to content

fix(cron): handle embedded null byte in lifecycle guard command tokens (#77988) - #78013

Closed
webtecnica wants to merge 1 commit into
NousResearch:mainfrom
webtecnica:fix/77988-lifecycle-nul-crash
Closed

fix(cron): handle embedded null byte in lifecycle guard command tokens (#77988)#78013
webtecnica wants to merge 1 commit into
NousResearch:mainfrom
webtecnica:fix/77988-lifecycle-nul-crash

Conversation

@webtecnica

Copy link
Copy Markdown
Contributor

Summary

Fixes #77988 — a ValueError: embedded null byte raised by cron/lifecycle_guard.py when a NUL byte appears in a terminal command token (executable or script-path string). The unhandled exception propagated out of the guard on every terminal-tool invocation once a NUL byte showed up in a command token, taking down completely benign commands ("Operacja nieudana: embedded null byte w lifecycle_guard").

Distinct from #77927 (NUL-byte security bypass in file content): this is the opposite failure direction — the guard crashes instead of silently letting something through.

Root Cause

Several sites in cron/lifecycle_guard.py construct a pathlib.Path (or call os.open) directly from a token parsed out of the raw command string, without guarding against ValueError (which pathlib/os raise for an embedded NUL byte):

  1. _resolve_terminal_script_path()Path(candidate).expanduser() unguarded; runs lazily inside the _iter_referenced_shell_scripts generator, so the try/except (OSError, ValueError) in _contains_unsafe_gateway_action wraps only the later resolve(strict=False) step and never catches the generator's own Path() construction.
  2. contains_launchctl_submit_command()Path(segment[index]).name unguarded.
  3. _iter_referenced_shell_scripts()Path(executable).name unguarded.
  4. _iter_shell_command_payloads()Path(segment[index]).name unguarded.
  5. _read_referenced_script()os.open(path, flags) catches only OSError, but a NUL byte in the path string makes os.open raise ValueError.

Sites 1–4 run on every terminal-tool invocation via contains_gateway_lifecycle_command_or_referenced_script() / contains_launchctl_submit_command(), which tools/terminal_tool.py calls unconditionally before executing any shell command.

Change

Guard must never crash on unresolvable/invalid path tokens — treat them as "nothing to scan" (mirrors the existing #76762 philosophy for binary content), not as a hard failure:

  • Add _safe_path_name(token) helper: like Path(token).name but returns None instead of raising on ValueError; use it at all three Path(...).name sites.
  • _resolve_terminal_script_path() now returns Optional[Path] and returns None on ValueError; the three yield sites in _iter_referenced_shell_scripts() skip None results.
  • _read_referenced_script() catches ValueError from os.open (NUL in the path string itself, distinct from NUL in file content handled below) and returns (None, False).

Verification

  • New regression tests in tests/hermes_cli/test_gateway_restart_loop.py:
    • test_nul_byte_in_command_token_does_not_crash_guard — the issue's exact repro (bash \x00engine/scripts/portfolio_report.py --date 2026-08-03) plus NUL in a -c payload token and a .sh path token all return False instead of raising.
    • test_nul_byte_in_launchctl_token_does_not_crash_guard — NUL in the launchctl executable token tolerated.
    • test_nul_byte_tolerance_does_not_weaken_guardhermes gateway restart still blocked, launchctl submit -l ai.hermes.svc-reload-tmp -- ... still blocked, benign ls -la /tmp still passes.
  • Full existing suite passes: tests/hermes_cli/test_gateway_restart_loop.py (85 tests) and tests/tools/test_terminal_tool_requirements.py + test_local_env_blocklist.py + test_approved_command_clean_slate.py (63 tests).

Closes #77988

@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 needs-decision Awaiting maintainer decision before any implementation sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 3, 2026
@xinbao-151

Copy link
Copy Markdown

生产事故实证:这个 bug 不是理论问题,我们在生产环境被它打崩过。

事故经过(2026-08-04,Hermes v0.20.0 / v2026.8.3,macOS):

  • gateway 内 agent 会话执行一条引用脚本的 terminal 命令,lifecycle_guard 递归扫描脚本内容时 token 化出含 NUL 字节的路径
  • _read_referenced_scriptos.open(path)ValueError: embedded null byte(只捕获了 OSError)
  • 异常穿透 terminal 工具 → agent 会话卡死 30 分钟(idle watchdog 才报警)→ QQ token 过期无法刷新 → 平台断连近 6 小时,用户完全联系不上 agent,只能回电脑手动重启 gateway

验证过的本地修复(与 #78013 方向一致):

# cron/lifecycle_guard.py _read_referenced_script
try:
    descriptor = os.open(path, flags)
except (OSError, ValueError):   # ValueError: embedded NUL byte in path
    return None, False

修复后模拟 null-byte 路径不再崩溃,gateway 恢复正常。

建议: 这个修复值得尽快合并——影响面是"任何引用脚本的 terminal 命令都可能触发",对 gateway 生产环境是单点故障。合并后我们的本地补丁会移除,跟随官方升级。

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

@webtecnica

Copy link
Copy Markdown
Contributor Author

Thanks @kshitijk4poor for the architecturally cleaner fix in #80258 — sanitizing path candidates once at the source beats per-callsite handling. Agreed on closing both; glad the whole bug class is covered.

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 needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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.

lifecycle_guard: unhandled ValueError ("embedded null byte") crashes every terminal-tool call once a NUL byte appears in a command token

4 participants