fix: allow dashboard host headers for trusted proxies - #28954
fix: allow dashboard host headers for trusted proxies#28954eco-botfleet-audit wants to merge 1 commit into
Conversation
|
Hey @eco-botfleet-audit — there are several competing dashboard-host-allowlist PRs and I left similar notes on #25173, #27113, #28578. One observation specific to yours: The if bound_host in {"0.0.0.0", "::"}:
return True
if host_only in _normalize_allowed_hosts(allowed_hosts):
return True
# Loopback bind: ...
bound_lc = bound_host.lower()
if bound_lc in _LOOPBACK_HOST_VALUES:
...So when the operator binds to a specific non-loopback hostname like Things in this PR I think are stronger than mine:
I opened #29195 with a narrower env-var-only surface but with the loopback-only placement. If the maintainer prefers your richer config surface, the placement tweak alone would lock down the non-loopback-bind case. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tackling the reverse-proxy dashboard Host allowlist. The premise still holds on current main: a loopback-bound dashboard only accepts loopback Host values in hermes_cli/web_server.py:342-345.
Problems
- In the PR diff, hermes_cli/web_server.py:231 checks allowed_hosts before the loopback-bind branch. That widens explicit non-loopback binds, which current main keeps strict via exact match at hermes_cli/web_server.py:347-348.
- The sibling WebSocket Host/Origin guard is not plumbed. Current main enforces that path at hermes_cli/web_server.py:10032 and :10049, outside HTTP middleware, so proxied dashboard chat traffic would still fail.
- hermes_cli/main.py:10186 adds HERMES_DASHBOARD_ALLOWED_HOSTS for a behavioral setting. AGENTS.md:102-107 says non-secret config should live in config.yaml rather than new user-facing HERMES_* env vars.
Suggested changes
- Move allowed_hosts acceptance inside the loopback branch only.
- Pass the normalized allowlist into WebSocket Host/Origin validation and add a regression test for proxied Host + Origin.
- Keep the public surface to dashboard.allowed_hosts and --allowed-host.
Automated hermes-sweeper review.
|
|
||
| # Explicit reverse-proxy names are exact matches only; wildcards are not | ||
| # supported because they would weaken the DNS-rebinding defence. | ||
| if host_only in _normalize_allowed_hosts(allowed_hosts): |
There was a problem hiding this comment.
This allowlist check needs to live inside the loopback-bind branch. As written, a server explicitly bound to a non-loopback hostname will also accept any configured proxy hostname, weakening the exact-match rule for that mode.
| dashboard_cfg = cfg.get("dashboard", {}) if isinstance(cfg, dict) else {} | ||
| allowed_hosts = [] | ||
| allowed_hosts.extend(_split_hosts(dashboard_cfg.get("allowed_hosts"))) | ||
| allowed_hosts.extend(_split_hosts(os.environ.get("HERMES_DASHBOARD_ALLOWED_HOSTS"))) |
There was a problem hiding this comment.
Please avoid adding a public HERMES_* env var for this non-secret behavior; AGENTS.md says behavioral settings should go through config.yaml rather than new user-facing environment variables.
Summary
--allowed-host/dashboard.allowed_hostssupport for Hermes Dashboard Host-header validationWhy
Cloudflare Access / Tunnel deployments commonly terminate the public hostname at the edge while forwarding to a loopback dashboard origin. The dashboard's DNS-rebinding Host-header guard should keep rejecting arbitrary hostnames, but operators need a narrow, explicit allowlist for the public proxy hostname instead of using
--host 0.0.0.0 --insecureas a workaround.Operational driver: Oss Gruppen audit dashboard issue https://github.com/Oss-Gruppen-AS/enterprise-continuum/issues/450
Test plan
python -m pytest tests/hermes_cli/test_web_server_host_header.py tests/hermes_cli/test_dashboard_lifecycle_flags.py -qaudit-kanban.scheel.noorigin:Host: audit-kanban.scheel.noto127.0.0.1:9119/api/statusreturns 200Host: evil.examplereturns 400127.0.0.1:9119, not0.0.0.0:9119