fix(dashboard-auth): warn when public_url override is silently rejected (#42780)#43214
Merged
Merged
Conversation
A non-empty HERMES_DASHBOARD_PUBLIC_URL / dashboard.public_url value that fails URL validation (overwhelmingly: a missing http(s):// scheme, e.g. "hermes.domain.com") was silently discarded by resolve_public_url(), falling back to reconstructing the OAuth redirect_uri from request headers. Behind a reverse proxy that doesn't forward X-Forwarded-Proto reliably, that yields an http:// callback even though the operator explicitly set the public URL — with no signal as to why (#42780). Emit a deduplicated operator-facing WARNING (once per distinct value, since resolve_public_url runs per request) naming the offending value and the required scheme. Turns a silent footgun into a self-diagnosing one; behaviour is otherwise unchanged. Tests assert the warning fires for a scheme-less value, is deduplicated across repeated calls, and stays silent for a valid value — all three fail without the fix.
1 task
Contributor
🔎 Lint report:
|
tonydwb
approved these changes
Jun 10, 2026
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Looks Good
- Clean fix: warn when HERMES_DASHBOARD_PUBLIC_URL is set but invalid or being silently rejected.
- Adds explicit warning rather than silently ignoring the misconfiguration, improving debuggability.
- 2 files changed, focused on dashboard auth.
- No security concerns - this is a warning improvement.
Reviewed by Hermes Agent
wachoo
pushed a commit
to wachoo/hermes-agent
that referenced
this pull request
Jun 10, 2026
…ed (NousResearch#43214) A non-empty HERMES_DASHBOARD_PUBLIC_URL / dashboard.public_url value that fails URL validation (overwhelmingly: a missing http(s):// scheme, e.g. "hermes.domain.com") was silently discarded by resolve_public_url(), falling back to reconstructing the OAuth redirect_uri from request headers. Behind a reverse proxy that doesn't forward X-Forwarded-Proto reliably, that yields an http:// callback even though the operator explicitly set the public URL — with no signal as to why (NousResearch#42780). Emit a deduplicated operator-facing WARNING (once per distinct value, since resolve_public_url runs per request) naming the offending value and the required scheme. Turns a silent footgun into a self-diagnosing one; behaviour is otherwise unchanged. Tests assert the warning fires for a scheme-less value, is deduplicated across repeated calls, and stays silent for a valid value — all three fail without the fix.
changman
pushed a commit
to changman/hermes-agent
that referenced
this pull request
Jun 10, 2026
…ed (NousResearch#43214) A non-empty HERMES_DASHBOARD_PUBLIC_URL / dashboard.public_url value that fails URL validation (overwhelmingly: a missing http(s):// scheme, e.g. "hermes.domain.com") was silently discarded by resolve_public_url(), falling back to reconstructing the OAuth redirect_uri from request headers. Behind a reverse proxy that doesn't forward X-Forwarded-Proto reliably, that yields an http:// callback even though the operator explicitly set the public URL — with no signal as to why (NousResearch#42780). Emit a deduplicated operator-facing WARNING (once per distinct value, since resolve_public_url runs per request) naming the offending value and the required scheme. Turns a silent footgun into a self-diagnosing one; behaviour is otherwise unchanged. Tests assert the warning fires for a scheme-less value, is deduplicated across repeated calls, and stays silent for a valid value — all three fail without the fix.
alt-glitch
pushed a commit
that referenced
this pull request
Jun 14, 2026
…ed (#43214) A non-empty HERMES_DASHBOARD_PUBLIC_URL / dashboard.public_url value that fails URL validation (overwhelmingly: a missing http(s):// scheme, e.g. "hermes.domain.com") was silently discarded by resolve_public_url(), falling back to reconstructing the OAuth redirect_uri from request headers. Behind a reverse proxy that doesn't forward X-Forwarded-Proto reliably, that yields an http:// callback even though the operator explicitly set the public URL — with no signal as to why (#42780). Emit a deduplicated operator-facing WARNING (once per distinct value, since resolve_public_url runs per request) naming the offending value and the required scheme. Turns a silent footgun into a self-diagnosing one; behaviour is otherwise unchanged. Tests assert the warning fires for a scheme-less value, is deduplicated across repeated calls, and stays silent for a valid value — all three fail without the fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
resolve_public_url()runs every operator-suppliedHERMES_DASHBOARD_PUBLIC_URL/dashboard.public_urlvalue through_normalise_public_url(), which returns""for anything that isn't a validhttp(s)://host[/path]. By design""is indistinguishable from "unset", so the caller silently falls back to reconstructing the OAuthredirect_urifrom request headers.The overwhelmingly common trip-wire is a missing scheme — an operator sets
HERMES_DASHBOARD_PUBLIC_URL=hermes.domain.com(nohttps://). The value is dropped without a peep, and behind a reverse proxy that doesn't forwardX-Forwarded-Protoreliably, the dashboard then generates anhttp://callback even though the operator explicitly declared the public URL. There is currently no log line telling them why their setting had no effect.This is the most likely root cause of #42780 (env var set, callback still
http://, only fixed by settingdashboard.public_urlin config.yaml — presumably written with the scheme there).Fix
Emit a deduplicated operator-facing
WARNINGwhen a non-empty value is rejected, naming the offending value and the required scheme:Dedup is keyed on (source, value) because
resolve_public_url()runs per request — without it a misconfigured deploy would flood the logs. Behaviour is otherwise unchanged: malformed values are still rejected, valid values still pass through.Tests
tests/hermes_cli/test_dashboard_auth_prefix.py:test_scheme_less_public_url_env_warns_operator— scheme-less value emits the warningtest_scheme_less_public_url_warning_is_deduplicated— fires at most once across 5 callstest_valid_public_url_emits_no_warning— no spurious warning for a valid valueAll three fail without the source change (verified by reverting prefix.py and re-running). Full prefix suite: 22 passed.
Scope
Pure dashboard-auth config-resolution behaviour (container/reverse-proxy deploys). Does not change the redirect-URI construction itself — complementary to #42937, which addresses the separate Tier-2/3 (no public_url set) scheme-mismatch path.