fix(dashboard): trust configured reverse proxies (#94126)

* fix(dashboard): trust configured reverse proxies * fix(dashboard): trust IPv6 loopback proxies
2026-08-27 11:35:22 -06:00
parent 39f1e1881a
commit 0dfba37b11
9 changed files with 220 additions and 5 deletions
@@ -1868,6 +1868,16 @@ updates:
 #   # default — works on Fly.io out of the box).
 #   #
 #   #   public_url: "https://example.com/hermes"
+#   #
+#   # Reverse proxies connecting from another container or host are not
+#   # trusted by default. Add only the proxy's exact IP address, or a bounded
+#   # CIDR for a dedicated proxy network, so X-Forwarded-Proto and
+#   # X-Forwarded-For can be honored. Loopback is always trusted. Wildcards
+#   # and /0 networks are rejected.
+#   #
+#   #   trusted_proxies:
+#   #     - "172.20.0.5"
+#   #     # - "172.20.0.0/24"  # dedicated network, if the IP is dynamic
 #
 # -----------------------------------------------------------------------------
 # Self-hosted OIDC dashboard auth (generic OpenID Connect — Authentik,
@@ -1633,6 +1633,11 @@ DEFAULT_CONFIG = {
        # Set this to True to re-enable the surfaces with the understanding
        # that the numbers are a local lower-bound estimate, not billing.
        "show_token_analytics": False,
+        # IP addresses or bounded CIDR networks of reverse proxies allowed to
+        # supply X-Forwarded-Proto / X-Forwarded-For. Loopback remains trusted
+        # automatically. Wildcards and /0 networks are rejected so arbitrary
+        # clients cannot spoof their scheme or source address.
+        "trusted_proxies": [],
        # WebSocket keepalive for the dashboard/desktop web server (#79635).
        # Applied to NON-loopback binds only: loopback always disables the
        # protocol ping (see hermes_cli/web_server.py — an event-loop stall
@@ -25,6 +25,7 @@ import hashlib
 import hmac
 import inspect
 import importlib.util
+import ipaddress
 import json
 import logging
 import math
@@ -19557,6 +19558,66 @@ def _report_port_in_use(host: str, port: int) -> None:
    )


+_DEFAULT_DASHBOARD_FORWARDED_ALLOW_IPS = ("127.0.0.1", "::1")
+
+
+def _dashboard_forwarded_allow_ips(dashboard_config: dict[str, Any]) -> list[str]:
+    """Return the bounded proxy addresses uvicorn may trust.
+
+    Uvicorn's default trusts loopback. Preserve that behavior and extend it
+    only with explicit IP addresses or CIDR networks from config. Invalid or
+    unbounded entries fail closed instead of turning arbitrary client-supplied
+    forwarding headers into request metadata.
+    """
+    configured = dashboard_config.get("trusted_proxies", [])
+    if configured in (None, ""):
+        configured = []
+    elif isinstance(configured, str):
+        configured = [configured]
+    elif not isinstance(configured, (list, tuple)):
+        _log.warning(
+            "dashboard.trusted_proxies must be a list of IP addresses or CIDR networks; "
+            "ignoring %r",
+            configured,
+        )
+        configured = []
+
+    trusted = list(_DEFAULT_DASHBOARD_FORWARDED_ALLOW_IPS)
+    for raw_entry in configured:
+        if not isinstance(raw_entry, str) or not raw_entry.strip():
+            _log.warning(
+                "Ignoring invalid dashboard.trusted_proxies entry %r; expected an IP "
+                "address or CIDR network",
+                raw_entry,
+            )
+            continue
+
+        entry = raw_entry.strip()
+        try:
+            if "/" in entry:
+                network = ipaddress.ip_network(entry, strict=False)
+                if network.prefixlen == 0:
+                    raise ValueError("unbounded network")
+                normalized = str(network)
+            else:
+                normalized = str(ipaddress.ip_address(entry))
+        except ValueError:
+            _log.warning(
+                "Ignoring unsafe dashboard.trusted_proxies entry %r; use a bounded IP "
+                "address or CIDR network, never '*' or a /0 network",
+                raw_entry,
+            )
+            continue
+
+        if normalized not in trusted:
+            trusted.append(normalized)
+
+    if trusted != list(_DEFAULT_DASHBOARD_FORWARDED_ALLOW_IPS):
+        _log.info("Dashboard trusted proxies: %s", ", ".join(trusted))
+
+    return trusted
+
+
 def start_server(
    host: str = "127.0.0.1",
    port: int = 9119,
@@ -19804,6 +19865,12 @@ def start_server(
        # decide cookie Secure flags, so we flip proxy_headers on for that
        # mode.
        proxy_headers=bool(app.state.auth_required),
+        # Keep uvicorn's loopback-only default unless the operator explicitly
+        # trusts the address or bounded network of an upstream proxy. This is
+        # what lets a separate-container TLS terminator supply HTTPS/client
+        # metadata without accepting spoofed X-Forwarded-* headers from every
+        # caller.
+        forwarded_allow_ips=_dashboard_forwarded_allow_ips(_dash_cfg),
        # Half-open detection for public binds only (see above). Loopback
        # disables the protocol ping (None) so an event-loop stall can never
        # trigger a false disconnect; a genuinely dead local client is still
@@ -117,7 +117,8 @@ dependencies = [
  # .gitignore-aware file matching for desktop build stamp.
  "pathspec==1.1.1",
  "fastapi>=0.104.0,<1",
-  "uvicorn[standard]>=0.24.0,<1",
+  # CIDR-aware forwarded_allow_ips requires uvicorn >=0.31.0.
+  "uvicorn[standard]>=0.31.0,<1",
  # Streaming multipart uploads for the dashboard file manager (NS-501).
  # FastAPI's UploadFile/Form depend on python-multipart; it is NOT pulled in
  # by fastapi itself, so the dashboard's multipart upload endpoint would 500
@@ -3,6 +3,9 @@
 Phase 0 — establish a baseline pin on the current (pre-OAuth) behavior so
 later phases can prove they didn't break loopback mode.
 """
+import asyncio
+import logging
+
 import pytest

 # Phase 5 / Phase 6: these tests mutate ``web_server.app.state.auth_required``
@@ -229,10 +232,102 @@ def test_start_server_gate_with_provider_proceeds_and_sets_proxy_headers(monkeyp
        assert web_server.app.state.auth_required is True
        assert captured["kwargs"].get("host") == "0.0.0.0"
        assert captured["kwargs"].get("proxy_headers") is True
+        assert captured["kwargs"].get("forwarded_allow_ips") == [
+            "127.0.0.1",
+            "::1",
+        ]
    finally:
        clear_providers()


+def test_start_server_passes_bounded_trusted_proxy_networks(monkeypatch, caplog):
+    """A configured proxy network reaches uvicorn without broadening to all peers."""
+    from hermes_cli.dashboard_auth import clear_providers, register_provider
+    from tests.hermes_cli.conftest_dashboard_auth import StubAuthProvider
+
+    clear_providers()
+    register_provider(StubAuthProvider())
+    captured = _stub_uvicorn_run(monkeypatch)
+    monkeypatch.setattr(
+        web_server,
+        "load_config",
+        lambda: {"dashboard": {"trusted_proxies": ["172.18.0.23/16"]}},
+    )
+    try:
+        with caplog.at_level(logging.INFO, logger=web_server._log.name):
+            web_server.start_server(
+                host="0.0.0.0", port=9119,
+                open_browser=False, allow_public=False,
+            )
+        assert captured["kwargs"]["forwarded_allow_ips"] == [
+            "127.0.0.1",
+            "::1",
+            "172.18.0.0/16",
+        ]
+        assert (
+            "Dashboard trusted proxies: 127.0.0.1, ::1, 172.18.0.0/16"
+            in caplog.text
+        )
+    finally:
+        clear_providers()
+
+
+def test_trusted_proxy_allowlist_rejects_unbounded_entries(caplog):
+    """Wildcard and whole-address-space trust must fail closed."""
+    trusted = web_server._dashboard_forwarded_allow_ips({
+        "trusted_proxies": ["*", "0.0.0.0/0", "::/0", "172.18.0.7"],
+    })
+
+    assert trusted == ["127.0.0.1", "::1", "172.18.0.7"]
+    assert "never '*' or a /0 network" in caplog.text
+
+
+def test_trusted_container_proxy_controls_https_detection():
+    """Only a configured bridge peer may turn X-Forwarded-Proto into HTTPS."""
+    from hermes_cli.dashboard_auth.cookies import detect_https
+    from starlette.requests import Request
+    from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware
+
+    trusted = web_server._dashboard_forwarded_allow_ips({
+        "trusted_proxies": ["172.18.0.0/16"],
+    })
+
+    async def detected_scheme(peer: str) -> bool:
+        observed: dict[str, bool] = {}
+
+        async def downstream(scope, receive, send):
+            observed["https"] = detect_https(Request(scope))
+
+        middleware = ProxyHeadersMiddleware(downstream, trusted_hosts=trusted)
+        scope = {
+            "type": "http",
+            "asgi": {"version": "3.0"},
+            "http_version": "1.1",
+            "method": "GET",
+            "scheme": "http",
+            "path": "/auth/login",
+            "raw_path": b"/auth/login",
+            "query_string": b"",
+            "root_path": "",
+            "headers": [(b"x-forwarded-proto", b"https")],
+            "client": (peer, 43120),
+            "server": ("hermes", 9119),
+        }
+
+        async def receive():
+            return {"type": "http.disconnect"}
+
+        async def send(message):
+            return None
+
+        await middleware(scope, receive, send)
+        return observed["https"]
+
+    assert asyncio.run(detected_scheme("172.18.0.9")) is True
+    assert asyncio.run(detected_scheme("::1")) is True
+    assert asyncio.run(detected_scheme("198.51.100.9")) is False
+
+
 def test_public_url_aware_gate_requires_auth_for_loopback_proxy(monkeypatch):
    """The shared gate decision includes an external browser-facing URL."""
    from hermes_cli.web_server import should_require_dashboard_auth
@@ -1942,7 +1942,7 @@ requires-dist = [
    { name = "ty", marker = "extra == 'dev'", specifier = "==0.0.21" },
    { name = "tzdata", marker = "sys_platform == 'win32'", specifier = "==2025.3" },
    { name = "urllib3", specifier = ">=2.7.0,<3" },
-    { name = "uvicorn", extras = ["standard"], specifier = ">=0.24.0,<1" },
+    { name = "uvicorn", extras = ["standard"], specifier = ">=0.31.0,<1" },
    { name = "uvicorn", extras = ["standard"], marker = "extra == 'web'", specifier = "==0.41.0" },
    { name = "vercel", marker = "extra == 'vercel'", specifier = "==0.7.2" },
    { name = "websockets", specifier = "==15.0.1" },
@@ -2630,6 +2630,7 @@ dashboard:
  theme: "default"            # "default" | "midnight" | "ember" | "mono" | "cyberpunk" | "rose"
  show_token_analytics: false # Re-enable the (local-estimate-only) token/cost analytics surfaces
  public_url: ""              # Full public authority for OAuth redirect_uri (env: HERMES_DASHBOARD_PUBLIC_URL)
+  trusted_proxies: []         # Proxy IPs/CIDRs allowed to supply X-Forwarded-* headers
  oauth:                      # Portal OAuth gate (engaged with --host and not --insecure)
    client_id: ""             # agent:{instance_id} — Portal provisions this
    portal_url: ""            # blank → plugin default (production Portal)
@@ -2651,6 +2652,7 @@ dashboard:
 - `theme` — dashboard visual theme.
 - `show_token_analytics` — off by default. The Analytics page and token/cost figures are a **local lower-bound estimate** (they exclude auxiliary calls, retries, fallbacks, and cache writes), so they can read far below the provider bill. Set `true` only if you understand they're not billing.
 - `public_url` — when set, this is the complete authority (scheme + host + optional path prefix) the OAuth `redirect_uri` is built from. Set it for deploys behind reverse proxies that don't reliably forward `X-Forwarded-*` headers. Leave empty to use proxy-header reconstruction.
+- `trusted_proxies` — IP addresses or bounded CIDR networks allowed to supply `X-Forwarded-Proto` and `X-Forwarded-For`. Loopback remains trusted automatically. Configure this when the TLS reverse proxy connects from another container or host. Prefer the proxy's exact IP; use a small dedicated network only when its address is dynamic. Wildcards and `/0` networks are rejected.
 - `oauth` / `basic_auth` / `drain_auth` — auth provider config read by the bundled dashboard-auth plugins. The drain secret itself is **not** set here; it's provisioned via the `HERMES_DASHBOARD_DRAIN_SECRET` env var. See [Web Dashboard](/user-guide/features/web-dashboard) for full auth setup.
 - `ws_ping_interval` / `ws_ping_timeout` — WebSocket keepalive tuning for non-loopback binds (loopback connections never ping). Raise these on high-latency links (Tailscale, distant SSH tunnels) where the 20 s defaults can manufacture spurious 1006 disconnects.
 - `ws_orphan_reap_grace_s` — how long a WS-detached session waits before the orphan reaper collects it. Raise alongside the keepalive values if clients reconnect slowly. (`HERMES_TUI_WS_ORPHAN_REAP_GRACE_S` remains as an internal override.)
@@ -138,6 +138,24 @@ There are three bundled ways to satisfy the second condition:

 Whichever you choose, the gate redirects callers to a login page before they can reach any protected route. See [Web Dashboard → Authentication](features/web-dashboard.md#authentication-gated-mode) for all three providers.

+When a reverse proxy such as Traefik or nginx runs in another container, its
+bridge-network address is not trusted by default. Set the dashboard's public
+URL and trust only that proxy's exact IP, or a bounded CIDR for a dedicated
+proxy network, in the mounted `config.yaml`:
+
+```yaml
+dashboard:
+  public_url: "https://dashboard.example.com"
+  trusted_proxies:
+    - "172.20.0.5"
+    # Or, if the proxy address is dynamic on a dedicated network:
+    # - "172.20.0.0/24"
+```
+
+This allows the proxy's `X-Forwarded-Proto: https` to control secure OAuth
+cookies while leaving forwarding headers from other peers untrusted. Do not
+use `*`, `0.0.0.0/0`, or `::/0`; Hermes rejects those unbounded entries.
+
 If no provider is registered and the bind is non-loopback, the dashboard **fails closed at startup** with a specific error pointing at the missing env var. There is no longer an escape hatch that serves the dashboard unauthenticated on a public bind: `HERMES_DASHBOARD_INSECURE=1` is now a deprecated no-op (it logs a warning and is ignored). Configure a provider, or bind `HERMES_DASHBOARD_HOST=127.0.0.1` and reach the dashboard over an SSH tunnel / Tailscale instead.

 :::warning Why `--insecure` was removed
@@ -935,6 +935,8 @@ For deploys behind reverse proxies that don't reliably forward those headers (ma
 ```yaml
 dashboard:
  public_url: "https://dashboard.example.com/hermes"
+  trusted_proxies:
+    - "172.20.0.5"
 ```

 When set, the OAuth callback URL becomes `<public_url>/auth/callback` verbatim — `X-Forwarded-Prefix` is ignored on that code path because the operator has explicitly declared the public URL. This is intentional: stacking the prefix on top would double-prefix the common case where the prefix is already baked into `public_url`.
@@ -950,8 +952,23 @@ Declaring a non-loopback `public_url` always engages the dashboard auth gate,
 even when the backend binds to loopback. Configure a password or OAuth provider
 first; without one, Hermes fails closed at startup. This prevents the local SPA
 session token from becoming a remote authentication mechanism through the
-proxy. Uvicorn also enables trusted proxy-header processing in this mode so a
-local TLS terminator can supply `X-Forwarded-Proto: https` for secure cookies.
+proxy. Uvicorn also enables proxy-header processing in this mode. Loopback
+proxies are trusted automatically. If the TLS terminator connects from another
+container or host, add its exact IP address to `dashboard.trusted_proxies`, or
+add a bounded CIDR for a dedicated proxy network when the address is dynamic:
+
+```yaml
+dashboard:
+  public_url: "https://dashboard.example.com/hermes"
+  trusted_proxies:
+    - "172.20.0.0/24"
+```
+
+Only listed peers may supply `X-Forwarded-Proto` and `X-Forwarded-For`.
+Hermes always preserves loopback trust and rejects `*`, `0.0.0.0/0`, and
+`::/0`. Trusting a network means every container or machine on that network
+can supply forwarding metadata, so prefer an exact proxy IP or a dedicated
+proxy-only network.

 ```bash
 # Backend remains reachable only on this machine.
@@ -978,7 +995,7 @@ Same precedence as the other dashboard settings — env wins over `config.yaml`:

 Validation rejects values without `http://` / `https://` scheme, without a host, or containing quote / angle / whitespace / control characters. A malformed value silently falls through to header reconstruction so the login flow keeps working rather than dispatching the user to a hostile URL.

-> **Note:** `public_url` overrides the OAuth callback URL only. The `Secure` cookie flag is still controlled by `request.url.scheme` (X-Forwarded-Proto under proxy_headers), so an `http://` `public_url` on a TLS-terminated public deploy will produce non-Secure cookies. This is an operator footgun — pair `public_url` with proper TLS termination upstream.
+> **Note:** `public_url` overrides the OAuth callback URL only. The `Secure` cookie flag is still controlled by `request.url.scheme`, using `X-Forwarded-Proto` only when the connecting peer is loopback or listed in `trusted_proxies`. Pair an HTTPS `public_url` with TLS termination and a bounded trusted-proxy entry when the proxy is not on loopback.

 ### OAuth flow