fix(cron): make the lifecycle guard total — sanitize at ingestion, not per-syscall

cron/lifecycle_guard.py

@@ -35,13 +35,16 @@
 
 from __future__ import annotations
 
+import logging
 import os
 import re
 import shlex
 import stat
 from pathlib import Path
 from typing import Callable, Iterator, Optional
 
+logger = logging.getLogger(__name__)
+
 
 class GatewayLifecycleBlocked(ValueError):
     """Raised when a cron job spec contains a gateway-lifecycle command."""
@@ -170,10 +173,41 @@ def contains_launchctl_submit_command(command: str) -> bool:
     return False
 
 
-def _resolve_terminal_script_path(candidate: str, cwd: Optional[str]) -> Path:
-    path = Path(candidate).expanduser()
+def _expand_candidate_path(candidate: str) -> Optional[Path]:
+    """Sanitize a tokenized path candidate at the ingestion boundary.
+
+    Candidate tokens come from shlex-splitting arbitrary command text —
+    including text recursively decoded from binaries or remote reads — so
+    they can carry NUL bytes or other junk no real filesystem path can
+    contain. Every OS-facing ``Path`` operation downstream (``expanduser``,
+    ``os.open``, ``resolve``) raises a *different* exception for the same
+    junk (``ValueError: embedded null byte``, ``RuntimeError: Could not
+    determine home directory`` when HOME is unset under launchd, OSError
+    for over-long paths). Rejecting here — once, before any OS call — is
+    the whole-class fix; catching per-syscall was the whack-a-mole that
+    produced #76762, #77703, #77780, and #78256.
+
+    Returns ``None`` for candidates that cannot be a real path (nothing to
+    scan), otherwise the ``expanduser()``-expanded ``Path``.
+    """
+    if not candidate or "\x00" in candidate:
+        return None
+    try:
+        return Path(candidate).expanduser()
+    except (ValueError, RuntimeError, OSError):
+        return None
+
+
+def _resolve_terminal_script_path(candidate: str, cwd: Optional[str]) -> Optional[Path]:
+    path = _expand_candidate_path(candidate)
+    if path is None:
+        return None
     if not path.is_absolute():
-        path = Path(cwd or Path.cwd()) / path
+        try:
+            path = Path(cwd or Path.cwd()) / path
+        except OSError:
+            # Path.cwd() can raise when the process cwd was deleted.
+            return None
     return path
 
 
