From d46abd445fa937b49900ef9d1d923c07f7f78ea0 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Fri, 18 Sep 2026 00:46:00 -0700 Subject: [PATCH] fix(cron): a data file only mentioned in an inert heredoc never fails the lifecycle guard closed The referenced-script walk reads paths named inside a masked (provably inert) interpreter body so `os.system('/x/restart.sh')` is still caught, but it turned every "could not scan" outcome for those mentions into a block: a <1 MiB JSON with one >64 KiB line exhausted the text budget at depth 1, a markdown table of paths pulled 64 remote-read misses and exhausted that budget, and a live SQLite database (`state.db` inside a running gateway) failed closed via LiveConnectionError with nothing logged. 22 of 36 in-gateway terminal blocks in one deployment were this class (#113944). Thread `executed` through `_contains_unsafe_gateway_action`: content reached only through a mention is still scanned for a literal lifecycle command, but budget/depth/size/device/live-DB/cloud there is "nothing to scan", never a verdict. Executed candidates still fail closed exactly as before. When the walk does fail closed for a non-lifecycle reason, record it on the budget (`scan_gateway_lifecycle` returns `(unsafe, refusal)`), log a WARNING naming the path and reason, and tell the model that reason in both the terminal guard and the cron `check_gateway_lifecycle` path instead of the generic "cannot restart, stop, or uninstall the gateway" text that made it reword and retry the same command. --- cron/lifecycle_guard.py | 128 +++++++++++++----- .../cron/test_lifecycle_guard_heredoc_walk.py | 44 ++++++ tests/hermes_cli/test_gateway_restart_loop.py | 27 ++++ tools/terminal_tool_guards.py | 18 ++- 4 files changed, 183 insertions(+), 34 deletions(-) diff --git a/cron/lifecycle_guard.py b/cron/lifecycle_guard.py index 0ff98ff083..6b48263c1a 100644 --- a/cron/lifecycle_guard.py +++ b/cron/lifecycle_guard.py @@ -318,9 +318,12 @@ _MAX_LIFECYCLE_SCAN_REMOTE_READS = 64 class _LifecycleScanBudget: - """Shared work budget for one complete referenced-script walk.""" + """Shared work budget for one complete referenced-script walk. ``refusal`` records why the walk + failed closed for a reason other than a lifecycle command (budget, size, device, live SQLite, + cloud placeholder) so the caller can tell the model the real reason (#113944).""" - __slots__ = ("bytes_remaining", "lines_remaining", "paths_remaining", "remote_reads_remaining") + __slots__ = ("bytes_remaining", "lines_remaining", "paths_remaining", "remote_reads_remaining", + "refusal") def __init__(self) -> None: # Read the module constants at construction so tests/operators can lower them at runtime. @@ -328,6 +331,7 @@ class _LifecycleScanBudget: self.lines_remaining = _MAX_LIFECYCLE_SCAN_LINES self.paths_remaining = _MAX_LIFECYCLE_SCAN_PATHS self.remote_reads_remaining = _MAX_LIFECYCLE_SCAN_REMOTE_READS + self.refusal: Optional[str] = None def charge_text(self, text: str) -> bool: """Charge *text* before tokenization; False when it does not fit.""" @@ -386,12 +390,35 @@ def lifecycle_scan_root_within_budget(text: str) -> bool: return False -def _budget_exhausted(what: str, depth: int) -> bool: +def _budget_exhausted(budget: _LifecycleScanBudget, what: str, depth: int) -> bool: logger.warning( "lifecycle guard scan budget exhausted (%s at depth %d); " "failing closed — see _MAX_LIFECYCLE_SCAN_* in cron/lifecycle_guard.py", what, depth, ) + budget.refusal = f"the scan budget was exhausted ({what} at depth {depth})" + return True + + +def _unreadable_reason(path: Path) -> str: + """Name why an *executed* script failed closed without being scanned (live SQLite, device, + oversized). Message-only: the fail-closed verdict itself came from the bounded reader.""" + from hermes_cli.sqlite_safe_read import has_live_connection + + if has_live_connection(path): + return f"`{path}` is a SQLite database open in this gateway process" + try: + metadata = os.stat(path) + except OSError: + return f"`{path}` could not be read" + if not stat.S_ISREG(metadata.st_mode): + return f"`{path}` is not a regular file" + return f"`{path}` is larger than the scan cap ({_MAX_REFERENCED_SCRIPT_BYTES} bytes) or the remaining walk budget" + + +def _refuse_unreadable(budget: _LifecycleScanBudget, path: Path, reason: str) -> bool: + logger.warning("lifecycle guard cannot scan referenced script %s: %s; failing closed", path, reason) + budget.refusal = reason return True @@ -905,20 +932,23 @@ def _read_script_for_scanning(script_path: str) -> str: def _contains_unsafe_gateway_action( command: str, *, cwd: Optional[str], depth: int, visited: set[Path], budget: _LifecycleScanBudget, - read_remote_script: Optional[_ReadRemoteScriptFn] = None, + read_remote_script: Optional[_ReadRemoteScriptFn] = None, executed: bool = True, ) -> bool: + """``executed=False`` means *command* is the content of a file that is only MENTIONED in inert + (masked) text: it is still scanned for a literal lifecycle command, but "could not scan" (budget, + depth, size, device, live SQLite, cloud) is "nothing to scan" there, never a block (#113944).""" # Charge BEFORE _direct_lifecycle_scan: every scan in it tokenizes with shlex. if not budget.charge_text(command): - return _budget_exhausted("text", depth) + return _budget_exhausted(budget, "text", depth) if executed else False if _direct_lifecycle_scan(command): return True if depth >= _MAX_REFERENCED_SCRIPT_DEPTH: - return True + return executed - def recurse(text: str, cwd: Optional[str]) -> bool: + def recurse(text: str, cwd: Optional[str], executed: bool) -> bool: return _contains_unsafe_gateway_action( text, cwd=cwd, depth=depth + 1, visited=visited, budget=budget, - read_remote_script=read_remote_script, + read_remote_script=read_remote_script, executed=executed, ) # The walks below must see the same masked view `_direct_lifecycle_scan` sees (#110422): a @@ -929,60 +959,75 @@ def _contains_unsafe_gateway_action( walk_command = strip_inert_heredoc_bodies(command) for payload in _iter_shell_command_payloads(walk_command): - if recurse(payload, cwd): + if recurse(payload, cwd, executed): return True # Paths named only inside a masked body are still READ: an interpreter body that hands # `/x/restart.sh` to os.system() executes it. Only the fail-closed verdicts (cloud placeholder, - # oversized/binary) stay restricted to the masked view — a mere data mention must not trip them. - candidates = [(path, True) for path in _iter_referenced_shell_scripts(walk_command, cwd=cwd)] + # oversized/binary, budget) stay restricted to the executed view — a mere data mention must not + # trip them. Executed candidates come first so a mention never starves a real script's budget. + candidates = [(path, executed) for path in _iter_referenced_shell_scripts(walk_command, cwd=cwd)] if walk_command != command: candidates += [(path, False) for path in _iter_referenced_shell_scripts(command, cwd=cwd)] - for script_path, executed in candidates: + for script_path, candidate_executed in candidates: # Do not touch a FileProvider path even to discover whether the file is hydrated. if _on_cloud_path(script_path): - if executed: - return True + if candidate_executed: + return _refuse_unreadable( + budget, script_path, + f"`{script_path}` lives on a cloud-synced path (iCloud Drive / " + "~/Library/CloudStorage) that the guard refuses to open", + ) continue resolved = _resolve_lenient(script_path) if resolved in visited: continue if not budget.charge_path(): - return _budget_exhausted("paths", depth) + if candidate_executed: + return _budget_exhausted(budget, "paths", depth) + break # remaining candidates are all mentions visited.add(resolved) # Never read more than the walk can still afford to tokenize; a file larger than the # remainder fails closed exactly like an oversized one. script_text, unsafe = _read_referenced_script(script_path, max_bytes=budget.bytes_remaining) if unsafe: - if executed: - return True + if candidate_executed: + return _refuse_unreadable(budget, script_path, _unreadable_reason(script_path)) continue if script_text is None and read_remote_script is not None: # Local path missing; the remote backend's output crosses the same trust boundary as a # local read — sanitize identically (binary skip + size fail-closed). if not budget.charge_remote_read(): - return _budget_exhausted("remote reads", depth) + if candidate_executed: + return _budget_exhausted(budget, "remote reads", depth) + break script_text, unsafe = _sanitize_remote_script_text( read_remote_script(str(script_path)), max_bytes=budget.bytes_remaining ) if unsafe: - if executed: - return True + if candidate_executed: + return _refuse_unreadable( + budget, script_path, + f"`{script_path}` read from the backend exceeds the scan cap " + f"({_MAX_REFERENCED_SCRIPT_BYTES} bytes) or the remaining walk budget", + ) continue if not script_text: continue # Relative references inside a script resolve against that script's directory, not the cwd. - if recurse(script_text, _resolve_script_directory(str(resolved)) or cwd): + if recurse(script_text, _resolve_script_directory(str(resolved)) or cwd, candidate_executed): return True return False -def contains_gateway_lifecycle_command_or_referenced_script( +def scan_gateway_lifecycle( command: str, *, cwd: Optional[str] = None, read_remote_script: Optional[_ReadRemoteScriptFn] = None, -) -> bool: - """Detect lifecycle/submit commands, including bounded nested scripts. +) -> tuple[bool, Optional[str]]: + """``(unsafe, refusal)``: *refusal* names a non-lifecycle reason the walk failed closed (budget, + size, device, live SQLite, cloud) so callers can tell the model; ``None`` when the verdict is a + real lifecycle command or the command is allowed. Total by construction: never raises. Direct scans are pure string ops; the referenced-script walk (filesystem, remote backends, shlex on decoded bytes) is best-effort defense-in-depth — an @@ -993,11 +1038,13 @@ def contains_gateway_lifecycle_command_or_referenced_script( every terminal command until the gateway restarts (#77780, #78256), which is strictly worse than either verdict. """ + budget = _LifecycleScanBudget() try: - return _contains_unsafe_gateway_action( - command, cwd=cwd, depth=0, visited=set(), budget=_LifecycleScanBudget(), + unsafe = _contains_unsafe_gateway_action( + command, cwd=cwd, depth=0, visited=set(), budget=budget, read_remote_script=read_remote_script, ) + return unsafe, budget.refusal if unsafe else None except Exception: logger.warning( "lifecycle guard referenced-script walk failed; " @@ -1005,10 +1052,20 @@ def contains_gateway_lifecycle_command_or_referenced_script( exc_info=True, ) try: - return _direct_lifecycle_scan(command) + return _direct_lifecycle_scan(command), None except Exception: # If even the data-argument masker fails, fall to raw regex + submit scan: stay total. - return contains_gateway_lifecycle_command(command) or contains_launchctl_submit_command(command) + return (contains_gateway_lifecycle_command(command) + or contains_launchctl_submit_command(command)), None + + +def contains_gateway_lifecycle_command_or_referenced_script( + command: str, *, cwd: Optional[str] = None, + read_remote_script: Optional[_ReadRemoteScriptFn] = None, +) -> bool: + """Detect lifecycle/submit commands, including bounded nested scripts (see + ``scan_gateway_lifecycle`` for the contract).""" + return scan_gateway_lifecycle(command, cwd=cwd, read_remote_script=read_remote_script)[0] def check_gateway_lifecycle(prompt: Optional[str], script: Optional[str] = None) -> None: @@ -1040,6 +1097,7 @@ def check_gateway_lifecycle(prompt: Optional[str], script: Optional[str] = None) if script_text: combined = f"{combined}\n{script_text}" + refusal: Optional[str] = None if python_script: # Python runs via the interpreter, never a POSIX shell, and the shell reference walk is a # false-positive generator on Python sources (pathlib "/" resolves to the filesystem root). @@ -1047,14 +1105,22 @@ def check_gateway_lifecycle(prompt: Optional[str], script: Optional[str] = None) # The data-exemption masker tokenizes with shlex, so it is charged against the walk budget. # The direct command regex below still scans the full text, so a literal `hermes gateway restart` # embedded in a .py script is still blocked. See #77131, #78398. - if not _LifecycleScanBudget().charge_text(combined): - unsafe = _budget_exhausted("text", 0) + budget = _LifecycleScanBudget() + if not budget.charge_text(combined): + unsafe = _budget_exhausted(budget, "text", 0) + refusal = budget.refusal else: unsafe = _lifecycle_command_scan_with_data_exemption(combined) else: - unsafe = contains_gateway_lifecycle_command_or_referenced_script( + unsafe, refusal = scan_gateway_lifecycle( combined, cwd=_resolve_script_directory(script) if script else None ) + if unsafe and refusal: + raise GatewayLifecycleBlocked( + f"Blocked: the lifecycle guard could not scan this cron job or referenced script: {refusal}. " + "Nothing in the job is known to contain a gateway lifecycle command, but a script " + "the job executes must be scannable (a regular text file under 1 MiB) before it can run." + ) if unsafe: raise GatewayLifecycleBlocked( "Blocked: cron job contains a gateway lifecycle command or persistent " diff --git a/tests/cron/test_lifecycle_guard_heredoc_walk.py b/tests/cron/test_lifecycle_guard_heredoc_walk.py index 9cd77fb4ee..061e72fada 100644 --- a/tests/cron/test_lifecycle_guard_heredoc_walk.py +++ b/tests/cron/test_lifecycle_guard_heredoc_walk.py @@ -48,3 +48,47 @@ def test_inert_heredoc_body_script_path_still_read(tmp_path): script.write_text("#!/bin/sh\nhermes gateway restart\n") command = f"python3 - <<'PY'\nimport os\nos.system('{script}')\nPY" assert guard(command, cwd=str(tmp_path)) is True + + +def test_mentioned_data_file_that_cannot_be_scanned_is_not_a_verdict(tmp_path, monkeypatch): + """A file only MENTIONED in an inert body may exhaust the text budget (one >64 KiB line), pull + in 64+ remote-read misses (a markdown table of paths) or be a live SQLite database: each is + "nothing to scan", never a block (#113944). The same file *executed* still fails closed.""" + import cron.lifecycle_guard as lifecycle_guard + from hermes_cli.sqlite_safe_read import connect_tracked + + minified = tmp_path / "minified.json" + minified.write_text("[" + "1," * 40000 + "1]") + notes = tmp_path / "notes.md" + notes.write_text("\n".join(f"| /opt/frag{i}/tool-{i}.md | note |" for i in range(70))) + db = tmp_path / "state.db" + conn = connect_tracked(db) + monkeypatch.setattr(lifecycle_guard, "_MAX_LIFECYCLE_SCAN_REMOTE_READS", 8) + remote_misses: list[str] = [] + + def remote(path: str): + remote_misses.append(path) + return None + + try: + for data in (minified, notes, db): + command = f"cd {tmp_path} && python3 - <<'PY'\nt = open('{data}').read()\nPY" + assert guard(command, cwd=str(tmp_path), read_remote_script=remote) is False, data.name + assert remote_misses # the notes table was walked and its misses were bounded, not fatal + unsafe, refusal = lifecycle_guard.scan_gateway_lifecycle(f"bash {minified}") + assert unsafe is True and "budget" in refusal + unsafe, refusal = lifecycle_guard.scan_gateway_lifecycle(f"bash {db}") + assert unsafe is True and "SQLite" in refusal + finally: + conn.close() + + +def test_mentioned_script_with_lifecycle_command_still_blocks(tmp_path): + """The lenient path only covers "could not scan": a mentioned script whose text IS a lifecycle + command is still a positive verdict, and the refusal reason stays empty (it is not a scan failure).""" + import cron.lifecycle_guard as lifecycle_guard + + script = tmp_path / "restart.sh" + script.write_text("#!/bin/sh\nhermes gateway restart\n") + command = f"cd {tmp_path} && python3 - <<'PY'\nimport os\nos.system('{script}')\nPY" + assert lifecycle_guard.scan_gateway_lifecycle(command, cwd=str(tmp_path)) == (True, None) diff --git a/tests/hermes_cli/test_gateway_restart_loop.py b/tests/hermes_cli/test_gateway_restart_loop.py index 423ae78088..ce185516de 100644 --- a/tests/hermes_cli/test_gateway_restart_loop.py +++ b/tests/hermes_cli/test_gateway_restart_loop.py @@ -1881,6 +1881,33 @@ class TestTerminalToolGatewayLifecycleGuardRemote: assert any("head -c" in c for c in calls) + def test_unscannable_executed_script_names_the_reason(self, monkeypatch, tmp_path): + """A script the command EXECUTES that the guard cannot scan (here a live SQLite database) + still fails closed, but the error names that reason instead of claiming a lifecycle + command the model then rewords and retries in a loop (#113944).""" + import tools.terminal_tool as tt + from hermes_cli.sqlite_safe_read import connect_tracked + + db = tmp_path / "state.db" + conn = connect_tracked(db) + + class _LocalEnv: + env = {} + cwd = str(tmp_path) + def execute(self, command, **kwargs): + return {"output": "", "returncode": 1} + + self._patch_env(monkeypatch, _LocalEnv(), inside_gateway=True) + try: + result = json.loads(tt.terminal_tool(command=f"bash {db}")) + finally: + conn.close() + + assert result["exit_code"] == 1 + assert "could not scan" in result["error"] and "SQLite" in result["error"] + assert "cannot restart, stop, or uninstall" not in result["error"] + + class TestCronCreateLifecycleBlockExtra: """Additional cron create lifecycle guard coverage.""" diff --git a/tools/terminal_tool_guards.py b/tools/terminal_tool_guards.py index 87823320d4..d01861bb16 100644 --- a/tools/terminal_tool_guards.py +++ b/tools/terminal_tool_guards.py @@ -204,9 +204,9 @@ def gateway_lifecycle_block( return None from cron.lifecycle_guard import ( _MAX_REFERENCED_SCRIPT_BYTES, - contains_gateway_lifecycle_command_or_referenced_script, contains_launchctl_submit_command, lifecycle_scan_root_within_budget, + scan_gateway_lifecycle, ) # Keep the specific launchctl diagnostic when this optional pre-scan fits the # budget. The full fail-closed guard below still runs when it does not, so @@ -227,11 +227,23 @@ def gateway_lifecycle_block( guard_cwd = _resolve_command_cwd( workdir=workdir, default_cwd=guard_cwd_base, session_key=session_key, env_type=env_type, ) - if contains_gateway_lifecycle_command_or_referenced_script( + unsafe, refusal = scan_gateway_lifecycle( command, cwd=guard_cwd, read_remote_script=lambda p: _read_script_for_guard(env, guard_cwd, p, _MAX_REFERENCED_SCRIPT_BYTES), - ): + ) + if unsafe and refusal: + # Not a lifecycle command: a script the command EXECUTES could not be scanned (budget, + # size, device, live SQLite, cloud placeholder). Say so, or the model rewords and retries + # the same command in a loop (#113944). + return _blocked_json( + f"Blocked: the lifecycle guard could not scan this command or referenced script: {refusal}. " + "Nothing in the command is known to contain a gateway lifecycle command, but a " + "script the command executes must be scannable (a regular text file under 1 MiB) " + "before it can run inside the gateway process.", + "error", + ) + if unsafe: return _blocked_json( "Blocked: command or referenced script cannot restart, stop, or " "uninstall the gateway from inside the gateway process. The gateway would "