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