Skip to content

fix(web_server): pass proxy_headers=False to uvicorn.run so the dashboard's loopback gate sees the real connection peer - #26834

Closed
davidcampbelldc wants to merge 1 commit into
NousResearch:mainfrom
davidcampbelldc:fix/uvicorn-proxy-headers-loopback-gate
Closed

fix(web_server): pass proxy_headers=False to uvicorn.run so the dashboard's loopback gate sees the real connection peer#26834
davidcampbelldc wants to merge 1 commit into
NousResearch:mainfrom
davidcampbelldc:fix/uvicorn-proxy-headers-loopback-gate

Conversation

@davidcampbelldc

Copy link
Copy Markdown
Contributor

Summary

_ws_client_is_allowed() enforces a loopback-only client check on every dashboard WebSocket upgrade (/api/ws, /api/events, /api/pty, /api/pub). The intent is: when bound to 127.0.0.1, only accept WS upgrades from loopback peers; public bind (--insecure) trades that for token-only auth.

However, uvicorn.run(app, host=host, port=port, log_level="warning") omits proxy_headers. In modern uvicorn (>= 0.20), proxy_headers defaults to True and forwarded_allow_ips defaults to "127.0.0.1". With those defaults, any reverse proxy connecting from loopback (nginx, in-cluster proxy, Cloudflare Tunnel sidecar in HTTP mode, K8s ingress-nginx) causes uvicorn to rewrite ws.client.host from the request's X-Forwarded-For header. So _ws_client_is_allowed sees a non-loopback client.host and returns False, closing every browser WS with code 4403 (which surfaces as HTTP 403 to the proxy).

Passing proxy_headers=False keeps the loopback gate's view of ws.client.host at the immediate transport peer (the proxy on 127.0.0.1), which is exactly what the gate is designed to check.

Reproduction

# On a host running the dashboard with --host 127.0.0.1, with the live session token in \$TOKEN:
curl -i -H "Connection: Upgrade" -H "Upgrade: websocket" \
     -H "Sec-WebSocket-Version: 13" -H "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==" \
     -H "X-Forwarded-For: 1.2.3.4" \
     "http://127.0.0.1:9119/api/ws?token=\$TOKEN"
Before this patch After this patch
With X-Forwarded-For: 1.2.3.4 HTTP/1.1 403 Forbidden HTTP/1.1 101 Switching Protocols
Without the header HTTP/1.1 101 HTTP/1.1 101

Single-variable trigger confirmed.

Real-world impact

