Skip to content

feat(dashboard): allow extra hostnames in Host validator via env var - #29195

Closed
StartupBros wants to merge 1 commit into
NousResearch:mainfrom
StartupBros-com:feat/dashboard-additional-hosts
Closed

StartupBros wants to merge 1 commit into
NousResearch:mainfrom
StartupBros-com:feat/dashboard-additional-hosts

Conversation

@StartupBros

@StartupBros StartupBros commented May 20, 2026

Copy link
Copy Markdown
Contributor

Problem

The dashboard's anti-DNS-rebinding Host validator (hermes_cli/web_server.py::_is_accepted_host) currently accepts only the bound interface — plus loopback aliases when bound to loopback. That's correct for the default deployment but blocks operators who run the dashboard behind:

  • a reverse proxy (caddy, nginx, traefik, etc.) terminating on a public hostname
  • tailscale serve mapping a public tailnet hostname back to 127.0.0.1:9119

In those setups the proxy forwards with the public hostname in the Host header, which the validator rejects with 400 "Invalid Host header. Dashboard requests must use the hostname the server was bound to."

Bypassing this today requires --insecure, which binds to all interfaces and exposes API keys on the network (the CLI help calls this out explicitly). That's a much larger blast radius than the operator needs — they just want loopback bind + one or two known proxy hostnames accepted.

Solution

A minimal env-var-gated extension:

HERMES_DASHBOARD_ADDITIONAL_HOSTS=host1.example,host2.example

When set, the loopback-bound dashboard also accepts requests whose Host header matches one of the listed hostnames. The DNS-rebinding defence is preserved for any host NOT in the list, so an attacker hostname that TTL-flips to 127.0.0.1 still gets rejected.

The opt-in is per-operator (env var), not a config-file default — so the safe behavior remains the default and operators consciously add hostnames they trust.

Use case (concrete)

I run the dashboard loopback-bound on a Debian VM, then sudo tailscale serve --bg --https=443 http://127.0.0.1:9119 to make it reachable from my other tailnet machines. Tailscale ACLs gate access to the tailnet; the dashboard's loopback bind keeps the API-key blast radius small; the env var bridges the gap so the validator doesn't reject the tailscale-MagicDNS Host header.

Without this patch the choice is --insecure (defeats the security model) or a local fork (drift risk on every upgrade).

Test plan

