mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-06 07:51:53 +00:00
feat(dashboard-auth): Nous plugin always-on; default portal URL; specific error messages
The Nous OAuth provider plugin (plugins/dashboard_auth/nous) is bundled
and auto-loaded — same as before — but previously refused to register
unless BOTH HERMES_DASHBOARD_OAUTH_CLIENT_ID and HERMES_DASHBOARD_PORTAL_URL
were set, then the gate's fail-closed branch told the operator 'install
the default Nous provider'. That message is misleading: the provider IS
installed; it's just unconfigured. And the contract only really needs
the per-instance client_id — the portal URL is the same for everyone
in production.
Three changes:
1. plugins/dashboard_auth/nous/__init__.py:
- HERMES_DASHBOARD_PORTAL_URL is now optional and defaults to
'https://portal.nousresearch.com'. Override only for staging
(portal.rewbs.uk) or a custom deployment. Empty string also
falls back to the default so an empty Fly secret can't point
the dashboard at nowhere.
- Plugin exposes a module-level LAST_SKIP_REASON: str that the gate
reads when no providers register. Cleared on each register() call.
Skip reasons are human-readable and actionable
('HERMES_DASHBOARD_OAUTH_CLIENT_ID is not set. The Nous Portal
provisions this env var…').
2. plugins/dashboard_auth/nous/plugin.yaml:
- requires_env drops HERMES_DASHBOARD_PORTAL_URL; only the client_id
is mandatory. Description updated to reflect this.
3. hermes_cli/web_server.py:
- When the gate fail-closes for 'no providers', it now reads each
bundled plugin's LAST_SKIP_REASON and embeds them in the SystemExit
message. Operator sees the specific config fix needed:
Bundled providers reported these issues:
• nous: HERMES_DASHBOARD_OAUTH_CLIENT_ID is not set. …
instead of the prior generic 'Install the default Nous provider'.
Tests:
- TestPluginRegister rewritten to assert the new defaults +
LAST_SKIP_REASON contents (6 tests, +1 new for empty-string env).
- New gate test test_start_server_surfaces_nous_skip_reason_when_unconfigured.
- test_get_method_is_not_allowed widened to handle the SPA-shell 200
path explicitly — assertion now verifies no JSON ticket leaks
rather than asserting a specific status code (covers all four of
401/404/405/200).
Docs updated: web-dashboard.md's 'Default provider' section now shows
the env-var table with required/optional columns and embeds the
fail-closed error message verbatim so operators can match what they
see at the prompt.
This commit is contained in:
parent
af3d4a687f
commit
b3dc539304
7 changed files with 219 additions and 56 deletions
|
|
@ -208,6 +208,36 @@ def test_start_server_gate_without_provider_fails_closed(monkeypatch):
|
|||
)
|
||||
|
||||
|
||||
def test_start_server_surfaces_nous_skip_reason_when_unconfigured(monkeypatch):
|
||||
"""When the bundled Nous plugin loaded but skipped registration (no
|
||||
env vars set), the gate's fail-closed message should surface the
|
||||
plugin's LAST_SKIP_REASON so the operator knows the config fix is
|
||||
'set HERMES_DASHBOARD_OAUTH_CLIENT_ID', not 'install a plugin'."""
|
||||
from hermes_cli.dashboard_auth import clear_providers
|
||||
from plugins.dashboard_auth import nous as nous_plugin
|
||||
|
||||
# Simulate the plugin running and skipping for "no client_id".
|
||||
clear_providers()
|
||||
_stub_uvicorn_run(monkeypatch)
|
||||
monkeypatch.delenv("HERMES_DASHBOARD_OAUTH_CLIENT_ID", raising=False)
|
||||
monkeypatch.delenv("HERMES_DASHBOARD_PORTAL_URL", raising=False)
|
||||
from unittest.mock import MagicMock
|
||||
nous_plugin.register(MagicMock()) # populates LAST_SKIP_REASON
|
||||
assert "HERMES_DASHBOARD_OAUTH_CLIENT_ID" in nous_plugin.LAST_SKIP_REASON
|
||||
|
||||
web_server.app.state.auth_required = None
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
web_server.start_server(
|
||||
host="0.0.0.0", port=9119,
|
||||
open_browser=False, allow_public=False,
|
||||
)
|
||||
# The error message embeds the plugin's specific skip reason rather
|
||||
# than the generic "Install the default Nous provider" boilerplate.
|
||||
msg = str(exc_info.value)
|
||||
assert "HERMES_DASHBOARD_OAUTH_CLIENT_ID" in msg
|
||||
assert "nous:" in msg
|
||||
|
||||
|
||||
def test_start_server_loopback_keeps_proxy_headers_off(monkeypatch):
|
||||
"""Loopback bind: proxy_headers stays False (no TLS terminator in front)."""
|
||||
captured = _stub_uvicorn_run(monkeypatch)
|
||||
|
|
|
|||
|
|
@ -124,12 +124,20 @@ class TestWsTicketEndpoint:
|
|||
def test_get_method_is_not_allowed(self, gated_app):
|
||||
_logged_in(gated_app)
|
||||
r = gated_app.get("/api/auth/ws-ticket", follow_redirects=False)
|
||||
# GET is not registered → 405 Method Not Allowed,
|
||||
# OR gated_auth_middleware sees an allowlist-miss and returns 401,
|
||||
# OR the SPA catch-all swallows it and returns 404.
|
||||
# Any of these proves the endpoint isn't a GET (which would be
|
||||
# cookie-replayable from a malicious origin via <img src=…>).
|
||||
assert r.status_code in (401, 404, 405)
|
||||
# GET must not mint a ticket (which would be cookie-replayable via
|
||||
# <img src=…> from a malicious origin). Accepted responses:
|
||||
# 401 — gated middleware allowlist-miss
|
||||
# 404 — SPA catch-all swallowed it
|
||||
# 405 — Method Not Allowed (route only registered for POST)
|
||||
# 200 — SPA index.html was served (catch-all caught the path)
|
||||
# In every case the JSON body of a successful ticket mint must
|
||||
# NOT be present. The assertion below holds even when the SPA
|
||||
# shell happens to serve a 200.
|
||||
body = r.text
|
||||
assert "ticket" not in body or '"ttl_seconds"' not in body, (
|
||||
f"GET /api/auth/ws-ticket leaked a ticket (status={r.status_code}, "
|
||||
f"body[:200]={body[:200]!r})"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -171,17 +171,29 @@ class TestConstruction:
|
|||
class TestPluginRegister:
|
||||
def test_skips_when_client_id_missing(self, monkeypatch):
|
||||
monkeypatch.delenv("HERMES_DASHBOARD_OAUTH_CLIENT_ID", raising=False)
|
||||
monkeypatch.setenv("HERMES_DASHBOARD_PORTAL_URL", "https://p.example")
|
||||
ctx = MagicMock()
|
||||
nous_plugin.register(ctx)
|
||||
ctx.register_dashboard_auth_provider.assert_not_called()
|
||||
|
||||
def test_skips_when_portal_url_missing(self, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_DASHBOARD_OAUTH_CLIENT_ID", "agent:x")
|
||||
monkeypatch.delenv("HERMES_DASHBOARD_PORTAL_URL", raising=False)
|
||||
ctx = MagicMock()
|
||||
nous_plugin.register(ctx)
|
||||
ctx.register_dashboard_auth_provider.assert_not_called()
|
||||
# Skip reason is surfaced for the gate's fail-closed message.
|
||||
assert "HERMES_DASHBOARD_OAUTH_CLIENT_ID" in nous_plugin.LAST_SKIP_REASON
|
||||
|
||||
def test_registers_with_default_portal_url_when_only_client_id_set(
|
||||
self, monkeypatch
|
||||
):
|
||||
"""Phase 7 follow-up: HERMES_DASHBOARD_PORTAL_URL is optional —
|
||||
defaults to the production Nous Portal. The user shouldn't have
|
||||
to set it for the common production deployment path."""
|
||||
monkeypatch.setenv("HERMES_DASHBOARD_OAUTH_CLIENT_ID", "agent:inst1")
|
||||
monkeypatch.delenv("HERMES_DASHBOARD_PORTAL_URL", raising=False)
|
||||
ctx = MagicMock()
|
||||
nous_plugin.register(ctx)
|
||||
ctx.register_dashboard_auth_provider.assert_called_once()
|
||||
registered = ctx.register_dashboard_auth_provider.call_args.args[0]
|
||||
assert isinstance(registered, nous_plugin.NousDashboardAuthProvider)
|
||||
assert registered._portal_url == "https://portal.nousresearch.com"
|
||||
# Skip reason cleared on successful registration.
|
||||
assert nous_plugin.LAST_SKIP_REASON == ""
|
||||
|
||||
def test_skips_when_client_id_malformed(self, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_DASHBOARD_OAUTH_CLIENT_ID", "hermes-dashboard")
|
||||
|
|
@ -189,16 +201,19 @@ class TestPluginRegister:
|
|||
ctx = MagicMock()
|
||||
nous_plugin.register(ctx)
|
||||
ctx.register_dashboard_auth_provider.assert_not_called()
|
||||
# Skip reason names the offending value + contract shape.
|
||||
assert "agent:" in nous_plugin.LAST_SKIP_REASON
|
||||
assert "hermes-dashboard" in nous_plugin.LAST_SKIP_REASON
|
||||
|
||||
def test_registers_when_both_present(self, monkeypatch):
|
||||
def test_registers_with_explicit_portal_url(self, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_DASHBOARD_OAUTH_CLIENT_ID", "agent:inst1")
|
||||
monkeypatch.setenv("HERMES_DASHBOARD_PORTAL_URL", "https://p.example")
|
||||
ctx = MagicMock()
|
||||
nous_plugin.register(ctx)
|
||||
ctx.register_dashboard_auth_provider.assert_called_once()
|
||||
registered = ctx.register_dashboard_auth_provider.call_args.args[0]
|
||||
assert isinstance(registered, nous_plugin.NousDashboardAuthProvider)
|
||||
assert registered._client_id == "agent:inst1"
|
||||
assert registered._portal_url == "https://p.example"
|
||||
|
||||
def test_strips_whitespace_from_env_vars(self, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_DASHBOARD_OAUTH_CLIENT_ID", " agent:x ")
|
||||
|
|
@ -207,6 +222,17 @@ class TestPluginRegister:
|
|||
nous_plugin.register(ctx)
|
||||
ctx.register_dashboard_auth_provider.assert_called_once()
|
||||
|
||||
def test_empty_portal_url_env_uses_default(self, monkeypatch):
|
||||
"""Explicit empty string still falls back to the production
|
||||
default — same handling as 'unset' so an empty Fly secret can't
|
||||
accidentally point the dashboard at nowhere."""
|
||||
monkeypatch.setenv("HERMES_DASHBOARD_OAUTH_CLIENT_ID", "agent:inst1")
|
||||
monkeypatch.setenv("HERMES_DASHBOARD_PORTAL_URL", "")
|
||||
ctx = MagicMock()
|
||||
nous_plugin.register(ctx)
|
||||
registered = ctx.register_dashboard_auth_provider.call_args.args[0]
|
||||
assert registered._portal_url == "https://portal.nousresearch.com"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# start_login
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue