From 320d884d8862360fbca7d0bfda70e487fcfa7bc9 Mon Sep 17 00:00:00 2001 From: Axel Vanni Date: Thu, 6 Aug 2026 20:39:43 +1000 Subject: [PATCH] fix(cron): cover bootout/remove/disable in the gateway lifecycle guard Branch B of _GATEWAY_LIFECYCLE_PATTERN enumerated launchd verbs but omitted `bootout` - the modern replacement for the `unload` it already listed, and the paired inverse of the `bootstrap` it already listed. `remove` (legacy sibling of bootout) and `disable` (what makes an unload durable) were missing for the same reason. This matters because the two enforcement layers are not interchangeable. In tools/terminal_tool.py under _HERMES_GATEWAY == "1": - the cron.lifecycle_guard hard block is documented as applying unconditionally ("force=True cannot help here") - detect_dangerous_command below it is explicitly skipped when force=True detect_dangerous_command already flags all three verbs, so the default path was covered - but with force=True inside the gateway they reached execution while stop/unload/kickstart did not. SIGTERM then propagates to the child before the command completes and the service may never come back, which is the state described in #74973. The label anchor (\bhermes[.\-]?gateway) is unchanged, so unrelated services such as `launchctl bootout gui/501/ai.hermes.update-checker` stay runnable. Adds TestLifecycleGuardLaunchctlParity, which pins the one-directional invariant: anything the bypassable approval layer flags, the unbypassable hard block must also catch. Deliberately not equality - the hard block is legitimately stricter (it also covers load/restart, which the approval layer leaves alone). Verified failing on the parent commit for exactly bootout, remove and disable. Closes #80260 Co-Authored-By: Claude Opus 5 --- cron/lifecycle_guard.py | 9 ++++- tests/tools/test_approval.py | 70 ++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/cron/lifecycle_guard.py b/cron/lifecycle_guard.py index 840e641e8c..4c42c05234 100644 --- a/cron/lifecycle_guard.py +++ b/cron/lifecycle_guard.py @@ -77,7 +77,14 @@ _GATEWAY_LIFECYCLE_PATTERN = re.compile( # 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)" + # `bootout`/`remove`/`disable` sit alongside `unload`: Apple deprecated + # load/unload in favour of bootstrap/bootout, so `bootout` is the modern + # spelling of an already-listed verb, `remove` is its legacy sibling, and + # `disable` is what makes an unload durable across boots. Omitting them + # left the bypassable approval layer (tools/approval.py, skipped on + # force=True) as the only cover, while this hard block — documented as + # "force=True cannot help here" — let them through (#80260). + r"|(?:launchctl\s+(?:kickstart|unload|load|stop|restart|submit|bootstrap|bootout|remove|disable)\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 diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index 258a991aae..24a74be21f 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -1782,3 +1782,73 @@ class TestCliApprovalTimeoutClassifiedSeparately: assert result.get("user_consent") is False assert "timed out without user response" in result["message"] assert "Silence is not consent" in result["message"] + + +# launchd verbs that stop, unload or deregister a running gateway. `disable` +# does not stop a live job on its own, but it is what makes an unload survive +# a reboot, so it belongs to the same family. +GATEWAY_LIFECYCLE_LAUNCHCTL = ( + "launchctl kickstart -k gui/501/ai.hermes.gateway", + "launchctl unload ~/Library/LaunchAgents/ai.hermes.gateway.plist", + "launchctl load ~/Library/LaunchAgents/ai.hermes.gateway.plist", + "launchctl stop ai.hermes.gateway", + "launchctl restart ai.hermes.gateway", + "launchctl bootout gui/501/ai.hermes.gateway", + "launchctl remove ai.hermes.gateway", + "launchctl disable gui/501/ai.hermes.gateway", +) + + +class TestLifecycleGuardLaunchctlParity: + """The in-gateway hard block must cover every launchd verb the approval + layer already treats as gateway lifecycle. + + These two layers are not interchangeable. In ``tools/terminal_tool.py`` + under ``_HERMES_GATEWAY == "1"``, the ``cron.lifecycle_guard`` block is + documented as applying unconditionally ("force=True cannot help here"), + while ``detect_dangerous_command`` below it is explicitly skipped when + ``force=True``. A verb covered only by the approval layer is therefore + reachable from inside the gateway, where SIGTERM propagates to the child + before the command completes and the service may never come back (#74973). + + ``bootout`` was missing exactly this way: it is the modern replacement for + the ``unload`` the guard already listed. See #80260. + """ + + def test_hard_block_covers_every_lifecycle_verb(self): + from cron.lifecycle_guard import contains_gateway_lifecycle_command + + for cmd in GATEWAY_LIFECYCLE_LAUNCHCTL: + assert contains_gateway_lifecycle_command(cmd) is True, cmd + + def test_bypassable_layer_is_never_stricter(self): + """One-directional invariant: anything ``detect_dangerous_command`` + flags as gateway lifecycle, the hard block must also catch. + + Not equality — the hard block is legitimately stricter (it also covers + ``load``/``restart``, which the approval layer leaves alone). What must + never happen is the reverse: a command stopped only by the layer that + ``force=True`` skips, leaving no cover inside the gateway.""" + from cron.lifecycle_guard import contains_gateway_lifecycle_command + + for cmd in GATEWAY_LIFECYCLE_LAUNCHCTL: + dangerous, _, _ = detect_dangerous_command(cmd) + if not dangerous: + continue + assert contains_gateway_lifecycle_command(cmd) is True, ( + f"approval layer flags this but the unbypassable hard block " + f"does not: {cmd}" + ) + + def test_unrelated_labels_are_not_blocked(self): + """The label anchor must still scope this to the gateway — unrelated + services, including other Hermes ones, stay runnable.""" + from cron.lifecycle_guard import contains_gateway_lifecycle_command + + for cmd in ( + "launchctl bootout gui/501/com.example.unrelated", + "launchctl remove ai.hermes.update-checker", + "launchctl disable gui/501/com.apple.WindowServer", + "launchctl print system/com.apple.WindowServer", + ): + assert contains_gateway_lifecycle_command(cmd) is False, cmd