diff --git a/tests/docker/conftest.py b/tests/docker/conftest.py index 4bd3ab3ff02..50c5269f7d6 100644 --- a/tests/docker/conftest.py +++ b/tests/docker/conftest.py @@ -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`` 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}" + ) diff --git a/tests/docker/test_container_restart.py b/tests/docker/test_container_restart.py index 6cc458a2b30..e56555bb73e 100644 --- a/tests/docker/test_container_restart.py +++ b/tests/docker/test_container_restart.py @@ -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" - - diff --git a/tests/docker/test_container_restart_stale_pid.py b/tests/docker/test_container_restart_stale_pid.py new file mode 100644 index 00000000000..72e15eeda69 --- /dev/null +++ b/tests/docker/test_container_restart_stale_pid.py @@ -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//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" diff --git a/tests/docker/test_dashboard.py b/tests/docker/test_dashboard.py index 7a7b762136d..7286e36ab9e 100644 --- a/tests/docker/test_dashboard.py +++ b/tests/docker/test_dashboard.py @@ -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`` 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." - ) diff --git a/tests/docker/test_dashboard_insecure_env.py b/tests/docker/test_dashboard_insecure_env.py new file mode 100644 index 00000000000..d3505ea1c81 --- /dev/null +++ b/tests/docker/test_dashboard_insecure_env.py @@ -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." + ) diff --git a/tests/docker/test_dashboard_oauth_gate.py b/tests/docker/test_dashboard_oauth_gate.py new file mode 100644 index 00000000000..de73ab441f1 --- /dev/null +++ b/tests/docker/test_dashboard_oauth_gate.py @@ -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}" + ) diff --git a/tests/docker/test_docker_exec_privilege_drop.py b/tests/docker/test_docker_exec_privilege_drop.py index 057abb737a1..1f4e6bbe90d 100644 --- a/tests/docker/test_docker_exec_privilege_drop.py +++ b/tests/docker/test_docker_exec_privilege_drop.py @@ -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 ) 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 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." - ) \ No newline at end of file diff --git a/tests/docker/test_docker_exec_privilege_e2e_login.py b/tests/docker/test_docker_exec_privilege_e2e_login.py new file mode 100644 index 00000000000..d369e708abf --- /dev/null +++ b/tests/docker/test_docker_exec_privilege_e2e_login.py @@ -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 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 hermes …`` (defaulting to root) gets dropped to the + hermes user before the real binary runs. +2. ``docker exec --user hermes 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 …``) 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 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 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." + )