fix(gateway): add submit/bootstrap to lifecycle guard Branch B and label-independent detection
Extends the shared _GATEWAY_LIFECYCLE_PATTERN (used by BOTH the cron creation-time guard in cron/lifecycle_guard.py and the terminal execution-time hard-block in tools/terminal_tool.py) so Branch B covers launchctl submit and bootstrap alongside kickstart/unload/load/stop/ restart, and normalizes POSIX shell line continuations before matching so the exact multi-line reported shape in #62891 cannot slip past. Also extends the execution-aware, label-independent detector (contains_launchctl_submit_command, cherry-picked from #63272) to cover launchctl bootstrap, since a neutral label like ai.hermes.svc-reload-tmp defeats any label-anchored regex — the second production reproduction. Regression tests cover both sites, including `launchctl submit -l com.foo -- /path/gateway` and the bootstrap variant, plus outside-gateway pass-through. Branch B regex extension and continuation normalization drawn from PR #62896; bootstrap coverage and test shapes drawn from PR #51003. Co-authored-by: JackJin <1037461232@qq.com> Co-authored-by: joelbrilliant <joelbrilliant1@gmail.com>
This commit is contained in:
parent
d8b041e58b
commit
56cf87432b
|
|
@ -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-<suffix> --
|
||||
# <helper-script>` (or `launchctl bootstrap gui/<uid> <plist>`) 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
|
||||
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue