Skip to content

fix(dashboard): allow configured reverse proxy hosts - #47903

Closed
jeffery-zhang wants to merge 1 commit into
NousResearch:mainfrom
jeffery-zhang:fix/dashboard-reverse-proxy-hosts
Closed

fix(dashboard): allow configured reverse proxy hosts#47903
jeffery-zhang wants to merge 1 commit into
NousResearch:mainfrom
jeffery-zhang:fix/dashboard-reverse-proxy-hosts

Conversation

@jeffery-zhang

Copy link
Copy Markdown

Summary

  • allow loopback-bound dashboards to opt into trusted reverse-proxy hostnames via HERMES_DASHBOARD_ALLOWED_HOSTS
  • keep the default DNS-rebinding Host/Origin rejection unchanged for unconfigured hosts
  • add focused coverage for both the reject-by-default and configured-allowlist paths

Why

A dashboard can be safely exposed through a local reverse proxy or Cloudflare Tunnel while the FastAPI server remains bound to 127.0.0.1. In that setup, browser WebSocket requests use the public Host / Origin (for example https://dashboard.example.com), but the backend currently compares them only against the loopback bind host and rejects /api/pty and /api/events upgrades with origin_mismatch.

Binding the dashboard to 0.0.0.0 --insecure works around this, but it weakens the deployment by exposing the dashboard listener more broadly. This patch keeps the listener loopback-only and adds a narrow, explicit hostname allowlist for reverse-proxied deployments.

Testing

  • python -m pytest tests/test_web_server.py -q
  • python -m py_compile hermes_cli/web_server.py
  • git diff --check

@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 duplicate This issue or pull request already exists labels Jun 17, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #20136 — this implements the same dashboard reverse-proxy Host/Origin allowlist feature (here via HERMES_DASHBOARD_ALLOWED_HOSTS while keeping the loopback bind and default DNS-rebinding rejection). It joins a heavily saturated cluster: canonical #20136 (config-based, oldest open), with #27113 (env-var approach) and #28954 also open. Marking duplicate of the canonical; maintainer picks the mechanism.

@alt-glitch alt-glitch added comp/dashboard Web dashboard / control panel UI (dashboard/, landing) and removed duplicate This issue or pull request already exists comp/cli CLI entry point, hermes_cli/, setup wizard labels Jun 26, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Correction to the earlier "Duplicate of #20136" note: #20136 is now closed, so this PR is no longer a duplicate of a closed item — the duplicate label has been removed. Re-classifying as related to the still-open, config-based competitor #47560 (dashboard.allowed_external_hosts), which solves the same reverse-proxy Host-header acceptance via a config key rather than this PR's HERMES_DASHBOARD_ALLOWED_HOSTS env var. Maintainer to pick one mechanism.

@Kinkoolino-Hermes

Copy link
Copy Markdown
Contributor

Triage note after a fresh live + cross-PR check:

If maintainers want the env-var mechanism, the useful next step here is probably a rebase and an explicit comparison against #48954. If the preferred API is config-based instead, this may be superseded by #47560 or another config-surface PR.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused reverse-proxy analysis and regression coverage. The underlying loopback Host/Origin rejection is real, but this implementation uses a configuration surface we do not accept.

This automated hermes-sweeper review is closing this under the standing configuration policy:

  • 9b286d5fda47 adds the user-facing non-secret setting HERMES_DASHBOARD_ALLOWED_HOSTS (hermes_cli/web_server.py:314).
  • AGENTS.md:102-107 requires behavioral settings to live in config.yaml, rather than a new HERMES_* environment variable.
  • Current dashboard configuration already uses the dashboard.* namespace (hermes_cli/config.py:2033-2053; website/docs/user-guide/features/web-dashboard.md:892-907).

A focused re-scope to a documented dashboard.* allowlist setting would be the supported path.


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.

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) P3 Low — cosmetic, nice to have sweeper:not-planned Sweeper: closed per standing maintainer policy (design direction) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants