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.
This commit is contained in:
@@ -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 "
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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."""
|
||||
|
||||
|
||||
@@ -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 "
|
||||
|
||||
Reference in New Issue
Block a user