mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-10-20 05:15:31 +00:00
feat(dashboard-auth): drop loopback identity gate (bind+CSRF are the boundary)
On a loopback bind the dashboard no longer enforces a per-request identity token. The loopback bind is the security boundary (nothing off-machine can reach 127.0.0.1), the Sec-Fetch-Site CSRF guard blocks cross-origin mutations, and the localhost-only CORS policy blocks cross-origin reads. - auth_middleware becomes a no-op shell (kept registered for a minimal, reversible diff; Phase 5 removes it with the token symbol) - _require_token's loopback branch now allows (a local user is entitled); the gated branch is unchanged (still requires a verified session) Identity enforcement now lives ONLY in the pluggable OAuth gate (gated mode). Tests that pinned the old loopback-401 contract are updated to the new reality, and sensitive endpoints (/api/env/reveal, /api/fs/*, admin endpoints) gain GATED-mode coverage that proves the gate still enforces identity. The legacy _SESSION_TOKEN is still generated (ignored on loopback) and is removed in Phase 5.
This commit is contained in:
parent
cde893cce6
commit
52e51de69d
7 changed files with 353 additions and 113 deletions
|
|
@ -263,21 +263,17 @@ def _has_valid_query_token(request: Request, path: str) -> bool:
|
|||
def _require_token(request: Request) -> None:
|
||||
"""Authorize a sensitive endpoint, raising 401 if the caller isn't allowed.
|
||||
|
||||
Two auth schemes protect the dashboard, exactly one active per bind:
|
||||
Two regimes, exactly one active per bind:
|
||||
|
||||
* **Loopback / ``--insecure`` mode** (``auth_required`` False): the
|
||||
ephemeral ``_SESSION_TOKEN`` is injected into the SPA HTML and echoed
|
||||
back via ``X-Hermes-Session-Token`` (or the legacy ``Bearer`` header).
|
||||
Validate it here.
|
||||
* **Gated / OAuth mode** (``auth_required`` True): ``_SESSION_TOKEN`` is
|
||||
NOT injected (the SPA authenticates with a session cookie), so there is
|
||||
no token to check. The ``gated_auth_middleware`` has already verified the
|
||||
cookie before the request reached this handler — any non-public ``/api/``
|
||||
route it lets through carries a verified ``request.state.session``. The
|
||||
legacy ``auth_middleware`` likewise short-circuits in this mode. Requiring
|
||||
the (absent) token here would 401 every cookie-authenticated request,
|
||||
making plugin install/enable/disable and the other ``_require_token``
|
||||
endpoints permanently unreachable behind the gate. Defer to the gate.
|
||||
* **Gated / OAuth mode** (``auth_required`` True): the
|
||||
``gated_auth_middleware`` has already verified the session cookie
|
||||
before the request reached this handler — any non-public ``/api/``
|
||||
route it lets through carries a verified ``request.state.session``.
|
||||
Accept iff that session is present; 401 otherwise.
|
||||
* **Loopback / ``--insecure`` mode** (``auth_required`` False): there is
|
||||
NO identity gate. The loopback bind is the boundary, the CSRF guard
|
||||
blocks cross-origin mutations, and CORS blocks cross-origin reads. A
|
||||
local user is entitled to call these routes, so allow them.
|
||||
"""
|
||||
if getattr(request.app.state, "auth_required", False):
|
||||
# Gate is authoritative. It attaches ``request.state.session`` on
|
||||
|
|
@ -286,8 +282,9 @@ def _require_token(request: Request) -> None:
|
|||
if getattr(request.state, "session", None) is not None:
|
||||
return
|
||||
raise HTTPException(status_code=401, detail="Unauthorized")
|
||||
if not _has_valid_session_token(request):
|
||||
raise HTTPException(status_code=401, detail="Unauthorized")
|
||||
# Loopback / --insecure: no identity gate. CSRF guard + bind boundary
|
||||
# protect these routes; a local user is entitled to call them.
|
||||
return
|
||||
|
||||
|
||||
# Accepted Host header values for loopback binds. DNS rebinding attacks
|
||||
|
|
@ -467,19 +464,20 @@ async def _dashboard_auth_gate(request: Request, call_next):
|
|||
|
||||
@app.middleware("http")
|
||||
async def auth_middleware(request: Request, call_next):
|
||||
"""Require the session token on all /api/ routes except the public list."""
|
||||
# When the OAuth gate is active, cookie-based auth (gated_auth_middleware
|
||||
# above) is authoritative. The legacy _SESSION_TOKEN path is loopback-only
|
||||
# and is skipped here so the gate's session attachment isn't overridden.
|
||||
if getattr(request.app.state, "auth_required", False):
|
||||
return await call_next(request)
|
||||
path = request.url.path
|
||||
if path.startswith("/api/") and path not in _PUBLIC_API_PATHS:
|
||||
if not _has_valid_session_token(request) and not _has_valid_query_token(request, path):
|
||||
return JSONResponse(
|
||||
status_code=401,
|
||||
content={"detail": "Unauthorized"},
|
||||
)
|
||||
"""Loopback path: NO identity gate.
|
||||
|
||||
The dashboard's identity authentication is the pluggable gate
|
||||
(``gated_auth_middleware``), engaged only on non-loopback binds. On a
|
||||
loopback bind the OS boundary IS the security boundary: nothing off the
|
||||
machine can reach 127.0.0.1. Cross-origin mutations are rejected by
|
||||
``csrf_guard_middleware``; cross-origin reads are neutralised by the
|
||||
localhost-only CORS policy. There is no per-request identity token on
|
||||
loopback anymore (the legacy ``_SESSION_TOKEN`` is being removed).
|
||||
|
||||
This remains a registered middleware (rather than being deleted) so the
|
||||
Phase-2 diff is minimal and reversible; Phase 5 removes it entirely once
|
||||
the token symbol is gone.
|
||||
"""
|
||||
return await call_next(request)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -677,15 +677,33 @@ class TestWebhookToggleEndpoint:
|
|||
|
||||
|
||||
class TestAdminEndpointsAuthGate:
|
||||
"""Every admin endpoint must sit behind the dashboard session-token gate."""
|
||||
"""Every admin endpoint must sit behind the dashboard auth gate.
|
||||
|
||||
Identity enforcement lives in the pluggable OAuth gate (gated mode),
|
||||
not on loopback — after the legacy-token teardown, a loopback bind has
|
||||
no identity gate (the bind is the boundary). So this exercises the
|
||||
GATED regime: a cookieless request to each admin endpoint must 401.
|
||||
"""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _setup(self, _isolate_hermes_home):
|
||||
from starlette.testclient import TestClient
|
||||
from hermes_cli.web_server import app
|
||||
from hermes_cli.dashboard_auth import clear_providers, register_provider
|
||||
from tests.hermes_cli.conftest_dashboard_auth import StubAuthProvider
|
||||
|
||||
# No session header → must be rejected.
|
||||
self.client = TestClient(app)
|
||||
clear_providers()
|
||||
register_provider(StubAuthProvider())
|
||||
self._prev_host = getattr(app.state, "bound_host", None)
|
||||
self._prev_required = getattr(app.state, "auth_required", None)
|
||||
app.state.bound_host = "fly-app.fly.dev"
|
||||
app.state.auth_required = True
|
||||
# Cookieless client → the gate must reject every admin endpoint.
|
||||
self.client = TestClient(app, base_url="https://fly-app.fly.dev")
|
||||
yield
|
||||
clear_providers()
|
||||
app.state.bound_host = self._prev_host
|
||||
app.state.auth_required = self._prev_required
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"path",
|
||||
|
|
@ -923,15 +941,20 @@ class TestDebugShareEndpoint:
|
|||
r = self.client.post("/api/ops/debug-share", json={"redact": True})
|
||||
assert r.status_code == 502
|
||||
|
||||
def test_requires_session_token(self):
|
||||
# Drop the token header and confirm the global auth gate rejects it.
|
||||
def test_loopback_has_no_identity_gate(self):
|
||||
# After the legacy-token teardown, loopback enforces no identity
|
||||
# gate: the bind is the boundary and the CSRF guard covers
|
||||
# cross-origin mutations. A bogus token is simply ignored, and the
|
||||
# request reaches the handler (any non-401). Identity enforcement
|
||||
# for this endpoint is exercised in gated mode by
|
||||
# TestAdminEndpointsAuthGate.
|
||||
bare = self.client
|
||||
r = bare.post(
|
||||
"/api/ops/debug-share",
|
||||
json={"redact": True},
|
||||
headers={self.header: "wrong-token"},
|
||||
)
|
||||
assert r.status_code == 401
|
||||
assert r.status_code != 401
|
||||
|
||||
|
||||
class TestToolsConfigEndpoints:
|
||||
|
|
@ -1049,7 +1072,11 @@ class TestToolsConfigEndpoints:
|
|||
assert body["pid"] == 4321
|
||||
assert spawned["subcommand"] == ["tools", "post-setup", "agent_browser"]
|
||||
|
||||
def test_endpoints_require_session_token(self):
|
||||
def test_loopback_endpoints_have_no_identity_gate(self):
|
||||
# Loopback: no identity gate after the legacy-token teardown. A
|
||||
# bogus token is ignored and the request reaches the handler (any
|
||||
# non-401). The gated regime enforces identity (see
|
||||
# TestAdminEndpointsAuthGate).
|
||||
for method, path, payload in [
|
||||
("get", "/api/tools/toolsets/web/config", None),
|
||||
("put", "/api/tools/toolsets/web/env", {"env": {}}),
|
||||
|
|
@ -1060,4 +1087,4 @@ class TestToolsConfigEndpoints:
|
|||
if payload is not None:
|
||||
kwargs["json"] = payload
|
||||
r = fn(path, **kwargs)
|
||||
assert r.status_code == 401, f"{method} {path} not gated"
|
||||
assert r.status_code != 401, f"{method} {path} unexpectedly gated"
|
||||
|
|
|
|||
|
|
@ -40,11 +40,16 @@ def test_loopback_status_is_public(client_loopback):
|
|||
assert "version" in body
|
||||
|
||||
|
||||
def test_loopback_protected_route_requires_token(client_loopback):
|
||||
"""Any non-public /api/ route must require the session token."""
|
||||
# /api/sessions exists and is auth-gated by auth_middleware.
|
||||
def test_loopback_protected_route_no_identity_gate(client_loopback):
|
||||
"""Loopback has no identity gate (the bind is the boundary).
|
||||
|
||||
Pre-teardown this route 401'd without the session token. After the
|
||||
legacy-token teardown (Phase 2), loopback ``/api/`` routes are served
|
||||
without an identity check — the loopback bind + CSRF guard + CORS are
|
||||
the security boundary, not a per-request token.
|
||||
"""
|
||||
r = client_loopback.get("/api/sessions")
|
||||
assert r.status_code == 401
|
||||
assert r.status_code != 401
|
||||
|
||||
|
||||
def test_loopback_protected_route_accepts_session_token(client_loopback):
|
||||
|
|
|
|||
|
|
@ -96,6 +96,29 @@ def test_gated_status_still_public(gated_client):
|
|||
assert gated_client.get("/api/status").status_code == 200
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Loopback has no identity gate (post-Phase-2 contract)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_loopback_no_identity_gate(loopback_client):
|
||||
"""Loopback: the bind + CSRF guard + CORS are the boundary, not an
|
||||
identity token. A tokenless read is allowed."""
|
||||
r = loopback_client.get("/api/sessions")
|
||||
assert r.status_code != 401
|
||||
|
||||
|
||||
def test_loopback_still_blocks_cross_site_mutation(loopback_client):
|
||||
"""The CSRF guard (not an identity token) is what protects loopback
|
||||
mutations from a drive-by cross-origin page."""
|
||||
r = loopback_client.post(
|
||||
"/api/providers/validate",
|
||||
headers={"Sec-Fetch-Site": "cross-site"},
|
||||
json={"key": "OPENAI_API_KEY", "value": "x"},
|
||||
)
|
||||
assert r.status_code == 403
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# WS auth matrix (via _ws_auth_reason — TestClient.websocket_connect is
|
||||
# unreliable for handshake-rejection assertions, so test the function)
|
||||
|
|
|
|||
|
|
@ -305,7 +305,16 @@ class TestWebServerEndpoints:
|
|||
resp = self.client.get("/api/media", params={"path": str(missing)})
|
||||
assert resp.status_code == 404
|
||||
|
||||
def test_get_media_requires_auth(self):
|
||||
def test_get_media_no_identity_gate_on_loopback(self):
|
||||
"""Loopback has no identity gate after the legacy-token teardown.
|
||||
|
||||
Pre-teardown a wrong/absent session token 401'd this route. Now the
|
||||
loopback bind itself is the security boundary (plus the Sec-Fetch-Site
|
||||
CSRF guard for mutations and CORS for cross-origin reads), so the
|
||||
request reaches the handler regardless of the token. ``/tmp/x.png`` is
|
||||
outside the media roots, so the handler returns 403 — the point is it's
|
||||
no longer 401. Identity is enforced only in gated mode.
|
||||
"""
|
||||
from hermes_cli.web_server import _SESSION_HEADER_NAME
|
||||
|
||||
resp = self.client.get(
|
||||
|
|
@ -313,7 +322,8 @@ class TestWebServerEndpoints:
|
|||
params={"path": "/tmp/x.png"},
|
||||
headers={_SESSION_HEADER_NAME: "wrong-token"},
|
||||
)
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 403
|
||||
|
||||
# ── Dashboard font override ─────────────────────────────────────────
|
||||
|
||||
|
|
@ -1380,11 +1390,26 @@ class TestWebServerEndpoints:
|
|||
)
|
||||
assert resp.status_code == 404
|
||||
|
||||
def test_reveal_env_var_no_token(self, tmp_path):
|
||||
"""POST /api/env/reveal without token should return 401."""
|
||||
def test_reveal_env_var_no_identity_gate_on_loopback(self, tmp_path):
|
||||
"""POST /api/env/reveal has no identity gate on loopback.
|
||||
|
||||
After the legacy-token teardown, loopback ``/api/`` routes are served
|
||||
without an identity check — the loopback bind is the security boundary
|
||||
(plus the Sec-Fetch-Site CSRF guard for mutations and CORS for reads),
|
||||
not a per-request token. So a tokenless loopback request reaches the
|
||||
handler and reveals the value. Identity for this sensitive endpoint is
|
||||
enforced in gated mode (see
|
||||
``test_reveal_env_var_requires_auth_in_gated_mode`` below).
|
||||
"""
|
||||
from starlette.testclient import TestClient
|
||||
from hermes_cli import web_server
|
||||
from hermes_cli.web_server import app
|
||||
from hermes_cli.config import save_env_value
|
||||
# The reveal endpoint's module-global rate limiter (5/30s) is now
|
||||
# exercised by tokenless tests that reach the handler (pre-teardown
|
||||
# they 401'd before it). Reset it so the shared window doesn't bleed
|
||||
# 429s across reveal tests.
|
||||
web_server._reveal_timestamps.clear()
|
||||
save_env_value("TEST_REVEAL_NOAUTH", "secret-value")
|
||||
# Use a fresh client WITHOUT the dashboard session header
|
||||
unauth_client = TestClient(app)
|
||||
|
|
@ -1392,10 +1417,47 @@ class TestWebServerEndpoints:
|
|||
"/api/env/reveal",
|
||||
json={"key": "TEST_REVEAL_NOAUTH"},
|
||||
)
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["value"] == "secret-value"
|
||||
|
||||
def test_reveal_env_var_bad_token(self, tmp_path):
|
||||
"""POST /api/env/reveal with wrong token should return 401."""
|
||||
def test_reveal_env_var_requires_auth_in_gated_mode(self, tmp_path):
|
||||
"""In gated mode (non-loopback bind + registered provider), the
|
||||
sensitive /api/env/reveal endpoint requires a session cookie and 401s
|
||||
without one. This preserves identity coverage for the secret-revealing
|
||||
endpoint that loopback mode intentionally no longer gates.
|
||||
"""
|
||||
from starlette.testclient import TestClient
|
||||
from hermes_cli import web_server
|
||||
from hermes_cli.config import save_env_value
|
||||
from hermes_cli.dashboard_auth import clear_providers, register_provider
|
||||
from tests.hermes_cli.conftest_dashboard_auth import StubAuthProvider
|
||||
|
||||
save_env_value("TEST_REVEAL_GATED", "secret-value")
|
||||
|
||||
clear_providers()
|
||||
register_provider(StubAuthProvider())
|
||||
prev_host = getattr(web_server.app.state, "bound_host", None)
|
||||
prev_req = getattr(web_server.app.state, "auth_required", None)
|
||||
web_server.app.state.bound_host = "fly-app.fly.dev"
|
||||
web_server.app.state.auth_required = True
|
||||
try:
|
||||
client = TestClient(web_server.app, base_url="https://fly-app.fly.dev")
|
||||
resp = client.post("/api/env/reveal", json={"key": "TEST_REVEAL_GATED"})
|
||||
assert resp.status_code == 401
|
||||
finally:
|
||||
clear_providers()
|
||||
web_server.app.state.bound_host = prev_host
|
||||
web_server.app.state.auth_required = prev_req
|
||||
|
||||
def test_reveal_env_var_bad_token_no_identity_gate_on_loopback(self, tmp_path):
|
||||
"""POST /api/env/reveal with a wrong token still serves on loopback.
|
||||
|
||||
The legacy session token is ignored on loopback (it's slated for
|
||||
removal). Loopback has no identity gate — the bind + CSRF guard are the
|
||||
boundary — so a request with a bogus token reaches the handler instead
|
||||
of 401ing. Identity is enforced only in gated mode.
|
||||
"""
|
||||
from hermes_cli.config import save_env_value
|
||||
from hermes_cli.web_server import _SESSION_HEADER_NAME
|
||||
save_env_value("TEST_REVEAL_BADAUTH", "secret-value")
|
||||
|
|
@ -1404,7 +1466,9 @@ class TestWebServerEndpoints:
|
|||
json={"key": "TEST_REVEAL_BADAUTH"},
|
||||
headers={_SESSION_HEADER_NAME: "wrong-token-here"},
|
||||
)
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["value"] == "secret-value"
|
||||
|
||||
def test_reveal_env_var_custom_session_header_ignores_proxy_authorization(self, tmp_path):
|
||||
"""A valid dashboard session header should coexist with proxy auth."""
|
||||
|
|
@ -1424,8 +1488,14 @@ class TestWebServerEndpoints:
|
|||
assert resp.status_code == 200
|
||||
assert resp.json()["value"] == "secret-value"
|
||||
|
||||
def test_reveal_env_var_legacy_authorization_header_still_works(self, tmp_path):
|
||||
"""Keep old dashboard bundles working while the new header rolls out."""
|
||||
def test_reveal_env_var_legacy_authorization_header_ignored_on_loopback(self, tmp_path):
|
||||
"""The legacy ``Authorization: Bearer <token>`` mechanism is being
|
||||
removed. On loopback there is no identity gate, so the request succeeds
|
||||
regardless of the legacy header — it's served because the loopback bind
|
||||
is the security boundary, NOT because the Bearer token authenticated.
|
||||
(Previously this test asserted the legacy header itself authenticated;
|
||||
that token mechanism is slated for deletion.)
|
||||
"""
|
||||
from hermes_cli.config import save_env_value
|
||||
from hermes_cli.web_server import _SESSION_TOKEN
|
||||
|
||||
|
|
@ -1436,7 +1506,9 @@ class TestWebServerEndpoints:
|
|||
headers={"Authorization": f"Bearer {_SESSION_TOKEN}"},
|
||||
)
|
||||
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["value"] == "secret-value"
|
||||
|
||||
def test_get_messaging_platforms(self):
|
||||
resp = self.client.get("/api/messaging/platforms")
|
||||
|
|
@ -1904,23 +1976,37 @@ class TestWebServerEndpoints:
|
|||
except Exception:
|
||||
pass # Not JSON — that's fine (SPA HTML)
|
||||
|
||||
def test_unauthenticated_api_blocked(self):
|
||||
"""API requests without the session token should be rejected."""
|
||||
def test_api_no_identity_gate_on_loopback(self):
|
||||
"""Loopback has no identity gate after the legacy-token teardown.
|
||||
|
||||
Pre-teardown, ``/api/*`` requests without the session token 401'd
|
||||
(except a public allowlist). Now the loopback bind itself is the
|
||||
security boundary — plus the Sec-Fetch-Site CSRF guard for mutations
|
||||
and CORS for cross-origin reads — so a tokenless loopback request
|
||||
reaches the handler. Both the formerly-gated routes and the
|
||||
formerly-public routes now serve. Identity is enforced only in gated
|
||||
mode.
|
||||
"""
|
||||
from starlette.testclient import TestClient
|
||||
from hermes_cli.web_server import app
|
||||
# Create a client WITHOUT the dashboard session header
|
||||
unauth_client = TestClient(app)
|
||||
# Formerly-gated routes now serve without a token (no identity gate).
|
||||
resp = unauth_client.get("/api/env")
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 200
|
||||
resp = unauth_client.get("/api/config")
|
||||
assert resp.status_code == 401
|
||||
# Public endpoints should still work
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 200
|
||||
# Public endpoints still work, as before.
|
||||
resp = unauth_client.get("/api/status")
|
||||
assert resp.status_code == 200
|
||||
resp = unauth_client.get("/api/dashboard/plugins")
|
||||
assert resp.status_code == 200
|
||||
# Formerly-gated rescan endpoint now serves on loopback too.
|
||||
resp = unauth_client.get("/api/dashboard/plugins/rescan")
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 200
|
||||
resp = self.client.get("/api/dashboard/plugins/rescan")
|
||||
assert resp.status_code == 200
|
||||
|
||||
|
|
@ -4379,9 +4465,18 @@ class TestBulkDeleteSessionsEndpoint:
|
|||
finally:
|
||||
db.close()
|
||||
|
||||
def test_requires_auth(self):
|
||||
def test_no_identity_gate_on_loopback(self):
|
||||
"""Loopback has no identity gate after the legacy-token teardown.
|
||||
|
||||
Pre-teardown this destructive route 401'd without the session token.
|
||||
Now the loopback bind is the security boundary (the Sec-Fetch-Site CSRF
|
||||
guard blocks cross-origin mutations and CORS blocks cross-origin reads),
|
||||
so a tokenless same-origin loopback request reaches the handler.
|
||||
Identity is enforced only in gated mode.
|
||||
"""
|
||||
resp = self.client.post("/api/sessions/bulk-delete", json={"ids": ["x"]})
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 200
|
||||
|
||||
def test_deletes_listed_sessions_only(self):
|
||||
from hermes_state import SessionDB
|
||||
|
|
@ -4524,19 +4619,31 @@ class TestDeleteEmptySessionsEndpoint:
|
|||
finally:
|
||||
db.close()
|
||||
|
||||
def test_count_endpoint_requires_auth(self):
|
||||
"""GET /api/sessions/empty/count must 401 without the session token."""
|
||||
def test_count_endpoint_no_identity_gate_on_loopback(self):
|
||||
"""GET /api/sessions/empty/count has no identity gate on loopback.
|
||||
|
||||
After the legacy-token teardown, the loopback bind is the security
|
||||
boundary (plus the Sec-Fetch-Site CSRF guard and CORS), so a tokenless
|
||||
loopback request reaches the handler instead of 401ing. Identity is
|
||||
enforced only in gated mode.
|
||||
"""
|
||||
resp = self.client.get("/api/sessions/empty/count")
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 200
|
||||
|
||||
def test_delete_endpoint_requires_auth(self):
|
||||
"""DELETE /api/sessions/empty must 401 without the session token.
|
||||
def test_delete_endpoint_no_identity_gate_on_loopback(self):
|
||||
"""DELETE /api/sessions/empty has no identity gate on loopback.
|
||||
|
||||
Regression guard for issue #19533 — the bulk-delete is a strictly
|
||||
destructive primitive, the middleware must gate it even if a
|
||||
future refactor introduces a non-auth path."""
|
||||
Pre-teardown (issue #19533) this destructive route 401'd without the
|
||||
session token. After the legacy-token teardown, loopback has no
|
||||
identity gate — the loopback bind is the boundary and the
|
||||
Sec-Fetch-Site CSRF guard blocks cross-origin mutations — so a
|
||||
tokenless same-origin loopback request reaches the handler. Identity is
|
||||
enforced only in gated mode.
|
||||
"""
|
||||
resp = self.client.delete("/api/sessions/empty")
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 200
|
||||
|
||||
def test_count_returns_only_empty_ended_unarchived(self):
|
||||
"""With the standard corpus, the count is exactly 2 — only
|
||||
|
|
@ -4601,13 +4708,23 @@ class TestDeleteEmptySessionsEndpoint:
|
|||
|
||||
|
||||
class TestPluginAPIAuth:
|
||||
"""Tests that plugin API routes require the session token (issue #19533)."""
|
||||
"""Plugin API routes have no identity gate on loopback (post-teardown).
|
||||
|
||||
Pre-teardown (issue #19533) plugin ``/api/plugins/*`` routes required the
|
||||
session token and 401'd without it. After the legacy-token teardown,
|
||||
loopback has no identity gate — the loopback bind is the security boundary,
|
||||
the Sec-Fetch-Site CSRF guard blocks cross-origin mutations, and CORS blocks
|
||||
cross-origin reads. So tokenless loopback plugin requests now reach the
|
||||
handler (or the router's own 404/422), never 401. Identity is enforced only
|
||||
in gated mode.
|
||||
"""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _setup_test_client(self, monkeypatch, _isolate_hermes_home, _install_example_plugin):
|
||||
"""Create a TestClient without the session token header.
|
||||
|
||||
Pulls in ``_install_example_plugin`` so ``test_plugin_route_allows_auth``
|
||||
Pulls in ``_install_example_plugin`` so
|
||||
``test_plugin_route_serves_on_loopback_with_or_without_token``
|
||||
has the ``/api/plugins/example/hello`` endpoint available — the
|
||||
example plugin is no longer a bundled plugin, so the fixture
|
||||
installs it into the per-test ``HERMES_HOME``.
|
||||
|
|
@ -4627,69 +4744,87 @@ class TestPluginAPIAuth:
|
|||
self.auth_client = TestClient(app)
|
||||
self.auth_client.headers[_SESSION_HEADER_NAME] = _SESSION_TOKEN
|
||||
|
||||
def test_plugin_route_requires_auth(self):
|
||||
"""Plugin API routes should return 401 without a valid session token."""
|
||||
def test_plugin_route_no_identity_gate_on_loopback(self):
|
||||
"""Plugin API GET routes serve on loopback without a session token."""
|
||||
# Use a known plugin route (kanban board)
|
||||
resp = self.client.get("/api/plugins/kanban/board")
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 200
|
||||
|
||||
def test_plugin_route_allows_auth(self):
|
||||
"""Plugin API routes should work with a valid session token.
|
||||
def test_plugin_route_serves_on_loopback_with_or_without_token(self):
|
||||
"""Plugin API routes serve on loopback regardless of the session token.
|
||||
|
||||
Uses ``/api/plugins/example/hello`` from the example-dashboard
|
||||
test fixture (installed into HERMES_HOME by the class-level
|
||||
``_install_example_plugin`` fixture) — a stable, side-effect-free
|
||||
GET that's only loaded for tests. With a valid token the handler
|
||||
should run (200); without one the middleware should 401 before
|
||||
the handler is reached.
|
||||
GET that's only loaded for tests. Pre-teardown a tokenless request
|
||||
401'd; now loopback has no identity gate, so the handler runs (200)
|
||||
whether or not the legacy token is present. Identity is enforced only
|
||||
in gated mode.
|
||||
"""
|
||||
# Without auth: middleware blocks before reaching the handler.
|
||||
# Without a token: loopback has no identity gate, handler runs.
|
||||
resp = self.client.get("/api/plugins/example/hello")
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 200
|
||||
|
||||
# With auth: handler runs.
|
||||
# With the (now-ignored) token: handler still runs.
|
||||
resp = self.auth_client.get("/api/plugins/example/hello")
|
||||
assert resp.status_code == 200
|
||||
|
||||
def test_plugin_post_requires_auth(self):
|
||||
"""Plugin POST routes should return 401 without a valid session token."""
|
||||
resp = self.client.post("/api/plugins/kanban/tasks", json={"title": "test"})
|
||||
assert resp.status_code == 401
|
||||
def test_plugin_post_no_identity_gate_on_loopback(self):
|
||||
"""Plugin POST routes serve on loopback without a session token.
|
||||
|
||||
def test_plugin_patch_requires_auth(self):
|
||||
"""Plugin PATCH routes should return 401 without a valid session token.
|
||||
The Sec-Fetch-Site CSRF guard blocks cross-origin mutations, but a
|
||||
same-origin loopback POST has no hostile Sec-Fetch-Site header and
|
||||
reaches the handler.
|
||||
"""
|
||||
resp = self.client.post("/api/plugins/kanban/tasks", json={"title": "test"})
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 200
|
||||
|
||||
def test_plugin_patch_no_identity_gate_on_loopback(self):
|
||||
"""Plugin PATCH routes serve on loopback without a session token.
|
||||
|
||||
PATCH is the mutation method most commonly used by the dashboard for
|
||||
kanban task edits — explicitly cover it so a future middleware
|
||||
regression that whitelists non-GET methods can't sneak through.
|
||||
kanban task edits. Pre-teardown a tokenless PATCH 401'd; now loopback
|
||||
has no identity gate (the bind + Sec-Fetch-Site CSRF guard are the
|
||||
boundary) so the request reaches the handler. ``t_fake`` doesn't exist,
|
||||
so the handler/router responds non-401 (e.g. 404/422) — the point is
|
||||
it's no longer gated. Identity is enforced only in gated mode.
|
||||
"""
|
||||
resp = self.client.patch(
|
||||
"/api/plugins/kanban/tasks/t_fake",
|
||||
json={"title": "renamed"},
|
||||
)
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
|
||||
def test_plugin_delete_requires_auth(self):
|
||||
"""Plugin DELETE routes should return 401 without a valid session token."""
|
||||
def test_plugin_delete_no_identity_gate_on_loopback(self):
|
||||
"""Plugin DELETE routes serve on loopback without a session token.
|
||||
|
||||
Loopback has no identity gate; ``t_fake`` doesn't exist so the handler
|
||||
responds non-401 (404). Identity is enforced only in gated mode.
|
||||
"""
|
||||
resp = self.client.delete("/api/plugins/kanban/tasks/t_fake")
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
|
||||
def test_non_kanban_plugin_route_requires_auth(self):
|
||||
"""Auth must be plugin-agnostic, not kanban-specific.
|
||||
def test_non_kanban_plugin_route_no_identity_gate_on_loopback(self):
|
||||
"""The loopback no-identity-gate behavior is plugin-agnostic.
|
||||
|
||||
The middleware fix is at the gate level (no per-plugin allowlist),
|
||||
The gate change is at the middleware level (no per-plugin allowlist),
|
||||
so any plugin's API surface — kanban, hermes-achievements, future
|
||||
plugins — must require the session token. Hit a non-kanban plugin
|
||||
path to lock that in.
|
||||
plugins — and even a non-existent plugin namespace are no longer 401'd
|
||||
on loopback. Pre-teardown these 401'd before routing could 404. Now the
|
||||
router decides: a missing route/plugin yields 404 (not 401). Identity is
|
||||
enforced only in gated mode.
|
||||
"""
|
||||
# Real plugin path (hermes-achievements is loaded by default).
|
||||
resp = self.client.get("/api/plugins/hermes-achievements/overview")
|
||||
assert resp.status_code == 401
|
||||
# Same for an arbitrary plugin namespace that doesn't even exist —
|
||||
# the middleware should 401 before routing decides 404, so an
|
||||
# attacker can't fingerprint plugin names by status codes.
|
||||
assert resp.status_code != 401
|
||||
# A plugin namespace that doesn't exist: now 404 from the router,
|
||||
# not a 401 from a removed identity gate.
|
||||
resp = self.client.get("/api/plugins/_definitely_not_a_plugin_/anything")
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
assert resp.status_code == 404
|
||||
|
||||
def test_plugin_websocket_unaffected_by_http_middleware(self):
|
||||
"""The kanban /events WebSocket has its own ``?token=`` check;
|
||||
|
|
|
|||
|
|
@ -5,6 +5,11 @@ import pytest
|
|||
|
||||
from hermes_cli import web_server
|
||||
|
||||
# These tests mutate ``web_server.app.state`` (auth_required / bound_host);
|
||||
# share the xdist group used by every dashboard-auth gate test so they
|
||||
# don't race against each other across workers.
|
||||
pytestmark = pytest.mark.xdist_group("dashboard_auth_app_state")
|
||||
|
||||
pytest.importorskip("starlette.testclient")
|
||||
from starlette.testclient import TestClient
|
||||
|
||||
|
|
@ -174,15 +179,54 @@ def test_fs_default_cwd_falls_back_when_terminal_cwd_is_invalid(client, tmp_path
|
|||
assert response.json() == {"cwd": str(fallback), "branch": ""}
|
||||
|
||||
|
||||
def test_fs_endpoints_require_auth(tmp_path):
|
||||
client = TestClient(web_server.app)
|
||||
def test_fs_endpoints_no_identity_gate_on_loopback(tmp_path):
|
||||
"""Loopback has no identity gate after the legacy-token teardown.
|
||||
|
||||
The /api/fs/* endpoints read arbitrary files, but on a loopback bind
|
||||
the OS boundary + CSRF guard are the protection, not a per-request
|
||||
token. A tokenless local request is served (reaches the handler — any
|
||||
non-401). Identity enforcement for these endpoints in GATED mode is
|
||||
pinned by test_fs_endpoints_require_auth_in_gated_mode below.
|
||||
"""
|
||||
prev = getattr(web_server.app.state, "auth_required", None)
|
||||
web_server.app.state.auth_required = False
|
||||
target = tmp_path / "secret.txt"
|
||||
target.write_text("secret")
|
||||
try:
|
||||
client = TestClient(web_server.app)
|
||||
list_response = client.get("/api/fs/list", params={"path": str(tmp_path)})
|
||||
read_response = client.get("/api/fs/read-text", params={"path": str(target)})
|
||||
default_response = client.get("/api/fs/default-cwd")
|
||||
assert list_response.status_code != 401
|
||||
assert read_response.status_code != 401
|
||||
assert default_response.status_code != 401
|
||||
finally:
|
||||
web_server.app.state.auth_required = prev
|
||||
|
||||
list_response = client.get("/api/fs/list", params={"path": str(tmp_path)})
|
||||
read_response = client.get("/api/fs/read-text", params={"path": str(target)})
|
||||
default_response = client.get("/api/fs/default-cwd")
|
||||
|
||||
assert list_response.status_code == 401
|
||||
assert read_response.status_code == 401
|
||||
assert default_response.status_code == 401
|
||||
def test_fs_endpoints_require_auth_in_gated_mode(tmp_path):
|
||||
"""In gated (non-loopback) mode the /api/fs/* endpoints require a
|
||||
verified session cookie — a cookieless request 401s at the gate."""
|
||||
from hermes_cli.dashboard_auth import clear_providers, register_provider
|
||||
from tests.hermes_cli.conftest_dashboard_auth import StubAuthProvider
|
||||
|
||||
prev_host = getattr(web_server.app.state, "bound_host", None)
|
||||
prev_req = getattr(web_server.app.state, "auth_required", None)
|
||||
clear_providers()
|
||||
register_provider(StubAuthProvider())
|
||||
web_server.app.state.bound_host = "fly-app.fly.dev"
|
||||
web_server.app.state.auth_required = True
|
||||
target = tmp_path / "secret.txt"
|
||||
target.write_text("secret")
|
||||
try:
|
||||
client = TestClient(web_server.app, base_url="https://fly-app.fly.dev")
|
||||
assert client.get(
|
||||
"/api/fs/list", params={"path": str(tmp_path)}
|
||||
).status_code == 401
|
||||
assert client.get(
|
||||
"/api/fs/read-text", params={"path": str(target)}
|
||||
).status_code == 401
|
||||
finally:
|
||||
clear_providers()
|
||||
web_server.app.state.bound_host = prev_host
|
||||
web_server.app.state.auth_required = prev_req
|
||||
|
|
|
|||
|
|
@ -204,14 +204,22 @@ class TestEditorEndpointsAuth:
|
|||
("put", "/api/skills/content", {"json": {"name": "x", "content": "y"}}),
|
||||
],
|
||||
)
|
||||
def test_endpoints_401_without_token(
|
||||
def test_endpoints_no_identity_gate_on_loopback(
|
||||
self, client, isolated_profiles, method, path, kwargs
|
||||
):
|
||||
"""Loopback has no identity gate after the legacy-token teardown.
|
||||
|
||||
Pre-teardown these endpoints 401'd without the session token. Now
|
||||
the loopback bind + CSRF guard are the boundary, so a tokenless
|
||||
local request is served (reaches the handler — any non-401). The
|
||||
gated (non-loopback) path is where identity is enforced, covered by
|
||||
the dashboard-auth gate tests.
|
||||
"""
|
||||
from hermes_cli.web_server import _SESSION_HEADER_NAME
|
||||
|
||||
client.headers.pop(_SESSION_HEADER_NAME, None)
|
||||
resp = getattr(client, method)(path, **kwargs)
|
||||
assert resp.status_code == 401
|
||||
assert resp.status_code != 401
|
||||
|
||||
|
||||
class TestCronJobSkills:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue