diff --git a/api/routes.py b/api/routes.py index e1580ce5c..7251111e3 100644 --- a/api/routes.py +++ b/api/routes.py @@ -5127,6 +5127,22 @@ 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. + + 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. + """ + origin = handler.headers.get("Origin", "").strip() + if not origin: + return "" + return origin if _check_same_origin_browser_request(handler) else "" + + def _csrf_exempt_path(path: str) -> bool: """Paths that cannot or must not carry a session CSRF token.""" return path in { diff --git a/server.py b/server.py index a74141c67..655cba9c8 100644 --- a/server.py +++ b/server.py @@ -109,7 +109,14 @@ 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 +from api.routes import ( + handle_delete, + handle_get, + handle_patch, + handle_post, + handle_put, + preflight_allow_origin, +) 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 @@ -435,12 +442,21 @@ class Handler(BaseHTTPRequestHandler): self._handle_write(handle_patch) def do_OPTIONS(self) -> None: - """Handle CORS preflight requests.""" + """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. + """ self._req_t0 = time.time() + allow_origin = preflight_allow_origin(self) self.send_response(200) - self.send_header("Access-Control-Allow-Origin", "*") - self.send_header("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS") - self.send_header("Access-Control-Allow-Headers", "Content-Type, Authorization") + 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") self.end_headers() def do_DELETE(self) -> None: diff --git a/tests/test_cors_preflight_allowlist.py b/tests/test_cors_preflight_allowlist.py new file mode 100644 index 000000000..bfbdc4013 --- /dev/null +++ b/tests/test_cors_preflight_allowlist.py @@ -0,0 +1,96 @@ +"""CORS preflight must not advertise wider access than the CSRF gate permits. + +server.py's do_OPTIONS previously answered every preflight with +``Access-Control-Allow-Origin: *``. It now echoes the request Origin only +when it is same-origin or explicitly allowlisted via +HERMES_WEBUI_ALLOWED_ORIGINS — reusing _check_same_origin_browser_request, +the same policy enforced for real requests. +""" + +from pathlib import Path +from types import SimpleNamespace + +from api.routes import preflight_allow_origin + +ROOT = Path(__file__).resolve().parent.parent + + +def _preflight(headers): + return preflight_allow_origin(SimpleNamespace(headers=headers)) + + +class TestPreflightAllowOrigin: + def test_no_origin_header_omits_cors(self): + """Non-browser OPTIONS (no Origin) gets no Allow-Origin header.""" + assert _preflight({}) == "" + + def test_same_origin_echoed(self): + assert _preflight({ + "Origin": "http://127.0.0.1:8787", + "Host": "127.0.0.1:8787", + }) == "http://127.0.0.1:8787" + + def test_cross_origin_rejected(self): + assert _preflight({ + "Origin": "http://evil.com", + "Host": "127.0.0.1:8787", + }) == "" + + def test_wildcard_never_returned(self): + """Even a same-origin match echoes the Origin, never '*'.""" + result = _preflight({ + "Origin": "http://localhost:8787", + "Host": "localhost:8787", + }) + assert result != "*" + + def test_sec_fetch_site_cross_site_rejected(self): + """Sec-Fetch-Site: cross-site is refused even if hosts happened to match.""" + assert _preflight({ + "Origin": "http://127.0.0.1:8787", + "Host": "127.0.0.1:8787", + "Sec-Fetch-Site": "cross-site", + }) == "" + + def test_allowlisted_public_origin_echoed(self, monkeypatch): + monkeypatch.setenv( + "HERMES_WEBUI_ALLOWED_ORIGINS", "https://myapp.example.com:8000" + ) + assert _preflight({ + "Origin": "https://myapp.example.com:8000", + "Host": "127.0.0.1:8787", + }) == "https://myapp.example.com:8000" + + def test_non_allowlisted_public_origin_rejected(self, monkeypatch): + monkeypatch.setenv( + "HERMES_WEBUI_ALLOWED_ORIGINS", "https://myapp.example.com:8000" + ) + assert _preflight({ + "Origin": "https://evil.example.com:8000", + "Host": "127.0.0.1:8787", + }) == "" + + def test_forwarded_host_untrusted_by_default(self, monkeypatch): + """Without HERMES_WEBUI_TRUST_FORWARDED_HOST, X-Forwarded-Host is ignored.""" + monkeypatch.delenv("HERMES_WEBUI_TRUST_FORWARDED_HOST", raising=False) + assert _preflight({ + "Origin": "https://webui.example.com", + "Host": "127.0.0.1:8787", + "X-Forwarded-Host": "webui.example.com:443", + }) == "" + + def test_forwarded_host_trusted_when_opted_in(self, monkeypatch): + monkeypatch.setenv("HERMES_WEBUI_TRUST_FORWARDED_HOST", "1") + assert _preflight({ + "Origin": "https://webui.example.com", + "Host": "127.0.0.1:8787", + "X-Forwarded-Host": "webui.example.com:443", + }) == "https://webui.example.com" + + +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") + assert '"Access-Control-Allow-Origin", "*"' not in src + assert "preflight_allow_origin" in src