Skip to content

fix(dashboard): secure loopback public URL proxy mode - #72127

Open
e-macgregor wants to merge 1 commit into
NousResearch:mainfrom
e-macgregor:fix/dashboard-loopback-public-url-auth
Open

fix(dashboard): secure loopback public URL proxy mode#72127
e-macgregor wants to merge 1 commit into
NousResearch:mainfrom
e-macgregor:fix/dashboard-loopback-public-url-auth

Conversation

@e-macgregor

@e-macgregor e-macgregor commented Jul 26, 2026

Copy link
Copy Markdown

Summary

  • Accept the exact hostname from dashboard.public_url as a trusted dashboard Host/Origin when the backend is bound to loopback.
  • Force the dashboard auth gate whenever a configured browser-facing URL exposes a loopback backend, and fail closed when no auth provider is registered.
  • Share the public-URL-aware gate decision with interactive auth setup so TTY operators are prompted before startup.
  • Apply the same boundary to HTTP and WebSocket upgrades, with regression coverage and docs for reverse-proxy deployments.

Testing

  • PYTHONPATH="$PWD" python -m pytest tests/hermes_cli/test_dashboard_auth_*.py tests/hermes_cli/test_dashboard_unified_launch.py tests/hermes_cli/test_web_server_host_header.py -o 'addopts=' -q — 142 passed
  • ruff check on all changed Python files
  • py_compile on all changed Python files
  • git diff --check origin/main...HEAD

Notes

  • This keeps DNS-rebinding Host validation enabled; it adds only an exact operator-declared browser-facing hostname.
  • Tailscale Serve is one motivating deployment: tailnet-only HTTPS on a *.ts.net hostname can proxy to a loopback dashboard without exposing it to the public internet.
  • This does not rely on wildcard hosts, 0.0.0.0, or unconditional trust in forwarded headers.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/config Config system, migrations, profiles P3 Low — cosmetic, nice to have needs-decision Awaiting maintainer decision before any implementation sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 26, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #42344 and #68251 implement the same configured-public-host family. This PR additionally forces the auth gate for an external declared public URL and covers HTTP plus WebSocket validation, so the maintainer should choose the intended proxy/auth contract rather than treating the implementations as duplicates.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused reverse-proxy hardening. The underlying gap is present on current main: HTTP validation only accepts loopback aliases for loopback binds at hermes_cli/web_server.py:464-505, and the WebSocket guard reuses that validator at hermes_cli/web_server.py:14353-14372.

Problems

  • The interactive auth setup path is not updated. hermes_cli/main.py:9808-9830 exits early for every loopback bind based only on should_require_auth(host), whereas this PR makes a loopback bind gated when dashboard.public_url declares an external hostname. A TTY operator with no provider would skip the setup prompt and then hit start_server()'s fail-closed error.

Suggested changes

  • Share the public-URL-aware gate decision with _maybe_setup_dashboard_auth_interactively() and add a regression test for that loopback-proxy configuration.
  • Relocate the hermes_cli/config.py comment hunk during salvage; current main has moved that surrounding configuration code.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
@e-macgregor
e-macgregor force-pushed the fix/dashboard-loopback-public-url-auth branch from 92daadf to 9d2915a Compare July 30, 2026 14:54
@e-macgregor

Copy link
Copy Markdown
Author

Addressed in 9d2915a

  • Added should_require_dashboard_auth() as the shared public-URL-aware gate decision and reused it in both start_server() and _maybe_setup_dashboard_auth_interactively().
  • Added regression coverage for an interactive loopback bind with an external dashboard.public_url, plus local-only compatibility and fail-closed startup coverage.
  • Relocated the configuration comment to hermes_cli/config_defaults.py after rebasing onto current main.
  • Documented Tailscale Serve as an example: tailnet-only HTTPS on a *.ts.net hostname can proxy to a loopback dashboard without making it public on the internet. The exact configured hostname remains protected by the auth gate and DNS-rebinding checks.

Validation

  • 142 targeted tests passed on current main.
  • Ruff passed.
  • py_compile passed.
  • git diff --check passed.

@egilewski

Copy link
Copy Markdown
Contributor

looks mergeable

The PR coherently closes the loopback reverse-proxy exposure: it snapshots the operator-declared public hostname, enables the existing auth gate before serving, removes SPA token injection in gated mode, and applies the same exact-host rule to HTTP and every dashboard WebSocket upgrade. Targeted gate, host, and unified-launch tests plus bytecode compilation passed; no source-backed security finding remains.

Security evidence:

  • trust boundary: The browser or public reverse proxy is untrusted at the request boundary; Host and Origin headers are attacker-controlled. The bind host and operator-declared dashboard.public_url are configuration trust inputs. The loopback reverse-proxy path must therefore use the public hostname only as an exact routing identity and must require the existing dashboard auth gate.
  • source/sink/invariant: Untrusted HTTP Host and WebSocket Origin values flow through _host_header_hostname and _is_accepted_host; only the bound host, loopback aliases, wildcard-bind policy, or the startup snapshot from resolve_public_url are accepted. start_server computes auth_required before binding, refuses a gated start with no provider, and the gated index path omits the legacy _SESSION_TOKEN.
  • current-main reproduction: Current-main source inspection shows start_server assigned auth_required from should_require_auth(host) alone and _is_accepted_host had no public-url allowlist. A loopback bind consequently remained token mode while an external proxy Host was rejected by the DNS-rebinding guard; the replay adds the missing gate and exact host snapshot.
  • PR-head or patch-replay validation: The reviewed tree is current-main HEAD with the PR patch applied in the staged replay. Four added auth-gate tests, two host-authority tests, one interactive-launch test, and compileall all completed successfully. The added WebSocket behavior was also exercised with a focused source-level probe.
  • positive/negative cases: Positive cases cover an external public URL on a loopback bind (gate enabled and declared host accepted) and a loopback public URL (local mode preserved). Negative cases cover suffix hosts, malformed authorities, evil origins, missing providers, and the old bind-only predicates; the new parser rejects them or the startup gate fails closed.
  • residual bypass search: Inspected HTTP middleware ordering, public API paths, cookie and token branches, URL/IPv6/port parsing, and all dashboard WebSocket handlers: audio speak-stream, console, PTY, gateway, publish, and events. Each reaches the shared auth and Host/Origin checks; no alternate route bypass was found.
  • reviewer validation: Performed an independent line-level review of the changed main, web-server, configuration, documentation, and test code, compared the current-main implementation, and adjudicated every inventory candidate against its source observation. No source-backed security issue remains.

Review setup: I reviewed a run-owned local rebase or patch replay against current GitHub main because the submitted branch is stale or conflicted; this does not mean the submitted branch itself merges cleanly.

Not checked:

  • Full target test-file suite
  • Live TLS reverse-proxy integration
  • ruff lint
  • CodeRabbit review

Signed: GPT-5.6-luna-max in Codex

@alt-glitch alt-glitch removed the needs-decision Awaiting maintainer decision before any implementation label Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants