mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-30 06:41:51 +00:00
PR #30136 review surfaced two issues, both rooted in the same audit gap: docker integration tests were running as root, not the unprivileged `hermes` user (UID 10000) that the runtime actually uses via `s6-setuidgid hermes`. Anything that probed PID-1 state or wrote to the s6 control surface worked as root in the tests but was inert in production. Fixes: 1. `_s6_running()` previously called `Path("/proc/1/exe").resolve()`, which is root-only readable. For UID 10000 the symlink yields PermissionError, `resolve()` silently returns the unresolved path, and `exe.name == "exe"` — so detection always returned False, the service-manager runtime-registration path was inert, and every `hermes profile create` / `hermes -p X gateway start` silently skipped the s6 hook. Replace with `/proc/1/comm` (world-readable) + `/run/s6/basedir` (s6-overlay-specific) — both required, fail closed. 2. `02-reconcile-profiles` now also chowns `/run/service/.s6-svscan/` {control,lock} to hermes so `s6-svscanctl -a/-an` works without root. Previously the directory chown stopped at `/run/service` and the FIFO inside stayed root-owned, so `register_profile_gateway` from hermes failed at the rescan-trigger step with EACCES — the wrapper in profiles.py caught the exception and printed a swallowed warning, so profile creation appeared to succeed while the slot was rolled back. Audit changes to flush this class of bug next time: - Add `docker_exec` / `docker_exec_sh` helpers to `tests/docker/conftest.py` that default to `-u hermes`. The module docstring explains why and flags `user="root"` as opt-in only for tests that explicitly need root (none currently do). - Refactor every `docker exec` call in tests/docker/ through the new helpers (test_dashboard.py, test_zombie_reaping.py, test_profile_gateway.py, test_container_restart.py, test_s6_profile_gateway_integration.py). - Add 5 unit tests covering `_s6_running` under various probe states (both signals present; comm wrong; basedir missing; PermissionError on /proc/1/comm; missing /proc — non-Linux). The PermissionError test is the explicit regression guard for the original bug. Known follow-up: the per-service `supervise/control` FIFO inside each `/run/service/gateway-<profile>/supervise/` is created root-owned by s6-supervise (which runs as root because s6-svscan is PID 1). `s6-svc -u/-d/-t` from the hermes user will get EACCES on those. The audit under `-u hermes` will reveal this in lifecycle tests — surfacing the issue cleanly so it can be fixed in a focused follow-up (likely via a small SUID helper or a polling chown loop in cont-init.d). The detection + svscanctl fixes here are independent and complete on their own.
149 lines
5.7 KiB
Python
149 lines
5.7 KiB
Python
"""Harness: dashboard opt-in via HERMES_DASHBOARD.
|
|
|
|
Today (tini): dashboard starts once when HERMES_DASHBOARD=1; if it crashes
|
|
it stays dead. After Phase 2 (s6): dashboard starts once; if it crashes
|
|
it is restarted under supervision. The restart-after-crash test lives in
|
|
Phase 2 Task 2.5; this file only locks the opt-in surface (which must
|
|
not change between tini and s6).
|
|
|
|
Every ``docker exec`` here runs as the unprivileged ``hermes`` user
|
|
(via :func:`docker_exec`/:func:`docker_exec_sh` in conftest), matching
|
|
the realistic runtime context. See the conftest module docstring.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import subprocess
|
|
import time
|
|
|
|
from tests.docker.conftest import docker_exec, docker_exec_sh
|
|
|
|
|
|
def _poll(container: str, probe: str, *, deadline_s: float = 30.0,
|
|
interval_s: float = 0.5) -> tuple[bool, str]:
|
|
"""Repeatedly run ``probe`` inside the container until it exits 0 or
|
|
``deadline_s`` elapses. Returns (success, last stdout)."""
|
|
end = time.monotonic() + deadline_s
|
|
last = ""
|
|
while time.monotonic() < end:
|
|
r = docker_exec_sh(container, probe, timeout=10)
|
|
last = r.stdout
|
|
if r.returncode == 0:
|
|
return True, last
|
|
time.sleep(interval_s)
|
|
return False, last
|
|
|
|
|
|
def test_dashboard_not_running_by_default(
|
|
built_image: str, container_name: str,
|
|
) -> None:
|
|
"""Without HERMES_DASHBOARD, no dashboard process should be running."""
|
|
subprocess.run(
|
|
["docker", "run", "-d", "--name", container_name, built_image,
|
|
"sleep", "60"],
|
|
check=True, capture_output=True, timeout=30,
|
|
)
|
|
# Give the entrypoint enough time to finish bootstrap; if a dashboard
|
|
# were going to start it'd be visible by now.
|
|
time.sleep(5)
|
|
r = docker_exec(container_name, "pgrep", "-f", "hermes dashboard")
|
|
# pgrep exits non-zero when no match found
|
|
assert r.returncode != 0, (
|
|
"Dashboard should not be running without HERMES_DASHBOARD"
|
|
)
|
|
|
|
|
|
def test_dashboard_opt_in_starts(
|
|
built_image: str, container_name: str,
|
|
) -> None:
|
|
"""With HERMES_DASHBOARD=1, a dashboard process should be visible."""
|
|
subprocess.run(
|
|
["docker", "run", "-d", "--name", container_name,
|
|
"-e", "HERMES_DASHBOARD=1", built_image, "sleep", "120"],
|
|
check=True, capture_output=True, timeout=30,
|
|
)
|
|
# Poll for the dashboard subprocess to appear — the entrypoint
|
|
# backgrounds it and bootstrap (skills sync etc.) can take a few
|
|
# seconds before the python process actually launches.
|
|
ok, _ = _poll(
|
|
container_name, "pgrep -f 'hermes dashboard'", deadline_s=30.0,
|
|
)
|
|
assert ok, "Dashboard should be running with HERMES_DASHBOARD=1"
|
|
|
|
|
|
def test_dashboard_port_override(
|
|
built_image: str, container_name: str,
|
|
) -> None:
|
|
"""HERMES_DASHBOARD_PORT changes the dashboard's listen port."""
|
|
subprocess.run(
|
|
["docker", "run", "-d", "--name", container_name,
|
|
"-e", "HERMES_DASHBOARD=1", "-e", "HERMES_DASHBOARD_PORT=9120",
|
|
built_image, "sleep", "120"],
|
|
check=True, capture_output=True, timeout=30,
|
|
)
|
|
# The dashboard process appearing in pgrep doesn't mean it's bound
|
|
# to the port yet — uvicorn takes another second or two to come up.
|
|
# The image doesn't ship ss/netstat, so probe /proc/net/tcp directly:
|
|
# port 9120 = 0x23A0, state 0A = LISTEN.
|
|
ok, stdout = _poll(
|
|
container_name,
|
|
"grep -E ' 0+:23A0 .* 0A ' /proc/net/tcp /proc/net/tcp6 "
|
|
"2>/dev/null",
|
|
deadline_s=60.0,
|
|
)
|
|
assert ok, f"Dashboard not listening on port 9120: stdout={stdout!r}"
|
|
|
|
|
|
def test_dashboard_restarts_after_crash(
|
|
built_image: str, container_name: str,
|
|
) -> None:
|
|
"""Phase 2 invariant: under s6 supervision, killing the dashboard
|
|
process should be recovered automatically.
|
|
|
|
Pre-s6 (tini) behavior was "stays dead" — the test wouldn't have
|
|
passed against that image. After the s6-overlay migration the
|
|
dashboard runs as a longrun s6-rc service and s6-supervise restarts
|
|
it after a ~1s backoff (the default).
|
|
"""
|
|
subprocess.run(
|
|
["docker", "run", "-d", "--name", container_name,
|
|
"-e", "HERMES_DASHBOARD=1", built_image, "sleep", "120"],
|
|
check=True, capture_output=True, timeout=30,
|
|
)
|
|
# Wait for the first dashboard to come up.
|
|
ok, _ = _poll(
|
|
container_name, "pgrep -f 'hermes dashboard'", deadline_s=30.0,
|
|
)
|
|
assert ok, "Dashboard never started initially"
|
|
|
|
# Grab the initial PID. s6 may briefly transition through restart
|
|
# state between our poll-success and the follow-up pgrep, so retry
|
|
# a couple of times before giving up.
|
|
first_pid: str | None = None
|
|
for _attempt in range(10):
|
|
first_pid_result = docker_exec(
|
|
container_name, "pgrep", "-f", "hermes dashboard",
|
|
)
|
|
first_pids = first_pid_result.stdout.strip().split()
|
|
if first_pids:
|
|
first_pid = first_pids[0]
|
|
break
|
|
time.sleep(0.5)
|
|
assert first_pid is not None, "Could not capture initial dashboard PID"
|
|
|
|
# Kill the dashboard. The dashboard process runs as hermes, so the
|
|
# hermes user can kill it (same UID).
|
|
docker_exec(container_name, "kill", "-9", first_pid)
|
|
|
|
# s6 backs off ~1s before restart; allow up to 15s for the new
|
|
# process to appear with a different PID.
|
|
deadline = time.monotonic() + 15.0
|
|
while time.monotonic() < deadline:
|
|
r = docker_exec(container_name, "pgrep", "-f", "hermes dashboard")
|
|
pids = r.stdout.strip().split() if r.returncode == 0 else []
|
|
if pids and pids[0] != first_pid:
|
|
return # success
|
|
time.sleep(0.5)
|
|
|
|
raise AssertionError(
|
|
f"Dashboard not restarted after kill (first_pid={first_pid})"
|
|
)
|