Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing a real reverse-proxy deployment gap. Current main rejects a proxy hostname on a loopback bind in hermes_cli/web_server.py:499-502, and applies the same check to WebSocket Host/Origin headers at hermes_cli/web_server.py:14388-14407.
Problems
- The headless exemption is only threaded into
start_server(). The earlier interactive preflight still callsshould_require_auth(host)athermes_cli/main.py:9905-9907. With extra hosts configured, an interactivehermes servecan enter auth setup even though the later server call is headless and leaves the gate off. HERMES_DASHBOARD_EXTRA_HOSTSis a new non-secret behavioral environment variable.AGENTS.md:102-105requires this kind of setting to useconfig.yamlinstead.- The PR changes only
hermes_cli/web_server.py; it does not adddashboard.extra_hoststohermes_cli/config_defaults.py, document it inwebsite/docs/user-guide/configuration.md, or add coverage for HTTP, WebSocket, gated dashboard, and headless-serve behavior.
Suggested changes
- Propagate the headless value through the auth preflight and test both dashboard and serve paths.
- Keep this as a documented
dashboard.extra_hostsconfig setting, without the new environment variable. - Add regression tests for listed and unlisted Host/Origin values and the auth/token split.
Automated hermes-sweeper review.
| @@ -17096,7 +17150,7 @@ def start_server( | |||
| # injection / WS-auth paths can branch on it consistently. Phase 3.5 | |||
There was a problem hiding this comment.
start_server() now passes headless, but hermes_cli/main.py:9905-9907 still calls should_require_auth(host) without it before this point. With dashboard.extra_hosts set, an interactive hermes serve enters auth setup even though this call sets auth_required=False. Thread the same headless value into that preflight and cover both paths.
There was a problem hiding this comment.
Fixed in d4d3a3d — the preflight now threads headless=getattr(args, "headless_backend", False) into should_require_auth, matching start_server. Both paths (serve no-prompt, dashboard prompt) covered by test_preflight_threads_headless_into_should_require_auth.
…sts) A loopback-bound dashboard behind a trusted reverse proxy (e.g. tailscale serve -> 127.0.0.1:9119) is unusable today: the browser's Host header carries the proxy hostname, which the DNS-rebinding allowlist rejects. And once the proxy is let through, "loopback bind == trusted operator" no longer holds — the dashboard is remote-reachable and must be auth-gated. - HERMES_DASHBOARD_EXTRA_HOSTS env var + dashboard.extra_hosts config list: operator-designated Host values accepted on any bind. Rebinding protection is preserved for every unlisted name. - Declaring any extra host re-engages the auth gate even on loopback binds (should_require_auth), closing the proxied-but-unauthenticated hole. - Scope that rule to non-headless (dashboard) processes. The headless serve path — the desktop app's own backend on an ephemeral loopback port the proxy never fronts — keeps the loopback token path; gating it breaks the desktop client, whose WS handshake uses the legacy ?token= credential that gated mode unconditionally rejects (_ws_auth_reason). A non-loopback headless bind still engages the gate via the existing fallback.
efbb6c6 to
16c206a
Compare
|
suggesting changes The extra-host path correctly rejects unlisted names and enables gated authentication for the normal dashboard, but the headless exception splits the security invariant: a configured proxy hostname is still accepted by Host and WebSocket-origin validation while headless loopback startup leaves
Security evidence:
Signed: GPT-5.6-sol-xhigh in Codex |
…s into preflight Review follow-up: - teknium1: _maybe_setup_dashboard_auth_interactively now calls should_require_auth(host, headless=...) with the same headless value start_server receives, so interactive 'hermes serve' no longer enters auth setup for a gate that will be off (and 'hermes dashboard' still prompts when extra_hosts engage the gate). - egilewski: Host acceptance and auth scoping are now atomic. _is_accepted_host() takes headless and rejects configured extra hosts for headless processes; app.state.headless is set by start_server and consulted by the HTTP middleware and _ws_host_origin_reason. A proxy routed to a headless loopback serve now gets 400/host_mismatch instead of reaching /api/status with auth_required=False. Rejection chosen over engaging the gate for headless: gated mode rejects the desktop app's legacy ?token= WS credential, and headless serve is never proxy-fronted by design. Tests: 11 new cases in test_dashboard_auth_gate.py (truth-table rows, _is_accepted_host accept/reject, HTTP 400 asserting no local-only /api/status fields, WS host_mismatch, preflight threading spy). scripts/run_tests.sh green on all four files touching these symbols.
|
Follow-up commit d4d3a3d addresses both reviews. @teknium1 — preflight threading: @egilewski — split invariant: closed by making Host acceptance and auth scoping atomic. I chose rejection over engaging the gate for headless (your either/or): gated mode unconditionally rejects the desktop app's legacy Tests: 11 new cases in |
|
Verification (swarm dedup campaign, 2026-08-03) — green-light with evidence, tested against current main
Residual asks from the sweeper review (
The security invariant itself (auth gate re-engages when extra hosts are declared; headless serve rejects proxy hosts at the boundary so /api/status never leaks local-only fields remotely) is verified correct. Recommend merge once 1-3 land. |
|
looks mergeable The security follow-up keeps Host acceptance and auth scoping atomic. Configured proxy hosts engage authentication for the interactive dashboard, while a headless loopback server rejects the same Host at both the HTTP and WebSocket Host/Origin boundaries; the interactive preflight now carries the same headless value into gate selection. Security evidence:
Not checked:
Signed: GPT-5.6-sol-xhigh in Codex |
|
I prepared a local follow-up patch for the three remaining configuration and documentation items identified in the review thread, based on this PR's current head, The patch keeps the existing Host/Origin and headless-auth design intact and only:
Patch relative to the PR head: Verification on the exact PR head plus this patch: Also passed:
I did not run the full repository suite. The local patch SHA-256 is: If this follow-up is useful, I can either paste the raw patch here or publish a commit on my fork for the author to cherry-pick. |
Problem
A loopback-bound dashboard fronted by a trusted reverse proxy (e.g.
tailscale serve→127.0.0.1:9119) is unusable: the browser'sHostheader carries the proxy hostname, which the DNS-rebinding allowlist rejects.And if you do let the proxy through, "loopback bind == trusted operator" no longer holds — the dashboard is remote-reachable and must be auth-gated.
Change
HERMES_DASHBOARD_EXTRA_HOSTSenv var +dashboard.extra_hostsconfig list: operator-designatedHostvalues accepted on any bind. Rebinding protection is preserved for every unlisted name.should_require_auth), closing the proxied-but-unauthenticated hole.headlessserve path — the desktop app's own backend on an ephemeral loopback port the proxy never fronts — keeps the loopback token path. Gating it breaks the desktop client, whose WS handshake uses the legacy?token=credential that gated mode unconditionally rejects (_ws_auth_reason). A non-loopback headless bind still engages the gate via the existing fallback.Verification
On a real install: the tailscale-proxied dashboard passes Host validation and requires auth; the desktop app boots and connects normally; a loopback dashboard with no extra hosts behaves exactly as before.
The headless scoping was found the hard way — without it the desktop app hard-fails at boot with "the WebSocket rejected the session token".