fix(dashboard): move CORSMiddleware outermost so OPTIONS preflight bypasses auth
app.add_middleware(CORSMiddleware, ...) was registered before the @app.middleware(http) auth decorators, making CORS the innermost layer in Starlette's onion model. A cross-origin OPTIONS preflight hit auth first → 401, and the CORS headers were never sent. Moving the add_middleware call after all @app.middleware registrations makes CORS outermost, so preflights are answered before auth runs. Fixes #59052
This commit is contained in:
committed by
Austin Pickett
parent
d59c736fcb
commit
727dbb17f2
@@ -438,16 +438,6 @@ _DASHBOARD_EMBEDDED_CHAT_ENABLED = True
|
||||
# uvicorn's 16 MiB default rejects files under the 256 MiB raw attach cap.
|
||||
_DESKTOP_ATTACHMENT_WS_MAX_BYTES = 384 * 1024 * 1024
|
||||
|
||||
|
||||
# CORS: localhost origins only — allow_origins=["*"] on 0.0.0.0 would let any
|
||||
# website read/modify config and secrets.
|
||||
app.add_middleware(
|
||||
CORSMiddleware,
|
||||
allow_origin_regex=r"^https?://(localhost|127\.0\.0\.1)(:\d+)?$",
|
||||
allow_methods=["*"],
|
||||
allow_headers=["*"],
|
||||
)
|
||||
|
||||
# Endpoints that do NOT require the session token; everything else under /api/
|
||||
# is gated below. Shared with the OAuth gate so the two allowlists cannot
|
||||
# drift (/api/status once 401'd under the OAuth gate, breaking the portal probe).
|
||||
@@ -787,7 +777,7 @@ DASHBOARD_HEALTH = DashboardHealth()
|
||||
|
||||
@app.middleware("http")
|
||||
async def _dashboard_health_middleware(request: Request, call_next):
|
||||
"""Outermost middleware (registered last): count unhandled exceptions and 5xx; re-raises, never alters."""
|
||||
"""Outermost non-CORS middleware: count unhandled exceptions and 5xx; re-raises, never alters."""
|
||||
try:
|
||||
response = await call_next(request)
|
||||
except Exception as exc:
|
||||
@@ -798,6 +788,22 @@ async def _dashboard_health_middleware(request: Request, call_next):
|
||||
return response
|
||||
|
||||
|
||||
# CORS: restrict to localhost origins only. The web UI is intended to run
|
||||
# locally; binding to 0.0.0.0 with allow_origins=["*"] would let any website
|
||||
# read/modify config and secrets.
|
||||
#
|
||||
# Registered AFTER all ``@app.middleware("http")`` decorators so it is the
|
||||
# *outermost* middleware (Starlette's onion: last-added runs first).
|
||||
# Without this, an OPTIONS preflight from a cross-origin SPA hits the auth
|
||||
# middlewares before CORS can answer, producing 401 instead of 204 + headers.
|
||||
app.add_middleware(
|
||||
CORSMiddleware,
|
||||
allow_origin_regex=r"^https?://(localhost|127\.0\.0\.1)(:\d+)?$",
|
||||
allow_methods=["*"],
|
||||
allow_headers=["*"],
|
||||
)
|
||||
|
||||
|
||||
# Authenticated-route self-test: one in-process request per minute against a
|
||||
# cheap DB-touching route, catching "liveness fine but every authed request 500s".
|
||||
_DASHBOARD_SELFTEST_INTERVAL_SECONDS = 60.0
|
||||
|
||||
121
tests/hermes_cli/test_web_server_cors_preflight.py
Normal file
121
tests/hermes_cli/test_web_server_cors_preflight.py
Normal file
@@ -0,0 +1,121 @@
|
||||
"""Tests for CORS preflight ordering on the dashboard web server.
|
||||
|
||||
Regression: ``CORSMiddleware`` was registered *before* the
|
||||
``@app.middleware("http")`` auth decorators, making it the innermost
|
||||
layer in Starlette's onion model. A cross-origin ``OPTIONS`` preflight
|
||||
hit the auth middleware first → 401, and the CORS headers were never
|
||||
sent. Moving the ``add_middleware(CORSMiddleware, …)`` call *after*
|
||||
all ``@app.middleware("http")`` registrations makes CORS outermost so
|
||||
preflights are answered before auth runs.
|
||||
|
||||
See: https://github.com/NousResearch/hermes-agent/issues/59052
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli import web_server
|
||||
|
||||
pytest.importorskip("starlette.testclient")
|
||||
from starlette.testclient import TestClient
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client():
|
||||
return TestClient(web_server.app)
|
||||
|
||||
|
||||
# ---- CORS preflight (OPTIONS) must not be blocked by auth ------------
|
||||
|
||||
|
||||
def test_options_preflight_returns_cors_headers(client):
|
||||
"""OPTIONS with Origin + Access-Control-Request-Method → 200 + CORS."""
|
||||
resp = client.options(
|
||||
"/api/skills",
|
||||
headers={
|
||||
"Origin": "http://127.0.0.1:8092",
|
||||
"Access-Control-Request-Method": "GET",
|
||||
"Access-Control-Request-Headers": "x-hermes-session-token",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert "access-control-allow-origin" in resp.headers
|
||||
assert resp.headers["access-control-allow-origin"] == "http://127.0.0.1:8092"
|
||||
assert "access-control-allow-methods" in resp.headers
|
||||
|
||||
|
||||
def test_options_preflight_allows_localhost_origin(client):
|
||||
"""Preflight from a different localhost port is allowed."""
|
||||
resp = client.options(
|
||||
"/api/status",
|
||||
headers={
|
||||
"Origin": "http://localhost:3000",
|
||||
"Access-Control-Request-Method": "GET",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.headers.get("access-control-allow-origin") == "http://localhost:3000"
|
||||
|
||||
|
||||
def test_options_preflight_rejects_disallowed_origin(client):
|
||||
"""Preflight from a non-localhost origin is rejected (no CORS headers)."""
|
||||
resp = client.options(
|
||||
"/api/skills",
|
||||
headers={
|
||||
"Origin": "http://evil.example.com",
|
||||
"Access-Control-Request-Method": "GET",
|
||||
},
|
||||
)
|
||||
# Starlette CORSMiddleware returns 400 for disallowed origins on preflight
|
||||
assert resp.status_code == 400
|
||||
assert "access-control-allow-origin" not in resp.headers
|
||||
|
||||
|
||||
# ---- gated (OAuth/cookie) mode: the plugin gate must not 401 preflights either
|
||||
|
||||
|
||||
def test_options_preflight_bypasses_gated_auth(client, monkeypatch):
|
||||
"""With auth_required on, the cookie gate still lets CORS answer preflights."""
|
||||
monkeypatch.setattr(web_server.app.state, "auth_required", True, raising=False)
|
||||
resp = client.options(
|
||||
"/api/skills",
|
||||
headers={
|
||||
"Origin": "http://localhost:8092",
|
||||
"Access-Control-Request-Method": "GET",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.headers.get("access-control-allow-origin") == "http://localhost:8092"
|
||||
|
||||
|
||||
# ---- auth still enforced on real requests ---------------------------
|
||||
|
||||
|
||||
def test_get_without_origin_has_no_cors_headers(client):
|
||||
"""A same-origin GET (no Origin header) has no CORS headers."""
|
||||
resp = client.get(
|
||||
"/api/status",
|
||||
headers={web_server._SESSION_HEADER_NAME: web_server._SESSION_TOKEN},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert "access-control-allow-origin" not in resp.headers
|
||||
|
||||
|
||||
def test_get_with_matching_origin_has_cors_headers(client):
|
||||
"""A cross-origin GET from an allowed origin has CORS headers."""
|
||||
resp = client.get(
|
||||
"/api/status",
|
||||
headers={
|
||||
"Origin": "http://127.0.0.1:8092",
|
||||
web_server._SESSION_HEADER_NAME: web_server._SESSION_TOKEN,
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.headers.get("access-control-allow-origin") == "http://127.0.0.1:8092"
|
||||
|
||||
|
||||
def test_get_without_token_still_401s(client):
|
||||
"""The token gate is untouched: a real unauthenticated GET still 401s."""
|
||||
resp = client.get("/api/skills", headers={"Origin": "http://127.0.0.1:8092"})
|
||||
assert resp.status_code == 401
|
||||
Reference in New Issue
Block a user