From 53736b3922604104379b5fdf7ffbed626ca1d288 Mon Sep 17 00:00:00 2001 From: Ben Date: Thu, 21 May 2026 15:31:40 +1000 Subject: [PATCH] feat(dashboard-auth): fail-closed on no providers; proxy_headers when gated; suppress _SESSION_TOKEN injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3, Task 3.5. Three changes to web_server.py: 1. start_server replaces the legacy SystemExit-refusing-to-bind guard with: if app.state.auth_required and no providers registered, exit with a clear message; otherwise log the gate-on banner. --insecure keeps its existing behaviour. 2. uvicorn proxy_headers flag is computed from app.state.auth_required. Loopback / --insecure keep it False (so _ws_client_is_allowed sees the real peer for the loopback gate); gated mode flips it True so X-Forwarded-Proto from Fly's TLS terminator is honoured for cookie Secure-flag decisions in detect_https(). 3. _serve_index no longer injects window.__HERMES_SESSION_TOKEN__ when the gate is on — the SPA reads identity from /api/auth/me using cookie auth instead. window.__HERMES_AUTH_REQUIRED__ flag lets the SPA pick between ticket-auth (gated) and token-auth (loopback) for /api/pty + /api/ws (Phase 5 will wire this in the React layer). 4 new behavioural tests; loopback regression harness still green. --- hermes_cli/web_server.py | 81 +++++++++++++++----- tests/hermes_cli/test_dashboard_auth_gate.py | 78 +++++++++++++++++-- 2 files changed, 134 insertions(+), 25 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 29512327548..6693f9c7cbf 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -3768,14 +3768,33 @@ def mount_spa(application: FastAPI): ``prefix`` is the normalised ``X-Forwarded-Prefix`` (e.g. ``/hermes``) or empty string when served at root. + + When the OAuth auth gate is active (``app.state.auth_required``), + the legacy ``_SESSION_TOKEN`` is NOT injected — the SPA reads + identity from ``/api/auth/me`` over cookie auth instead. The + ``__HERMES_AUTH_REQUIRED__`` flag lets the SPA pick the right + auth scheme for /api/pty and /api/ws (ticket vs token). """ html = _index_path.read_text() chat_js = "true" if _DASHBOARD_EMBEDDED_CHAT_ENABLED else "false" - token_script = ( - f'' - ) + gated = bool(getattr(app.state, "auth_required", False)) + gated_js = "true" if gated else "false" + if gated: + bootstrap_script = ( + f"" + ) + else: + bootstrap_script = ( + f'" + ) if prefix: # Rewrite absolute asset URLs baked into the Vite build so the # browser fetches them through the same proxy prefix. @@ -3785,7 +3804,7 @@ def mount_spa(application: FastAPI): html = html.replace('href="/fonts/', f'href="{prefix}/fonts/') html = html.replace('href="/ds-assets/', f'href="{prefix}/ds-assets/') html = html.replace('src="/ds-assets/', f'src="{prefix}/ds-assets/') - html = html.replace("", f"{token_script}", 1) + html = html.replace("", f"{bootstrap_script}", 1) return HTMLResponse( html, headers={"Cache-Control": "no-store, no-cache, must-revalidate"}, @@ -4744,19 +4763,35 @@ def start_server( _DASHBOARD_EMBEDDED_CHAT_ENABLED = embedded_chat # Phase 0: stash the auth-gate flag on app.state so middleware / SPA-token - # injection / WS-auth paths can branch on it consistently. At Phase 0 the - # flag is set but nothing reads it yet — later phases register the gate - # middleware and the gated /auth/* routes. + # injection / WS-auth paths can branch on it consistently. Phase 3.5 + # uses this to decide whether to refuse the bind, log the gate-on + # banner, and enable uvicorn proxy_headers. app.state.auth_required = should_require_auth(host, allow_public) - _LOCALHOST = ("127.0.0.1", "localhost", "::1") - if host not in _LOCALHOST and not allow_public: - raise SystemExit( - f"Refusing to bind to {host} — the dashboard exposes API keys " - f"and config without robust authentication.\n" - f"Use --insecure to override (NOT recommended on untrusted networks)." + if app.state.auth_required: + # Phase 3.5: the gate engages on non-loopback binds. The legacy + # "refusing to bind" guard is replaced by "require at least one + # provider to be registered, else fail closed". + from hermes_cli.dashboard_auth import list_providers + if not list_providers(): + raise SystemExit( + f"Refusing to bind dashboard to {host} — the OAuth auth " + f"gate engages on non-loopback binds, but no auth providers " + f"are registered.\n" + f"Install the default Nous provider " + f"(plugins/dashboard-auth-nous) or another " + f"DashboardAuthProvider plugin.\n" + f"Or pass --insecure to skip the auth gate (NOT recommended " + f"on untrusted networks)." + ) + _log.info( + "Dashboard binding to %s with OAuth auth gate enabled. " + "Providers: %s", + host, + ", ".join(p.name for p in list_providers()), ) - if host not in _LOCALHOST: + elif host not in _LOOPBACK_HOST_VALUES and allow_public: + # --insecure path — no auth, loud warning. _log.warning( "Binding to %s with --insecure — the dashboard has no robust " "authentication. Only use on trusted networks.", host, @@ -4801,7 +4836,13 @@ def start_server( ) print(f" Hermes Web UI → http://{host}:{port}") - # proxy_headers=False so _ws_client_is_allowed sees the real connection peer - # rather than X-Forwarded-For's rewritten value (which would defeat the - # loopback gate when behind a reverse proxy). - uvicorn.run(app, host=host, port=port, log_level="warning", proxy_headers=False) + # proxy_headers defaults to False so _ws_client_is_allowed sees the real + # connection peer rather than X-Forwarded-For's rewritten value (which + # would defeat the loopback gate when behind a reverse proxy). When the + # OAuth gate is active we are explicitly running behind a TLS terminator + # (Fly.io) and need X-Forwarded-Proto to decide cookie Secure flags, so + # we flip proxy_headers on for that mode. + uvicorn.run( + app, host=host, port=port, log_level="warning", + proxy_headers=bool(app.state.auth_required), + ) diff --git a/tests/hermes_cli/test_dashboard_auth_gate.py b/tests/hermes_cli/test_dashboard_auth_gate.py index 8d8c6b94977..e2576cb3b3f 100644 --- a/tests/hermes_cli/test_dashboard_auth_gate.py +++ b/tests/hermes_cli/test_dashboard_auth_gate.py @@ -137,13 +137,14 @@ def test_start_server_insecure_public_sets_auth_required_false(monkeypatch): def test_start_server_public_without_insecure_records_auth_required(monkeypatch): - """Public bind without --insecure: the gate is meant to engage. + """Public bind without --insecure: the gate engages and auth_required=True. - Until Phase 3 lands, start_server still raises SystemExit on this path - (the legacy "refusing to bind" guard). We must still observe the - auth_required flag being set on app.state BEFORE the exit happens, so - the rest of the system can branch on it consistently. + With no providers registered, this fails closed with SystemExit. The + flag-stashing happens BEFORE the exit so the rest of the system can + branch on it. (See task 3.5 tests below for the with-provider path.) """ + from hermes_cli.dashboard_auth import clear_providers + clear_providers() _stub_uvicorn_run(monkeypatch) web_server.app.state.auth_required = None with pytest.raises(SystemExit): @@ -152,3 +153,70 @@ def test_start_server_public_without_insecure_records_auth_required(monkeypatch) open_browser=False, allow_public=False, ) assert web_server.app.state.auth_required is True + + +# --------------------------------------------------------------------------- +# Task 3.5: start_server fail-closed + proxy_headers + index-token suppression +# --------------------------------------------------------------------------- + + +def test_start_server_gate_with_provider_proceeds_and_sets_proxy_headers(monkeypatch): + """With at least one provider, public bind + no --insecure starts the server. + + The SystemExit-refusing-to-bind guard is REPLACED in gated mode by + "the gate engages", so as long as a provider is registered the bind + succeeds. uvicorn is called with proxy_headers=True so X-Forwarded-Proto + from Fly's TLS terminator is honoured for cookie Secure-flag decisions. + """ + from hermes_cli.dashboard_auth import clear_providers, register_provider + from tests.hermes_cli.conftest_dashboard_auth import StubAuthProvider + + clear_providers() + register_provider(StubAuthProvider()) + captured = _stub_uvicorn_run(monkeypatch) + try: + web_server.app.state.auth_required = None + web_server.start_server( + host="0.0.0.0", port=9119, + open_browser=False, allow_public=False, + ) + assert web_server.app.state.auth_required is True + assert captured["kwargs"].get("host") == "0.0.0.0" + assert captured["kwargs"].get("proxy_headers") is True + finally: + clear_providers() + + +def test_start_server_gate_without_provider_fails_closed(monkeypatch): + """No providers + gate would activate → SystemExit with a clear message.""" + from hermes_cli.dashboard_auth import clear_providers + + clear_providers() + _stub_uvicorn_run(monkeypatch) + web_server.app.state.auth_required = None + with pytest.raises(SystemExit, match=r"no auth providers"): + web_server.start_server( + host="0.0.0.0", port=9119, + open_browser=False, allow_public=False, + ) + + +def test_start_server_loopback_keeps_proxy_headers_off(monkeypatch): + """Loopback bind: proxy_headers stays False (no TLS terminator in front).""" + captured = _stub_uvicorn_run(monkeypatch) + web_server.start_server( + host="127.0.0.1", port=9119, + open_browser=False, allow_public=False, + ) + assert captured["kwargs"].get("proxy_headers") is False + + +def test_start_server_insecure_keeps_proxy_headers_off(monkeypatch): + """--insecure: gate stays off, proxy_headers stays off.""" + captured = _stub_uvicorn_run(monkeypatch) + web_server.start_server( + host="0.0.0.0", port=9119, + open_browser=False, allow_public=True, + ) + assert web_server.app.state.auth_required is False + assert captured["kwargs"].get("proxy_headers") is False