From cde893cce6cd263ad572750a3754b3cd93d025e9 Mon Sep 17 00:00:00 2001 From: Ben Date: Tue, 16 Jun 2026 12:09:10 +1000 Subject: [PATCH] feat(dashboard-auth): add Sec-Fetch-Site CSRF guard on mutating /api routes Credential-free, browser-asserted CSRF defense that applies in both auth regimes. Rejects a PRESENT hostile Sec-Fetch-Site (cross-site/same-site) on POST/PUT/PATCH/DELETE under /api/*; fails open on an absent header so non-browser clients (curl, NAS probe, desktop) are unaffected. Reads stay CORS-covered (mutations-only scope, plan Q2). This is the replacement for the legacy _SESSION_TOKEN's only load-bearing job, installed BEFORE the token is removed so there's never a window with neither defense. --- hermes_cli/web_server.py | 59 ++++++++ tests/hermes_cli/test_csrf_sec_fetch_guard.py | 136 ++++++++++++++++++ 2 files changed, 195 insertions(+) create mode 100644 tests/hermes_cli/test_csrf_sec_fetch_guard.py diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 14e2a8a5ecc..6443897667f 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -391,6 +391,65 @@ async def host_header_middleware(request: Request, call_next): return await call_next(request) +# --------------------------------------------------------------------------- +# CSRF guard — reject cross-origin state-changing requests via Sec-Fetch-Site. +# +# This is the credential-free replacement for the legacy ``_SESSION_TOKEN``'s +# only robust contribution: blocking drive-by CSRF from a web page the user +# visits. It applies in BOTH auth regimes (loopback and gated). +# +# Middleware order note: ``@app.middleware`` prepends, so the runtime order +# (outermost→innermost) is auth_middleware → _dashboard_auth_gate → +# csrf_guard_middleware → host_header_middleware → CORS → route. So an +# UNAUTHENTICATED cross-site mutation is already rejected by the outer auth +# layer (401 token in loopback, 401 cookie in gated); the CSRF guard's job is +# to reject an AUTHENTICATED cross-site mutation (403) — the genuine CSRF +# case where the victim's own credentials ride along. Both regimes covered. +# --------------------------------------------------------------------------- + +# Methods whose side effects a cross-origin page could trigger WITHOUT a CORS +# preflight ("simple requests" plus anything the browser will send cross-site). +# Reads are not guarded here — the CORSMiddleware (localhost-only origin regex, +# allow_credentials off) already prevents a foreign origin from reading any +# /api/* response body, so a cross-origin GET leaks nothing. +_CSRF_GUARDED_METHODS: frozenset = frozenset({"POST", "PUT", "PATCH", "DELETE"}) + +# Sec-Fetch-Site values that indicate a same-origin or user-initiated request. +# ``Sec-Fetch-Site`` is a forbidden header name (browser-set, JS cannot forge +# it), Baseline-available since 2023. ``none`` covers user navigation AND the +# packaged desktop renderer's file:// origin. +_CSRF_SAFE_FETCH_SITES: frozenset = frozenset({"same-origin", "none"}) + + +@app.middleware("http") +async def csrf_guard_middleware(request: Request, call_next): + """Reject cross-origin state-changing requests via Sec-Fetch-Site. + + Fail-open on an ABSENT header so non-browser clients (curl, the NAS + liveness probe, the desktop main process) are unaffected — those carry + no CSRF risk and the real auth gate (cookie / Origin guard) still + applies to them. Only a PRESENT, hostile value (``cross-site`` / + ``same-site``) is rejected. + """ + if ( + request.method in _CSRF_GUARDED_METHODS + and request.url.path.startswith("/api/") + ): + sfs = request.headers.get("sec-fetch-site") + if sfs is not None and sfs not in _CSRF_SAFE_FETCH_SITES: + return JSONResponse( + status_code=403, + content={ + "error": "cross_origin_blocked", + "detail": ( + "Cross-origin state-changing request rejected. The " + "dashboard only accepts mutations from its own origin." + ), + }, + ) + return await call_next(request) + + # --------------------------------------------------------------------------- # Dashboard OAuth auth gate — engaged only when start_server flags the # bind as non-loopback-without-insecure. No-op pass-through in loopback diff --git a/tests/hermes_cli/test_csrf_sec_fetch_guard.py b/tests/hermes_cli/test_csrf_sec_fetch_guard.py new file mode 100644 index 00000000000..c746c9139b8 --- /dev/null +++ b/tests/hermes_cli/test_csrf_sec_fetch_guard.py @@ -0,0 +1,136 @@ +"""Sec-Fetch-Site CSRF guard on mutating /api/* routes. + +The guard replaces the legacy ``_SESSION_TOKEN``'s only robust +contribution — blocking drive-by CSRF from a web page the user visits — +with a credential-free, browser-asserted check that applies in BOTH auth +regimes. ``Sec-Fetch-Site`` is a forbidden header name (JS cannot forge +it), so a cross-origin page cannot spoof ``same-origin``. + +Scope decision (plan Q2): mutating methods only. Reads are already +neutralised by the CORSMiddleware (localhost-only origin regex, +allow_credentials off), which prevents a foreign origin from reading any +``/api/*`` response body. +""" +from __future__ import annotations + +import pytest + +pytestmark = pytest.mark.xdist_group("dashboard_auth_app_state") + +from fastapi.testclient import TestClient + +from hermes_cli import web_server + + +@pytest.fixture +def loopback_client(): + 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.auth_required = False + web_server.app.state.bound_host = "127.0.0.1" + web_server.app.state.bound_port = 9119 + client = TestClient(web_server.app, base_url="http://127.0.0.1:9119") + yield client + web_server.app.state.bound_host = prev_host + web_server.app.state.bound_port = prev_port + web_server.app.state.auth_required = prev_required + + +# A real state-changing route. The CSRF guard runs BEFORE auth, so the +# blocked cases 403 regardless of token; the allowed cases carry a valid +# token so a non-403 proves the guard let them through to auth+handler. +_MUTATING_ROUTE = "/api/providers/validate" + + +@pytest.mark.parametrize("sfs", ["cross-site", "same-site"]) +def test_cross_origin_mutation_blocked(loopback_client, sfs): + r = loopback_client.post( + _MUTATING_ROUTE, + headers={ + "X-Hermes-Session-Token": web_server._SESSION_TOKEN, + "Sec-Fetch-Site": sfs, + }, + json={"key": "OPENAI_API_KEY", "value": "x"}, + ) + assert r.status_code == 403 + assert r.json().get("error") == "cross_origin_blocked" + + +@pytest.mark.parametrize("sfs", ["same-origin", "none"]) +def test_same_origin_mutation_allowed(loopback_client, sfs): + r = loopback_client.post( + _MUTATING_ROUTE, + headers={ + "X-Hermes-Session-Token": web_server._SESSION_TOKEN, + "Sec-Fetch-Site": sfs, + }, + json={"key": "OPENAI_API_KEY", "value": "x"}, + ) + # Reaches the handler (any non-403): the CSRF guard let it through. + assert r.status_code != 403 + + +def test_absent_header_fails_open(loopback_client): + """Non-browser clients (curl, NAS probe, desktop) send no + Sec-Fetch-Site and must NOT be blocked.""" + r = loopback_client.post( + _MUTATING_ROUTE, + headers={"X-Hermes-Session-Token": web_server._SESSION_TOKEN}, + json={"key": "OPENAI_API_KEY", "value": "x"}, + ) + assert r.status_code != 403 + + +def test_cross_site_get_not_blocked(loopback_client): + """Reads are CORS-covered, not CSRF-guarded (mutations-only scope).""" + r = loopback_client.get( + "/api/status", headers={"Sec-Fetch-Site": "cross-site"} + ) + assert r.status_code == 200 + + +def test_guard_applies_in_gated_mode(): + """The guard is mode-agnostic: a cross-site mutation from an + AUTHENTICATED session is still blocked in gated mode by the CSRF guard. + + A cookieless gated request 401s at the cookie gate before the CSRF + guard runs (Starlette runs last-registered-middleware outermost, so + the auth gate is outer). To prove the CSRF guard actually fires in + gated mode we must carry a valid session cookie so the request gets + past the gate and reaches the guard, which then 403s the cross-site + mutation. + """ + from hermes_cli.dashboard_auth import clear_providers, register_provider + from hermes_cli.dashboard_auth.cookies import SESSION_AT_COOKIE + from tests.hermes_cli.conftest_dashboard_auth import StubAuthProvider + + prev_host = getattr(web_server.app.state, "bound_host", None) + prev_required = getattr(web_server.app.state, "auth_required", None) + clear_providers() + provider = StubAuthProvider() + register_provider(provider) + web_server.app.state.auth_required = True + web_server.app.state.bound_host = "fly-app.fly.dev" + try: + # Mint a real session via the stub's login round trip. + start = provider.start_login(redirect_uri="https://fly-app.fly.dev/auth/callback") + state = start.cookie_payload["hermes_session_pkce"].split("state=")[1].split(";")[0] + verifier = start.cookie_payload["hermes_session_pkce"].split("verifier=")[1] + session = provider.complete_login( + code="stub_code", state=state, code_verifier=verifier, + redirect_uri="https://fly-app.fly.dev/auth/callback", + ) + client = TestClient(web_server.app, base_url="https://fly-app.fly.dev") + client.cookies.set(SESSION_AT_COOKIE, session.access_token) + r = client.post( + _MUTATING_ROUTE, + headers={"Sec-Fetch-Site": "cross-site"}, + json={"key": "OPENAI_API_KEY", "value": "x"}, + ) + assert r.status_code == 403 + assert r.json().get("error") == "cross_origin_blocked" + finally: + clear_providers() + web_server.app.state.auth_required = prev_required + web_server.app.state.bound_host = prev_host