-
Notifications
You must be signed in to change notification settings - Fork 52.6k
fix(dashboard): accept HERMES_DASHBOARD_PUBLIC_URL hostname in Host-header validation #68251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| webtecnica |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -397,6 +397,27 @@ def _require_token(request: Request) -> None: | |
| }) | ||
|
|
||
|
|
||
| def _resolve_allowed_hosts() -> frozenset: | ||
| """Return hostnames from ``dashboard.public_url`` / | ||
| ``HERMES_DASHBOARD_PUBLIC_URL`` as a frozenset. | ||
|
|
||
| Used by ``_is_accepted_host`` to accept the declared public hostname | ||
| through a loopback-bound dashboard behind a reverse proxy (the most | ||
| common self-host deploy pattern). | ||
| """ | ||
| try: | ||
| from hermes_cli.dashboard_auth.prefix import resolve_public_url | ||
| url = resolve_public_url() | ||
| if url: | ||
| parsed = urllib.parse.urlparse(url) | ||
| hostname = parsed.hostname | ||
| if hostname: | ||
| return frozenset({hostname.lower()}) | ||
| except Exception: | ||
| pass | ||
| return frozenset() | ||
|
|
||
|
|
||
| def should_require_auth(host: str, allow_public: bool = False) -> bool: | ||
| """Return True iff the dashboard auth gate must be active. | ||
|
|
||
|
|
@@ -419,14 +440,18 @@ def should_require_auth(host: str, allow_public: bool = False) -> bool: | |
| return host not in _LOOPBACK_HOST_VALUES | ||
|
|
||
|
|
||
| def _is_accepted_host(host_header: str, bound_host: str) -> bool: | ||
| def _is_accepted_host(host_header: str, bound_host: str, | ||
| extra_hosts: frozenset = frozenset()) -> bool: | ||
| """True if the Host header targets the interface we bound to. | ||
|
|
||
| Accepts: | ||
| - Exact bound host (with or without port suffix) | ||
| - Loopback aliases when bound to loopback | ||
| - Any host when bound to 0.0.0.0 (explicit opt-in to non-loopback, | ||
| no protection possible at this layer) | ||
| - ``extra_hosts`` hostnames (e.g. ``dashboard.public_url``) when | ||
| bound to loopback — lets a reverse-proxy on the same machine | ||
| forward a public hostname through 127.0.0.1. | ||
| """ | ||
| if not host_header: | ||
| return False | ||
|
|
@@ -438,7 +463,7 @@ def _is_accepted_host(host_header: str, bound_host: str) -> bool: | |
| # 127.0.0.1:9119 | ||
| h = host_header.strip() | ||
| if h.startswith("["): | ||
| # IPv6 bracketed — port (if any) follows "]:" | ||
| # IPv6 bracketed — port (if any) follows "]:"" | ||
| close = h.find("]") | ||
| if close != -1: | ||
| host_only = h[1:close] # strip brackets | ||
|
|
@@ -454,10 +479,15 @@ def _is_accepted_host(host_header: str, bound_host: str) -> bool: | |
| if bound_host in {"0.0.0.0", "::"}: | ||
| return True | ||
|
|
||
| # Loopback bind: accept the loopback names | ||
| # Loopback bind: accept the loopback names plus any extra hosts | ||
| # declared via dashboard.public_url / HERMES_DASHBOARD_PUBLIC_URL. | ||
| bound_lc = bound_host.lower() | ||
| if bound_lc in _LOOPBACK_HOST_VALUES: | ||
| return host_only in _LOOPBACK_HOST_VALUES | ||
| if host_only in _LOOPBACK_HOST_VALUES: | ||
| return True | ||
| if extra_hosts and host_only in extra_hosts: | ||
| return True | ||
| return False | ||
|
|
||
| # Explicit non-loopback bind: require exact host match | ||
| return host_only == bound_lc | ||
|
|
@@ -480,7 +510,8 @@ async def host_header_middleware(request: Request, call_next): | |
| bound_host = getattr(app.state, "bound_host", None) | ||
| if bound_host: | ||
| host_header = request.headers.get("host", "") | ||
| if not _is_accepted_host(host_header, bound_host): | ||
| extra_hosts = getattr(app.state, "allowed_hosts", frozenset()) | ||
| if not _is_accepted_host(host_header, bound_host, extra_hosts=extra_hosts): | ||
| return JSONResponse( | ||
| status_code=400, | ||
| content={ | ||
|
|
@@ -19270,6 +19301,7 @@ def start_server( | |
| # Record the bound host so host_header_middleware can validate incoming | ||
| # Host headers against it. Defends against DNS rebinding (GHSA-ppp5-vxwm-4cf7). | ||
| app.state.bound_host = host | ||
| app.state.allowed_hosts = _resolve_allowed_hosts() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes an externally reachable hostname trusted after |
||
|
|
||
| # ── Start uvicorn with direct Server API ───────────────────────── | ||
| # We use uvicorn.Server directly (not uvicorn.run) so we can split | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This allowlist reaches only
host_header_middleware. WebSocket upgrades independently call_is_accepted_host()for Host and Origin (hermes_cli/web_server.py:17112,17129on current main), so a browser using this proxy hostname still fails its WebSocket handshake. Thread the same app-state allowlist through that guard and add WebSocket coverage.