mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
test(conftest): plug every gateway-kill leak path (#23486)
The existing _live_system_guard (PR #23397) blocked os.kill / os.killpg and a narrow subset of subprocess invocations. Tests still SIGTERMed the live gateway today (May 10) because the guard had structural holes. Plug them all: - subprocess: also wrap getoutput, getstatusoutput - os.system, os.popen - completely unwrapped before - pty.spawn - completely unwrapped before - asyncio.create_subprocess_exec / create_subprocess_shell - bypassed the subprocess module entirely; now wrapped - Subprocess command inspection now looks at the WHOLE command string, not just tokens[0]. Catches sudo systemctl, env systemctl, bash -c 'systemctl', setsid systemctl, /usr/bin/systemctl, etc. - New process-killer block: pkill / killall / taskkill / fuser targeting hermes/python patterns is now refused - os.kill PID 0 (own group) allowed; PID -1 (every process we can signal) refused - subprocess.Popen wrapper preserves __class_getitem__ so third-party packages that use Popen[bytes] as a type annotation still import Coverage is locked in by tests/test_live_system_guard_self_test.py - exercises every primitive against a guaranteed-foreign PID and asserts the guard fires. Adding a new kill primitive without updating the guard breaks CI. scripts/run_tests.sh now also force-loads ~/.hermes/pytest_live_guard.py when present (developer-machine convenience), so even worktrees that predate this commit get the protection on subsequent test runs through the canonical wrapper.
This commit is contained in:
Teknium
GitHub
parent
e5bce320db
commit
771b8c4a36
3 changed files with 514 additions and 42 deletions
|
|
@ -665,12 +665,27 @@ def _live_system_guard(request, monkeypatch):
|
|||
See block comment above for the why. Tests that genuinely need
|
||||
real signal delivery (e.g. PTY tests that SIGINT their own child)
|
||||
can opt out with ``@pytest.mark.live_system_guard_bypass``.
|
||||
|
||||
Coverage (every primitive that can deliver a signal to or otherwise
|
||||
terminate a foreign process):
|
||||
• os.kill, os.killpg (POSIX)
|
||||
• subprocess.run / Popen / call / check_call / check_output
|
||||
• subprocess.getoutput / getstatusoutput
|
||||
• os.system / os.popen
|
||||
• pty.spawn
|
||||
• asyncio.create_subprocess_exec / create_subprocess_shell
|
||||
Subprocess inspection looks at the WHOLE command string (not just
|
||||
tokens[0]), so ``bash -c "systemctl restart hermes-gateway"``,
|
||||
``sudo systemctl ...``, ``env systemctl ...``, ``setsid systemctl ...``
|
||||
are all caught. ``pkill``/``killall``/``taskkill`` invocations
|
||||
targeting hermes/python patterns are also blocked.
|
||||
"""
|
||||
if request.node.get_closest_marker(_LIVE_SYSTEM_GUARD_BYPASS_MARK):
|
||||
yield
|
||||
return
|
||||
|
||||
import os as _os
|
||||
import shlex as _shlex
|
||||
import subprocess as _subprocess
|
||||
|
||||
test_pid = _os.getpid()
|
||||
|
|
@ -687,6 +702,14 @@ def _live_system_guard(request, monkeypatch):
|
|||
_initial_children = set()
|
||||
|
||||
def _is_own_subtree(pid: int) -> bool:
|
||||
# PID 0 means "our own process group"; -1 means "every process we
|
||||
# can signal". Both are dangerous when paired with SIGTERM/SIGKILL,
|
||||
# but pid 0 is technically scoped to our group so allow it; pid -1
|
||||
# is treated as foreign (refuse).
|
||||
if pid == 0:
|
||||
return True
|
||||
if pid < 0:
|
||||
return False
|
||||
if pid == test_pid or pid in _initial_children:
|
||||
return True
|
||||
if _psutil is None:
|
||||
|
|
@ -742,55 +765,133 @@ def _live_system_guard(request, monkeypatch):
|
|||
|
||||
monkeypatch.setattr(_os, "killpg", _guarded_killpg)
|
||||
|
||||
# ── Subprocess command-string inspection (whole-line) ──────────
|
||||
_HERMES_TOKENS = (
|
||||
"hermes-gateway",
|
||||
"hermes.service",
|
||||
"hermes_cli.main gateway",
|
||||
"hermes_cli/main.py gateway",
|
||||
"gateway/run.py",
|
||||
"hermes gateway",
|
||||
)
|
||||
_MUTATING_VERBS = (
|
||||
"restart", "start", "stop", "kill", "reload",
|
||||
"reset-failed", "enable", "disable", "mask", "unmask",
|
||||
"daemon-reload", "try-restart", "reload-or-restart",
|
||||
)
|
||||
_PROCESS_KILLERS = ("pkill", "killall", "taskkill", "skill", "fuser")
|
||||
|
||||
def _cmd_to_string(cmd) -> str:
|
||||
if cmd is None:
|
||||
return ""
|
||||
if isinstance(cmd, (bytes, bytearray)):
|
||||
try:
|
||||
return bytes(cmd).decode(errors="replace")
|
||||
except Exception:
|
||||
return ""
|
||||
if isinstance(cmd, str):
|
||||
return cmd
|
||||
if isinstance(cmd, (list, tuple)):
|
||||
try:
|
||||
return " ".join(str(t) for t in cmd)
|
||||
except Exception:
|
||||
return ""
|
||||
return str(cmd)
|
||||
|
||||
def _matches_hermes_gateway(cmd_str: str) -> bool:
|
||||
low = cmd_str.lower()
|
||||
return any(tok in low for tok in _HERMES_TOKENS)
|
||||
|
||||
def _is_blocked_systemctl(cmd) -> bool:
|
||||
cmd_str = _cmd_to_string(cmd)
|
||||
if "systemctl" not in cmd_str:
|
||||
return False
|
||||
if not _matches_hermes_gateway(cmd_str):
|
||||
return False
|
||||
try:
|
||||
tokens = _shlex.split(cmd_str)
|
||||
except ValueError:
|
||||
tokens = cmd_str.split()
|
||||
return any(verb in tokens for verb in _MUTATING_VERBS)
|
||||
|
||||
def _is_process_killer(cmd) -> bool:
|
||||
cmd_str = _cmd_to_string(cmd)
|
||||
try:
|
||||
tokens = _shlex.split(cmd_str)
|
||||
except ValueError:
|
||||
tokens = cmd_str.split()
|
||||
if not tokens:
|
||||
return False
|
||||
for tok in tokens:
|
||||
head = tok.rsplit("/", 1)[-1].rsplit("\\", 1)[-1]
|
||||
if head in _PROCESS_KILLERS:
|
||||
low = cmd_str.lower()
|
||||
# pkill -f pattern: catch hermes-themed patterns + a
|
||||
# plain "python" -f which would catch the live gateway
|
||||
# whose cmdline contains "python -m hermes_cli.main".
|
||||
if (
|
||||
"hermes" in low
|
||||
or "gateway" in low
|
||||
or ("python" in low and "-f" in tokens)
|
||||
):
|
||||
return True
|
||||
return False
|
||||
|
||||
def _check_subprocess_cmd(name, cmd):
|
||||
if _is_blocked_systemctl(cmd):
|
||||
raise RuntimeError(
|
||||
f"tests/conftest.py live-system guard: blocked "
|
||||
f"subprocess.{name}({cmd!r}) — would mutate the "
|
||||
"live hermes-gateway systemd unit. Mock "
|
||||
"subprocess.run / _run_systemctl in the test, or "
|
||||
"mark with @pytest.mark.live_system_guard_bypass."
|
||||
)
|
||||
if _is_process_killer(cmd):
|
||||
raise RuntimeError(
|
||||
f"tests/conftest.py live-system guard: blocked "
|
||||
f"subprocess.{name}({cmd!r}) — process-killer command "
|
||||
"targeting hermes/python could hit the live gateway. "
|
||||
"Mark with @pytest.mark.live_system_guard_bypass if "
|
||||
"intentional."
|
||||
)
|
||||
|
||||
def _wrap_subprocess(name, real):
|
||||
def _guarded(cmd, *args, **kwargs):
|
||||
_check_subprocess_cmd(name, cmd)
|
||||
return real(cmd, *args, **kwargs)
|
||||
_guarded.__name__ = f"_guarded_{name}"
|
||||
# Make the wrapper subscriptable like the wrapped callable when
|
||||
# the wrapped object is. ``subprocess.Popen[bytes]`` is used as
|
||||
# a type annotation in third-party packages (mcp, etc.); replacing
|
||||
# ``Popen`` with a plain function breaks ``Popen[bytes]`` at
|
||||
# import time. Defer ``__class_getitem__`` to the original.
|
||||
if hasattr(real, "__class_getitem__"):
|
||||
_guarded.__class_getitem__ = real.__class_getitem__
|
||||
return _guarded
|
||||
|
||||
def _wrap_popen():
|
||||
"""Subclass Popen so isinstance checks AND Popen[bytes] still work."""
|
||||
real = _subprocess.Popen
|
||||
|
||||
class _GuardedPopen(real): # type: ignore[misc, valid-type]
|
||||
def __init__(self, cmd, *args, **kwargs):
|
||||
_check_subprocess_cmd("Popen", cmd)
|
||||
super().__init__(cmd, *args, **kwargs)
|
||||
|
||||
_GuardedPopen.__name__ = "Popen"
|
||||
_GuardedPopen.__qualname__ = "Popen"
|
||||
return _GuardedPopen
|
||||
|
||||
real_run = _subprocess.run
|
||||
real_popen = _subprocess.Popen
|
||||
real_call = _subprocess.call
|
||||
real_check_call = _subprocess.check_call
|
||||
real_check_output = _subprocess.check_output
|
||||
|
||||
def _is_blocked_systemctl(cmd) -> bool:
|
||||
if isinstance(cmd, (list, tuple)):
|
||||
tokens = [str(t) for t in cmd]
|
||||
elif isinstance(cmd, (bytes, bytearray)):
|
||||
tokens = bytes(cmd).decode(errors="replace").split()
|
||||
elif isinstance(cmd, str):
|
||||
tokens = cmd.split()
|
||||
else:
|
||||
return False
|
||||
if not tokens:
|
||||
return False
|
||||
head = tokens[0].rsplit("/", 1)[-1]
|
||||
if head != "systemctl":
|
||||
return False
|
||||
joined = " ".join(tokens).lower()
|
||||
# Block any systemctl invocation that targets a hermes-gateway
|
||||
# unit AND would change its run state. Status reads, list-units,
|
||||
# and show calls remain allowed because they're side-effect-free.
|
||||
if "hermes-gateway" not in joined and "hermes.service" not in joined:
|
||||
return False
|
||||
mutating = (
|
||||
"restart", "start", "stop", "kill", "reload",
|
||||
"reset-failed", "enable", "disable", "mask", "unmask",
|
||||
"daemon-reload",
|
||||
)
|
||||
return any(verb in tokens for verb in mutating)
|
||||
|
||||
def _wrap_subprocess(name, real):
|
||||
def _guarded(cmd, *args, **kwargs):
|
||||
if _is_blocked_systemctl(cmd):
|
||||
raise RuntimeError(
|
||||
f"tests/conftest.py live-system guard: blocked "
|
||||
f"subprocess.{name}({cmd!r}) — would mutate the "
|
||||
"live hermes-gateway systemd unit. Mock "
|
||||
"subprocess.run / _run_systemctl in the test, or "
|
||||
"mark with @pytest.mark.live_system_guard_bypass."
|
||||
)
|
||||
return real(cmd, *args, **kwargs)
|
||||
_guarded.__name__ = f"_guarded_{name}"
|
||||
return _guarded
|
||||
real_getoutput = _subprocess.getoutput
|
||||
real_getstatusoutput = _subprocess.getstatusoutput
|
||||
|
||||
monkeypatch.setattr(_subprocess, "run", _wrap_subprocess("run", real_run))
|
||||
monkeypatch.setattr(_subprocess, "Popen", _wrap_subprocess("Popen", real_popen))
|
||||
monkeypatch.setattr(_subprocess, "Popen", _wrap_popen())
|
||||
monkeypatch.setattr(_subprocess, "call", _wrap_subprocess("call", real_call))
|
||||
monkeypatch.setattr(
|
||||
_subprocess, "check_call", _wrap_subprocess("check_call", real_check_call)
|
||||
|
|
@ -800,5 +901,65 @@ def _live_system_guard(request, monkeypatch):
|
|||
"check_output",
|
||||
_wrap_subprocess("check_output", real_check_output),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
_subprocess, "getoutput", _wrap_subprocess("getoutput", real_getoutput)
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
_subprocess,
|
||||
"getstatusoutput",
|
||||
_wrap_subprocess("getstatusoutput", real_getstatusoutput),
|
||||
)
|
||||
|
||||
# os.system / os.popen — same risk class, completely unwrapped before.
|
||||
real_os_system = _os.system
|
||||
real_os_popen = _os.popen
|
||||
|
||||
def _guarded_os_system(command):
|
||||
_check_subprocess_cmd("os.system", command)
|
||||
return real_os_system(command)
|
||||
|
||||
def _guarded_os_popen(cmd, *args, **kwargs):
|
||||
_check_subprocess_cmd("os.popen", cmd)
|
||||
return real_os_popen(cmd, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(_os, "system", _guarded_os_system)
|
||||
monkeypatch.setattr(_os, "popen", _guarded_os_popen)
|
||||
|
||||
# pty.spawn — POSIX-only.
|
||||
try:
|
||||
import pty as _pty
|
||||
if hasattr(_pty, "spawn"):
|
||||
real_pty_spawn = _pty.spawn
|
||||
|
||||
def _guarded_pty_spawn(argv, *args, **kwargs):
|
||||
_check_subprocess_cmd("pty.spawn", argv)
|
||||
return real_pty_spawn(argv, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(_pty, "spawn", _guarded_pty_spawn)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# asyncio.create_subprocess_* — bypasses subprocess module entirely.
|
||||
try:
|
||||
import asyncio as _asyncio
|
||||
real_async_exec = _asyncio.create_subprocess_exec
|
||||
real_async_shell = _asyncio.create_subprocess_shell
|
||||
|
||||
async def _guarded_async_exec(program, *args, **kwargs):
|
||||
_check_subprocess_cmd(
|
||||
"asyncio.create_subprocess_exec", [program, *args]
|
||||
)
|
||||
return await real_async_exec(program, *args, **kwargs)
|
||||
|
||||
async def _guarded_async_shell(cmd, *args, **kwargs):
|
||||
_check_subprocess_cmd("asyncio.create_subprocess_shell", cmd)
|
||||
return await real_async_shell(cmd, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(_asyncio, "create_subprocess_exec", _guarded_async_exec)
|
||||
monkeypatch.setattr(
|
||||
_asyncio, "create_subprocess_shell", _guarded_async_shell
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
yield
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue