Skip to content

fix(dashboard): trust configured public URL host - #42344

Open
StartupBros wants to merge 1 commit into
NousResearch:mainfrom
StartupBros-com:fix/dashboard-trusted-proxy-hosts
Open

fix(dashboard): trust configured public URL host#42344
StartupBros wants to merge 1 commit into
NousResearch:mainfrom
StartupBros-com:fix/dashboard-trusted-proxy-hosts

Conversation

@StartupBros

@StartupBros StartupBros commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • reuse existing dashboard.public_url / HERMES_DASHBOARD_PUBLIC_URL as the canonical trusted reverse-proxy dashboard host
  • keep the DNS-rebinding Host/Origin guard exact-match and fail-closed for malformed or suffix-trick Host headers
  • add unit, HTTP middleware, and WebSocket coverage for public-url host acceptance and attacker-host rejection

Why

Trusted reverse proxies such as Tailscale Serve can keep the dashboard bound to 127.0.0.1 while preserving the browser-facing Host header. In that deployment, requests arrive as dashboard.tailnet.example.ts.net even though the server is bound to loopback, so the current Host guard rejects legitimate dashboard traffic with Invalid Host header.

dashboard.public_url already declares the browser-facing dashboard URL for OAuth redirects. This patch reuses its hostname for the dashboard Host/Origin guard instead of adding a second env-only allowlist. Matching remains exact; wildcards and suffixes like dashboard.example.evil.example are still rejected.

Tests

  • uv run --with pytest --with pytest-timeout pytest tests/hermes_cli/test_web_server_host_header.py -q
  • uv run --with pytest --with pytest-timeout pytest tests/hermes_cli/test_web_server_host_header.py tests/hermes_cli/test_dashboard_auth_prefix.py tests/hermes_cli/test_dashboard_auth_ws_auth.py tests/hermes_cli/test_dashboard_auth_gate.py -q
  • uv run ruff check hermes_cli/web_server.py hermes_cli/config.py tests/hermes_cli/test_web_server_host_header.py

@alt-glitch alt-glitch added type/feature New feature or request comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels Jun 8, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Clean review — no issues found.

What's good:

  • Exact-match semantics for the allowlist (no wildcards) — DNS-rebinding attacker hosts still fail closed
  • _host_header_hostname() correctly handles IPv6 bracket notation, port stripping, and URL-prefix inputs
  • Negative test cases cover suffix attacks (dashboard.tailnet.example.ts.net.evil.example) and unrelated hosts
  • Clean refactor: extracted the hostname normalization from the inline IPv4/IPv6 port-stripping logic, reducing duplication

One observation (not a blocker): HERMES_DASHBOARD_ADDITIONAL_HOSTS is env-var only with no config.yaml equivalent. This is fine for a trusted-proxy use case where operators control the process environment, but worth documenting in the config reference if it gets wider adoption.

@StartupBros
StartupBros force-pushed the fix/dashboard-trusted-proxy-hosts branch from 3bcc441 to c453467 Compare June 8, 2026 21:17
@StartupBros StartupBros changed the title fix(dashboard): allow trusted proxy host headers fix(dashboard): trust configured public URL host Jun 8, 2026
@StartupBros

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I agreed with the env-only observation and reworked the PR accordingly: it no longer introduces HERMES_DASHBOARD_ADDITIONAL_HOSTS. The patch now reuses the existing dashboard.public_url / HERMES_DASHBOARD_PUBLIC_URL contract as the single browser-facing dashboard URL, and adds HTTP + WebSocket tests for that path.

@alt-glitch alt-glitch added type/bug Something isn't working comp/dashboard Web dashboard / control panel UI (dashboard/, landing) sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data and removed type/feature New feature or request labels Jun 26, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused reverse-proxy fix. Current main still rejects the configured public hostname for both HTTP and WebSocket handshakes: hermes_cli/web_server.py:415-456 only accepts loopback aliases for a loopback bind, and the WebSocket guard reuses that helper at hermes_cli/web_server.py:14390-14409.

Problems

  • dashboard.public_url gains a security-relevant Host/Origin trust meaning, but its public documentation remains OAuth-only. website/docs/user-guide/features/web-dashboard.md:911 currently states that public_url “overrides the OAuth callback URL only”; that statement would be inaccurate after this patch. The environment-variable reference likewise describes HERMES_DASHBOARD_PUBLIC_URL only as OAuth callback construction at website/docs/reference/environment-variables.md:502.

Suggested changes

  • Update those documentation entries to describe exact configured-host acceptance for loopback-bound reverse-proxy deployments, alongside the existing OAuth behavior and malformed-value fallback.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@aSchaber2026

Copy link
Copy Markdown

Tested this PR's intended design in a real Tailscale Serve deployment and ported commit c45346712 onto current origin/main (9fc12bf7). The design works as intended: loopback-bound dashboard, TLS-terminated reverse proxy, authenticated HTTPS + WSS, exact public hostname, unrelated hosts rejected. The same deployment passed backend restart, sleep/wake, and home-network → mobile hotspot → home-network transitions without changing the Desktop URL.

Current-main port notes:

  • The cherry-pick now conflicts in hermes_cli/config.py because defaults moved to hermes_cli/config_defaults.py. Resolve config.py entirely in favor of current main; dashboard.public_url already exists in config_defaults.py.
  • In hermes_cli/web_server.py, retain current main's should_require_auth(host, allow_public=False) signature/body and add the PR's _host_header_hostname(), _dashboard_public_hostname(), and _is_accepted_host() changes.
  • The test file applies cleanly.

Verification on that resolved current-main port:

uv run --with ruff ruff check hermes_cli/web_server.py tests/hermes_cli/test_web_server_host_header.py
All checks passed!

pytest tests/hermes_cli/test_web_server_host_header.py -o addopts= -q
13 passed

git diff --check
clean

I also applied the sweeper's requested documentation update locally. The needed wording changes are:

  1. website/docs/user-guide/features/web-dashboard.md: state that public_url also declares the exact HTTP Host/WebSocket Origin accepted for a loopback-bound reverse proxy, while unrelated/suffix hosts remain rejected; replace the OAuth-only note accordingly.
  2. website/docs/reference/environment-variables.md: describe HERMES_DASHBOARD_PUBLIC_URL as both OAuth callback input and exact Host/Origin trust declaration.
  3. website/docs/user-guide/configuration.md: update the dashboard.public_url example/comment and detailed bullet with the same exact-match semantics.

This PR is the right upstream shape for Tailscale Serve: it reuses the existing validated public_url contract and does not add an env-only behavioral allowlist.

@alt-glitch alt-glitch added P2 Medium — degraded but workaround exists area/config Config system, migrations, profiles needs-decision Awaiting maintainer decision before any implementation and removed P3 Low — cosmetic, nice to have labels Aug 1, 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) needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists 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/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants