mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
perf(tests): split boot-heavy docker test files for parallel boots
The docker suite's wall time was max(whale files): four files each serialized 2-3 ~110s container boots internally while 21 fast files finished in seconds (P50 9.9s vs max 341s on the ARC runners). The per-file parallel runner can only overlap what lives in separate files. - test_dashboard.py -> 3 files (one boot each); shared _http_probe helper moves to conftest - test_container_restart.py -> 2 files (restart_container fixture travels via the shared header; per-file container isolation is the point of the split) - test_docker_exec_privilege_drop.py -> boot-heavy e2e login test split out; the two fast tests stay together - test_config_migration.py: single test, unchanged 53 tests before and after, zero assertions changed — pure file reorganization. Local (-j4, same cap as CI): 374.6s -> 58.0s wall, slowest file 341.7s -> 18.6s.
This commit is contained in:
parent
f87bc50dc6
commit
e4458ee7d9
8 changed files with 476 additions and 270 deletions
|
|
@ -329,3 +329,72 @@ def wait_for_docker_logs(
|
|||
return last
|
||||
time.sleep(interval_s)
|
||||
raise AssertionError(f"Didn't see `{needle}` in docker logs within {deadline_s} in container {container}")
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _http_probe — in-container HTTP probe shared by the dashboard test files
|
||||
# (split into per-boot files so the parallel runner overlaps container
|
||||
# boots; the shared probe lives here).
|
||||
# ---------------------------------------------------------------------------
|
||||
def _http_probe(
|
||||
container: str,
|
||||
path: str,
|
||||
*,
|
||||
deadline_s: float = 60.0,
|
||||
) -> tuple[int, str]:
|
||||
"""Poll ``http://127.0.0.1:9119<path>`` from inside the container.
|
||||
|
||||
Returns ``(status_code, body)`` as soon as the dashboard answers any
|
||||
HTTP response — 200, 401, 503, anything. The image doesn't ship
|
||||
``curl`` but the venv's stdlib ``urllib`` is good enough; we use a
|
||||
proper ``try``/``except`` to intercept ``HTTPError`` because
|
||||
``urlopen`` raises on 4xx/5xx, and we treat those as legitimate
|
||||
responses (the OAuth gate's 401 IS the success signal for the
|
||||
gate-engaged test).
|
||||
|
||||
Connection errors (uvicorn still starting, fail-closed exited) keep
|
||||
the poll loop running until ``deadline_s`` elapses.
|
||||
|
||||
The probe Python program is fed over stdin (``python -``) rather
|
||||
than ``python -c`` so we can use proper multi-line syntax with
|
||||
``try``/``except`` blocks without escaping hell.
|
||||
|
||||
Raises ``AssertionError`` on timeout.
|
||||
"""
|
||||
py_program = f"""\
|
||||
import urllib.request, urllib.error
|
||||
req = urllib.request.Request("http://127.0.0.1:9119{path}")
|
||||
try:
|
||||
r = urllib.request.urlopen(req, timeout=5)
|
||||
print(r.status)
|
||||
print(r.read().decode(), end="")
|
||||
except urllib.error.HTTPError as h:
|
||||
print(h.code)
|
||||
print(h.read().decode(), end="")
|
||||
"""
|
||||
# Feed the program over stdin via a heredoc so docker_exec_sh's
|
||||
# single bash string stays clean. The 'PY' delimiter is quoted to
|
||||
# disable shell expansion inside the heredoc body.
|
||||
probe = (
|
||||
"/opt/hermes/.venv/bin/python - <<'PY'\n"
|
||||
f"{py_program}"
|
||||
"PY"
|
||||
)
|
||||
end = time.monotonic() + deadline_s
|
||||
last_err = ""
|
||||
while time.monotonic() < end:
|
||||
r = docker_exec_sh(container, probe, timeout=10)
|
||||
if r.returncode == 0 and r.stdout.strip():
|
||||
lines = r.stdout.split("\n", 1)
|
||||
try:
|
||||
status = int(lines[0].strip())
|
||||
body = lines[1] if len(lines) > 1 else ""
|
||||
return status, body
|
||||
except (ValueError, IndexError) as exc:
|
||||
last_err = f"parse: {exc!r} / stdout={r.stdout!r}"
|
||||
else:
|
||||
last_err = f"rc={r.returncode} stderr={r.stderr!r}"
|
||||
time.sleep(0.5)
|
||||
raise AssertionError(
|
||||
f"Probe of {path} never returned HTTP within {deadline_s}s; "
|
||||
f"last error: {last_err}"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -75,8 +75,6 @@ def restart_container(request, built_image: str):
|
|||
_docker("volume", "rm", "-f", volume)
|
||||
|
||||
|
||||
|
||||
|
||||
def test_stopped_gateway_stays_stopped_after_restart(restart_container: str) -> None:
|
||||
container = restart_container
|
||||
|
||||
|
|
@ -102,35 +100,3 @@ def test_stopped_gateway_stays_stopped_after_restart(restart_container: str) ->
|
|||
# Down marker present.
|
||||
r = docker_exec_sh(container, "test -f /run/service/gateway-writer/down")
|
||||
assert r.returncode == 0, "down marker missing despite prior_state=stopped"
|
||||
|
||||
|
||||
def test_stale_gateway_pid_cleaned_up_on_restart(restart_container: str) -> None:
|
||||
"""A dead container's gateway.pid + processes.json must NOT
|
||||
survive the restart — a numerically-equal live PID in the new
|
||||
container is a different process and would confuse the gateway
|
||||
process-mismatch checks."""
|
||||
container = restart_container
|
||||
|
||||
docker_exec(container, "hermes", "profile", "create", "ghost").check_returncode()
|
||||
|
||||
# Stamp stale runtime files alongside a 'running' state so the
|
||||
# reconciler walks this profile.
|
||||
stamp = (
|
||||
"import json, pathlib; "
|
||||
"p = pathlib.Path('/opt/data/profiles/ghost'); "
|
||||
"(p / 'gateway_state.json').write_text(json.dumps({'gateway_state': 'stopped', 'timestamp': 1})); "
|
||||
"(p / 'gateway.pid').write_text(json.dumps({'pid': 99999, 'host': 'old'})); "
|
||||
"(p / 'processes.json').write_text('[]')"
|
||||
)
|
||||
docker_exec(container, "python3", "-c", stamp, timeout=10).check_returncode()
|
||||
|
||||
_docker("restart", container, timeout=60).check_returncode()
|
||||
_wait_for_reconcile_log_mention(container, "ghost", deadline_s=30.0)
|
||||
|
||||
# Stale runtime files swept.
|
||||
r = docker_exec_sh(container, "test -f /opt/data/profiles/ghost/gateway.pid")
|
||||
assert r.returncode != 0, "stale gateway.pid survived restart"
|
||||
r = docker_exec_sh(container, "test -f /opt/data/profiles/ghost/processes.json")
|
||||
assert r.returncode != 0, "stale processes.json survived restart"
|
||||
|
||||
|
||||
|
|
|
|||
110
tests/docker/test_container_restart_stale_pid.py
Normal file
110
tests/docker/test_container_restart_stale_pid.py
Normal file
|
|
@ -0,0 +1,110 @@
|
|||
"""Container-restart survives per-profile gateway registrations.
|
||||
|
||||
Split from test_container_restart.py so the per-file parallel runner
|
||||
overlaps the ~110s container boots instead of serializing them in one
|
||||
file. The restart_container fixture travels with the shared header.
|
||||
|
||||
|
||||
The s6 dynamic scandir at /run/service/ lives on tmpfs and is wiped
|
||||
on every container restart. Phase 4 Task 4.0's container_boot module
|
||||
+ cont-init.d/02-reconcile-profiles regenerate the service slots from
|
||||
$HERMES_HOME/profiles/<name>/gateway_state.json on every boot and
|
||||
auto-start only those whose last state was `running`.
|
||||
|
||||
These tests stand up a container with a named volume, create profiles
|
||||
inside it in various gateway states, restart the container, and
|
||||
assert the reconciler did the right thing.
|
||||
|
||||
Every ``docker exec`` here runs as the unprivileged ``hermes`` user
|
||||
(via :func:`docker_exec` / :func:`docker_exec_sh` in conftest); see
|
||||
the conftest module docstring.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
from tests.docker.conftest import docker_exec, docker_exec_sh, wait_for_path, wait_for_log, wait_for_docker_logs, poll_container
|
||||
|
||||
|
||||
def _docker(*args: str, **kw) -> subprocess.CompletedProcess[str]:
|
||||
return subprocess.run(
|
||||
["docker", *args],
|
||||
capture_output=True, text=True, timeout=kw.pop("timeout", 60),
|
||||
**kw,
|
||||
)
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
def _wait_for_reconcile_log_mention(
|
||||
container: str,
|
||||
profile: str,
|
||||
*,
|
||||
deadline_s: float = 30.0,
|
||||
interval_s: float = 0.25,
|
||||
) -> str:
|
||||
"""Poll until /opt/data/logs/container-boot.log mentions `profile`.
|
||||
"""
|
||||
return wait_for_log(container, "/opt/data/logs/container-boot.log", f"profile={profile}")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def restart_container(request, built_image: str):
|
||||
"""A long-running container with a named volume so docker restart
|
||||
preserves $HERMES_HOME/profiles/."""
|
||||
safe = request.node.name.replace("[", "_").replace("]", "_")
|
||||
name = f"hermes-restart-{safe}"
|
||||
volume = f"hermes-restart-vol-{safe}"
|
||||
_docker("rm", "-f", name)
|
||||
_docker("volume", "rm", "-f", volume)
|
||||
_docker("volume", "create", volume, timeout=10).check_returncode()
|
||||
r = _docker(
|
||||
"run", "-d", "--name", name,
|
||||
"-v", f"{volume}:/opt/data",
|
||||
built_image, "sleep", "infinity",
|
||||
timeout=30,
|
||||
)
|
||||
r.check_returncode()
|
||||
# Wait for s6 + stage2 + 02-reconcile to publish the boot log so
|
||||
# the test can rely on the default slot being registered before
|
||||
# it starts issuing commands. The reconciler always writes one
|
||||
# 'default' line on every boot (PR #30136 item I1) — that's our
|
||||
# readiness signal.
|
||||
wait_for_log(name, "/opt/data/logs/container-boot.log", "profile=default")
|
||||
yield name
|
||||
_docker("rm", "-f", name)
|
||||
_docker("volume", "rm", "-f", volume)
|
||||
|
||||
|
||||
def test_stale_gateway_pid_cleaned_up_on_restart(restart_container: str) -> None:
|
||||
"""A dead container's gateway.pid + processes.json must NOT
|
||||
survive the restart — a numerically-equal live PID in the new
|
||||
container is a different process and would confuse the gateway
|
||||
process-mismatch checks."""
|
||||
container = restart_container
|
||||
|
||||
docker_exec(container, "hermes", "profile", "create", "ghost").check_returncode()
|
||||
|
||||
# Stamp stale runtime files alongside a 'running' state so the
|
||||
# reconciler walks this profile.
|
||||
stamp = (
|
||||
"import json, pathlib; "
|
||||
"p = pathlib.Path('/opt/data/profiles/ghost'); "
|
||||
"(p / 'gateway_state.json').write_text(json.dumps({'gateway_state': 'stopped', 'timestamp': 1})); "
|
||||
"(p / 'gateway.pid').write_text(json.dumps({'pid': 99999, 'host': 'old'})); "
|
||||
"(p / 'processes.json').write_text('[]')"
|
||||
)
|
||||
docker_exec(container, "python3", "-c", stamp, timeout=10).check_returncode()
|
||||
|
||||
_docker("restart", container, timeout=60).check_returncode()
|
||||
_wait_for_reconcile_log_mention(container, "ghost", deadline_s=30.0)
|
||||
|
||||
# Stale runtime files swept.
|
||||
r = docker_exec_sh(container, "test -f /opt/data/profiles/ghost/gateway.pid")
|
||||
assert r.returncode != 0, "stale gateway.pid survived restart"
|
||||
r = docker_exec_sh(container, "test -f /opt/data/profiles/ghost/processes.json")
|
||||
assert r.returncode != 0, "stale processes.json survived restart"
|
||||
|
|
@ -15,7 +15,7 @@ from __future__ import annotations
|
|||
import json
|
||||
import time
|
||||
|
||||
from tests.docker.conftest import docker_exec, docker_exec_sh, start_container, poll_container
|
||||
from tests.docker.conftest import docker_exec, docker_exec_sh, start_container, poll_container, _http_probe
|
||||
|
||||
|
||||
def test_dashboard_not_running_by_default(
|
||||
|
|
@ -48,180 +48,3 @@ def test_dashboard_not_running_by_default(
|
|||
# static-text guard lives in tests/test_docker_home_override_scripts.py;
|
||||
# this is the behavioural end-to-end check.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _http_probe(
|
||||
container: str,
|
||||
path: str,
|
||||
*,
|
||||
deadline_s: float = 60.0,
|
||||
) -> tuple[int, str]:
|
||||
"""Poll ``http://127.0.0.1:9119<path>`` from inside the container.
|
||||
|
||||
Returns ``(status_code, body)`` as soon as the dashboard answers any
|
||||
HTTP response — 200, 401, 503, anything. The image doesn't ship
|
||||
``curl`` but the venv's stdlib ``urllib`` is good enough; we use a
|
||||
proper ``try``/``except`` to intercept ``HTTPError`` because
|
||||
``urlopen`` raises on 4xx/5xx, and we treat those as legitimate
|
||||
responses (the OAuth gate's 401 IS the success signal for the
|
||||
gate-engaged test).
|
||||
|
||||
Connection errors (uvicorn still starting, fail-closed exited) keep
|
||||
the poll loop running until ``deadline_s`` elapses.
|
||||
|
||||
The probe Python program is fed over stdin (``python -``) rather
|
||||
than ``python -c`` so we can use proper multi-line syntax with
|
||||
``try``/``except`` blocks without escaping hell.
|
||||
|
||||
Raises ``AssertionError`` on timeout.
|
||||
"""
|
||||
py_program = f"""\
|
||||
import urllib.request, urllib.error
|
||||
req = urllib.request.Request("http://127.0.0.1:9119{path}")
|
||||
try:
|
||||
r = urllib.request.urlopen(req, timeout=5)
|
||||
print(r.status)
|
||||
print(r.read().decode(), end="")
|
||||
except urllib.error.HTTPError as h:
|
||||
print(h.code)
|
||||
print(h.read().decode(), end="")
|
||||
"""
|
||||
# Feed the program over stdin via a heredoc so docker_exec_sh's
|
||||
# single bash string stays clean. The 'PY' delimiter is quoted to
|
||||
# disable shell expansion inside the heredoc body.
|
||||
probe = (
|
||||
"/opt/hermes/.venv/bin/python - <<'PY'\n"
|
||||
f"{py_program}"
|
||||
"PY"
|
||||
)
|
||||
end = time.monotonic() + deadline_s
|
||||
last_err = ""
|
||||
while time.monotonic() < end:
|
||||
r = docker_exec_sh(container, probe, timeout=10)
|
||||
if r.returncode == 0 and r.stdout.strip():
|
||||
lines = r.stdout.split("\n", 1)
|
||||
try:
|
||||
status = int(lines[0].strip())
|
||||
body = lines[1] if len(lines) > 1 else ""
|
||||
return status, body
|
||||
except (ValueError, IndexError) as exc:
|
||||
last_err = f"parse: {exc!r} / stdout={r.stdout!r}"
|
||||
else:
|
||||
last_err = f"rc={r.returncode} stderr={r.stderr!r}"
|
||||
time.sleep(0.5)
|
||||
raise AssertionError(
|
||||
f"Probe of {path} never returned HTTP within {deadline_s}s; "
|
||||
f"last error: {last_err}"
|
||||
)
|
||||
|
||||
|
||||
def test_dashboard_oauth_gate_engages_on_non_loopback_bind(
|
||||
built_image: str, container_name: str,
|
||||
) -> None:
|
||||
"""The s6 dashboard run script must NOT auto-add ``--insecure`` when the
|
||||
dashboard binds to ``0.0.0.0``. The OAuth auth gate engages on its own
|
||||
when a ``DashboardAuthProvider`` is registered (the bundled nous
|
||||
provider activates whenever ``HERMES_DASHBOARD_OAUTH_CLIENT_ID`` is
|
||||
set).
|
||||
|
||||
Regression guard for the wildcard-subdomain rollout where every
|
||||
portal-provisioned agent binds ``0.0.0.0`` and relies on the OAuth
|
||||
gate to authenticate browser callers. Before this fix, the run script
|
||||
flipped ``--insecure`` on for any non-loopback bind, which routed
|
||||
``start_server`` straight back into the legacy ``allow_public=True``
|
||||
branch and disabled the gate every time.
|
||||
|
||||
We verify two independent observable consequences of the gate being
|
||||
on:
|
||||
|
||||
1. ``/api/auth/providers`` (publicly reachable through the gate so
|
||||
the login page can bootstrap) returns 200 with ``nous`` in the
|
||||
provider list — proves the bundled provider registered.
|
||||
2. ``/api/sessions`` (a gated route under both the legacy
|
||||
``_SESSION_TOKEN`` middleware and the OAuth gate) returns 401
|
||||
to an unauthenticated caller — proves the OAuth gate is actively
|
||||
intercepting browser traffic. We deliberately probe a gated route
|
||||
here rather than ``/api/status``: status sits in the shared
|
||||
``PUBLIC_API_PATHS`` allowlist (portal liveness probe target) and
|
||||
responds 200 without a cookie under both gates, so it cannot
|
||||
distinguish "gate on" from "gate off".
|
||||
"""
|
||||
start_container(
|
||||
built_image, container_name,
|
||||
"HERMES_DASHBOARD=1",
|
||||
"HERMES_DASHBOARD_HOST=0.0.0.0",
|
||||
"HERMES_DASHBOARD_OAUTH_CLIENT_ID=agent:test-instance",
|
||||
cmd="sleep 120",
|
||||
)
|
||||
|
||||
# (1) Provider registry visible via the public bootstrap endpoint.
|
||||
status_code, body = _http_probe(container_name, "/api/auth/providers")
|
||||
assert status_code == 200, (
|
||||
f"/api/auth/providers should return 200 when a provider is "
|
||||
f"registered; got {status_code} body={body!r}"
|
||||
)
|
||||
payload = json.loads(body)
|
||||
provider_names = [p.get("name") for p in payload.get("providers", [])]
|
||||
assert "nous" in provider_names, (
|
||||
"Bundled dashboard_auth/nous provider should register when "
|
||||
f"HERMES_DASHBOARD_OAUTH_CLIENT_ID is set. Got: {payload!r}"
|
||||
)
|
||||
|
||||
# (2) A gated route (``/api/sessions``) returns 401 to an
|
||||
# unauthenticated caller — the OAuth gate is intercepting.
|
||||
status_code, body = _http_probe(container_name, "/api/sessions")
|
||||
assert status_code == 401, (
|
||||
"OAuth gate must intercept gated /api/* routes on 0.0.0.0 bind "
|
||||
"when a provider is registered and HERMES_DASHBOARD_INSECURE "
|
||||
f"is unset. Got: status={status_code} body={body!r}"
|
||||
)
|
||||
|
||||
# (3) ``/api/status`` remains 200 under the gate — it's in the shared
|
||||
# ``PUBLIC_API_PATHS`` allowlist so NAS's wildcard-subdomain
|
||||
# liveness probe (``fly-provider.ts`` ``getInstanceRuntimeStatus``)
|
||||
# can reach it without a cookie. Regression guard: this allowlist
|
||||
# drifted once already and surfaced every healthy agent as
|
||||
# STARTING/down in the portal UI.
|
||||
status_code, body = _http_probe(container_name, "/api/status")
|
||||
assert status_code == 200, (
|
||||
"/api/status must remain publicly reachable under the OAuth gate "
|
||||
"— the portal uses it as the wildcard-subdomain liveness probe. "
|
||||
f"Got: status={status_code} body={body!r}"
|
||||
)
|
||||
status = json.loads(body)
|
||||
assert status.get("auth_required") is True, (
|
||||
"/api/status must report auth_required=True when the OAuth gate "
|
||||
f"is engaged so the SPA/portal can distinguish modes. Got: {status!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_dashboard_insecure_env_var_no_longer_bypasses_gate(
|
||||
built_image: str, container_name: str,
|
||||
) -> None:
|
||||
"""``HERMES_DASHBOARD_INSECURE=1`` NO LONGER disables the auth gate
|
||||
(June 2026 hardening). With insecure set on a 0.0.0.0 bind and NO auth
|
||||
provider registered, start_server fails closed — the dashboard never
|
||||
binds, so ``/api/status`` is unreachable. This proves the unauthenticated
|
||||
public-dashboard escape hatch is gone: there is no env that serves the
|
||||
dashboard on a public bind without an auth provider.
|
||||
"""
|
||||
start_container(
|
||||
built_image, container_name,
|
||||
"HERMES_DASHBOARD=1",
|
||||
"HERMES_DASHBOARD_HOST=0.0.0.0",
|
||||
"HERMES_DASHBOARD_INSECURE=1",
|
||||
cmd="sleep 120",
|
||||
)
|
||||
# Fail-closed: the dashboard process must NOT successfully serve. Probe
|
||||
# for a few seconds; /api/status should never become reachable because
|
||||
# start_server raised SystemExit before binding.
|
||||
ok, _ = poll_container(
|
||||
container_name,
|
||||
"curl -fsS -m 2 http://127.0.0.1:9119/api/status >/dev/null 2>&1",
|
||||
deadline_s=12.0,
|
||||
)
|
||||
assert not ok, (
|
||||
"Dashboard must NOT serve on a public bind with --insecure and no "
|
||||
"auth provider — the gate fails closed. /api/status became reachable, "
|
||||
"meaning the unauthenticated escape hatch is still open."
|
||||
)
|
||||
|
|
|
|||
44
tests/docker/test_dashboard_insecure_env.py
Normal file
44
tests/docker/test_dashboard_insecure_env.py
Normal file
|
|
@ -0,0 +1,44 @@
|
|||
"""Split from test_dashboard.py: each boot-heavy test lives in its own
|
||||
file so the per-file parallel runner (scripts/run_tests_parallel.py)
|
||||
can overlap container boots across workers instead of serializing
|
||||
~110s boots inside one file. Shared docstring/context: see the
|
||||
original header in test_dashboard.py.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import time
|
||||
|
||||
from tests.docker.conftest import docker_exec, docker_exec_sh, start_container, poll_container, _http_probe
|
||||
|
||||
|
||||
def test_dashboard_insecure_env_var_no_longer_bypasses_gate(
|
||||
built_image: str, container_name: str,
|
||||
) -> None:
|
||||
"""``HERMES_DASHBOARD_INSECURE=1`` NO LONGER disables the auth gate
|
||||
(June 2026 hardening). With insecure set on a 0.0.0.0 bind and NO auth
|
||||
provider registered, start_server fails closed — the dashboard never
|
||||
binds, so ``/api/status`` is unreachable. This proves the unauthenticated
|
||||
public-dashboard escape hatch is gone: there is no env that serves the
|
||||
dashboard on a public bind without an auth provider.
|
||||
"""
|
||||
start_container(
|
||||
built_image, container_name,
|
||||
"HERMES_DASHBOARD=1",
|
||||
"HERMES_DASHBOARD_HOST=0.0.0.0",
|
||||
"HERMES_DASHBOARD_INSECURE=1",
|
||||
cmd="sleep 120",
|
||||
)
|
||||
# Fail-closed: the dashboard process must NOT successfully serve. Probe
|
||||
# for a few seconds; /api/status should never become reachable because
|
||||
# start_server raised SystemExit before binding.
|
||||
ok, _ = poll_container(
|
||||
container_name,
|
||||
"curl -fsS -m 2 http://127.0.0.1:9119/api/status >/dev/null 2>&1",
|
||||
deadline_s=12.0,
|
||||
)
|
||||
assert not ok, (
|
||||
"Dashboard must NOT serve on a public bind with --insecure and no "
|
||||
"auth provider — the gate fails closed. /api/status became reachable, "
|
||||
"meaning the unauthenticated escape hatch is still open."
|
||||
)
|
||||
92
tests/docker/test_dashboard_oauth_gate.py
Normal file
92
tests/docker/test_dashboard_oauth_gate.py
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
"""Split from test_dashboard.py: each boot-heavy test lives in its own
|
||||
file so the per-file parallel runner (scripts/run_tests_parallel.py)
|
||||
can overlap container boots across workers instead of serializing
|
||||
~110s boots inside one file. Shared docstring/context: see the
|
||||
original header in test_dashboard.py.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import time
|
||||
|
||||
from tests.docker.conftest import docker_exec, docker_exec_sh, start_container, poll_container, _http_probe
|
||||
|
||||
|
||||
def test_dashboard_oauth_gate_engages_on_non_loopback_bind(
|
||||
built_image: str, container_name: str,
|
||||
) -> None:
|
||||
"""The s6 dashboard run script must NOT auto-add ``--insecure`` when the
|
||||
dashboard binds to ``0.0.0.0``. The OAuth auth gate engages on its own
|
||||
when a ``DashboardAuthProvider`` is registered (the bundled nous
|
||||
provider activates whenever ``HERMES_DASHBOARD_OAUTH_CLIENT_ID`` is
|
||||
set).
|
||||
|
||||
Regression guard for the wildcard-subdomain rollout where every
|
||||
portal-provisioned agent binds ``0.0.0.0`` and relies on the OAuth
|
||||
gate to authenticate browser callers. Before this fix, the run script
|
||||
flipped ``--insecure`` on for any non-loopback bind, which routed
|
||||
``start_server`` straight back into the legacy ``allow_public=True``
|
||||
branch and disabled the gate every time.
|
||||
|
||||
We verify two independent observable consequences of the gate being
|
||||
on:
|
||||
|
||||
1. ``/api/auth/providers`` (publicly reachable through the gate so
|
||||
the login page can bootstrap) returns 200 with ``nous`` in the
|
||||
provider list — proves the bundled provider registered.
|
||||
2. ``/api/sessions`` (a gated route under both the legacy
|
||||
``_SESSION_TOKEN`` middleware and the OAuth gate) returns 401
|
||||
to an unauthenticated caller — proves the OAuth gate is actively
|
||||
intercepting browser traffic. We deliberately probe a gated route
|
||||
here rather than ``/api/status``: status sits in the shared
|
||||
``PUBLIC_API_PATHS`` allowlist (portal liveness probe target) and
|
||||
responds 200 without a cookie under both gates, so it cannot
|
||||
distinguish "gate on" from "gate off".
|
||||
"""
|
||||
start_container(
|
||||
built_image, container_name,
|
||||
"HERMES_DASHBOARD=1",
|
||||
"HERMES_DASHBOARD_HOST=0.0.0.0",
|
||||
"HERMES_DASHBOARD_OAUTH_CLIENT_ID=agent:test-instance",
|
||||
cmd="sleep 120",
|
||||
)
|
||||
|
||||
# (1) Provider registry visible via the public bootstrap endpoint.
|
||||
status_code, body = _http_probe(container_name, "/api/auth/providers")
|
||||
assert status_code == 200, (
|
||||
f"/api/auth/providers should return 200 when a provider is "
|
||||
f"registered; got {status_code} body={body!r}"
|
||||
)
|
||||
payload = json.loads(body)
|
||||
provider_names = [p.get("name") for p in payload.get("providers", [])]
|
||||
assert "nous" in provider_names, (
|
||||
"Bundled dashboard_auth/nous provider should register when "
|
||||
f"HERMES_DASHBOARD_OAUTH_CLIENT_ID is set. Got: {payload!r}"
|
||||
)
|
||||
|
||||
# (2) A gated route (``/api/sessions``) returns 401 to an
|
||||
# unauthenticated caller — the OAuth gate is intercepting.
|
||||
status_code, body = _http_probe(container_name, "/api/sessions")
|
||||
assert status_code == 401, (
|
||||
"OAuth gate must intercept gated /api/* routes on 0.0.0.0 bind "
|
||||
"when a provider is registered and HERMES_DASHBOARD_INSECURE "
|
||||
f"is unset. Got: status={status_code} body={body!r}"
|
||||
)
|
||||
|
||||
# (3) ``/api/status`` remains 200 under the gate — it's in the shared
|
||||
# ``PUBLIC_API_PATHS`` allowlist so NAS's wildcard-subdomain
|
||||
# liveness probe (``fly-provider.ts`` ``getInstanceRuntimeStatus``)
|
||||
# can reach it without a cookie. Regression guard: this allowlist
|
||||
# drifted once already and surfaced every healthy agent as
|
||||
# STARTING/down in the portal UI.
|
||||
status_code, body = _http_probe(container_name, "/api/status")
|
||||
assert status_code == 200, (
|
||||
"/api/status must remain publicly reachable under the OAuth gate "
|
||||
"— the portal uses it as the wildcard-subdomain liveness probe. "
|
||||
f"Got: status={status_code} body={body!r}"
|
||||
)
|
||||
status = json.loads(body)
|
||||
assert status.get("auth_required") is True, (
|
||||
"/api/status must report auth_required=True when the OAuth gate "
|
||||
f"is engaged so the SPA/portal can distinguish modes. Got: {status!r}"
|
||||
)
|
||||
|
|
@ -146,12 +146,6 @@ def test_shim_drops_root_to_hermes_uid(sleep_container: str) -> None:
|
|||
)
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
def test_main_cmd_path_unaffected(built_image: str) -> None:
|
||||
"""The CMD path (docker run <image> <args>) must still work.
|
||||
|
||||
|
|
@ -172,55 +166,3 @@ def test_main_cmd_path_unaffected(built_image: str) -> None:
|
|||
)
|
||||
assert r.returncode == 0, f"CMD path broken by shim: stderr={r.stderr!r}"
|
||||
assert "Traceback" not in r.stderr
|
||||
|
||||
|
||||
def test_e2e_login_then_supervised_gateway_can_read_auth(
|
||||
sleep_container: str,
|
||||
) -> None:
|
||||
"""End-to-end regression for the original bug.
|
||||
|
||||
Pre-shim: ``docker exec <c> hermes login`` (root) wrote
|
||||
/opt/data/auth.json as root:root 0600. The supervised gateway (UID
|
||||
10000) couldn't read it, _load_auth_store swallowed PermissionError
|
||||
as a parse failure, and resolve_nous_runtime_credentials raised
|
||||
"Hermes is not logged into Nous Portal" on every message.
|
||||
|
||||
We can't do a real OAuth login in a unit test, but we can stand in
|
||||
for it by writing the same file shape via `hermes config set`-style
|
||||
writes — what matters is the *file ownership invariant* downstream
|
||||
of `_save_auth_store`. If the shim works, every file the
|
||||
`docker exec` path produces is hermes-readable.
|
||||
|
||||
Specifically: pretend the operator ran `hermes login` (writes
|
||||
auth.json) and verify (a) the file exists and (b) it's readable by
|
||||
the hermes UID. We use `hermes auth list` since that touches the
|
||||
auth store on the read side and would fail with the same
|
||||
'not logged in' shape if the file was unreadable to uid 10000.
|
||||
"""
|
||||
# Have the shim-protected `docker exec` write the auth store.
|
||||
# `hermes auth list` is read-only but still exercises _load_auth_store
|
||||
# under the shim's UID. We invoke `hermes config set` first to
|
||||
# provoke a write into HERMES_HOME so we have something concrete to
|
||||
# owner-check.
|
||||
r = subprocess.run(
|
||||
["docker", "exec", sleep_container,
|
||||
"hermes", "config", "set", "_test.e2e_marker", "1"],
|
||||
capture_output=True, text=True, timeout=30,
|
||||
)
|
||||
assert r.returncode == 0, f"config set failed: {r.stderr}"
|
||||
|
||||
# The supervised UID (10000) must be able to read everything under
|
||||
# HERMES_HOME that docker exec just wrote.
|
||||
r = subprocess.run(
|
||||
["docker", "exec", "--user", "hermes", sleep_container,
|
||||
"find", "/opt/data", "-maxdepth", "2", "-type", "f",
|
||||
"!", "-readable", "-print"],
|
||||
capture_output=True, text=True, timeout=15,
|
||||
)
|
||||
assert r.returncode == 0, f"find failed: {r.stderr}"
|
||||
unreadable = [ln for ln in r.stdout.splitlines() if ln.strip()]
|
||||
assert not unreadable, (
|
||||
"Files written by `docker exec` are unreadable to the hermes user "
|
||||
f"(supervised gateway UID): {unreadable}. The shim failed to drop "
|
||||
"privileges before the write."
|
||||
)
|
||||
160
tests/docker/test_docker_exec_privilege_e2e_login.py
Normal file
160
tests/docker/test_docker_exec_privilege_e2e_login.py
Normal file
|
|
@ -0,0 +1,160 @@
|
|||
"""Regression tests for the docker-exec privilege-drop shim.
|
||||
|
||||
Split from test_docker_exec_privilege_drop.py so its boot-heavy e2e
|
||||
test parallelizes against the other whales instead of serializing
|
||||
inside one file. Shared helpers/fixture travel via the common header.
|
||||
|
||||
|
||||
The shim (docker/hermes-exec-shim.sh, installed at /opt/hermes/bin/hermes)
|
||||
exists to prevent the auth.json ownership-mismatch bug where
|
||||
`docker exec <c> hermes login` would write /opt/data/auth.json as
|
||||
root:root mode 0600, leaving the supervised gateway (UID 10000) unable
|
||||
to read its own credentials and returning "Provider authentication
|
||||
failed: Hermes is not logged into Nous Portal" on every message.
|
||||
|
||||
These tests verify:
|
||||
|
||||
1. ``docker exec <c> hermes …`` (defaulting to root) gets dropped to the
|
||||
hermes user before the real binary runs.
|
||||
2. ``docker exec --user hermes <c> hermes …`` (already non-root) short-
|
||||
circuits and doesn't try to drop again.
|
||||
3. Files written under $HERMES_HOME from a ``docker exec`` session land
|
||||
as hermes:hermes — the actual user-visible invariant.
|
||||
4. The HERMES_DOCKER_EXEC_AS_ROOT opt-out lets diagnostic sessions keep
|
||||
running as root deliberately.
|
||||
5. The main CMD path (``docker run <image> …``) is unaffected by the
|
||||
PATH-shim ordering — no recursion, no behavior change.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
from tests.docker.conftest import docker_exec
|
||||
|
||||
import subprocess
|
||||
import time
|
||||
from collections.abc import Iterator
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# How long to give a `docker run -d` container before declaring it not ready.
|
||||
# Generous because under arm64 QEMU emulation cont-init (a Python config
|
||||
# migration + chowns) runs several times slower than on native amd64.
|
||||
_RUN_READY_TIMEOUT_S = 60
|
||||
|
||||
|
||||
def _wait_for_cont_init(container: str) -> None:
|
||||
"""Block until s6 cont-init has fully finished, not merely until
|
||||
``docker exec`` is responsive.
|
||||
|
||||
The earlier ``_wait_for_init`` only polled ``docker exec <c> true``,
|
||||
which succeeds almost immediately on s6-overlay — long before the
|
||||
``01-hermes-setup`` cont-init hook (docker/stage2-hook.sh) has
|
||||
finished seeding + ``chown hermes:hermes`` config.yaml and running the
|
||||
Python config migration. A test that wipes config.yaml and then writes
|
||||
it as root would then race that boot-time chown: on native amd64
|
||||
stage2-hook wins in a blink and the test always passed, but under arm64
|
||||
QEMU emulation the slow Python migration was still in flight and
|
||||
clobbered the root-written file's ownership back to hermes:hermes,
|
||||
failing ``test_shim_opt_out_keeps_root`` non-deterministically.
|
||||
|
||||
The reliable "cont-init is done" signal is
|
||||
``$HERMES_HOME/logs/container-boot.log``: it is written by
|
||||
``02-reconcile-profiles`` (hermes_cli.container_boot), which s6 runs
|
||||
*strictly after* ``01-hermes-setup`` in lexicographic order. The
|
||||
reconciler always logs at least one ``profile=default`` line even for a
|
||||
bare ``sleep infinity`` container, so once that marker appears every
|
||||
stage2-hook side effect (seed, chown, migrate) is guaranteed complete.
|
||||
Mirrors the readiness pattern in test_container_restart.py.
|
||||
"""
|
||||
deadline = time.monotonic() + _RUN_READY_TIMEOUT_S
|
||||
last = ""
|
||||
while time.monotonic() < deadline:
|
||||
r = subprocess.run(
|
||||
["docker", "exec", container,
|
||||
"cat", "/opt/data/logs/container-boot.log"],
|
||||
capture_output=True, text=True, timeout=5,
|
||||
)
|
||||
if r.returncode == 0:
|
||||
last = r.stdout
|
||||
if "profile=default" in last:
|
||||
return
|
||||
time.sleep(0.2)
|
||||
pytest.fail(
|
||||
f"container {container} did not finish cont-init within "
|
||||
f"{_RUN_READY_TIMEOUT_S}s (container-boot.log so far: {last!r})"
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def sleep_container(built_image: str, container_name: str) -> Iterator[str]:
|
||||
"""Long-lived container running `sleep infinity` so we can docker exec into it."""
|
||||
subprocess.run(
|
||||
["docker", "rm", "-f", container_name],
|
||||
capture_output=True, check=False,
|
||||
)
|
||||
r = subprocess.run(
|
||||
["docker", "run", "-d", "--name", container_name, built_image,
|
||||
"sleep", "infinity"],
|
||||
capture_output=True, text=True, timeout=30,
|
||||
)
|
||||
assert r.returncode == 0, f"docker run failed: {r.stderr}"
|
||||
try:
|
||||
_wait_for_cont_init(container_name)
|
||||
yield container_name
|
||||
finally:
|
||||
subprocess.run(
|
||||
["docker", "rm", "-f", container_name],
|
||||
capture_output=True, check=False,
|
||||
)
|
||||
|
||||
|
||||
def test_e2e_login_then_supervised_gateway_can_read_auth(
|
||||
sleep_container: str,
|
||||
) -> None:
|
||||
"""End-to-end regression for the original bug.
|
||||
|
||||
Pre-shim: ``docker exec <c> hermes login`` (root) wrote
|
||||
/opt/data/auth.json as root:root 0600. The supervised gateway (UID
|
||||
10000) couldn't read it, _load_auth_store swallowed PermissionError
|
||||
as a parse failure, and resolve_nous_runtime_credentials raised
|
||||
"Hermes is not logged into Nous Portal" on every message.
|
||||
|
||||
We can't do a real OAuth login in a unit test, but we can stand in
|
||||
for it by writing the same file shape via `hermes config set`-style
|
||||
writes — what matters is the *file ownership invariant* downstream
|
||||
of `_save_auth_store`. If the shim works, every file the
|
||||
`docker exec` path produces is hermes-readable.
|
||||
|
||||
Specifically: pretend the operator ran `hermes login` (writes
|
||||
auth.json) and verify (a) the file exists and (b) it's readable by
|
||||
the hermes UID. We use `hermes auth list` since that touches the
|
||||
auth store on the read side and would fail with the same
|
||||
'not logged in' shape if the file was unreadable to uid 10000.
|
||||
"""
|
||||
# Have the shim-protected `docker exec` write the auth store.
|
||||
# `hermes auth list` is read-only but still exercises _load_auth_store
|
||||
# under the shim's UID. We invoke `hermes config set` first to
|
||||
# provoke a write into HERMES_HOME so we have something concrete to
|
||||
# owner-check.
|
||||
r = subprocess.run(
|
||||
["docker", "exec", sleep_container,
|
||||
"hermes", "config", "set", "_test.e2e_marker", "1"],
|
||||
capture_output=True, text=True, timeout=30,
|
||||
)
|
||||
assert r.returncode == 0, f"config set failed: {r.stderr}"
|
||||
|
||||
# The supervised UID (10000) must be able to read everything under
|
||||
# HERMES_HOME that docker exec just wrote.
|
||||
r = subprocess.run(
|
||||
["docker", "exec", "--user", "hermes", sleep_container,
|
||||
"find", "/opt/data", "-maxdepth", "2", "-type", "f",
|
||||
"!", "-readable", "-print"],
|
||||
capture_output=True, text=True, timeout=15,
|
||||
)
|
||||
assert r.returncode == 0, f"find failed: {r.stderr}"
|
||||
unreadable = [ln for ln in r.stdout.splitlines() if ln.strip()]
|
||||
assert not unreadable, (
|
||||
"Files written by `docker exec` are unreadable to the hermes user "
|
||||
f"(supervised gateway UID): {unreadable}. The shim failed to drop "
|
||||
"privileges before the write."
|
||||
)
|
||||
Loading…
Add table
Add a link
Reference in a new issue