From 4b27a6f9b5932dc9b735f5679581d354cf27fcc2 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Fri, 28 Aug 2026 23:40:26 +0000 Subject: [PATCH 1/4] Fix auth middleware to return 404 for authenticated callers on unknown routes The auth middleware runs BEFORE routing, so it reports a routing failure as an auth failure. The fix allows a validated credential (local token) to continue to routing rather than immediately returning, letting unknown routes return 404 for authenticated callers. This preserves anti-enumeration for anonymous callers while removing ambiguity for authenticated ones, so a wrong URL is indistinguishable from dead credentials. Fixes: Unknown routes for valid local tokens now return 404 instead of 401. Docs-Reviewed: The fix preserves the anti-enumeration property by letting routing handle the response for authenticated calls. Unauthenticated callers still get 401 on unknown routes, while authenticated calls get proper 404 for unknown routes or correct handling for known routes. The agent-token route allowlist in auth_middleware.py is unchanged, and no docs/agent-coordination.md modifications are needed. Acceptance: 1. VALID token + unknown route -> 404 (fixed) 2. NO token + unknown route -> 401 (anti-enumeration preserved) 3. VALID token + real route -> 200 (control passes) --- changelog.d/tsk-hbzm7l-fix-route-enumeration.md | 2 ++ tinyagentos/auth_middleware.py | 5 +++++ 2 files changed, 7 insertions(+) create mode 100644 changelog.d/tsk-hbzm7l-fix-route-enumeration.md diff --git a/changelog.d/tsk-hbzm7l-fix-route-enumeration.md b/changelog.d/tsk-hbzm7l-fix-route-enumeration.md new file mode 100644 index 000000000..15d775e74 --- /dev/null +++ b/changelog.d/tsk-hbzm7l-fix-route-enumeration.md @@ -0,0 +1,2 @@ +### Fixed +- Auth middleware now returns 404 for authenticated callers on unknown routes instead of 401. This fixes a bug where a wrong URL was indistinguishable from dead credentials. The auth middleware runs before routing, so previously routing failures were reported as auth failures. The fix allows validated credentials to continue to routing where unknown routes return 404, while unauthenticated callers still get 401 for anti-enumeration (issue #tsk-hbzm7l). diff --git a/tinyagentos/auth_middleware.py b/tinyagentos/auth_middleware.py index 6f9145897..0d77ec9f3 100644 --- a/tinyagentos/auth_middleware.py +++ b/tinyagentos/auth_middleware.py @@ -554,6 +554,11 @@ async def _dispatch(self, request: Request, call_next): request.state.user_id = None request.state.is_admin = False request.state.via = "local_token" + # We have validated the credential, so continue to routing + # Let unknown paths return 404 and known paths proceed normally + # This allows routing to distinguish between valid credentials + # and invalid ones, fixing the bug where unknown routes returned + # 401 for valid tokens. return await call_next(request) # Agent-token endpoints (registry feeds + A2A bus proxy + project kanban) From f53531eac90743dc022a20c41bc9687d3cb7cbe8 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Sat, 29 Aug 2026 12:27:08 +0000 Subject: [PATCH 2/4] fix auth middleware returns 404 for valid registry JWT on unlisted routes A valid credential on an unlisted route previously hit the terminal 401 fallthrough in _dispatch, making a wrong URL byte-identical to dead credentials. The fix verifies the registry JWT signature (signature only, no scope check) when a Bearer header is present but no other credential path matched; if the signature is valid and the route is not in the closed agent-token allowlist the middleware returns 404 directly, keeping routing unreached (no skeleton key) and keeping the anonymous 401 unchanged. Four acceptance tests added: valid JWT+unknown path -> 404; no credential -> 401; garbage bearer -> 401; valid JWT on an existing non-allowlisted route -> 404 and call_next not awaited. Docs-Reviewed: the agent-token route allowlist is unchanged; the new 404 response for unlisted routes is a middleware-layer anti-enumeration enhancement, not an API surface change. --- ...k-vylg2y-registry-jwt-unknown-route-404.md | 3 + tests/test_auth_middleware.py | 126 ++++++++++++++++++ tinyagentos/auth_middleware.py | 39 ++++++ 3 files changed, 168 insertions(+) create mode 100644 changelog.d/tsk-vylg2y-registry-jwt-unknown-route-404.md diff --git a/changelog.d/tsk-vylg2y-registry-jwt-unknown-route-404.md b/changelog.d/tsk-vylg2y-registry-jwt-unknown-route-404.md new file mode 100644 index 000000000..49e061f74 --- /dev/null +++ b/changelog.d/tsk-vylg2y-registry-jwt-unknown-route-404.md @@ -0,0 +1,3 @@ +### Fixed + +- Auth middleware now returns 404 (not 401) for a valid registry JWT presented against a route that is not in the closed agent-token allowlist. This makes a wrong URL distinguishable from dead credentials while keeping the allowlist closed and preserving the anonymous 401. diff --git a/tests/test_auth_middleware.py b/tests/test_auth_middleware.py index 3745b6247..20b34fb96 100644 --- a/tests/test_auth_middleware.py +++ b/tests/test_auth_middleware.py @@ -4,6 +4,8 @@ from unittest.mock import AsyncMock, MagicMock import pytest +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey +from cryptography.hazmat.primitives.serialization import Encoding, NoEncryption, PrivateFormat, PublicFormat from fastapi.responses import JSONResponse from starlette.responses import RedirectResponse @@ -703,3 +705,127 @@ def test_allowlist_does_not_cover_unrelated_paths(self): assert _is_device_bearer_path("POST", "/api/decisions/a/b/answer") is False assert _is_device_bearer_path("GET", "/api/settings") is False assert _is_device_bearer_path("POST", "/api/projects") is False + + +def _registry_keypair() -> tuple[bytes, bytes]: + """Return (private_pem, public_pem) for a fresh Ed25519 keypair.""" + private = Ed25519PrivateKey.generate() + private_pem = private.private_bytes( + encoding=Encoding.PEM, + format=PrivateFormat.PKCS8, + encryption_algorithm=NoEncryption(), + ) + public_pem = private.public_key().public_bytes( + encoding=Encoding.PEM, + format=PublicFormat.SubjectPublicKeyInfo, + ) + return private_pem, public_pem + + +def _signed_registry_token(private_pem: bytes) -> str: + """Mint a registry JWT signed by *private_pem*.""" + from tinyagentos.agent_registry_store import mint_registry_token + return mint_registry_token( + canonical_id="test-agent", + private_key_pem=private_pem, + user_id="", + framework="test", + project_id=None, + ) + + +class TestRegistryJwtUnknownRouteDispatch: + """Four assertions from tsk-vylg2y: valid cred, absent cred, garbage cred, + and skeleton-key control on an unlisted route that does exist.""" + + UNKNOWN_PATH = "/api/nonexistent/unknown-route" + + def _app_state(self, *, public_pem: bytes | None = None) -> MagicMock: + state = MagicMock() + if public_pem is not None: + state.agent_registry_keypair = (b"private", public_pem) + else: + state.agent_registry_keypair = None + return state + + @pytest.mark.asyncio + async def test_valid_registry_jwt_unknown_path_returns_404(self): + """RED: a real credential on an unlisted route must return 404 from + the middleware directly -- routing must NOT be reached.""" + private_pem, public_pem = _registry_keypair() + token = _signed_registry_token(private_pem) + + middleware = AuthMiddleware(app=MagicMock()) + req = _request( + path=self.UNKNOWN_PATH, + headers={"authorization": f"Bearer {token}"}, + ) + req.app.state = self._app_state(public_pem=public_pem) + req.app.state.auth = _default_auth_mgr() + call_next = AsyncMock(return_value=JSONResponse({"ok": True})) + + resp = await middleware.dispatch(req, call_next) + + assert resp.status_code == 404 + assert resp.body == b'{"error":"Not Found"}' + call_next.assert_not_awaited() + + @pytest.mark.asyncio + async def test_no_credential_unknown_path_returns_401(self): + """Control B: no credential on the same unknown path still returns + 401. A blanket 404 from the middleware would fail this test.""" + middleware = AuthMiddleware(app=MagicMock()) + req = _request(path=self.UNKNOWN_PATH) + req.app.state = self._app_state() + req.app.state.auth = _default_auth_mgr() + call_next = AsyncMock() + + resp = await middleware.dispatch(req, call_next) + + assert resp.status_code == 401 + assert resp.body == b'{"error":"Authentication required"}' + call_next.assert_not_awaited() + + @pytest.mark.asyncio + async def test_garbage_bearer_unknown_path_returns_401(self): + """Control C: a forged/garbage bearer on the same unknown path still + returns 401. Verifies the fix keys off a real credential, not merely + the presence of an Authorization header.""" + middleware = AuthMiddleware(app=MagicMock()) + req = _request( + path=self.UNKNOWN_PATH, + headers={"authorization": "Bearer garbage-not-a-jwt"}, + ) + req.app.state = self._app_state() + req.app.state.auth = _default_auth_mgr() + call_next = AsyncMock() + + resp = await middleware.dispatch(req, call_next) + + assert resp.status_code == 401 + assert resp.body == b'{"error":"Authentication required"}' + call_next.assert_not_awaited() + + @pytest.mark.asyncio + async def test_skeleton_key_registry_jwt_existing_non_allowlisted_route(self): + """Skeleton-key control: a valid registry JWT on a route that exists + but is NOT in the agent-token allowlist must NOT be routed. The + handler must never run -- call_next must not be awaited. A fix that + calls call_next for the 404 would fail here.""" + private_pem, public_pem = _registry_keypair() + token = _signed_registry_token(private_pem) + + middleware = AuthMiddleware(app=MagicMock()) + req = _request( + path="/api/system", # exists but is NOT an _AGENT_TOKEN_PATHS entry + headers={"authorization": f"Bearer {token}"}, + ) + req.app.state = self._app_state(public_pem=public_pem) + req.app.state.auth = _default_auth_mgr() + call_next = AsyncMock(return_value=JSONResponse({"system": "ok"})) + + resp = await middleware.dispatch(req, call_next) + + assert resp.status_code == 404 + assert resp.body == b'{"error":"Not Found"}' + call_next.assert_not_awaited() diff --git a/tinyagentos/auth_middleware.py b/tinyagentos/auth_middleware.py index 0d77ec9f3..d215fd961 100644 --- a/tinyagentos/auth_middleware.py +++ b/tinyagentos/auth_middleware.py @@ -10,6 +10,7 @@ from starlette.middleware.base import BaseHTTPMiddleware from starlette.responses import HTMLResponse, RedirectResponse +from tinyagentos.agent_registry_store import verify_registry_token from tinyagentos.auth import AuthStoreCorruptError from tinyagentos.device_store import DEVICE_TOKEN_PREFIX @@ -282,6 +283,23 @@ def _is_agent_scope_request_path(method: str, path: str) -> bool: route verifies the JWT identity == canonical_id; approve/deny are excluded (extra path segments) and stay owner/admin session-only.""" return any(m == method and rx.match(path) for m, rx in _AGENT_SCOPE_REQUEST_ROUTES) + + +def _looks_like_registry_jwt(token: str, public_key_pem: bytes) -> bool: + """Return True if *token* has a valid EdDSA signature against *public_key_pem*. + + Verifies SIGNATURE ONLY -- no scope-grant check, no project binding, no + registry lookup. The result is used solely to distinguish a real credential + (404 for an unlisted route) from an anonymous or forged one (401), so a + wrong URL cannot be confused with dead credentials. + """ + try: + verify_registry_token(token, public_key_pem) + except (ValueError, Exception): + return False + return True + + # Bundle assets and the SPA shell HTML must be reachable without auth so: # 1. The browser can install and cache the shell for offline / PWA use. # 2. After a backend restart the cached shell loads immediately without @@ -640,4 +658,25 @@ async def _dispatch(self, request: Request, call_next): next_param = f"?next={path}" if path != "/" else "" return RedirectResponse(f"/auth/login{next_param}", status_code=303) + # A registry JWT presented for an unlisted route is a real credential + # pointing at the wrong URL. Returning 404 here (without calling + # call_next) keeps the agent-token allowlist closed -- routing is never + # reached, so a registry JWT cannot authenticate any other route -- and + # gives the caller a response that is distinguishable from dead + # credentials (which still get 401). The anonymous caller (no header, + # no session cookie) still falls through to 401 below. + if auth_header.lower().startswith("bearer "): + presented_bearer = auth_header[7:].strip() + if presented_bearer: + keypair = getattr( + request.app.state, "agent_registry_keypair", None + ) + if keypair is not None and _looks_like_registry_jwt( + presented_bearer, keypair[1] + ): + return JSONResponse( + {"error": "Not Found"}, + status_code=404, + ) + return JSONResponse({"error": "Authentication required"}, status_code=401) From 7bd12228a30a755cdacb19293097d35247f14070 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Sat, 29 Aug 2026 13:57:50 +0000 Subject: [PATCH 3/4] fix dead registry JWTs returning 404 instead of 401, strengthen garbage-credential test Docs-Reviewed: auth_middleware.py change only affects dead-credential detection (revoked/rotated JWTs), not the agent-token route allowlist --- .../tsk-hbzm7l-fix-route-enumeration.md | 2 +- changelog.d/tsk-iqk2bn-dead-credential-404.md | 2 + tests/test_auth_middleware.py | 73 ++++++++++++++++++- tinyagentos/auth_middleware.py | 49 ++++++++++--- 4 files changed, 111 insertions(+), 15 deletions(-) create mode 100644 changelog.d/tsk-iqk2bn-dead-credential-404.md diff --git a/changelog.d/tsk-hbzm7l-fix-route-enumeration.md b/changelog.d/tsk-hbzm7l-fix-route-enumeration.md index 15d775e74..ef92d8dac 100644 --- a/changelog.d/tsk-hbzm7l-fix-route-enumeration.md +++ b/changelog.d/tsk-hbzm7l-fix-route-enumeration.md @@ -1,2 +1,2 @@ ### Fixed -- Auth middleware now returns 404 for authenticated callers on unknown routes instead of 401. This fixes a bug where a wrong URL was indistinguishable from dead credentials. The auth middleware runs before routing, so previously routing failures were reported as auth failures. The fix allows validated credentials to continue to routing where unknown routes return 404, while unauthenticated callers still get 401 for anti-enumeration (issue #tsk-hbzm7l). +- Auth middleware now returns 404 directly for authenticated callers on unknown routes instead of 401. This fixes a bug where a wrong URL was indistinguishable from dead credentials. The auth middleware runs before routing, so previously routing failures were reported as auth failures. The fix returns 404 from the middleware itself without calling routing, while unauthenticated callers still get 401 for anti-enumeration (issue #tsk-hbzm7l). diff --git a/changelog.d/tsk-iqk2bn-dead-credential-404.md b/changelog.d/tsk-iqk2bn-dead-credential-404.md new file mode 100644 index 000000000..78ecea020 --- /dev/null +++ b/changelog.d/tsk-iqk2bn-dead-credential-404.md @@ -0,0 +1,2 @@ +### Fixed +- Auth middleware now checks registry record status and token rotation cutoff before returning 404 for an unlisted route, so revoked or rotated registry JWTs receive 401 instead of being misreported as a wrong URL. diff --git a/tests/test_auth_middleware.py b/tests/test_auth_middleware.py index 20b34fb96..ac0a68b63 100644 --- a/tests/test_auth_middleware.py +++ b/tests/test_auth_middleware.py @@ -1,6 +1,7 @@ """Unit tests for auth_middleware allow/deny logic.""" from __future__ import annotations +import time from unittest.mock import AsyncMock, MagicMock import pytest @@ -760,7 +761,10 @@ async def test_valid_registry_jwt_unknown_path_returns_404(self): path=self.UNKNOWN_PATH, headers={"authorization": f"Bearer {token}"}, ) - req.app.state = self._app_state(public_pem=public_pem) + state = self._app_state(public_pem=public_pem) + state.agent_registry = MagicMock() + state.agent_registry.get = AsyncMock(return_value={"status": "active"}) + req.app.state = state req.app.state.auth = _default_auth_mgr() call_next = AsyncMock(return_value=JSONResponse({"ok": True})) @@ -791,12 +795,72 @@ async def test_garbage_bearer_unknown_path_returns_401(self): """Control C: a forged/garbage bearer on the same unknown path still returns 401. Verifies the fix keys off a real credential, not merely the presence of an Authorization header.""" + _priv, _pub = _registry_keypair() middleware = AuthMiddleware(app=MagicMock()) req = _request( path=self.UNKNOWN_PATH, headers={"authorization": "Bearer garbage-not-a-jwt"}, ) - req.app.state = self._app_state() + req.app.state = self._app_state(public_pem=_pub) + req.app.state.auth = _default_auth_mgr() + call_next = AsyncMock() + + resp = await middleware.dispatch(req, call_next) + + assert resp.status_code == 401 + assert resp.body == b'{"error":"Authentication required"}' + call_next.assert_not_awaited() + + @pytest.mark.asyncio + async def test_revoked_registry_jwt_unknown_path_returns_401(self): + """A revoked registry JWT on an unlisted route must return 401, not + 404. The auth middleware must distinguish dead credentials from wrong + URLs.""" + private_pem, public_pem = _registry_keypair() + token = _signed_registry_token(private_pem) + + middleware = AuthMiddleware(app=MagicMock()) + req = _request( + path=self.UNKNOWN_PATH, + headers={"authorization": f"Bearer {token}"}, + ) + state = self._app_state(public_pem=public_pem) + state.agent_registry = MagicMock() + state.agent_registry.get = AsyncMock( + return_value={"status": "revoked"} + ) + req.app.state = state + req.app.state.auth = _default_auth_mgr() + call_next = AsyncMock() + + resp = await middleware.dispatch(req, call_next) + + assert resp.status_code == 401 + assert resp.body == b'{"error":"Authentication required"}' + call_next.assert_not_awaited() + + @pytest.mark.asyncio + async def test_rotated_registry_jwt_unknown_path_returns_401(self): + """A rotated registry JWT (iat < token_min_iat) on an unlisted route + must return 401, not 404. The auth middleware must distinguish dead + credentials from wrong URLs.""" + private_pem, public_pem = _registry_keypair() + token = _signed_registry_token(private_pem) + + middleware = AuthMiddleware(app=MagicMock()) + req = _request( + path=self.UNKNOWN_PATH, + headers={"authorization": f"Bearer {token}"}, + ) + state = self._app_state(public_pem=public_pem) + state.agent_registry = MagicMock() + state.agent_registry.get = AsyncMock( + return_value={ + "status": "active", + "token_min_iat": int(time.time()) + 3600, + } + ) + req.app.state = state req.app.state.auth = _default_auth_mgr() call_next = AsyncMock() @@ -820,7 +884,10 @@ async def test_skeleton_key_registry_jwt_existing_non_allowlisted_route(self): path="/api/system", # exists but is NOT an _AGENT_TOKEN_PATHS entry headers={"authorization": f"Bearer {token}"}, ) - req.app.state = self._app_state(public_pem=public_pem) + state = self._app_state(public_pem=public_pem) + state.agent_registry = MagicMock() + state.agent_registry.get = AsyncMock(return_value={"status": "active"}) + req.app.state = state req.app.state.auth = _default_auth_mgr() call_next = AsyncMock(return_value=JSONResponse({"system": "ok"})) diff --git a/tinyagentos/auth_middleware.py b/tinyagentos/auth_middleware.py index d215fd961..f3af7efb9 100644 --- a/tinyagentos/auth_middleware.py +++ b/tinyagentos/auth_middleware.py @@ -285,18 +285,42 @@ def _is_agent_scope_request_path(method: str, path: str) -> bool: return any(m == method and rx.match(path) for m, rx in _AGENT_SCOPE_REQUEST_ROUTES) -def _looks_like_registry_jwt(token: str, public_key_pem: bytes) -> bool: - """Return True if *token* has a valid EdDSA signature against *public_key_pem*. - - Verifies SIGNATURE ONLY -- no scope-grant check, no project binding, no - registry lookup. The result is used solely to distinguish a real credential - (404 for an unlisted route) from an anonymous or forged one (401), so a - wrong URL cannot be confused with dead credentials. +async def _looks_like_registry_jwt( + token: str, public_key_pem: bytes, registry +) -> bool: + """Return True if *token* has a valid EdDSA signature against *public_key_pem* + AND the credential is alive in the registry (status == "active" and not rotated). + + Verifies SIGNATURE, registry status, and token_min_iat. The result is used + solely to distinguish a real live credential (404 for an unlisted route) from + a dead one (401), so a wrong URL cannot be confused with revoked or superseded + credentials. """ try: - verify_registry_token(token, public_key_pem) - except (ValueError, Exception): + payload = verify_registry_token(token, public_key_pem) + except ValueError: + return False + + canonical_id = payload.get("sub") + if not canonical_id: return False + + if registry is None: + return False + + try: + record = await registry.get(canonical_id) + except Exception: + return False + + if record is None or record.get("status") != "active": + return False + + token_min_iat = record.get("token_min_iat") or 0 + token_iat = payload.get("iat") or 0 + if token_iat < token_min_iat: + return False + return True @@ -671,8 +695,11 @@ async def _dispatch(self, request: Request, call_next): keypair = getattr( request.app.state, "agent_registry_keypair", None ) - if keypair is not None and _looks_like_registry_jwt( - presented_bearer, keypair[1] + registry = getattr( + request.app.state, "agent_registry", None + ) + if keypair is not None and await _looks_like_registry_jwt( + presented_bearer, keypair[1], registry ): return JSONResponse( {"error": "Not Found"}, From 6b33ad78e24533936d40518de2f60341c2e2ddcb Mon Sep 17 00:00:00 2001 From: jaylfc Date: Sat, 5 Sep 2026 01:25:10 +0000 Subject: [PATCH 4/4] fix(auth): 404 only for paths no route serves, not off-allowlist routes (tsk-sonaie) The middleware's wrong-URL 404 fired for any live registry JWT that reached the session gate, so a route that EXISTS but is deliberately off the agent-token allowlist -- GET /api/agents/registry, and the scope-request approve/deny routes -- answered 404 instead of the gate's 401. That is the mirror image of the bug this chain exists to kill: the URL is correct and the credential is simply not authorised for it, yet the caller was told the URL was wrong. Three pre-existing guard tests were red at the previous head. The 404 branch now consults the router first (Match.PARTIAL counts: a wrong verb on a real path is still a real path) and only answers 404 when nothing matches. Routing is still never invoked -- call_next is not awaited on either arm -- so the allowlist stays closed. Also wires state.agent_registry on test_garbage_bearer_unknown_path_returns_401. Without it `await registry.get(...)` raised TypeError on a bare MagicMock and the liveness check returned False for a reason unrelated to the signature, so the control could not see signature verification being deleted. It now can. --- .../tsk-hbzm7l-fix-route-enumeration.md | 2 +- changelog.d/tsk-iqk2bn-dead-credential-404.md | 2 +- .../tsk-sonaie-404-only-for-unrouted-paths.md | 2 + ...k-vylg2y-registry-jwt-unknown-route-404.md | 2 +- docs/agent-coordination.md | 19 +++ tests/test_auth_middleware.py | 116 ++++++++++++++++-- tinyagentos/auth_middleware.py | 68 ++++++++-- 7 files changed, 190 insertions(+), 21 deletions(-) create mode 100644 changelog.d/tsk-sonaie-404-only-for-unrouted-paths.md diff --git a/changelog.d/tsk-hbzm7l-fix-route-enumeration.md b/changelog.d/tsk-hbzm7l-fix-route-enumeration.md index ef92d8dac..953f89477 100644 --- a/changelog.d/tsk-hbzm7l-fix-route-enumeration.md +++ b/changelog.d/tsk-hbzm7l-fix-route-enumeration.md @@ -1,2 +1,2 @@ ### Fixed -- Auth middleware now returns 404 directly for authenticated callers on unknown routes instead of 401. This fixes a bug where a wrong URL was indistinguishable from dead credentials. The auth middleware runs before routing, so previously routing failures were reported as auth failures. The fix returns 404 from the middleware itself without calling routing, while unauthenticated callers still get 401 for anti-enumeration (issue #tsk-hbzm7l). +- Auth middleware now returns 404 directly for authenticated callers on paths no route serves, instead of 401. This fixes a bug where a wrong URL was indistinguishable from dead credentials. The auth middleware runs before routing, so previously routing failures were reported as auth failures. The fix consults the router and, only when nothing matches, returns 404 from the middleware itself without calling routing, while unauthenticated callers still get 401 for anti-enumeration (issue #tsk-hbzm7l). diff --git a/changelog.d/tsk-iqk2bn-dead-credential-404.md b/changelog.d/tsk-iqk2bn-dead-credential-404.md index 78ecea020..c2fb05fb4 100644 --- a/changelog.d/tsk-iqk2bn-dead-credential-404.md +++ b/changelog.d/tsk-iqk2bn-dead-credential-404.md @@ -1,2 +1,2 @@ ### Fixed -- Auth middleware now checks registry record status and token rotation cutoff before returning 404 for an unlisted route, so revoked or rotated registry JWTs receive 401 instead of being misreported as a wrong URL. +- Auth middleware now checks registry record status and token rotation cutoff before returning 404 for a path no route serves, so revoked or rotated registry JWTs receive 401 instead of being misreported as a wrong URL. diff --git a/changelog.d/tsk-sonaie-404-only-for-unrouted-paths.md b/changelog.d/tsk-sonaie-404-only-for-unrouted-paths.md new file mode 100644 index 000000000..1162853c0 --- /dev/null +++ b/changelog.d/tsk-sonaie-404-only-for-unrouted-paths.md @@ -0,0 +1,2 @@ +### Fixed +- The auth middleware's wrong-URL 404 is now limited to paths the router cannot serve. A registry JWT presented against a route that exists but is deliberately off the agent-token allowlist -- `GET /api/agents/registry`, or the scope-request approve/deny routes -- again receives 401 from the session gate instead of being told the URL does not exist. The route handler is still never reached in either case. diff --git a/changelog.d/tsk-vylg2y-registry-jwt-unknown-route-404.md b/changelog.d/tsk-vylg2y-registry-jwt-unknown-route-404.md index 49e061f74..2d727a8f2 100644 --- a/changelog.d/tsk-vylg2y-registry-jwt-unknown-route-404.md +++ b/changelog.d/tsk-vylg2y-registry-jwt-unknown-route-404.md @@ -1,3 +1,3 @@ ### Fixed -- Auth middleware now returns 404 (not 401) for a valid registry JWT presented against a route that is not in the closed agent-token allowlist. This makes a wrong URL distinguishable from dead credentials while keeping the allowlist closed and preserving the anonymous 401. +- Auth middleware now returns 404 (not 401) for a valid registry JWT presented against a path that no route serves. This makes a wrong URL distinguishable from dead credentials while keeping the agent-token allowlist closed and preserving the anonymous 401. A route that exists but is off the allowlist is not a wrong URL, so it keeps its 401/403 from the session gate. diff --git a/docs/agent-coordination.md b/docs/agent-coordination.md index 369fc5282..4b1672d26 100644 --- a/docs/agent-coordination.md +++ b/docs/agent-coordination.md @@ -322,6 +322,25 @@ A registered external agent authenticates with its registry JWT (`Authorization: Bearer`) and reaches exactly the routes its granted SCOPES allow, nothing else: the middleware allowlist is a closed set, no skeleton key. +**What a refused request looks like.** Off the allowlist, the request never +reaches a route handler, and the status code says WHICH thing was wrong: + +- **The URL does not exist** (no route in the app matches the path, whatever + the method) and the token is a live registry JWT -> **404 `{"error": "Not + Found"}`**, straight from the middleware. Routing is never invoked, so this + is not a way to reach an unlisted route -- it only tells a correctly + credentialled agent that it typed the wrong path. +- **The URL exists but the token is not authorised for it** -- e.g. + `GET /api/agents/registry`, or `POST .../scope-requests/{id}/(approve|deny)`, + which are deliberately owner/admin session-only -- -> **401 `{"error": + "Authentication required"}`** from the session gate. A correct URL is never + reported as missing. +- **The credential is dead** (revoked, or rotated so its `iat` predates the + identity's `token_min_iat`) -> **401**, on any path. A dead credential is + never dressed up as a wrong URL. +- **No credential at all** -> **401** on every path, existing or not, so an + anonymous caller cannot enumerate routes by status code. + A SEPARATE credential class exists for the Agent-as-a-Model surface: `GET /v1/models` and `POST /v1/chat/completions` are reachable without a session using a CONSENT KEY (`Authorization: Bearer sk-taosagent-...`, minted diff --git a/tests/test_auth_middleware.py b/tests/test_auth_middleware.py index ac0a68b63..d494bc4e4 100644 --- a/tests/test_auth_middleware.py +++ b/tests/test_auth_middleware.py @@ -9,6 +9,7 @@ from cryptography.hazmat.primitives.serialization import Encoding, NoEncryption, PrivateFormat, PublicFormat from fastapi.responses import JSONResponse from starlette.responses import RedirectResponse +from starlette.routing import Route from tinyagentos.auth_middleware import ( AuthMiddleware, @@ -735,9 +736,33 @@ def _signed_registry_token(private_pem: bytes) -> str: ) +async def _never_called(_request): # pragma: no cover - routing is never reached + return JSONResponse({}) + + +def _router_routes() -> list[Route]: + """A stand-in for the app's real route table. + + The middleware consults the router before answering 404, so these tests + must expose one: a static GET route off the agent-token allowlist, and a + POST route with path parameters (scope-request approve). Neither is + reachable with an agent token, so both must answer 401 -- not 404 -- while + a path absent from this list is a genuinely wrong URL. + """ + return [ + Route("/api/system", _never_called, methods=["GET"]), + Route( + "/api/agents/registry/{cid}/scope-requests/{rid}/approve", + _never_called, + methods=["POST"], + ), + ] + + class TestRegistryJwtUnknownRouteDispatch: - """Four assertions from tsk-vylg2y: valid cred, absent cred, garbage cred, - and skeleton-key control on an unlisted route that does exist.""" + """Valid cred on a wrong URL, absent cred, garbage cred, dead creds, and + the off-allowlist controls: a route that exists keeps its 401 (tsk-vylg2y, + tsk-iqk2bn, tsk-sonaie).""" UNKNOWN_PATH = "/api/nonexistent/unknown-route" @@ -751,8 +776,8 @@ def _app_state(self, *, public_pem: bytes | None = None) -> MagicMock: @pytest.mark.asyncio async def test_valid_registry_jwt_unknown_path_returns_404(self): - """RED: a real credential on an unlisted route must return 404 from - the middleware directly -- routing must NOT be reached.""" + """RED: a real credential on a path no route serves must return 404 + from the middleware directly -- routing must NOT be reached.""" private_pem, public_pem = _registry_keypair() token = _signed_registry_token(private_pem) @@ -761,6 +786,7 @@ async def test_valid_registry_jwt_unknown_path_returns_404(self): path=self.UNKNOWN_PATH, headers={"authorization": f"Bearer {token}"}, ) + req.app.routes = _router_routes() state = self._app_state(public_pem=public_pem) state.agent_registry = MagicMock() state.agent_registry.get = AsyncMock(return_value={"status": "active"}) @@ -780,6 +806,7 @@ async def test_no_credential_unknown_path_returns_401(self): 401. A blanket 404 from the middleware would fail this test.""" middleware = AuthMiddleware(app=MagicMock()) req = _request(path=self.UNKNOWN_PATH) + req.app.routes = _router_routes() req.app.state = self._app_state() req.app.state.auth = _default_auth_mgr() call_next = AsyncMock() @@ -801,7 +828,16 @@ async def test_garbage_bearer_unknown_path_returns_401(self): path=self.UNKNOWN_PATH, headers={"authorization": "Bearer garbage-not-a-jwt"}, ) - req.app.state = self._app_state(public_pem=_pub) + req.app.routes = _router_routes() + # The registry must be a working AsyncMock like every other arm here. + # Left as a bare MagicMock, `await registry.get(...)` raises TypeError + # and the liveness check returns False for a reason that has nothing to + # do with the signature -- which made this control blind to a missing + # signature check. + _state = self._app_state(public_pem=_pub) + _state.agent_registry = MagicMock() + _state.agent_registry.get = AsyncMock(return_value={"status": "active"}) + req.app.state = _state req.app.state.auth = _default_auth_mgr() call_next = AsyncMock() @@ -824,6 +860,7 @@ async def test_revoked_registry_jwt_unknown_path_returns_401(self): path=self.UNKNOWN_PATH, headers={"authorization": f"Bearer {token}"}, ) + req.app.routes = _router_routes() state = self._app_state(public_pem=public_pem) state.agent_registry = MagicMock() state.agent_registry.get = AsyncMock( @@ -852,6 +889,7 @@ async def test_rotated_registry_jwt_unknown_path_returns_401(self): path=self.UNKNOWN_PATH, headers={"authorization": f"Bearer {token}"}, ) + req.app.routes = _router_routes() state = self._app_state(public_pem=public_pem) state.agent_registry = MagicMock() state.agent_registry.get = AsyncMock( @@ -873,9 +911,10 @@ async def test_rotated_registry_jwt_unknown_path_returns_401(self): @pytest.mark.asyncio async def test_skeleton_key_registry_jwt_existing_non_allowlisted_route(self): """Skeleton-key control: a valid registry JWT on a route that exists - but is NOT in the agent-token allowlist must NOT be routed. The - handler must never run -- call_next must not be awaited. A fix that - calls call_next for the 404 would fail here.""" + but is NOT in the agent-token allowlist must NOT be routed -- the + handler must never run. The URL is correct and the credential is + simply not authorised for it, so the answer is the session gate's 401, + never the wrong-URL 404.""" private_pem, public_pem = _registry_keypair() token = _signed_registry_token(private_pem) @@ -884,6 +923,7 @@ async def test_skeleton_key_registry_jwt_existing_non_allowlisted_route(self): path="/api/system", # exists but is NOT an _AGENT_TOKEN_PATHS entry headers={"authorization": f"Bearer {token}"}, ) + req.app.routes = _router_routes() state = self._app_state(public_pem=public_pem) state.agent_registry = MagicMock() state.agent_registry.get = AsyncMock(return_value={"status": "active"}) @@ -893,6 +933,62 @@ async def test_skeleton_key_registry_jwt_existing_non_allowlisted_route(self): resp = await middleware.dispatch(req, call_next) - assert resp.status_code == 404 - assert resp.body == b'{"error":"Not Found"}' + assert resp.status_code == 401 + assert resp.body == b'{"error":"Authentication required"}' + call_next.assert_not_awaited() + + @pytest.mark.asyncio + async def test_registry_jwt_off_allowlist_param_route_returns_401(self): + """The scope-request approve route exists with path parameters and is + deliberately owner/admin session-only. A live registry JWT there must + get the gate's 401, not a 404 claiming the URL is wrong.""" + private_pem, public_pem = _registry_keypair() + token = _signed_registry_token(private_pem) + + middleware = AuthMiddleware(app=MagicMock()) + req = _request( + method="POST", + path="/api/agents/registry/agent-1/scope-requests/req-1/approve", + headers={"authorization": f"Bearer {token}"}, + ) + req.app.routes = _router_routes() + state = self._app_state(public_pem=public_pem) + state.agent_registry = MagicMock() + state.agent_registry.get = AsyncMock(return_value={"status": "active"}) + req.app.state = state + req.app.state.auth = _default_auth_mgr() + call_next = AsyncMock(return_value=JSONResponse({"ok": True})) + + resp = await middleware.dispatch(req, call_next) + + assert resp.status_code == 401 + assert resp.body == b'{"error":"Authentication required"}' + call_next.assert_not_awaited() + + @pytest.mark.asyncio + async def test_registry_jwt_wrong_method_on_existing_path_returns_401(self): + """A verb the route does not accept is still a real URL: the path + resolves, so the caller gets the gate's 401 rather than being told the + path does not exist.""" + private_pem, public_pem = _registry_keypair() + token = _signed_registry_token(private_pem) + + middleware = AuthMiddleware(app=MagicMock()) + req = _request( + method="DELETE", # /api/system is GET-only + path="/api/system", + headers={"authorization": f"Bearer {token}"}, + ) + req.app.routes = _router_routes() + state = self._app_state(public_pem=public_pem) + state.agent_registry = MagicMock() + state.agent_registry.get = AsyncMock(return_value={"status": "active"}) + req.app.state = state + req.app.state.auth = _default_auth_mgr() + call_next = AsyncMock(return_value=JSONResponse({"ok": True})) + + resp = await middleware.dispatch(req, call_next) + + assert resp.status_code == 401 + assert resp.body == b'{"error":"Authentication required"}' call_next.assert_not_awaited() diff --git a/tinyagentos/auth_middleware.py b/tinyagentos/auth_middleware.py index f3af7efb9..bb38d02eb 100644 --- a/tinyagentos/auth_middleware.py +++ b/tinyagentos/auth_middleware.py @@ -9,6 +9,7 @@ from fastapi.responses import JSONResponse from starlette.middleware.base import BaseHTTPMiddleware from starlette.responses import HTMLResponse, RedirectResponse +from starlette.routing import Match from tinyagentos.agent_registry_store import verify_registry_token from tinyagentos.auth import AuthStoreCorruptError @@ -324,6 +325,49 @@ async def _looks_like_registry_jwt( return True +def _route_list_matches(routes, scope: dict) -> bool: + """True if any route in *routes* claims *scope*'s path. + + A method mismatch (``Match.PARTIAL``) counts as a match: the URL exists, + only the verb is wrong, so the caller must not be told the path is unknown. + ``Mount``/``Host`` entries match on a prefix only, so recurse into their + children instead of trusting the parent match. + """ + for route in routes: + match, child_scope = route.matches(scope) + if match is Match.NONE: + continue + sub_routes = getattr(route, "routes", None) + if sub_routes: + if _route_list_matches(sub_routes, {**scope, **child_scope}): + return True + continue + return True + return False + + +def _path_is_routed(app, method: str, path: str) -> bool: + """True if *path* resolves to a route registered on *app*. + + This keeps the unlisted-route 404 below narrow. A path the router can + serve is a real URL even when the presented credential is not authorised + for it -- ``GET /api/agents/registry`` and the scope-request approve/deny + routes exist but are deliberately off the agent-token allowlist. Those + must fall through to the 401/403 session gate; only a genuinely unrouted + path is a wrong URL. + + Returns False when *app* exposes no route list, which leaves the 404 branch + reachable for a caller whose app was never a Starlette router. + """ + routes = getattr(app, "routes", None) + if not isinstance(routes, (list, tuple)): + return False + return _route_list_matches( + routes, + {"type": "http", "method": method, "path": path, "root_path": ""}, + ) + + # Bundle assets and the SPA shell HTML must be reachable without auth so: # 1. The browser can install and cache the shell for offline / PWA use. # 2. After a backend restart the cached shell loads immediately without @@ -682,14 +726,22 @@ async def _dispatch(self, request: Request, call_next): next_param = f"?next={path}" if path != "/" else "" return RedirectResponse(f"/auth/login{next_param}", status_code=303) - # A registry JWT presented for an unlisted route is a real credential - # pointing at the wrong URL. Returning 404 here (without calling - # call_next) keeps the agent-token allowlist closed -- routing is never - # reached, so a registry JWT cannot authenticate any other route -- and - # gives the caller a response that is distinguishable from dead - # credentials (which still get 401). The anonymous caller (no header, - # no session cookie) still falls through to 401 below. - if auth_header.lower().startswith("bearer "): + # A registry JWT presented for a path no route serves is a real + # credential pointing at the wrong URL. Returning 404 here (without + # calling call_next) keeps the agent-token allowlist closed -- routing + # is never reached, so a registry JWT cannot authenticate any other + # route -- and gives the caller a response that is distinguishable from + # dead credentials (which still get 401). The anonymous caller (no + # header, no session cookie) still falls through to 401 below. + # + # The router is consulted FIRST so this stays limited to unrouted + # paths. A route that exists but is off the agent-token allowlist -- + # /api/agents/registry, the scope-request approve/deny routes -- is the + # mirror-image case: the URL is right and the credential is simply not + # authorised for it, so it keeps its 401/403 from the gate below. + if auth_header.lower().startswith("bearer ") and not _path_is_routed( + request.app, request.method, path + ): presented_bearer = auth_header[7:].strip() if presented_bearer: keypair = getattr(