diff --git a/cron/lifecycle_guard.py b/cron/lifecycle_guard.py index c515fa73789a8..b09635c998bb9 100644 --- a/cron/lifecycle_guard.py +++ b/cron/lifecycle_guard.py @@ -61,7 +61,15 @@ _GATEWAY_LIFECYCLE_PATTERN = re.compile( # 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 @@ -71,11 +79,25 @@ _GATEWAY_LIFECYCLE_PATTERN = re.compile( ) +# 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"}) @@ -128,14 +150,22 @@ def _command_token_index(segment: list[str]) -> Optional[int]: def contains_launchctl_submit_command(command: str) -> bool: - """Detect an executed ``launchctl submit``, not quoted/comment-only text.""" + """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() == "submit": + if arguments and arguments[0].lower() in {"submit", "bootstrap"}: return True return False diff --git a/tests/hermes_cli/test_gateway_restart_loop.py b/tests/hermes_cli/test_gateway_restart_loop.py index 274401ca43433..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 @@ class TestGatewayLifecyclePattern: # 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 @@ class TestTerminalToolGatewayLifecycleGuard: "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): @@ -287,6 +327,51 @@ class TestTerminalToolGatewayLifecycleGuard: 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 ): @@ -510,6 +595,21 @@ class TestLifecycleGuardModule: 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.""" diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index 7b50d4bcd5ee7..6fa0cfe7e3dfa 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -2468,10 +2468,10 @@ def terminal_tool( "output": "", "exit_code": 1, "error": ( - "Blocked: launchctl submit creates 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." + "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)