fix(approval): extend gateway-lifecycle guard to launchctl and pidof-based kills

The dangerous-command approval layer already blocks `hermes gateway
(stop|restart)`, `pkill/killall hermes|gateway`, and `kill ... $(pgrep ...)`.
A reporter noted on #33071 that the agent can still achieve the same
effect by driving launchd directly against the gateway's service label
(`launchctl stop ai.hermes.gateway`, `launchctl kickstart -k
system/ai.hermes.gateway`, etc.) or by substituting `pidof` for `pgrep`
in the kill-expansion form.

This widens the "Gateway lifecycle protection" block in
`tools/approval.py` to cover both vectors:

- `launchctl (stop|kickstart|bootout|unload|kill|disable|remove)`
  scoped to commands that target a Hermes label (`hermes`,
  `ai.hermes`). Read-only inspection (`launchctl print …`,
  `launchctl list`) and operations against unrelated labels remain
  unflagged.
- `kill ... $(pidof …)` and the backtick form, alongside the existing
  `pgrep` expansion. `pidof` is the BSD/Linux equivalent and is
  equally opaque to the `(pkill|killall) … hermes` name pattern.

Intentionally left out of scope: plain `kill -TERM <numeric_pid>` with
a PID looked up out-of-band. Catching that would require runtime PID
state and would break the existing
`TestPgrepKillExpansion::test_safe_kill_pid_not_flagged` contract,
which guarantees that a plain literal-PID `kill 12345` stays safe.
This commit is contained in:
briandevans 2026-05-27 00:13:07 -07:00 committed by Teknium
parent ba7026c376
commit 3c8d3ecfa0
2 changed files with 64 additions and 2 deletions

View file

@ -1121,6 +1121,61 @@ class TestPgrepKillExpansion:
dangerous, _, _ = detect_dangerous_command(cmd)
assert dangerous is False
def test_kill_dollar_pidof_detected(self):
"""`kill $(pidof hermes)` is the BSD/Linux equivalent of the
pgrep expansion and bypasses the pkill/killall name pattern
in the same way. See issue #33071."""
cmd = "kill -TERM $(pidof hermes_cli.main)"
dangerous, _, desc = detect_dangerous_command(cmd)
assert dangerous is True
assert "pidof" in desc.lower() or "pgrep" in desc.lower()
def test_kill_backtick_pidof_detected(self):
cmd = "kill -9 `pidof hermes`"
dangerous, _, _ = detect_dangerous_command(cmd)
assert dangerous is True
class TestLaunchctlGatewayLifecycle:
"""launchctl stop/kickstart/bootout/unload against the Hermes service
label achieves the same effect as `hermes gateway stop|restart` and
must require the same approval. See issue #33071.
"""
def test_launchctl_stop_hermes_detected(self):
cmd = "launchctl stop ai.hermes.gateway"
dangerous, _, desc = detect_dangerous_command(cmd)
assert dangerous is True
assert "launchd" in desc.lower() or "hermes" in desc.lower()
def test_launchctl_kickstart_hermes_detected(self):
cmd = "launchctl kickstart -k system/ai.hermes.gateway"
dangerous, _, _ = detect_dangerous_command(cmd)
assert dangerous is True
def test_launchctl_bootout_hermes_detected(self):
cmd = "launchctl bootout system/ai.hermes.gateway"
dangerous, _, _ = detect_dangerous_command(cmd)
assert dangerous is True
def test_launchctl_unload_hermes_detected(self):
cmd = "launchctl unload ~/Library/LaunchAgents/ai.hermes.gateway.plist"
dangerous, _, _ = detect_dangerous_command(cmd)
assert dangerous is True
def test_launchctl_print_unrelated_not_flagged(self):
"""Read-only inspection of an unrelated launchd label must stay safe."""
cmd = "launchctl print system/com.apple.WindowServer"
dangerous, _, _ = detect_dangerous_command(cmd)
assert dangerous is False
def test_launchctl_stop_unrelated_not_flagged(self):
"""`launchctl stop` on a non-Hermes label is out of scope for the
gateway-lifecycle guard."""
cmd = "launchctl stop com.example.unrelated"
dangerous, _, _ = detect_dangerous_command(cmd)
assert dangerous is False
class TestGitDestructiveOps:
"""git reset --hard, push --force, clean -f, branch -D can destroy

View file

@ -443,8 +443,15 @@ DANGEROUS_PATTERNS = [
# The name-based pattern above catches `pkill hermes` but not
# `kill -9 $(pgrep -f hermes)` because the substitution is opaque
# to regex at detection time. Catch the structural pattern instead.
(r'\bkill\b.*\$\(\s*pgrep\b', "kill process via pgrep expansion (self-termination)"),
(r'\bkill\b.*`\s*pgrep\b', "kill process via backtick pgrep expansion (self-termination)"),
# `pidof` is the BSD/Linux alternative to `pgrep` and is equally
# opaque, so include it in the same alternation.
(r'\bkill\b.*\$\(\s*(pgrep|pidof)\b', "kill process via pgrep/pidof expansion (self-termination)"),
(r'\bkill\b.*`\s*(pgrep|pidof)\b', "kill process via backtick pgrep/pidof expansion (self-termination)"),
# launchctl-driven gateway stop/restart on macOS. The agent can bypass
# the `hermes gateway stop|restart` pattern above by driving launchd
# directly against the service label (commonly `ai.hermes.gateway`).
# Catch the operations that stop, restart, or unload it.
(r'\blaunchctl\s+(stop|kickstart|bootout|unload|kill|disable|remove)\b.*\b(hermes|ai\.hermes)\b', "stop/restart hermes launchd service (kills running agents)"),
# File copy/move/edit into sensitive system paths (/etc/ and macOS
# /private/etc/ mirror).
(rf'\b(cp|mv|install)\b.*\s{_SYSTEM_CONFIG_PATH}', "copy/move file into system config path"),