diff --git a/api/routes.py b/api/routes.py index 7251111e..3403e210 100644 --- a/api/routes.py +++ b/api/routes.py @@ -5127,20 +5127,26 @@ def _check_same_origin_browser_request(handler, *, require_provenance: bool = Fa return True -def preflight_allow_origin(handler) -> str: - """CORS preflight: return the Origin value to echo back, or '' to omit. +def apply_cors_preflight_headers(handler) -> None: + """Emit CORS preflight headers on ``handler`` for a same-origin/allowlisted + request; emit nothing for a disallowed origin (browser treats the header-less + 200 as a preflight denial). - Echo the request Origin only when it is same-origin or explicitly - allowlisted via HERMES_WEBUI_ALLOWED_ORIGINS — the exact policy the CSRF - gate enforces for real requests. Reuses _check_same_origin_browser_request - so the preflight can never advertise wider access (`*`) than an actual - request would be granted. A wildcard here would let any site read - authenticated responses on a deployment with no password set. + Echoes the request Origin only when it is same-origin or explicitly + allowlisted via HERMES_WEBUI_ALLOWED_ORIGINS — the exact policy the CSRF gate + enforces for real requests. Reuses _check_same_origin_browser_request so the + preflight can never advertise wider access (`*`) than an actual request would + be granted. A wildcard here would let any site read authenticated responses + on a deployment with no password set. Kept in api/ so server.py stays a thin + dispatcher. """ origin = handler.headers.get("Origin", "").strip() - if not origin: - return "" - return origin if _check_same_origin_browser_request(handler) else "" + if not origin or not _check_same_origin_browser_request(handler): + return + handler.send_header("Access-Control-Allow-Origin", origin) + handler.send_header("Vary", "Origin") + handler.send_header("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS") + handler.send_header("Access-Control-Allow-Headers", "Content-Type, Authorization") def _csrf_exempt_path(path: str) -> bool: diff --git a/server.py b/server.py index 655cba9c..48129542 100644 --- a/server.py +++ b/server.py @@ -109,14 +109,7 @@ from api.helpers import ( _CLIENT_DISCONNECT_ERRORS, ) from api.profiles import set_request_profile, clear_request_profile -from api.routes import ( - handle_delete, - handle_get, - handle_patch, - handle_post, - handle_put, - preflight_allow_origin, -) +from api.routes import handle_delete, handle_get, handle_patch, handle_post, handle_put, apply_cors_preflight_headers from api.startup import auto_install_agent_deps, fix_credential_permissions from api.updates import WEBUI_VERSION from api.crash_visibility import install_crash_visibility @@ -442,21 +435,10 @@ class Handler(BaseHTTPRequestHandler): self._handle_write(handle_patch) def do_OPTIONS(self) -> None: - """Handle CORS preflight requests. - - Echo the request Origin only when it is same-origin or allowlisted - via HERMES_WEBUI_ALLOWED_ORIGINS (same policy as the CSRF gate) — - never advertise `*`. A disallowed origin gets a 200 with no CORS - headers, which the browser treats as a preflight denial. - """ + """Handle CORS preflight requests (headers emitted by api.routes).""" self._req_t0 = time.time() - allow_origin = preflight_allow_origin(self) self.send_response(200) - if allow_origin: - self.send_header("Access-Control-Allow-Origin", allow_origin) - self.send_header("Vary", "Origin") - self.send_header("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS") - self.send_header("Access-Control-Allow-Headers", "Content-Type, Authorization") + apply_cors_preflight_headers(self) self.end_headers() def do_DELETE(self) -> None: diff --git a/tests/test_cors_preflight_allowlist.py b/tests/test_cors_preflight_allowlist.py index bfbdc401..13521f4f 100644 --- a/tests/test_cors_preflight_allowlist.py +++ b/tests/test_cors_preflight_allowlist.py @@ -8,15 +8,35 @@ the same policy enforced for real requests. """ from pathlib import Path -from types import SimpleNamespace -from api.routes import preflight_allow_origin +from api.routes import apply_cors_preflight_headers ROOT = Path(__file__).resolve().parent.parent +class _FakeHandler: + """Captures send_header() calls so tests can assert emitted CORS headers.""" + + def __init__(self, headers): + self.headers = headers + self.sent = {} + + def send_header(self, key, value): + self.sent[key] = value + + def _preflight(headers): - return preflight_allow_origin(SimpleNamespace(headers=headers)) + """Return the Access-Control-Allow-Origin value the preflight would emit, + or '' when no CORS headers are emitted (disallowed / no-origin).""" + h = _FakeHandler(headers) + apply_cors_preflight_headers(h) + return h.sent.get("Access-Control-Allow-Origin", "") + + +def _preflight_headers(headers): + h = _FakeHandler(headers) + apply_cors_preflight_headers(h) + return h.sent class TestPreflightAllowOrigin: @@ -92,5 +112,27 @@ class TestServerNoWildcard: def test_server_source_has_no_wildcard_allow_origin(self): """Regression guard: the literal `*` Allow-Origin must not come back.""" src = (ROOT / "server.py").read_text(encoding="utf-8") + routes_src = (ROOT / "api" / "routes.py").read_text(encoding="utf-8") assert '"Access-Control-Allow-Origin", "*"' not in src - assert "preflight_allow_origin" in src + assert '"Access-Control-Allow-Origin", "*"' not in routes_src + # server.py stays a thin dispatcher: the preflight logic lives in api/routes. + assert "apply_cors_preflight_headers" in src + assert "def apply_cors_preflight_headers" in routes_src + + def test_allowed_preflight_sets_vary_origin(self): + """An allowed preflight must set Vary: Origin (correct for caching an + origin-conditional response).""" + sent = _preflight_headers({ + "Origin": "http://127.0.0.1:8787", + "Host": "127.0.0.1:8787", + }) + assert sent.get("Vary") == "Origin" + assert sent.get("Access-Control-Allow-Origin") == "http://127.0.0.1:8787" + + def test_denied_preflight_emits_no_cors_headers(self): + """A disallowed origin gets zero CORS headers (browser preflight denial).""" + sent = _preflight_headers({ + "Origin": "http://evil.com", + "Host": "127.0.0.1:8787", + }) + assert sent == {}