@@ -192,7 +226,9 @@ def _iter_referenced_shell_scripts(
 
         if executable_name in {".", "source"}:
             if len(segment) > index + 1:
-                yield _resolve_terminal_script_path(segment[index + 1], cwd)
+                resolved = _resolve_terminal_script_path(segment[index + 1], cwd)
+                if resolved is not None:
+                    yield resolved
             continue
 
         if executable_name in _SHELL_EXECUTABLES:
@@ -216,7 +252,9 @@ def _iter_referenced_shell_scripts(
                 "-c",
                 "--command",
             }:
-                yield _resolve_terminal_script_path(arguments[arg_index], cwd)
+                resolved = _resolve_terminal_script_path(arguments[arg_index], cwd)
+                if resolved is not None:
+                    yield resolved
             continue
 
         # A bare "/" token is pathlib's division operator in Python sources
@@ -226,7 +264,9 @@ def _iter_referenced_shell_scripts(
         # (#77131). Skip pure-separator tokens.
         if executable.strip("/"):
             if "/" in executable or executable.endswith((".sh", ".bash", ".zsh")):
-                yield _resolve_terminal_script_path(executable, cwd)
+                resolved = _resolve_terminal_script_path(executable, cwd)
+                if resolved is not None:
+                    yield resolved
 
 
 def _iter_shell_command_payloads(command: str) -> Iterator[str]:
@@ -246,7 +286,7 @@ def _resolve_script_directory(script_path: str) -> Optional[str]:
     """Return the directory *script_path* resolves to, handling relative names."""
     try:
         path = _resolve_script_path(script_path)
-        if path.is_absolute():
+        if path is not None and path.is_absolute():
             return str(path.parent)
     except Exception:
         pass
@@ -290,6 +330,32 @@ def _read_referenced_script(path: Path) -> tuple[Optional[str], bool]:
     return data.decode("utf-8", errors="replace"), False
 
 
+def _sanitize_remote_script_text(text: Optional[str]) -> tuple[Optional[str], bool]:
+    """Apply the local-read contract to text from a ``read_remote_script`` callback.
+
+    The recursion boundary must not trust its callbacks: any backend (SSH,
+    Modal, Daytona, or a future one) can hand back raw binary bytes decoded
+    as text, or arbitrarily large output. Mirror
+    ``_read_referenced_script``'s semantics exactly — NUL bytes mean binary
+    (nothing to scan, checked first, #77703), oversized text fails closed
+    like an oversized local file (#76762) — so remote and local reads can
+    never diverge again. The size check re-encodes to compare *bytes*
+    (matching the local read and the ``head -c`` wire bound): a >1 MiB
+    multibyte file truncated at the byte cap decodes to fewer characters
+    than bytes, and a character-count check would scan the truncated text
+    instead of failing closed. Enforced here rather than inside each
+    callback so the guarantee holds for every callback, not just the ones
+    we hardened.
+    """
+    if not text:
+        return None, False
+    if "\x00" in text:
+        return None, False
+    if len(text.encode("utf-8", errors="replace")) > _MAX_REFERENCED_SCRIPT_BYTES:
+        return None, True
+    return text, False
+
+
 def _contains_unsafe_gateway_action(
     command: str,
     *,
@@ -331,13 +397,20 @@ def _contains_unsafe_gateway_action(
             return True
         if script_text is None and read_remote_script is not None:
             # Local path missing; try the remote backend if one is available.
-            script_text = read_remote_script(str(script_path))
+            # The callback's output crosses the same trust boundary as a
+            # local read — sanitize it identically before it enters the
+            # recursion (binary skip + size fail-closed).
+            script_text, unsafe = _sanitize_remote_script_text(
+                read_remote_script(str(script_path))
+            )
+            if unsafe:
+                return True
         if not script_text:
             continue
         # Relative references inside a script resolve against that script's
         # directory, not the original command's cwd.
         script_dir = _resolve_script_directory(str(resolved)) or cwd
-        if script_text and _contains_unsafe_gateway_action(
+        if _contains_unsafe_gateway_action(
             script_text,
             cwd=script_dir,
             depth=depth + 1,
@@ -354,19 +427,45 @@ def contains_gateway_lifecycle_command_or_referenced_script(
     cwd: Optional[str] = None,
     read_remote_script: Optional[_ReadRemoteScriptFn] = None,
 ) -> bool:
-    """Detect lifecycle/submit commands, including bounded nested scripts."""
-    return _contains_unsafe_gateway_action(
-        command,
-        cwd=cwd,
-        depth=0,
-        visited=set(),
-        read_remote_script=read_remote_script,
-    )
+    """Detect lifecycle/submit commands, including bounded nested scripts.
+
+    Total by construction: this function returns a verdict for *every*
+    input and never raises. The direct scans below are pure string
+    operations; the referenced-script walk touches the filesystem, remote
+    backends, and shlex on arbitrary decoded bytes, so it is best-effort
+    defense-in-depth — any unexpected failure inside it is logged and
+    treated as "walk found nothing" rather than crashing the caller.
+
+    This is the contract #76762 established ("a guarded path must never
+    crash the guard") enforced at the boundary instead of per-syscall: a
+    guard crash propagates out of ``tools/terminal_tool.py`` and breaks
+    every terminal command until the gateway restarts (#77780, #78256),
+    which is strictly worse than either verdict.
+    """
+    try:
+        # Includes the direct regex/submit scans at depth 0.
+        return _contains_unsafe_gateway_action(
+            command,
+            cwd=cwd,
+            depth=0,
+            visited=set(),
+            read_remote_script=read_remote_script,
+        )
+    except Exception:
+        logger.warning(
+            "lifecycle guard referenced-script walk failed; "
+            "falling back to direct-scan verdict",
+            exc_info=True,
+        )
+        # Pure string scans of the top-level command — cannot raise.
+        return contains_gateway_lifecycle_command(
+            command
+        ) or contains_launchctl_submit_command(command)
 
 
 
 
-def _resolve_script_path(script_path: str) -> Path:
+def _resolve_script_path(script_path: str) -> Optional[Path]:
     """Resolve a cron ``script`` value the same way the scheduler does.
 
     The scheduler (``cron.scheduler``) resolves a bare/relative script path
@@ -376,23 +475,39 @@ def _resolve_script_path(script_path: str) -> Path:
     (``~/.hermes/scripts/restart.sh``) but is passed as the bare name
     ``restart.sh`` would read as a nonexistent relative path and silently
     scan prompt-only content, letting the command through.
+
+    Returns ``None`` for values that cannot be a real path (NUL bytes,
+    unexpandable ``~``) — the same ingestion contract as
+    ``_expand_candidate_path``; such a value can never name a file the
+    scheduler would execute, so there is nothing to scan.
     """
     from hermes_constants import get_hermes_home
 
-    raw = Path(script_path).expanduser()
+    raw = _expand_candidate_path(script_path)
+    if raw is None:
+        return None
     if raw.is_absolute():
         return raw
-    return get_hermes_home() / "scripts" / raw
+    try:
+        return get_hermes_home() / "scripts" / raw
+    except (RuntimeError, OSError):
+        # get_hermes_home() falls back to Path.home(), which raises when
+        # neither HERMES_HOME nor HOME is resolvable (launchd/systemd
+        # environments) — same ingestion contract: nothing to scan.
+        return None
 
 
 def _read_script_for_scanning(script_path: str) -> str:
     """Read a cron script with the bounded terminal-script scanner.
 
     Non-regular or oversized inputs fail closed by returning a lifecycle-shaped
-    sentinel, while missing/unreadable paths remain empty so ordinary scheduler
-    path validation can report them.
+    sentinel, while missing/unreadable/unresolvable paths remain empty so
+    ordinary scheduler path validation can report them.
     """
-    script_text, unsafe = _read_referenced_script(_resolve_script_path(script_path))
+    resolved = _resolve_script_path(script_path)
+    if resolved is None:
+        return ""
+    script_text, unsafe = _read_referenced_script(resolved)
     if unsafe:
         return "hermes gateway restart"
     return script_text or ""
@@ -417,7 +532,8 @@ def check_gateway_lifecycle(
     combined = prompt or ""
     python_script = False
     if script:
-        python_script = _resolve_script_path(script).suffix == ".py"
+        resolved_script = _resolve_script_path(script)
+        python_script = resolved_script is not None and resolved_script.suffix == ".py"
         script_text = _read_script_for_scanning(script)
         if script_text:
             combined = f"{combined}\n{script_text}"

cron/scheduler.py

@@ -2251,7 +2251,16 @@ def _run_job_script(
     scripts_dir.mkdir(parents=True, exist_ok=True)
     scripts_dir_resolved = scripts_dir.resolve()
 
-    raw = Path(script_path).expanduser()
+    try:
+        raw = Path(script_path).expanduser()
+    except (ValueError, RuntimeError, OSError):
+        # Same ingestion contract as cron.lifecycle_guard: a NUL-bearing
+        # value (ValueError) or an unexpandable ``~`` (RuntimeError with no
+        # resolvable HOME) can never name a real script. The creation-time
+        # guard tolerates such values as "nothing to scan", so they can
+        # reach fire time — fail the run with a report instead of crashing
+        # the scheduler with an unhandled exception.
+        return False, f"Blocked: script path is not a valid filesystem path: {script_path!r}"
     if raw.is_absolute():
         path = raw.resolve()
     else:

tests/cron/test_cron_no_agent.py

@@ -118,3 +118,16 @@ def test_run_job_script_path_traversal_still_blocked(hermes_env):
     ok, output = _run_job_script("/etc/passwd")
     assert ok is False
     assert "Blocked" in output or "outside" in output
+
+
+def test_run_job_script_nul_path_fails_cleanly(hermes_env):
+    """Sibling of the lifecycle-guard ingestion fix: a NUL-bearing script
+    value can survive to fire time (the creation-time guard treats it as
+    "nothing to scan"), and ``Path.expanduser()`` raises ValueError — not
+    OSError — on it. The scheduler must fail the run with a report, not
+    crash with an unhandled exception."""
+    from cron.scheduler import _run_job_script
+
+    ok, output = _run_job_script("~user\x00bad.sh")
+    assert ok is False
+    assert "Blocked" in output