A new TestHostHeaderAdditionalHosts class in tests/hermes_cli/test_web_server_host_header.py (the file annotated for GHSA-ppp5-vxwm-4cf7) covers:

  • Listed hosts accepted on every loopback bind (127.0.0.1, localhost, ::1), with and without port suffix
  • Core safety property: attacker hostnames still rejected with env var set — including suffix attacks (magic.tailnet.ts.net.attacker.test) and subdomain attacks (attacker.magic.tailnet.ts.net)
  • Case-insensitive comparison (env entries normalized to lowercase, header matched per RFC 7230)
  • Comma-separated parsing with whitespace tolerance
  • Empty / unset / whitespace-only env var is a true no-op
  • Env var ignored on explicit non-loopback bind (doesn't weaken specific binds)
  • 0.0.0.0 (--insecure) behavior unchanged

All 15 tests in the file pass (5 existing + 7 new + 3 middleware).

Notes

  • Self-contained: 16 lines of production code in one file, 102 lines of tests in one file
  • HERMES_DASHBOARD_ADDITIONAL_HOSTS follows the existing HERMES_* env-var convention (HERMES_QWEN_BASE_URL, HERMES_DOCKER_BINARY, HERMES_HUMAN_DELAY_MODE, etc.)
  • Helper function isolated so it's trivially testable in isolation
  • No config schema change; no new CLI flag
  • Comment in the new helper explains intent + security rationale

@alt-glitch alt-glitch added type/feature New feature or request duplicate This issue or pull request already exists comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels May 20, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

Duplicate of #20136 (oldest, config-based). Competes with #27113 (env-var HERMES_DASHBOARD_ALLOWED_HOSTS), #25173 (dup of #20136), #28578 (bundled), #28954 (CLI flag + config). 7th+ competing implementation for dashboard reverse-proxy host allowlisting.

The dashboard's anti-DNS-rebinding Host validator
(`hermes_cli/web_server.py::_is_accepted_host`) accepts only the bound
interface — plus loopback aliases when bound to loopback. That's correct
for the default deployment but blocks operators who run the dashboard
behind a reverse proxy or `tailscale serve` mapping a public tailnet
hostname back to `127.0.0.1:9119`: in those setups the proxy forwards
with the public hostname in the Host header, which the validator rejects.

`HERMES_DASHBOARD_ADDITIONAL_HOSTS=host1.example,host2.example` adds
specific hostnames to the loopback accept list. Hosts NOT in the list
still get rejected, so a DNS-rebinding attacker host that TTL-flips to
127.0.0.1 is still blocked. The opt-in is per-operator (env var), so the
default stays safe.

Tests in tests/hermes_cli/test_web_server_host_header.py cover: listed
hosts accepted on every loopback bind, attacker hosts still rejected with
env set (incl. suffix/subdomain variants), case-insensitive matching,
comma-separated parsing with whitespace, empty/unset env is a no-op, env
var ignored on explicit non-loopback bind, and 0.0.0.0 unaffected.
@StartupBros

Copy link
Copy Markdown
Contributor Author

Status re: @alt-glitch's duplicate note above:
Reviewed all 5 competing PRs (#20136, #25173, #27113, #28578, #28954) and posted differentiation comments on each. Four of them have the same placement bug — the extras check runs before bind discrimination, so the env var widens explicit non-loopback binds too. This PR gates the extras inside the loopback-bind branch and has a regression test for the non-loopback case (test_additional_hosts_not_consulted_on_non_loopback_bind). #20136 is mostly orthogonal (also covers WebSocket peer allowlist + Origin validation, which I suggested splitting out).

@StartupBros

Copy link
Copy Markdown
Contributor Author

Correction to my earlier status comment: I said "Reviewed all 5 competing PRs" — but that was the set the duplicate-detector bot enumerated. A wider search turned up more PRs in the dashboard-host space that the bot didn't flag:

Posted a brief acknowledgment comment on #20871 since that's the closest direct comparator. For the maintainer's triage: this PR remains the narrowest host-allowlist-only diff with the most focused test coverage, but #20871 has equivalent placement logic if a single host-allowlist + voice multi-fix is preferred.

@alt-glitch alt-glitch added 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 comp/cli CLI entry point, hermes_cli/, setup wizard labels Jun 26, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused implementation and for preserving exact-match handling, loopback-only scope, and regression coverage.

  • This is an automated hermes-sweeper review. The PR adds HERMES_DASHBOARD_ADDITIONAL_HOSTS as a user-facing behavioral setting (hermes_cli/web_server.py, proposed line 162).
  • The standing project policy rejects new non-secret HERMES_* variables for behavioral configuration; these settings belong in config.yaml (AGENTS.md:102-107). This matches the env-var-for-config policy directly.
  • The underlying limitation is real on current main: loopback binds only accept loopback Host aliases (hermes_cli/web_server.py:450-456). Current documentation also supports Tailscale deployment by binding to the Tailscale IP with the authentication gate (website/docs/user-guide/features/web-dashboard.md:1030).

If this capability is revisited, please re-scope it around a config.yaml dashboard setting and its setup/config UX rather than a new environment variable.


Closed as not-planned per standing maintainer policy (env-var-for-config). This is a design-direction decision, not a code-quality judgment — see the Contribution Rubric in AGENTS.md for what the project is looking for. If you believe this policy was misapplied to your change, comment here and a maintainer will take a look.

@teknium1 teknium1 closed this Jul 13, 2026
@teknium1 teknium1 added the sweeper:not-planned Sweeper: closed per standing maintainer policy (design direction) label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/dashboard Web dashboard / control panel UI (dashboard/, landing) duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:not-planned Sweeper: closed per standing maintainer policy (design direction) sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants