mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-11 18:20:30 +00:00
fix(security): CORS preflight echoes only same-origin or allowlisted origins
do_OPTIONS answered every CORS preflight with Access-Control-Allow-Origin: *, advertising broader cross-origin access than a real request is granted — on a password-less deployment that lets any site read authenticated responses. It now echoes the request Origin only when _check_same_origin_browser_request approves it (the exact same-origin / HERMES_WEBUI_ALLOWED_ORIGINS policy the CSRF gate enforces for real requests), adds Vary: Origin, and never emits *. A disallowed origin gets a 200 with no CORS headers (browser treats as preflight denial). Default deployments (no allowlist) are unaffected. Contributor stage; gate-pass. Co-authored-by: mo7al876any <mo7al876any@users.noreply.github.com>
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user