Skip to content

fix(dashboard): allow reverse proxy hosts - #25173

Closed
farena-ai wants to merge 2 commits into
NousResearch:mainfrom
farena-ai:fix/dashboard-proxy-hosts
Closed

fix(dashboard): allow reverse proxy hosts#25173
farena-ai wants to merge 2 commits into
NousResearch:mainfrom
farena-ai:fix/dashboard-proxy-hosts

Conversation

@farena-ai

Copy link
Copy Markdown

Summary

  • Adds explicit dashboard reverse-proxy host allowlisting via HERMES_DASHBOARD_ALLOWED_HOSTS.
  • Keeps unlisted hosts blocked and avoids wildcard-style bypass behavior.
  • Adds focused tests for allowed proxy host parsing and matching.

Files changed

  • hermes_cli/web_server.py
  • tests/hermes_cli/test_dashboard_allowed_hosts.py

Test commands/results

python3 -m py_compile hermes_cli/web_server.py tests/hermes_cli/test_dashboard_allowed_hosts.py
venv/bin/python -m pytest tests/hermes_cli/test_dashboard_allowed_hosts.py -q

Result:

4 passed in 3.02s

Live impact

  • Enables operator-approved reverse proxy hostnames for the dashboard.
  • Does not set any live environment variables.
  • Does not restart or modify any running service.

Restart/reload needed

  • If the dashboard/web server is already running, restart that dashboard/web process after merge/deploy.
  • If the dashboard is served through a long-running Hermes process, restart that process after approval.

Known caveats

  • Security-sensitive Host header behavior; reviewers should verify allowlist semantics carefully.
  • No live env var was set and no live dashboard process was restarted.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles labels May 13, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #20136, which implements the same reverse proxy host allowlisting via config (dashboard.allowed_hosts + trusted_proxy_hosts). Also related to #22437 (env-var approach) and #21104.

@StartupBros

Copy link
Copy Markdown
Contributor

Hey @farena-ai — flagged by the duplicate-detector bot alongside several other PRs solving similar dashboard-host-allowlist problems. Wanted to share one observation in case it's useful.

The placement of the extras check matters for the DNS-rebinding defence. In this PR the check runs before the bind discrimination:

if host_only in extra_hosts:
    return True

if bound_host in {"0.0.0.0", "::"}:
    return True
# ... loopback branch ...
# ... explicit non-loopback branch ...

That means setting HERMES_DASHBOARD_ALLOWED_HOSTS=proxy.example widens the accept list on every bind mode — including the explicit-non-loopback bind (line 232-203 of the unmodified _is_accepted_host). An operator who deliberately bound to my-server.corp.net now also accepts proxy.example unexpectedly.

I opened #29195 which gates the extras check inside the loopback-bind branch so the explicit-non-loopback case stays strict, with a regression test for that exact scenario (test_additional_hosts_not_consulted_on_non_loopback_bind). Same env var idea, just a tighter placement. Linked the discussion in case it's helpful — happy to consolidate either way.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused allowlist implementation and targeted tests.

  • Automated hermes-sweeper review found that this PR adds HERMES_DASHBOARD_ALLOWED_HOSTS as a user-facing behavioral setting in hermes_cli/web_server.py (PR hunk at -176,+176).
  • The maintainer policy in AGENTS.md:102-107 is that new non-secret HERMES_* behavioral settings do not land as environment-variable configuration; they belong in config.yaml.
  • The current validator is security-sensitive: hermes_cli/web_server.py:455-456 preserves exact matching for explicit non-loopback binds. The proposed early allowlist return would bypass that distinction whenever the new variable is set.

Please re-scope any follow-up around a documented dashboard configuration key, with the allowlist applied only in the intended bind mode and regression coverage for explicit non-loopback binds. This is an automated hermes-sweeper review.


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

area/config Config system, migrations, profiles 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