Any Hermes deployment that uses a reverse proxy with default X-Forwarded-For forwarding (nginx's \$proxy_add_x_forwarded_for, Traefik, K8s ingress-nginx, Cloudflare Tunnel sidecars in HTTP mode) sees this as: chat tab opens, events feed banner shows "disconnected — tool calls may not appear", /api/ws + /api/events + /api/pty all return HTTP 403 from the dashboard. Browsers report NS_ERROR_WEBSOCKET_CONNECTION_REFUSED (Firefox) or equivalent.

The bug is invisible in development (no reverse proxy → no X-Forwarded-Forws.client.host stays at the real loopback peer). It surfaces only in proxied production deployments — which is exactly the case the loopback gate is meant to handle safely.

Alternative considered

Reading ws.scope["client"] (ASGI transport peer, not rewritten by uvicorn's proxy_headers logic) inside _ws_client_is_allowed:

```python
def _ws_client_is_allowed(ws):
if _is_public_bind():
return True
scope_client = ws.scope.get("client")
client_host = scope_client[0] if scope_client else ""
if not client_host:
return True
return client_host in _LOOPBACK_HOSTS
```

Arguably more correct (expresses intent — "check the transport-layer peer" — without depending on uvicorn config), but it's a larger semantic change, only protects the WS gate, and leaves HTTP-side handlers (which trust request.client.host for client-IP logging etc.) intact. The proxy_headers=False change is more conservative and globally consistent. Happy to send either or both — let me know.

Tests

The existing tests/hermes_cli/test_web_server.py suite uses Starlette's TestClient, which sets ws.client.host to "testclient" (already in _LOOPBACK_HOSTS) and bypasses uvicorn entirely. Existing tests pass either way. Could add a regression test that monkeypatches ws.client.host to a non-loopback value to lock in the fix — happy to include if useful.

Discovered by

Diagnosing why the Hermes dashboard at mandy.loadmagic.ai (behind nginx + Cloudflare Tunnel + CF Access) refused all browser WS upgrades despite Access app config (SameSite=None / binding cookie / HttpOnly) matching a known-working sibling deployment. Three-way bisect — through nginx with curl + on-box with curl + same-token comparison — narrowed to the XFF header as the single triggering variable.

🤖 Generated with Claude Code

…oard's loopback gate sees the real connection peer

`_ws_client_is_allowed()` enforces a loopback-only client check on every
dashboard WebSocket upgrade (`/api/ws`, `/api/events`, `/api/pty`,
`/api/pub`):

    def _ws_client_is_allowed(ws):
        if _is_public_bind():
            return True
        client_host = ws.client.host if ws.client else ""
        if not client_host:
            return True
        return client_host in _LOOPBACK_HOSTS

The intent is: when bound to 127.0.0.1, only accept WS upgrades from
loopback peers. Public bind (--insecure) trades that for token-only.

However, `uvicorn.run(app, host=host, port=port, log_level="warning")`
omits `proxy_headers`. In modern uvicorn (>= 0.20) `proxy_headers`
defaults to True and `forwarded_allow_ips` defaults to "127.0.0.1".
With those defaults, any reverse proxy connecting from loopback (nginx,
in-cluster proxy, Cloudflare Tunnel sidecar in HTTP mode, K8s
ingress-nginx) causes uvicorn to rewrite `ws.client.host` from the
request's `X-Forwarded-For` header. So the gate sees the original
client's IP (a public address) instead of the loopback peer, returns
False, and closes every browser WS with code=4403 (surfaces as HTTP
403 to the proxy).

Passing `proxy_headers=False` keeps the loopback gate's view of
`ws.client.host` at the immediate transport peer (the proxy on
127.0.0.1), which is exactly what the gate is designed to check.

The bug is invisible in dev (no proxy → no XFF → ws.client.host stays
loopback). It surfaces in proxied production: dashboard chat tab opens,
events feed banner shows "disconnected — tool calls may not appear",
all WS endpoints return 403. Reproduces with:

    curl -i -H "Connection: Upgrade" -H "Upgrade: websocket" \
         -H "Sec-WebSocket-Version: 13" -H "Sec-WebSocket-Key: ..." \
         -H "X-Forwarded-For: 1.2.3.4" \
         "http://127.0.0.1:9119/api/ws?token=\$TOKEN"
    # Before: HTTP/1.1 403 Forbidden
    # After:  HTTP/1.1 101 Switching Protocols

Without the XFF header, both behave the same (101) — confirming the
single-variable trigger.

Discovered while diagnosing why the Hermes dashboard at
mandy.loadmagic.ai (behind nginx + Cloudflare Tunnel + CF Access)
refused all browser WS upgrades despite Access app config matching a
known-working sibling deployment (Simone, which doesn't have nginx in
the path).
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists labels May 16, 2026
teknium1 added a commit that referenced this pull request May 17, 2026
…tors

Final LHF run group. Adds release-note attribution mappings for:
- @bird (PR #25219)
- @davidcampbelldc (PR #26834)

(zccyman, wesleysimplicio already mapped from prior groups.)
teknium1 added a commit that referenced this pull request May 17, 2026
…tors

Final LHF run group. Adds release-note attribution mappings for:
- @bird (PR #25219)
- @davidcampbelldc (PR #26834)

(zccyman, wesleysimplicio already mapped from prior groups.)
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #27550 — your commit was cherry-picked onto current main as part of the final batch of the May LHF salvage run. Authorship preserved (fix(web_server): pass proxy_headers=False to uvicorn.run). Thanks for the contribution.

@teknium1 teknium1 closed this May 17, 2026
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
…tors

Final LHF run group. Adds release-note attribution mappings for:
- @bird (PR NousResearch#25219)
- @davidcampbelldc (PR NousResearch#26834)

(zccyman, wesleysimplicio already mapped from prior groups.)
Seven74AI pushed a commit to Seven74AI/hermes-agent that referenced this pull request Jun 13, 2026
…tors

Final LHF run group. Adds release-note attribution mappings for:
- @bird (PR NousResearch#25219)
- @davidcampbelldc (PR NousResearch#26834)

(zccyman, wesleysimplicio already mapped from prior groups.)
alt-glitch pushed a commit that referenced this pull request Jun 14, 2026
…tors

Final LHF run group. Adds release-note attribution mappings for:
- @bird (PR #25219)
- @davidcampbelldc (PR #26834)

(zccyman, wesleysimplicio already mapped from prior groups.)
T02200059 pushed a commit to T02200059/hermes-agent that referenced this pull request Jun 18, 2026
…tors

Final LHF run group. Adds release-note attribution mappings for:
- @bird (PR NousResearch#25219)
- @davidcampbelldc (PR NousResearch#26834)

(zccyman, wesleysimplicio already mapped from prior groups.)
liuchanchen pushed a commit to liuchanchen/hermes-agent that referenced this pull request Jun 23, 2026
…tors

Final LHF run group. Adds release-note attribution mappings for:
- @bird (PR NousResearch#25219)
- @davidcampbelldc (PR NousResearch#26834)

(zccyman, wesleysimplicio already mapped from prior groups.)
donbowman pushed a commit to donbowman/hermes-agent that referenced this pull request Jul 13, 2026
…tors

Final LHF run group. Adds release-note attribution mappings for:
- @bird (PR NousResearch#25219)
- @davidcampbelldc (PR NousResearch#26834)

(zccyman, wesleysimplicio already mapped from prior groups.)
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…tors

Final LHF run group. Adds release-note attribution mappings for:
- @bird (PR NousResearch#25219)
- @davidcampbelldc (PR NousResearch#26834)

(zccyman, wesleysimplicio already mapped from prior groups.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants