From 52e51de69d5f70812972f533211e31b00622cad9 Mon Sep 17 00:00:00 2001 From: Ben Date: Tue, 16 Jun 2026 12:42:44 +1000 Subject: [PATCH] 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. --- hermes_cli/web_server.py | 56 ++-- .../test_dashboard_admin_endpoints.py | 43 ++- tests/hermes_cli/test_dashboard_auth_gate.py | 13 +- .../test_legacy_token_teardown_baseline.py | 23 ++ tests/hermes_cli/test_web_server.py | 259 +++++++++++++----- tests/hermes_cli/test_web_server_fs.py | 60 +++- .../test_web_server_skill_editor.py | 12 +- 7 files changed, 353 insertions(+), 113 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 6443897667f1..c7cbe4fc0e0a 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -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) diff --git a/tests/hermes_cli/test_dashboard_admin_endpoints.py b/tests/hermes_cli/test_dashboard_admin_endpoints.py index 3eb2ca37d2d1..3a5d0499a14b 100644 --- a/tests/hermes_cli/test_dashboard_admin_endpoints.py +++ b/tests/hermes_cli/test_dashboard_admin_endpoints.py @@ -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" diff --git a/tests/hermes_cli/test_dashboard_auth_gate.py b/tests/hermes_cli/test_dashboard_auth_gate.py index c39356bbb43e..2db03e05c010 100644 --- a/tests/hermes_cli/test_dashboard_auth_gate.py +++ b/tests/hermes_cli/test_dashboard_auth_gate.py @@ -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): diff --git a/tests/hermes_cli/test_legacy_token_teardown_baseline.py b/tests/hermes_cli/test_legacy_token_teardown_baseline.py index 16ac6ac12583..e2cd8f6bd7d8 100644 --- a/tests/hermes_cli/test_legacy_token_teardown_baseline.py +++ b/tests/hermes_cli/test_legacy_token_teardown_baseline.py @@ -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) diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index 8f6842b6b503..5b0fd59fc914 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -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 `` 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; diff --git a/tests/hermes_cli/test_web_server_fs.py b/tests/hermes_cli/test_web_server_fs.py index c2baab00ff3b..4dae98c9c130 100644 --- a/tests/hermes_cli/test_web_server_fs.py +++ b/tests/hermes_cli/test_web_server_fs.py @@ -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 diff --git a/tests/hermes_cli/test_web_server_skill_editor.py b/tests/hermes_cli/test_web_server_skill_editor.py index c89142ae0dff..a367dbdcf802 100644 --- a/tests/hermes_cli/test_web_server_skill_editor.py +++ b/tests/hermes_cli/test_web_server_skill_editor.py @@ -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: