diff --git a/cron/lifecycle_guard.py b/cron/lifecycle_guard.py index 6c70c1af8ae87..b09635c998bb9 100644 --- a/cron/lifecycle_guard.py +++ b/cron/lifecycle_guard.py @@ -23,19 +23,24 @@ rate without preventing the actual foot-gun, which requires a real command shape. -This is a defence-in-depth layer. ``tools/terminal_tool.py`` already -blocks these commands at *execution* time when ``_HERMES_GATEWAY=1``, and -``hermes gateway stop|restart`` refuse to self-target from inside the -gateway. Blocking at *creation* time as well means the agent gets an -immediate, informative rejection instead of scheduling a job that will -only fail (silently) when it fires. +This is a defence-in-depth layer. ``tools/terminal_tool.py`` blocks direct +commands and shell scripts they reference when ``_HERMES_GATEWAY=1``. It also +rejects ``launchctl submit`` in gateway sessions because launchd treats that +primitive as a persistent KeepAlive job, not a one-shot task. ``hermes gateway +stop|restart`` separately refuse to self-target from inside the gateway. +Blocking cron specs at creation time as well means the agent gets an immediate, +informative rejection instead of scheduling a job that will only fail +(silently) when it fires. """ from __future__ import annotations +import os import re +import shlex +import stat from pathlib import Path -from typing import Optional +from typing import Callable, Iterator, Optional class GatewayLifecycleBlocked(ValueError): @@ -56,7 +61,15 @@ class GatewayLifecycleBlocked(ValueError): # labels look like `ai.hermes.gateway` / `hermes-gateway`. Requiring the # gateway identifier prevents blocking unrelated hermes services (e.g. # `launchctl unload ai.hermes.update-checker.plist`). - r"|(?:launchctl\s+(?:kickstart|unload|load|stop|restart)\b[^\n]*\bhermes[.\-]?gateway)" + # `submit` and `bootstrap` are included alongside the direct verbs + # (kickstart/etc.): `launchctl submit -l ai.hermes.gateway- -- + # ` (or `launchctl bootstrap gui/ `) creates + # a NEW keepalive job wrapping an arbitrary helper, which is how a + # blocked direct restart/kill gets laundered into a persistent restart + # loop instead (#62891) — same foot-gun, indirect shape. Neutral-label + # submissions that dodge this text anchor are caught separately by + # `contains_launchctl_submit_command` (execution-aware, label-independent). + r"|(?:launchctl\s+(?:kickstart|unload|load|stop|restart|submit|bootstrap)\b[^\n]*\bhermes[.\-]?gateway)" # Branch C: systemctl ops on a hermes-gateway unit. r"|(?:systemctl\s+(?:-\S+\s+)*(?:restart|stop|start)\b[^\n]*\bhermes[.\-]?gateway)" # Branch D: pkill / kill targeting the hermes gateway process. Both @@ -66,11 +79,268 @@ class GatewayLifecycleBlocked(ValueError): ) +# A backslash immediately followed by a newline is a POSIX shell line +# continuation — the shell joins the two lines before parsing. Every branch +# above uses `[^\n]*` between its verb and the gateway identifier so the +# match can't span unrelated lines of a longer cron prompt/script, but that +# also means a real multi-line shell invocation split across continuation +# lines (e.g. `launchctl submit \` / ` -l ai.hermes.gateway-... \` / ` -- ...`, +# the exact reported shape in #62891) would otherwise slip past. Collapse +# continuations to a single space before matching, mirroring what the shell +# itself does, rather than loosening `[^\n]*` and risking false positives +# across genuinely separate lines. +_SHELL_LINE_CONTINUATION = re.compile(r"\\\r?\n[ \t]*") + + def contains_gateway_lifecycle_command(text: str) -> bool: """Return True if *text* contains a gateway lifecycle command pattern.""" if not text: return False - return bool(_GATEWAY_LIFECYCLE_PATTERN.search(text)) + normalized = _SHELL_LINE_CONTINUATION.sub(" ", text) + return bool(_GATEWAY_LIFECYCLE_PATTERN.search(normalized)) + + +_SHELL_EXECUTABLES = frozenset({"sh", "bash", "dash", "ksh", "zsh"}) +_SHELL_OPTIONS_WITH_VALUES = frozenset({"-O", "+O", "-o", "+o"}) +_MAX_REFERENCED_SCRIPT_BYTES = 1024 * 1024 +_MAX_REFERENCED_SCRIPT_DEPTH = 8 +_CONTROL_CHARS = frozenset(";&|()") + + + + +_ReadRemoteScriptFn = Callable[[str], Optional[str]] + + +def _iter_command_segments(command: str) -> Iterator[list[str]]: + """Yield shell-tokenized command segments, honoring quotes and comments.""" + normalized = command.replace("\\\n", "") + for line in normalized.splitlines() or [normalized]: + try: + lexer = shlex.shlex( + line, + posix=True, + punctuation_chars=";&|()", + ) + lexer.whitespace_split = True + lexer.commenters = "#" + tokens = list(lexer) + except ValueError: + continue + + segment: list[str] = [] + for token in tokens: + if token and set(token) <= _CONTROL_CHARS: + if segment: + yield segment + segment = [] + continue + segment.append(token) + if segment: + yield segment + + +def _command_token_index(segment: list[str]) -> Optional[int]: + """Return the executable token index after simple env assignments.""" + for index, token in enumerate(segment): + if re.match(r"^[A-Za-z_][A-Za-z0-9_]*=", token): + continue + return index + return None + + +def contains_launchctl_submit_command(command: str) -> bool: + """Detect an executed ``launchctl submit``/``bootstrap``, not quoted text. + + Label-independent by design: the label of a submitted/bootstrapped job is + chosen by whoever writes it, so a neutral name (``ai.hermes.svc-reload-tmp``) + defeats any label-anchored regex (#62891, second reproduction). Both verbs + register a NEW persistent launchd job (``submit`` jobs get KeepAlive + semantics; ``bootstrap`` loads an arbitrary plist), which is never safe to + do from inside the gateway process. + """ + for segment in _iter_command_segments(command): + index = _command_token_index(segment) + if index is None: + continue + if Path(segment[index]).name == "launchctl": + arguments = segment[index + 1 :] + if arguments and arguments[0].lower() in {"submit", "bootstrap"}: + return True + return False + + +def _resolve_terminal_script_path(candidate: str, cwd: Optional[str]) -> Path: + path = Path(candidate).expanduser() + if not path.is_absolute(): + path = Path(cwd or Path.cwd()) / path + return path + + +def _iter_referenced_shell_scripts( + command: str, + *, + cwd: Optional[str] = None, +) -> Iterator[Path]: + """Yield scripts executed directly or through a POSIX shell.""" + for segment in _iter_command_segments(command): + index = _command_token_index(segment) + if index is None: + continue + executable = segment[index] + executable_name = Path(executable).name + + if executable_name in {".", "source"}: + if len(segment) > index + 1: + yield _resolve_terminal_script_path(segment[index + 1], cwd) + continue + + if executable_name in _SHELL_EXECUTABLES: + arguments = segment[index + 1 :] + arg_index = 0 + while arg_index < len(arguments): + argument = arguments[arg_index] + if argument == "--": + arg_index += 1 + break + if argument in {"-c", "--command"}: + break + if argument in _SHELL_OPTIONS_WITH_VALUES: + arg_index += 2 + continue + if argument.startswith("-"): + arg_index += 1 + continue + break + if arg_index < len(arguments) and arguments[arg_index] not in { + "-c", + "--command", + }: + yield _resolve_terminal_script_path(arguments[arg_index], cwd) + continue + + if "/" in executable or executable.endswith((".sh", ".bash", ".zsh")): + yield _resolve_terminal_script_path(executable, cwd) + + +def _iter_shell_command_payloads(command: str) -> Iterator[str]: + """Yield code passed through ``sh|bash|... -c`` for recursive scanning.""" + for segment in _iter_command_segments(command): + index = _command_token_index(segment) + if index is None or Path(segment[index]).name not in _SHELL_EXECUTABLES: + continue + arguments = segment[index + 1 :] + for arg_index, argument in enumerate(arguments[:-1]): + if argument in {"-c", "--command"}: + yield arguments[arg_index + 1] + break + + +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(): + return str(path.parent) + except Exception: + pass + return None + + +def _read_referenced_script(path: Path) -> tuple[Optional[str], bool]: + """Return ``(text, unsafe)`` using bounded, regular-file-only reads.""" + flags = os.O_RDONLY | getattr(os, "O_NONBLOCK", 0) + try: + descriptor = os.open(path, flags) + except OSError: + return None, False + try: + metadata = os.fstat(descriptor) + if not stat.S_ISREG(metadata.st_mode): + return None, True + if metadata.st_size > _MAX_REFERENCED_SCRIPT_BYTES: + return None, True + data = os.read(descriptor, _MAX_REFERENCED_SCRIPT_BYTES + 1) + except OSError: + return None, False + finally: + os.close(descriptor) + if len(data) > _MAX_REFERENCED_SCRIPT_BYTES: + return None, True + return data.decode("utf-8", errors="replace"), False + + +def _contains_unsafe_gateway_action( + command: str, + *, + cwd: Optional[str], + depth: int, + visited: set[Path], + read_remote_script: Optional[_ReadRemoteScriptFn] = None, +) -> bool: + if contains_gateway_lifecycle_command(command) or contains_launchctl_submit_command( + command + ): + return True + if depth >= _MAX_REFERENCED_SCRIPT_DEPTH: + return True + + for payload in _iter_shell_command_payloads(command): + if _contains_unsafe_gateway_action( + payload, + cwd=cwd, + depth=depth + 1, + visited=visited, + read_remote_script=read_remote_script, + ): + return True + + for script_path in _iter_referenced_shell_scripts(command, cwd=cwd): + try: + resolved = script_path.resolve(strict=False) + except OSError: + resolved = script_path + if resolved in visited: + continue + visited.add(resolved) + script_text, unsafe = _read_referenced_script(script_path) + if unsafe: + 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)) + 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( + script_text, + cwd=script_dir, + depth=depth + 1, + visited=visited, + read_remote_script=read_remote_script, + ): + return True + return False + + +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.""" + return _contains_unsafe_gateway_action( + command, + cwd=cwd, + depth=0, + visited=set(), + read_remote_script=read_remote_script, + ) + + def _resolve_script_path(script_path: str) -> Path: @@ -93,20 +363,16 @@ def _resolve_script_path(script_path: str) -> Path: def _read_script_for_scanning(script_path: str) -> str: - """Read a script file for lifecycle-pattern scanning. + """Read a cron script with the bounded terminal-script scanner. - Decodes with ``errors="replace"`` so binary or non-UTF-8 content does not - silently bypass the check — a plain text-mode read raises - ``UnicodeDecodeError`` on such files, and swallowing that error would let - an attacker hide the command in binary noise. Returns an empty string - only when the file cannot be read at all. + 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. """ - try: - return _resolve_script_path(script_path).read_bytes().decode( - "utf-8", errors="replace" - ) - except OSError: - return "" + script_text, unsafe = _read_referenced_script(_resolve_script_path(script_path)) + if unsafe: + return "hermes gateway restart" + return script_text or "" def check_gateway_lifecycle( @@ -131,10 +397,14 @@ def check_gateway_lifecycle( if script_text: combined = f"{combined}\n{script_text}" - if contains_gateway_lifecycle_command(combined): + script_dir = _resolve_script_directory(script) if script else None + if contains_gateway_lifecycle_command_or_referenced_script( + combined, + cwd=script_dir, + ): raise GatewayLifecycleBlocked( - "Blocked: cron job contains a gateway lifecycle command " - "(restart/stop/kill). This is blocked to prevent agent-driven " + "Blocked: cron job contains a gateway lifecycle command or persistent " + "launchctl submit operation. This is blocked to prevent agent-driven " "SIGTERM-respawn loops under launchd/systemd supervision " "(#30719). Run `hermes gateway restart` from a shell outside " "the running gateway instead." diff --git a/tests/hermes_cli/test_gateway_restart_loop.py b/tests/hermes_cli/test_gateway_restart_loop.py index 59c61da114feb..840e26a0dfad5 100644 --- a/tests/hermes_cli/test_gateway_restart_loop.py +++ b/tests/hermes_cli/test_gateway_restart_loop.py @@ -35,6 +35,37 @@ class TestGatewayLifecyclePattern: def test_hermes_gateway_commands(self, text): assert _contains_gateway_lifecycle_command(text), f"Should match: {text!r}" + @pytest.mark.parametrize("text", [ + # #62891: a blocked direct restart/kill laundered through a NEW + # launchd keepalive job wrapping a helper script, instead of a + # direct kickstart/unload/stop/restart on the existing service. + "launchctl submit -l ai.hermes.gateway-hard-restart-no-photon-notice -- /bin/sh ~/.hermes/scripts/hard_restart_gateway_no_photon_notice.sh", + "launchctl submit -l hermes-gateway-restart-helper -- /bin/sh helper.sh", + # bootstrap loads an arbitrary plist — same laundering shape. + "launchctl bootstrap gui/501 ~/Library/LaunchAgents/ai.hermes.gateway.restart-once.plist", + # The exact reported shape: split across shell line-continuations + # (`\` immediately followed by a newline). `[^\n]*` alone can't span + # that, so the verb and the gateway-label token land on different + # physical lines unless continuations are normalized first. + ( + "launchctl submit \\\n" + " -l ai.hermes.gateway-hard-restart-no-photon-notice \\\n" + " -- /bin/sh ~/.hermes/scripts/hard_restart_gateway_no_photon_notice.sh" + ), + ]) + def test_launchctl_submit_bootstrap_commands(self, text): + assert _contains_gateway_lifecycle_command(text), f"Should match: {text!r}" + + def test_line_continuation_does_not_bridge_unrelated_lines(self): + # A backslash-newline is only normalized when it's a real shell + # continuation. Two genuinely separate lines of a longer prompt + # (no trailing backslash) must not be bridged into a false match. + text = ( + "this restarts the payment gateway\n" + "unrelated hermes note on the next line" + ) + assert not _contains_gateway_lifecycle_command(text), f"Should NOT match: {text!r}" + @pytest.mark.parametrize("text", [ "restart the server application", @@ -55,6 +86,11 @@ def test_hermes_gateway_commands(self, text): # hermes token). "launchctl unload ai.hermes.update-checker.plist", "launchctl restart ai.hermes.daemon", + # `submit` on an unrelated launchd label must not match the text + # pattern (a cron PROMPT is prose fed to an LLM). The execution-aware + # `contains_launchctl_submit_command` handles neutral-label submits + # at the terminal/cron-script chokepoints instead. + "launchctl submit -l com.example.backup -- /bin/sh backup.sh", "systemctl restart hermes-meta.service", "systemctl restart hermes-cron-helper", # Regression (#30728 follow-up): legit prompts that merely mention an @@ -234,6 +270,10 @@ def _patch_env(self, monkeypatch, fake_env, *, inside_gateway: bool): "systemctl stop hermes-gateway.service", "hermes gateway restart", "launchctl kickstart gui/501/ai.hermes.gateway", + # #62891 exact reported shape and its bootstrap sibling. + "launchctl submit -l ai.hermes.gateway-hard-restart-no-photon-notice -- /bin/sh ~/.hermes/scripts/hard_restart_gateway_no_photon_notice.sh", + "launchctl submit -l com.foo -- /path/gateway", + "launchctl bootstrap gui/501 ~/Library/LaunchAgents/ai.hermes.gateway.restart-once.plist", "pkill -f hermes.*gateway", ]) def test_blocks_lifecycle_commands_inside_gateway(self, monkeypatch, cmd): @@ -256,6 +296,250 @@ def test_force_true_cannot_bypass_block(self, monkeypatch): assert result["exit_code"] == 1 assert "Blocked" in result["error"] + def test_blocks_lifecycle_command_hidden_in_referenced_script( + self, monkeypatch, tmp_path + ): + import tools.terminal_tool as tt + + script = tmp_path / "delayed-ops.sh" + script.write_text("#!/bin/bash\nsleep 45\nhermes gateway restart\n") + self._patch_env(monkeypatch, self._make_fake_env(), inside_gateway=True) + + result = json.loads(tt.terminal_tool(command=f"/bin/bash {script}")) + + assert result["exit_code"] == 1 + assert "referenced script" in result["error"] + + def test_blocks_launchctl_submit_inside_gateway(self, monkeypatch, tmp_path): + import tools.terminal_tool as tt + + script = tmp_path / "health-check.sh" + script.write_text("#!/bin/bash\nprintf 'healthy\\n'\n") + self._patch_env(monkeypatch, self._make_fake_env(), inside_gateway=True) + + result = json.loads(tt.terminal_tool( + command=( + "launchctl submit -l ai.hermes.delayed-ops -- " + f"/bin/bash {script}" + ) + )) + + assert result["exit_code"] == 1 + assert "KeepAlive" in result["error"] + + @pytest.mark.parametrize("command", [ + # Neutral, non-hermes label: label-independent detection is the point + # (#62891 second reproduction used `ai.hermes.svc-reload-tmp`). + "launchctl submit -l com.foo -- /path/gateway", + "launchctl submit -l ai.hermes.svc-reload-tmp -- /bin/sh /tmp/h-svc-reload.sh", + # bootstrap variant: loads an arbitrary plist as a persistent job. + "launchctl bootstrap gui/501 /tmp/com.foo.plist", + ]) + def test_blocks_neutral_label_submit_and_bootstrap(self, monkeypatch, command): + import tools.terminal_tool as tt + + self._patch_env(monkeypatch, self._make_fake_env(), inside_gateway=True) + + result = json.loads(tt.terminal_tool(command=command)) + + assert result["exit_code"] == 1 + assert "KeepAlive" in result["error"] + + @pytest.mark.parametrize("command", [ + "launchctl submit -l com.foo -- /path/gateway", + "launchctl bootstrap gui/501 /tmp/com.foo.plist", + ]) + def test_submit_and_bootstrap_allowed_outside_gateway(self, monkeypatch, command): + """The label-independent block applies only inside the gateway process.""" + import tools.terminal_tool as tt + + calls = [] + + class _FakeEnv: + env = {} + + def execute(self, cmd, **kwargs): + calls.append(cmd) + return {"output": "", "returncode": 0} + + self._patch_env(monkeypatch, _FakeEnv(), inside_gateway=False) + monkeypatch.setattr( + tt, "_check_all_guards", lambda cmd, env, **kwargs: {"approved": True} + ) + + result = json.loads(tt.terminal_tool(command=command)) + + assert result["exit_code"] == 0 + assert calls == [command] + + def test_blocks_launchctl_submit_hidden_in_referenced_script( + self, monkeypatch, tmp_path + ): + import tools.terminal_tool as tt + + script = tmp_path / "wrapper.sh" + script.write_text( + "#!/bin/bash\nlaunchctl submit -l ai.hermes.loop -- /bin/true\n" + ) + self._patch_env(monkeypatch, self._make_fake_env(), inside_gateway=True) + + result = json.loads(tt.terminal_tool(command=f"/bin/bash {script}")) + + assert result["exit_code"] == 1 + assert "referenced script" in result["error"] + + def test_relative_script_uses_live_session_cwd(self, monkeypatch, tmp_path): + import tools.terminal_tool as tt + + script = tmp_path / "relative.sh" + script.write_text("#!/bin/bash\nhermes gateway restart\n") + + class _FakeEnv: + env = {} + cwd = str(tmp_path) + def execute(self, command, **kwargs): # pragma: no cover + raise AssertionError("execute must not be reached") + + self._patch_env(monkeypatch, _FakeEnv(), inside_gateway=True) + + result = json.loads(tt.terminal_tool(command="/bin/bash relative.sh")) + + assert result["exit_code"] == 1 + assert "referenced script" in result["error"] + + def test_blocks_executable_shebang_script(self, monkeypatch, tmp_path): + import tools.terminal_tool as tt + + script = tmp_path / "delayed.sh" + script.write_text("#!/bin/bash\nhermes gateway stop\n") + script.chmod(0o700) + self._patch_env(monkeypatch, self._make_fake_env(), inside_gateway=True) + + result = json.loads(tt.terminal_tool(command=str(script))) + + assert result["exit_code"] == 1 + + def test_launchctl_submit_parser_handles_shell_quoting(self, monkeypatch): + import tools.terminal_tool as tt + + self._patch_env(monkeypatch, self._make_fake_env(), inside_gateway=True) + result = json.loads(tt.terminal_tool( + command="launchctl sub\"\"mit -l ai.hermes.loop -- /bin/true" + )) + + assert result["exit_code"] == 1 + assert "KeepAlive" in result["error"] + + def test_shell_option_with_value_still_scans_script(self, monkeypatch, tmp_path): + import tools.terminal_tool as tt + + script = tmp_path / "options.sh" + script.write_text("#!/bin/bash\nhermes gateway restart\n") + self._patch_env(monkeypatch, self._make_fake_env(), inside_gateway=True) + + result = json.loads(tt.terminal_tool( + command=f"/bin/bash -O extglob {script}" + )) + + assert result["exit_code"] == 1 + + def test_shell_c_payload_recursively_scans_script(self, monkeypatch, tmp_path): + import tools.terminal_tool as tt + + script = tmp_path / "nested.sh" + script.write_text("#!/bin/bash\nlaunchctl submit -l ai.hermes.loop -- /bin/true\n") + + class _FakeEnv: + env = {} + cwd = str(tmp_path) + def execute(self, command, **kwargs): # pragma: no cover + raise AssertionError("execute must not be reached") + + self._patch_env(monkeypatch, _FakeEnv(), inside_gateway=True) + + result = json.loads(tt.terminal_tool( + command="/bin/bash -c '/bin/bash nested.sh'" + )) + + assert result["exit_code"] == 1 + + def test_nested_wrapper_script_is_scanned(self, monkeypatch, tmp_path): + import tools.terminal_tool as tt + + inner = tmp_path / "inner.sh" + inner.write_text("#!/bin/bash\nhermes gateway restart\n") + outer = tmp_path / "outer.sh" + outer.write_text("#!/bin/bash\n/bin/bash inner.sh\n") + + class _FakeEnv: + env = {} + cwd = str(tmp_path) + def execute(self, command, **kwargs): # pragma: no cover + raise AssertionError("execute must not be reached") + + self._patch_env(monkeypatch, _FakeEnv(), inside_gateway=True) + + result = json.loads(tt.terminal_tool(command=f"/bin/bash {outer}")) + + assert result["exit_code"] == 1 + + def test_non_regular_referenced_script_fails_closed(self, monkeypatch, tmp_path): + import tools.terminal_tool as tt + + fifo = tmp_path / "script.fifo" + os.mkfifo(fifo) + self._patch_env(monkeypatch, self._make_fake_env(), inside_gateway=True) + + result = json.loads(tt.terminal_tool(command=f"/bin/bash {fifo}")) + + assert result["exit_code"] == 1 + + def test_quoted_launchctl_submit_text_is_not_blocked(self, monkeypatch): + import tools.terminal_tool as tt + + calls = [] + + class _FakeEnv: + env = {} + def execute(self, command, **kwargs): + calls.append(command) + return {"output": "launchctl submit is persistent", "returncode": 0} + + self._patch_env(monkeypatch, _FakeEnv(), inside_gateway=True) + monkeypatch.setattr( + tt, "_check_all_guards", lambda cmd, env, **kwargs: {"approved": True} + ) + command = "printf '%s\\n' 'launchctl submit is persistent'" + + result = json.loads(tt.terminal_tool(command=command)) + + assert result["exit_code"] == 0 + assert calls == [command] + + def test_safe_referenced_script_passes_through(self, monkeypatch, tmp_path): + import tools.terminal_tool as tt + + calls = [] + script = tmp_path / "health-check.sh" + script.write_text("#!/bin/bash\nprintf 'healthy\\n'\n") + + class _FakeEnv: + env = {} + def execute(self, command, **kwargs): + calls.append(command) + return {"output": "healthy", "returncode": 0} + + self._patch_env(monkeypatch, _FakeEnv(), inside_gateway=True) + monkeypatch.setattr( + tt, "_check_all_guards", lambda cmd, env, **kwargs: {"approved": True} + ) + command = f"/bin/bash {script}" + + result = json.loads(tt.terminal_tool(command=command)) + + assert result["exit_code"] == 0 + assert calls == [command] + def test_safe_systemctl_commands_pass_through(self, monkeypatch): """Non-hermes systemctl commands must not be blocked by this guard.""" import tools.terminal_tool as tt @@ -290,6 +574,50 @@ def test_prompt_with_command_raises(self): check_gateway_lifecycle("please run hermes gateway restart", None) assert "#30719" in str(exc.value) + def test_clean_prompt_does_not_raise(self): + from cron.lifecycle_guard import check_gateway_lifecycle + check_gateway_lifecycle("research the gateway architecture", None) + check_gateway_lifecycle("check server health and restart watchers", None) + + def test_script_with_command_raises(self, tmp_path, monkeypatch): + from cron.lifecycle_guard import GatewayLifecycleBlocked, check_gateway_lifecycle + script = tmp_path / "restart.sh" + script.write_text("#!/bin/bash\nhermes gateway restart\n") + with pytest.raises(GatewayLifecycleBlocked): + check_gateway_lifecycle("clean prompt", str(script)) + + def test_script_with_launchctl_submit_raises(self, tmp_path): + from cron.lifecycle_guard import GatewayLifecycleBlocked, check_gateway_lifecycle + script = tmp_path / "persistent.sh" + script.write_text( + "#!/bin/bash\nlaunchctl submit -l ai.hermes.loop -- /bin/true\n" + ) + with pytest.raises(GatewayLifecycleBlocked): + check_gateway_lifecycle("clean prompt", str(script)) + + @pytest.mark.parametrize("line", [ + # #62891: neutral labels defeat any label-anchored regex, so cron + # scripts get the same label-independent submit/bootstrap block. + "launchctl submit -l com.foo -- /path/gateway", + "launchctl bootstrap gui/501 /tmp/com.foo.plist", + ]) + def test_script_with_neutral_label_submit_or_bootstrap_raises( + self, tmp_path, line + ): + from cron.lifecycle_guard import GatewayLifecycleBlocked, check_gateway_lifecycle + script = tmp_path / "persistent.sh" + script.write_text(f"#!/bin/bash\n{line}\n") + with pytest.raises(GatewayLifecycleBlocked): + check_gateway_lifecycle("clean prompt", str(script)) + + def test_split_across_prompt_and_script_still_blocks(self, tmp_path): + """Concatenated scan prevents splitting the command between prompt and + script to slip through.""" + from cron.lifecycle_guard import GatewayLifecycleBlocked, check_gateway_lifecycle + script = tmp_path / "ops.sh" + script.write_text("hermes gateway stop\n") + with pytest.raises(GatewayLifecycleBlocked): + check_gateway_lifecycle("daily ops job", str(script)) def test_binary_script_does_not_silently_bypass(self, tmp_path): """Non-UTF-8 bytes used to be swallowed by UnicodeDecodeError; now we @@ -390,3 +718,80 @@ def test_clear_resets(self): rlg.check_and_record(3, 60, now=1001.0) rlg.clear() assert rlg.check_and_record(3, 60, now=1002.0) is False + +class TestTerminalToolGatewayLifecycleGuardRemote: + """Remote-backend and two-session cwd regression coverage.""" + + def _patch_env(self, monkeypatch, fake_env, *, inside_gateway: bool): + import tools.terminal_tool as tt + eid = "default" + monkeypatch.setattr(tt, "_active_environments", {eid: fake_env}) + monkeypatch.setattr(tt, "_last_activity", {eid: 0.0}) + monkeypatch.setattr(tt, "_task_env_overrides", {}) + monkeypatch.setattr(tt, "_get_env_config", lambda: {"env_type": "local", "cwd": "/tmp", "timeout": 60, "lifetime_seconds": 3600}) + if inside_gateway: + monkeypatch.setenv("_HERMES_GATEWAY", "1") + else: + monkeypatch.delenv("_HERMES_GATEWAY", raising=False) + + def test_remote_backend_script_read_uses_env_execute(self, monkeypatch, tmp_path): + import tools.terminal_tool as tt + + # Path only exists on the remote backend; locally it is absent, so the + # guard must fall back to env.execute('cat ...') to scan it. + script = "/remote/workspace/remote.sh" + calls = [] + + class _RemoteEnv: + env = {} + cwd = str(tmp_path) + def execute(self, command, **kwargs): + calls.append(command) + if "cat" in command and "/remote/workspace/remote.sh" in command: + return {"output": "#!/bin/bash\\nhermes gateway restart\\n", "returncode": 0} + return {"output": "", "returncode": 0} + + fake_env = _RemoteEnv() + fake_env.cwd = "/remote/workspace" + self._patch_env(monkeypatch, fake_env, inside_gateway=True) + + result = json.loads(tt.terminal_tool(command=f"/bin/bash {script}")) + + assert result["exit_code"] == 1 + assert "referenced script" in result["error"] + assert any("cat" in c for c in calls) + + +class TestCronCreateLifecycleBlockExtra: + """Additional cron create lifecycle guard coverage.""" + + @pytest.fixture(autouse=True) + def _setup_cron_dir(self, tmp_path, monkeypatch): + monkeypatch.setattr("cron.jobs.CRON_DIR", tmp_path / "cron") + monkeypatch.setattr("cron.jobs.JOBS_FILE", tmp_path / "cron" / "jobs.json") + monkeypatch.setattr("cron.jobs.OUTPUT_DIR", tmp_path / "cron" / "output") + + def test_cron_nested_wrapper_script_is_scanned(self, tmp_path, capsys, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + scripts_dir = tmp_path / ".hermes" / "scripts" + scripts_dir.mkdir(parents=True) + (scripts_dir / "inner.sh").write_text("#!/bin/bash\nhermes gateway restart\n") + (scripts_dir / "outer.sh").write_text("#!/bin/bash\n/bin/bash inner.sh\n") + args = Namespace( + cron_command="create", + schedule="1h", + prompt=None, + name=None, + deliver=None, + repeat=None, + skill=None, + skills=None, + script="outer.sh", + workdir=None, + profile=None, + no_agent=True, + ) + rc = cron_command(args) + assert rc == 1 + out = capsys.readouterr().out + assert "Blocked" in out diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index 6e06e55b2cb95..d9c165bdf6eb4 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -39,6 +39,8 @@ import os import platform import re +import shlex +import stat import time import threading import atexit @@ -2322,7 +2324,7 @@ def terminal_tool( # Use a per-task creation lock so concurrent tool calls for the same # task_id wait for the first one to finish creating the sandbox, # instead of each creating their own (wasting Modal resources). - env = None + env: Any = None with _env_lock: # Prefer the collapsed container id, but fall back to an env cached # under the raw task_id. Per-session surfaces (ACP/gateway/dashboard) @@ -2426,15 +2428,15 @@ def terminal_tool( env = new_env logger.info("%s environment ready for task %s", env_type, effective_task_id[:8]) - if env is None: - # Unreachable in practice (either the cached branch or the creation - # branch assigned env above); guard for type-safety and so a future - # refactor of the branches can't fall through to an AttributeError. - return json.dumps({ - "output": "", - "exit_code": -1, - "error": "Terminal environment unavailable (creation raced cleanup)", - }, ensure_ascii=False) + assert env is not None # all creation failure paths return above + + # The session key that drives cwd records: get_current_session_key()'s + # contextvar doesn't cross tool-worker threads, so fall back to the raw + # task_id (which IS the session_key for the top-level agent) — a + # stable, thread-safe anchor. + from tools.approval import get_current_session_key + + session_key = get_current_session_key(default="") or (task_id or "") # Hard-block: gateway lifecycle commands (systemctl/launchctl/hermes # restart|stop targeting hermes-gateway) must never run inside the @@ -2444,17 +2446,75 @@ def terminal_tool( # hermes_cli/gateway.py and the cron-path guard in hermes_cli/cron.py, # but applies unconditionally (force=True cannot help here). if os.environ.get("_HERMES_GATEWAY") == "1": - from hermes_cli.cron import _contains_gateway_lifecycle_command - if _contains_gateway_lifecycle_command(command): + from cron.lifecycle_guard import ( + contains_gateway_lifecycle_command_or_referenced_script, + contains_launchctl_submit_command, + ) + if contains_launchctl_submit_command(command): return json.dumps({ "output": "", "exit_code": 1, "error": ( - "Blocked: cannot restart or stop the gateway from inside the " - "gateway process. The gateway would kill this command before " - "it could complete (SIGTERM propagates to child processes). " - "Run `hermes gateway restart` from a separate shell outside " - "the running gateway." + "Blocked: launchctl submit/bootstrap registers a persistent " + "KeepAlive job and is unsafe from inside the gateway process. " + "Use Hermes cron for one-shot delayed work, or install an " + "explicit LaunchAgent from a separate shell." + ), + "status": "error", + }, ensure_ascii=False) + guard_cwd_base = get_session_cwd(session_key) + if guard_cwd_base is None: + guard_cwd_base = getattr(env, "cwd", None) or cwd + guard_cwd = _resolve_command_cwd( + workdir=workdir, + default_cwd=guard_cwd_base, + session_key=session_key, + ) + + def _read_script_in_env(script_path: str) -> Optional[str]: + """Best-effort script read; uses env.execute only when local read fails. + + For local backends the script path is on the host filesystem. For + SSH/Modal/Daytona the same path is remote; the local read misses, so we + fall back to ``env.execute('cat ...')``. + """ + if env is None: + return None + try: + local_path = Path(script_path).expanduser() + if not local_path.is_absolute(): + local_path = Path(guard_cwd) / local_path + if local_path.is_file(): + metadata = local_path.stat() + if stat.S_ISREG(metadata.st_mode) and metadata.st_size <= 1024 * 1024: + data = local_path.read_bytes() + if len(data) <= 1024 * 1024: + 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", "") + except Exception: + pass + return None + + if contains_gateway_lifecycle_command_or_referenced_script( + command, + cwd=guard_cwd, + read_remote_script=_read_script_in_env, + ): + return json.dumps({ + "output": "", + "exit_code": 1, + "error": ( + "Blocked: command or referenced script cannot restart or stop " + "the gateway from inside the gateway process. The gateway would " + "kill this command before it could complete (SIGTERM propagates " + "to child processes). Run `hermes gateway restart` from a " + "separate shell outside the running gateway." ), "status": "error", }, ensure_ascii=False) @@ -2533,14 +2593,7 @@ def terminal_tool( "EOF." ) - # The session key that drives cwd records: get_current_session_key()'s - # contextvar doesn't cross tool-worker threads, so fall back to the raw - # task_id (which IS the session_key for the top-level agent) — a - # stable, thread-safe anchor. - from tools.approval import get_current_session_key - - session_key = get_current_session_key(default="") or (task_id or "") - + # The session key is already computed above the gateway guard. if background: # Spawn a tracked background process via the process registry. # For local backends: uses subprocess.Popen with output buffering.