hermes-agent/tests/hermes_cli/test_dashboard_auth_prefix.py
Teknium 39975613b1
test: prune wave 2 + speed fixes — 28,106 → 19,757 test functions, suite wall 315s → 294s
Second, deeper pass over tools/gateway/hermes_cli plus first pass over
the trees wave 1 missed (acp, acp_adapter, skills, computer_use, docker,
dashboard, conformance, monitoring, secret_sources, hermes_state,
providers). Same rubric as wave 1 (AGENTS.md test policy); security,
alternation/caching invariants, issue-number regressions, and E2E kept.

Real test-quality fixes found and rooted out along the way:
- tests/tools/test_command_guards.py made real auxiliary-LLM HTTPS calls
  (DEFAULT_CONFIG smart-approval leaked in) — pinned approval
  mode=manual via autouse fixture: 17.4s → 0.4s.
- test_model_switch_custom_providers.py / test_user_providers_model_switch.py
  silently probed live provider catalogs (~2s/test) — stubbed
  cached_provider_model_ids/provider_model_ids/fetch_api_models.
- test_telegram_noise_filter.py: 15-platform copy-paste matrix over
  shared gateway.run logic → 3 representative platforms (55s → 3.9s).
- test_gateway_shutdown.py: stop()'s 5s interrupt-deadline loop spun on
  MagicMock agents — interrupt.side_effect now clears _running_agents
  (22s → 1.0s).
- test_gateway_inactivity_timeout.py poll-harness timings shrunk 3-5x
  (24s → 1.1s); test_mcp_stability.py backoff/SIGTERM-grace sleeps
  patched (15.4s → 2.5s); test_async_delegation.py negative-drain wait
  5s → 0.5s.
- test_telegram_init_deadline.py: loop-block margin restored to 1.0s
  with rationale comment — the watchdog-dump assertion needs the loop
  blocked well past deadline+grace under parallel load (flaked once in
  the 40-worker verification run at a 0.2s margin).

Verification: full hermetic suite via scripts/run_tests.sh —
2,438 files, 21,718 tests passed, 0 failed, 293.9s wall.
Suite totals vs original baseline: 46,820 → 19,757 test functions
(−57.8%), wall 583.5s → 293.9s (−50%), subprocess CPU 13,564s → 11,623s.
2026-07-29 13:39:40 -07:00

527 lines
21 KiB
Python

"""Path-prefix (X-Forwarded-Prefix) awareness for the dashboard-auth gate.
Mission-control style deployments reverse-proxy the dashboard at a path
prefix (e.g. ``mission-control.tilos.com/hermes/*`` -> local Caddy ->
:9119), injecting ``X-Forwarded-Prefix: /hermes`` on every request.
The dashboard already honours this for the SPA bundle (rewriting asset
URLs and the bootstrap ``__HERMES_BASE_PATH__``). The OAuth gate must
honour it too:
1. The gate's ``Location:`` redirect to /login (in
``_unauth_response``) needs to be ``/hermes/login`` so the browser
follows it through the proxy.
2. The 401 JSON envelope's ``login_url`` needs the same prefix so the
SPA's full-page navigation lands at the proxied login page.
3. ``_redirect_uri`` (the OAuth callback URL handed to the IDP) must
reconstruct the public URL including the prefix, otherwise the IDP
redirects back to ``/auth/callback`` instead of
``/hermes/auth/callback`` and the user gets 404.
4. Cookies must use ``Path=/hermes`` when behind a prefix so they
don't leak to other apps on the same origin AND so they get sent
back to the dashboard on subsequent requests under the prefix.
5. The ``__Host-`` cookie prefix requires ``Path=/`` — when behind an
X-Forwarded-Prefix we use ``__Secure-`` instead (matches every
hardening property except scope, which the explicit ``Path``
covers).
These tests document the wire-level contract so a regression in any of
those rules surfaces before a Mission Control deploy.
"""
from __future__ import annotations
import logging
import pytest
from fastapi.testclient import TestClient
from hermes_cli import web_server
from hermes_cli.dashboard_auth import clear_providers, register_provider
from hermes_cli.dashboard_auth import prefix as prefix_mod
from tests.hermes_cli.conftest_dashboard_auth import StubAuthProvider
HA_INGRESS_DASHBOARD_PREFIX = (
"/api/hassio_ingress/8AbCdEfGhIjKlMnOpQrStUvWxYz0123456789AbCdEf"
"/dashboard"
)
@pytest.fixture
def gated_app_proxied():
"""web_server.app configured for gated mode with proxy_headers + a
public Host that simulates the Mission Control reverse proxy.
The ``base_url`` sets ``host:scheme`` defaults so we don't have to
pass them on every request. ``X-Forwarded-Prefix`` is passed
per-request because the TestClient doesn't have a way to default
request headers.
"""
clear_providers()
register_provider(StubAuthProvider())
prev_host = getattr(web_server.app.state, "bound_host", None)
prev_port = getattr(web_server.app.state, "bound_port", None)
prev_required = getattr(web_server.app.state, "auth_required", None)
web_server.app.state.bound_host = "mission-control.tilos.com"
web_server.app.state.bound_port = 443
web_server.app.state.auth_required = True
client = TestClient(
web_server.app,
base_url="https://mission-control.tilos.com",
)
yield client
clear_providers()
web_server.app.state.bound_host = prev_host
web_server.app.state.bound_port = prev_port
web_server.app.state.auth_required = prev_required
@pytest.fixture
def gated_app_direct():
"""web_server.app configured for gated mode WITHOUT a proxy prefix,
for the Fly-direct deploy shape (no path mounting).
"""
clear_providers()
register_provider(StubAuthProvider())
prev_host = getattr(web_server.app.state, "bound_host", None)
prev_port = getattr(web_server.app.state, "bound_port", None)
prev_required = getattr(web_server.app.state, "auth_required", None)
web_server.app.state.bound_host = "fly-app.fly.dev"
web_server.app.state.bound_port = 443
web_server.app.state.auth_required = True
client = TestClient(
web_server.app,
base_url="https://fly-app.fly.dev",
)
yield client
clear_providers()
web_server.app.state.bound_host = prev_host
web_server.app.state.bound_port = prev_port
web_server.app.state.auth_required = prev_required
# ---------------------------------------------------------------------------
# X-Forwarded-Prefix normalisation
# ---------------------------------------------------------------------------
class TestForwardedPrefixNormalisation:
def test_home_assistant_ingress_prefix_with_subpath_is_accepted(
self, caplog
):
"""Home Assistant Supervisor ingress prefixes are 63 chars before
add-ons append their own mount path. They must survive validation so
the SPA receives the correct __HERMES_BASE_PATH__ and asset prefix."""
prefix_mod._warned_malformed_prefixes.clear()
assert len(HA_INGRESS_DASHBOARD_PREFIX) > 64
with caplog.at_level(logging.WARNING, logger=prefix_mod.__name__):
result = prefix_mod.normalise_prefix(HA_INGRESS_DASHBOARD_PREFIX)
assert result == HA_INGRESS_DASHBOARD_PREFIX
assert not [
r for r in caplog.records
if r.levelno == logging.WARNING
and "X-Forwarded-Prefix" in r.getMessage()
]
def test_overlong_prefix_is_rejected_with_deduplicated_warning(
self, caplog
):
"""Keep a bounded header budget, but make rejected non-empty
prefixes diagnosable instead of silently producing root-relative
dashboard URLs."""
prefix_mod._warned_malformed_prefixes.clear()
too_long = "/" + ("a" * 257)
with caplog.at_level(logging.WARNING, logger=prefix_mod.__name__):
for _ in range(3):
assert prefix_mod.normalise_prefix(too_long) == ""
warnings = [
r for r in caplog.records
if r.levelno == logging.WARNING
and "X-Forwarded-Prefix" in r.getMessage()
]
assert len(warnings) == 1
assert "longer than 256 characters" in warnings[0].getMessage()
# ---------------------------------------------------------------------------
# Gate middleware: Location: header and 401 envelope respect prefix
# ---------------------------------------------------------------------------
class TestGateRedirectsCarryPrefix:
def test_html_redirect_to_login_carries_prefix(self, gated_app_proxied):
r = gated_app_proxied.get(
"/sessions",
headers={"x-forwarded-prefix": "/hermes"},
follow_redirects=False,
)
assert r.status_code == 302
# Phase 1 (cloud-auto-discovery): a single-provider unauth HTML load
# auto-initiates the OAuth redirect to /auth/login. That redirect must
# ALSO carry the prefix, or the browser follows it to
# mission-control.tilos.com/auth/login (which the proxy doesn't route
# to the dashboard). The prefix-carrying invariant is what's under
# test; only the target path moved from /login to /auth/login.
assert r.headers["location"].startswith("/hermes/auth/login"), (
f"Location header lost prefix: {r.headers['location']!r}"
)
def test_api_401_envelope_login_url_carries_prefix(self, gated_app_proxied):
r = gated_app_proxied.get(
"/api/sessions",
headers={"x-forwarded-prefix": "/hermes"},
follow_redirects=False,
)
assert r.status_code == 401
body = r.json()
# SPA does window.location.assign(body.login_url); this MUST
# include the prefix.
assert body["login_url"].startswith("/hermes/login"), (
f"401 envelope login_url lost prefix: {body['login_url']!r}"
)
def test_malformed_prefix_header_is_ignored(self, gated_app_proxied):
"""A hostile proxy injects ``X-Forwarded-Prefix: <script>``;
the normaliser rejects it and the gate falls back to unprefixed
URLs. Defence against header-injection HTML inside Location."""
r = gated_app_proxied.get(
"/sessions",
headers={"x-forwarded-prefix": "<script>alert(1)</script>"},
follow_redirects=False,
)
assert r.status_code == 302
assert "<script>" not in r.headers["location"]
# Phase 1: malformed prefix dropped → unprefixed auto-SSO redirect.
assert r.headers["location"].startswith("/auth/login")
# ---------------------------------------------------------------------------
# /auth/login: the OAuth redirect_uri reflects the proxy prefix
# ---------------------------------------------------------------------------
class TestOAuthRedirectUriRespectsPrefix:
def test_redirect_uri_includes_prefix_in_authorize_url(
self, gated_app_proxied
):
"""The IDP returns the user to the redirect_uri we sent. If we
don't include the prefix, the IDP redirects to
``https://mission-control.tilos.com/auth/callback`` instead of
``https://mission-control.tilos.com/hermes/auth/callback`` — the
former routes to the MC frontend, not the dashboard, so the
user gets 404."""
r = gated_app_proxied.get(
"/auth/login?provider=stub",
headers={"x-forwarded-prefix": "/hermes"},
follow_redirects=False,
)
assert r.status_code == 302
location = r.headers["location"]
# The stub IDP's redirect_url echoes the redirect_uri back. The
# real IDP would consume it and later use it to redirect the
# user, so the byte-exact value MUST include the prefix.
from urllib.parse import urlparse
# Stub returns ``{redirect_uri}?code=stub_code&state=...`` — so
# we read up to the first ``?``.
redirect_uri = location.split("?", 1)[0]
# Absolute https URL including prefix.
parsed = urlparse(redirect_uri)
assert parsed.scheme == "https"
assert parsed.netloc == "mission-control.tilos.com"
assert parsed.path == "/hermes/auth/callback", (
f"redirect_uri dropped prefix: {redirect_uri!r}"
)
def test_redirect_uri_no_prefix_when_direct_deploy(
self, gated_app_direct
):
r = gated_app_direct.get(
"/auth/login?provider=stub", follow_redirects=False
)
assert r.status_code == 302
redirect_uri = r.headers["location"].split("?", 1)[0]
from urllib.parse import urlparse
parsed = urlparse(redirect_uri)
assert parsed.netloc == "fly-app.fly.dev"
assert parsed.path == "/auth/callback"
# ---------------------------------------------------------------------------
# HERMES_DASHBOARD_PUBLIC_URL / dashboard.public_url override
# ---------------------------------------------------------------------------
class TestPublicUrlOverride:
"""``dashboard.public_url`` (env override:
``HERMES_DASHBOARD_PUBLIC_URL``) lets an operator force the absolute
base URL the OAuth ``redirect_uri`` is built from.
When set, it is the *complete authority* — scheme + host + optional
path prefix. ``X-Forwarded-Prefix`` is ignored on that code path
because the operator has explicitly declared the public URL and we
no longer need to guess from proxy headers. This is the relief
valve for deploys behind reverse proxies that don't set
``X-Forwarded-Host`` / ``X-Forwarded-Proto`` / ``X-Forwarded-Prefix``
correctly (or at all) — manual nginx setups, on-prem ingresses,
Fly.io deploys with custom domains where the proxy header chain is
incomplete.
When unset, the existing ``proxy_headers=True`` + X-Forwarded-Prefix
reconstruction path runs untouched. Existing Fly.io deploys
continue to work without configuration.
Precedence (mirrors ``client_id``):
env (non-empty) > config.yaml > reconstructed from request
"""
@pytest.fixture
def patch_config(self, monkeypatch):
"""Replace ``hermes_cli.config.load_config`` with a stub
returning the given ``public_url``. Pass ``None`` to set no
config-side value."""
def _set(public_url) -> None:
cfg = {}
if public_url is not None:
cfg = {"dashboard": {"public_url": public_url}}
monkeypatch.setattr(
"hermes_cli.config.load_config", lambda: cfg
)
return _set
def _redirect_uri(self, gated_app, *, headers=None) -> str:
"""Drive /auth/login and read the redirect_uri the IDP saw."""
r = gated_app.get(
"/auth/login?provider=stub",
headers=headers or {},
follow_redirects=False,
)
assert r.status_code == 302, r.text
# Stub IDP echoes redirect_uri back as the prefix of the
# Location header (`{redirect_uri}?code=stub_code&state=…`).
return r.headers["location"].split("?", 1)[0]
def test_env_overrides_config_public_url(
self, gated_app_direct, patch_config, monkeypatch
):
"""Precedence pin — env wins over config.yaml. Fly.io / CI
secret injection depends on this ordering."""
monkeypatch.setenv(
"HERMES_DASHBOARD_PUBLIC_URL", "https://from-env.example",
)
patch_config("https://from-config.example")
redirect_uri = self._redirect_uri(gated_app_direct)
assert redirect_uri == "https://from-env.example/auth/callback", (
"env var must override config.yaml — Fly secret injection "
"depends on this precedence"
)
def test_malformed_public_url_falls_through_to_reconstruction(
self, gated_app_direct, patch_config, monkeypatch
):
"""Defence against header injection: a public_url that doesn't
parse as ``http(s)://host[/path]`` is dropped and we fall back
to request reconstruction. The login flow continues to work
rather than dispatching the user to a hostile URL."""
from urllib.parse import urlparse
patch_config(None)
for bad in [
"javascript:alert(1)",
"ftp://example.com",
"example.com", # missing scheme
"https://", # missing host
'https://example.com/"injected', # quote char
"https://example.com/\nhttps://evil", # CRLF injection
]:
monkeypatch.setenv("HERMES_DASHBOARD_PUBLIC_URL", bad)
redirect_uri = self._redirect_uri(gated_app_direct)
# Fell through to request reconstruction — netloc is the
# bound host, NOT the hostile value.
parsed = urlparse(redirect_uri)
assert parsed.netloc == "fly-app.fly.dev", (
f"malformed public_url={bad!r} leaked into redirect_uri: "
f"{redirect_uri!r}"
)
assert parsed.path == "/auth/callback"
def test_scheme_less_public_url_env_warns_operator(
self, patch_config, monkeypatch, caplog
):
"""A non-empty env var that's missing its scheme (the #1 cause
of "I set HERMES_DASHBOARD_PUBLIC_URL but the callback is still
http://") must emit an operator-facing WARNING rather than being
silently discarded. Regression for #42780."""
import logging
from hermes_cli.dashboard_auth import prefix as prefix_mod
# Reset the per-value dedup cache so the warning fires in-test
# regardless of test ordering.
prefix_mod._warned_malformed_public_urls.clear()
patch_config(None)
monkeypatch.setenv("HERMES_DASHBOARD_PUBLIC_URL", "hermes.domain.com")
with caplog.at_level(logging.WARNING, logger=prefix_mod.__name__):
result = prefix_mod.resolve_public_url()
assert result == "" # scheme-less value is still rejected
warnings = [
r.getMessage()
for r in caplog.records
if r.levelno == logging.WARNING
]
assert any(
"HERMES_DASHBOARD_PUBLIC_URL" in m
and "hermes.domain.com" in m
and "scheme" in m
for m in warnings
), f"expected a scheme warning, got: {warnings!r}"
# ---------------------------------------------------------------------------
# Cookies: Path attribute + __Host- / __Secure- prefix rules
# ---------------------------------------------------------------------------
class TestCookiePathRespectsPrefix:
"""Cookies must use ``Path=<prefix>`` when behind a proxy so they:
a) get sent back to the dashboard on subsequent requests (browser
only sends a cookie if the request path starts with the cookie's
Path attribute);
b) don't leak to other apps mounted alongside the dashboard
(e.g. ``mission-control.tilos.com/billing/...``).
When the cookie's Path can be ``/`` (no prefix, Fly-direct), we use
the ``__Host-`` cookie prefix for additional hardening — it binds
the cookie to the exact host (no Domain attribute) and requires Secure.
"""
def test_pkce_cookie_uses_secure_prefix_when_proxied(
self, gated_app_proxied
):
"""Behind a proxy with Path != /, ``__Host-`` is disallowed
(the spec requires Path=/). Fall back to ``__Secure-``, which
carries the same Secure-required guarantee but allows any Path.
"""
r = gated_app_proxied.get(
"/auth/login?provider=stub",
headers={"x-forwarded-prefix": "/hermes"},
follow_redirects=False,
)
cookies = r.headers.get_list("set-cookie")
# The PKCE cookie name carries the __Secure- prefix.
pkce_candidates = [
c for c in cookies
if c.startswith("__Secure-hermes_session_pkce=")
]
assert pkce_candidates, (
f"PKCE cookie missing __Secure- prefix: {cookies!r}"
)
def test_loopback_cookies_unprefixed(self):
"""Loopback HTTP dev: no Secure, no __Host- / __Secure-.
The bare cookie name is the right choice — neither prefix is
spec-compatible without Secure."""
from fastapi import FastAPI
from fastapi.responses import Response
from hermes_cli.dashboard_auth.cookies import set_pkce_cookie
app = FastAPI()
@app.get("/set")
def _set():
r = Response("ok")
set_pkce_cookie(r, payload="x", use_https=False)
return r
client = TestClient(app)
r = client.get("/set")
cookies = r.headers.get_list("set-cookie")
# Bare cookie name, no prefix.
assert any(c.startswith("hermes_session_pkce=") for c in cookies), (
f"Loopback cookie should be bare-named: {cookies!r}"
)
# And no __Host- / __Secure- variant accidentally emitted.
assert not any(
c.startswith("__Host-") or c.startswith("__Secure-")
for c in cookies
)
def test_cookies_read_back_round_trip_through_prefix(
self, gated_app_proxied
):
"""The end-to-end property: after a successful OAuth round
trip via the proxy, the session-AT cookie carries the
__Secure- prefix AND Path=/hermes, so the next request under
the same prefix is authenticated.
Note on TestClient semantics: starlette's TestClient sees the
literal request path (``/auth/login``, ``/auth/callback``) —
not the public path the proxy displays to the browser
(``/hermes/auth/login``, ``/hermes/auth/callback``). A cookie
set with ``Path=/hermes`` would therefore NOT be sent back on
the second request through TestClient even though it WOULD be
sent by a real browser hitting ``/hermes/auth/callback``. To
avoid baking that mismatch into the test, we inspect the
``Set-Cookie`` header on the callback's response WITHOUT
depending on the PKCE cookie round-tripping through
TestClient's jar — we drive /auth/callback with an explicit
Cookie header that carries the PKCE value from /auth/login.
"""
# /auth/login sets the PKCE cookie. Capture it from Set-Cookie.
r1 = gated_app_proxied.get(
"/auth/login?provider=stub",
headers={"x-forwarded-prefix": "/hermes"},
follow_redirects=False,
)
pkce_set = next(
c for c in r1.headers.get_list("set-cookie")
if "hermes_session_pkce" in c
)
# Parse "__Secure-hermes_session_pkce=...; HttpOnly; ...".
pkce_kv = pkce_set.split(";", 1)[0] # "__Secure-hermes_session_pkce=value"
state = r1.headers["location"].split("state=")[1]
# Round-trip the cookie by hand because TestClient's jar won't
# automatically send a Path=/hermes cookie to a /auth/callback
# request path.
r2 = gated_app_proxied.get(
f"/auth/callback?code=stub_code&state={state}",
headers={
"x-forwarded-prefix": "/hermes",
"cookie": pkce_kv,
},
follow_redirects=False,
)
assert r2.status_code == 302, r2.text
cookies = r2.headers.get_list("set-cookie")
at_cookies = [
c for c in cookies
if c.startswith("__Secure-hermes_session_at=")
]
assert at_cookies, (
f"session_at missing __Secure- prefix: {cookies!r}"
)
assert "Path=/hermes" in at_cookies[0]
assert "Secure" in at_cookies[0]
assert "HttpOnly" in at_cookies[0]