fix(mcp): stop rejecting valid API keys on non-200 upstream statuses

src/late/mcp/auth.py

@@ -1,10 +1,16 @@
 """Authentication module for Zernio MCP HTTP server."""
 
+import hashlib
+import logging
 import os
+import time
+from collections import OrderedDict
+from enum import Enum
 from urllib.parse import urlparse
 
 import httpx
 from fastmcp.server.auth import AccessToken, RemoteAuthProvider, TokenVerifier
+from starlette.authentication import AuthenticationError
 from starlette.requests import Request
 
 from late.mcp.constants import (
@@ -58,43 +64,171 @@ def is_allowed_origin(request: Request) -> bool:
     return any(host == s or host.endswith("." + s) for s in _allowed_origin_suffixes())
 
 
-async def verify_late_api_key(api_key: str) -> bool:
+logger = logging.getLogger(__name__)
+
+_VERIFY_URL = "https://zernio.com/api/v1/accounts"
+_VERIFY_TIMEOUT = 5.0
+
+# Positive-only verification cache: sha256(token) -> monotonic timestamp of the
+# last upstream confirmation. Positives only, so an attacker cannot grow it by
+# spraying invalid bearers. LRU-capped anyway: the container is long-lived.
+# Per process on purpose: Railway runs one uvicorn worker (Dockerfile CMD, no
+# --workers), and a shared store would put a new dependency in front of auth,
+# which is how you rebuild this same outage.
+_VERIFIED_AT: OrderedDict[str, float] = OrderedDict()
+_VERIFIED_CACHE_MAX = 10_000
+# Mirrors the 60s Redis TTL the API itself keeps on apikey:{keyHash}
+# (Schedule-Posts-API libs/api-auth.ts:471), so this adds no revocation window
+# the platform does not already have.
+_FRESH_TTL_SECONDS = 60.0
+# Only used when upstream could not answer. Blast radius is bounded to
+# nothing: the MCP stores no data and every tool call re-presents the same
+# key to the same API, so a revoked key honoured here still cannot read or
+# write anything.
+_GRACE_TTL_SECONDS = 3600.0
+
+
+class Verification(Enum):
+    """Upstream's verdict on a bearer token.
+
+    Three-valued on purpose: the incident this replaces collapsed every
+    non-200 into False, so 402, 429, 5xx and timeouts all reached the user as
+    "your token is invalid, clear it and re-register".
     """
-    Verify Zernio API key by making a test request to Zernio API.
 
-    Function name kept as verify_late_api_key for backwards compatibility.
+    VALID = "valid"
+    INVALID = "invalid"
+    UNKNOWN = "unknown"
+
+
+def _classify(status_code: int) -> Verification:
+    """Map an upstream status to a verdict about the TOKEN, not the request.
+
+    401/403 are the only statuses that say anything about token validity.
+    402 and 429 are emitted only after the API has already resolved the key
+    to a principal (Schedule-Posts-API libs/api-auth.ts:829 "Valid
+    credential, but the billing owner is payment-suspended. 402 (not 401) so
+    integrators can tell 'fix your card' apart from 'bad key'", and
+    authenticateWithRateLimit at :665 rate-limits only after authenticate()
+    succeeds), so both PROVE the key is good. Billing and limits are
+    enforced per call by the endpoint that owns them. Everything else tells
+    us nothing and must not be reported as an auth failure.
+    """
+    if status_code == 200:
+        return Verification.VALID
+    if status_code in (401, 403):
+        return Verification.INVALID
+    if status_code in (402, 429):
+        return Verification.VALID
+    return Verification.UNKNOWN
 
-    Args:
-        api_key: The Zernio API key to verify.
 
-    Returns:
-        True if API key is valid, False otherwise.
+def _remember(token_key: str, now: float) -> None:
+    _VERIFIED_AT[token_key] = now
+    _VERIFIED_AT.move_to_end(token_key)
+    while len(_VERIFIED_AT) > _VERIFIED_CACHE_MAX:
+        _VERIFIED_AT.popitem(last=False)
+
+
+async def verify_late_api_key(
+    api_key: str, *, client: httpx.AsyncClient | None = None
+) -> Verification:
+    """Ask the Zernio API whether this bearer is a real credential.
+
+    Function name kept as verify_late_api_key for backwards compatibility.
+    Returns a three-valued Verification, NOT a bool: never write
+    ``if await verify_late_api_key(...)``, every Enum member is truthy.
+
+    client is injected by tests (httpx.MockTransport); production opens a
+    per-call client and must not close an injected one.
     """
+    owns_client = client is None
+    client = client or httpx.AsyncClient()
     try:
-        async with httpx.AsyncClient() as client:
-            response = await client.get(
-                "https://zernio.com/api/v1/accounts",
-                headers={"Authorization": f"Bearer {api_key}"},
-                timeout=5.0,
-            )
-            return response.status_code == 200
-    except Exception:
-        return False
+        response = await client.get(
+            _VERIFY_URL,
+            headers={"Authorization": f"Bearer {api_key}"},
+            timeout=_VERIFY_TIMEOUT,
+        )
+    except Exception as exc:
+        # Timeouts, DNS and TLS failures say nothing about the token. Logged
+        # because the previous silent `return False` is why a 26h auth
+        # outage ran with a green /health and no alert.
+        logger.warning("Zernio token verification unreachable: %s", type(exc).__name__)
+        return Verification.UNKNOWN
+    finally:
+        if owns_client:
+            await client.aclose()
+
+    verdict = _classify(response.status_code)
+    if verdict is Verification.UNKNOWN:
+        logger.warning(
+            "Zernio token verification inconclusive: HTTP %s",
+            response.status_code,
+        )
+    return verdict
 
 
 class ZernioTokenVerifier(TokenVerifier):
     """Resource-server token verification for the Zernio MCP server.
 
     Accepts BOTH plain Zernio API keys and OAuth access tokens: both arrive as
     the same bearer string and are validated the same way — a live GET to the
-    Zernio API (verify_late_api_key). HTTP 200 => valid. Deliberately
-    format-agnostic (no JWT decode), which is why a static API key works here
-    just as well as an issued OAuth token.
+    Zernio API (verify_late_api_key). 402 and 429 also count as valid: the
+    Zernio API only emits them after it has already authenticated the
+    credential. Deliberately format-agnostic (no JWT decode), which is why a
+    static API key works here just as well as an issued OAuth token.
     """
 
+    def __init__(self, *, client: httpx.AsyncClient | None = None) -> None:
+        super().__init__()
+        self._client = client
+
     async def verify_token(self, token: str) -> AccessToken | None:
-        if not await verify_late_api_key(token):
+        token_key = hashlib.sha256(token.encode()).hexdigest()
+        now = time.monotonic()
+        verified_at = _VERIFIED_AT.get(token_key)
+
+        if verified_at is not None and now - verified_at < _FRESH_TTL_SECONDS:
+            _VERIFIED_AT.move_to_end(token_key)
+            return self._grant(token)
+
+        verdict = await verify_late_api_key(token, client=self._client)
+
+        if verdict is Verification.VALID:
+            _remember(token_key, now)
+            return self._grant(token)
+
+        if verdict is Verification.INVALID:
+            _VERIFIED_AT.pop(token_key, None)
             return None
+
+        # UNKNOWN. Honour a token upstream confirmed recently; refuse to
+        # guess about any other one, so an outage never becomes an auth
+        # bypass. Raising AuthenticationError yields HTTP 400 with this
+        # message. Returning None instead would emit the 401 invalid_token
+        # whose text tells users to clear their tokens and re-register,
+        # which is the exact damage this fix exists to stop. Any exception
+        # type OTHER than AuthenticationError escapes as a 500 with the
+        # message swallowed.
+        if verified_at is not None and now - verified_at < _GRACE_TTL_SECONDS:
+            # Guarded because the await above can suspend for the full verify
+            # timeout, and a concurrent request may LRU-evict this key while
+            # we are parked. A bare move_to_end would raise KeyError, which is
+            # not AuthenticationError and so escapes as a 500.
+            if token_key in _VERIFIED_AT:
+                _VERIFIED_AT.move_to_end(token_key)
+            logger.warning(
+                "Granting %s... from cache: upstream unreachable",
+                token_key[:12],
+            )
+            return self._grant(token)
+        raise AuthenticationError(
+            "Zernio API is temporarily unreachable, so this token could not "
+            "be verified. Your credentials are fine; retry in a moment."
+        )
+
+    def _grant(self, token: str) -> AccessToken:
         return AccessToken(token=token, client_id="zernio", scopes=list(OAUTH_SCOPES))
 
 

tests/test_mcp_auth_verification.py

@@ -0,0 +1,168 @@
+"""
+Regression tests for the Zernio MCP auth outage: auth.py used to collapse
+every non-200 upstream status into "invalid token" (``return
+response.status_code == 200``) and swallow timeouts (``except Exception:
+return False``). ``verify_token`` then returned ``None`` and fastmcp emitted a
+401 ``invalid_token`` telling users to clear their credentials and
+re-register, even though the Zernio API only emits 402/429 for a credential
+it has already authenticated.
+
+These tests drive the real ``_classify`` / ``ZernioTokenVerifier`` against a
+fake transport (``httpx.MockTransport`` -- no network, no mocking of our own
+functions).
+"""
+
+from __future__ import annotations
+
+import hashlib
+import time
+
+import httpx
+import pytest
+from starlette.authentication import AuthenticationError
+
+from late.mcp import auth
+from late.mcp.auth import Verification, ZernioTokenVerifier, _classify
+
+
+@pytest.fixture(autouse=True)
+def _clear_verification_cache():
+    auth._VERIFIED_AT.clear()
+    yield
+    auth._VERIFIED_AT.clear()
+
+
+@pytest.mark.parametrize(
+    "status_code,expected",
+    [
+        (200, Verification.VALID),
+        (401, Verification.INVALID),
+        (403, Verification.INVALID),
+        (402, Verification.VALID),
+        (429, Verification.VALID),
+        (500, Verification.UNKNOWN),
+        (502, Verification.UNKNOWN),
+        (503, Verification.UNKNOWN),
+        (504, Verification.UNKNOWN),
+        (404, Verification.UNKNOWN),
+    ],
+)
+def test_classify_maps_upstream_status_to_outcome(status_code, expected):
+    assert _classify(status_code) is expected
+
+
+async def test_verify_token_accepts_payment_required_key():
+    def handler(_request: httpx.Request) -> httpx.Response:
+        return httpx.Response(402)
+
+    client = httpx.AsyncClient(transport=httpx.MockTransport(handler))
+    verifier = ZernioTokenVerifier(client=client)
+
+    access_token = await verifier.verify_token("some-api-key")
+
+    assert access_token is not None
+    await client.aclose()
+
+
+async def test_verify_token_raises_unavailable_for_unknown_token_when_upstream_is_down():
+    def handler(_request: httpx.Request) -> httpx.Response:
+        raise httpx.ConnectTimeout("boom")
+
+    client = httpx.AsyncClient(transport=httpx.MockTransport(handler))
+    verifier = ZernioTokenVerifier(client=client)
+
+    with pytest.raises(AuthenticationError):
+        await verifier.verify_token("never-seen-before")
+
+    await client.aclose()
+
+
+async def test_verify_token_honours_recently_valid_token_during_outage():
+    def ok_handler(_request: httpx.Request) -> httpx.Response:
+        return httpx.Response(200)
+
+    warm_client = httpx.AsyncClient(transport=httpx.MockTransport(ok_handler))
+    verifier = ZernioTokenVerifier(client=warm_client)
+    token = "a-recently-valid-key"
+    warm_token = await verifier.verify_token(token)
+    assert warm_token is not None
+    await warm_client.aclose()
+
+    # Push the cached timestamp past the fresh window (60s) but still inside
+    # the grace window (3600s), so the second call below actually exercises
+    # the UNKNOWN + grace-cache branch instead of short-circuiting on the
+    # fresh-cache check.
+    token_key = hashlib.sha256(token.encode()).hexdigest()
+    auth._VERIFIED_AT[token_key] = time.monotonic() - (auth._FRESH_TTL_SECONDS + 1)
+
+    def down_handler(_request: httpx.Request) -> httpx.Response:
+        raise httpx.ConnectTimeout("boom")
+
+    down_client = httpx.AsyncClient(transport=httpx.MockTransport(down_handler))
+    verifier._client = down_client
+
+    access_token = await verifier.verify_token(token)
+
+    assert access_token is not None
+    await down_client.aclose()
+
+
+async def test_verify_token_rejects_a_genuinely_bad_key():
+    def handler(_request: httpx.Request) -> httpx.Response:
+        return httpx.Response(401)
+
+    client = httpx.AsyncClient(transport=httpx.MockTransport(handler))
+    verifier = ZernioTokenVerifier(client=client)
+
+    assert await verifier.verify_token("a-revoked-key") is None
+    await client.aclose()
+
+
+async def test_invalid_verdict_evicts_the_cached_entry():
+    """A key that upstream later rejects must not survive on the grace path."""
+
+    def ok_handler(_request: httpx.Request) -> httpx.Response:
+        return httpx.Response(200)
+
+    token = "a-key-that-gets-revoked"
+    token_key = hashlib.sha256(token.encode()).hexdigest()
+
+    warm_client = httpx.AsyncClient(transport=httpx.MockTransport(ok_handler))
+    verifier = ZernioTokenVerifier(client=warm_client)
+    assert await verifier.verify_token(token) is not None
+    await warm_client.aclose()
+
+    # Past the fresh window so the next call really re-verifies upstream.
+    auth._VERIFIED_AT[token_key] = time.monotonic() - (auth._FRESH_TTL_SECONDS + 1)
+
+    def revoked_handler(_request: httpx.Request) -> httpx.Response:
+        return httpx.Response(401)
+
+    revoked_client = httpx.AsyncClient(transport=httpx.MockTransport(revoked_handler))
+    verifier._client = revoked_client
+    assert await verifier.verify_token(token) is None
+    await revoked_client.aclose()
+
+    assert token_key not in auth._VERIFIED_AT
+
+    # With the entry evicted, an upstream outage must refuse the token rather
+    # than honour it from the grace cache.
+    def down_handler(_request: httpx.Request) -> httpx.Response:
+        raise httpx.ConnectTimeout("boom")
+
+    down_client = httpx.AsyncClient(transport=httpx.MockTransport(down_handler))
+    verifier._client = down_client
+    with pytest.raises(AuthenticationError):
+        await verifier.verify_token(token)
+    await down_client.aclose()
+
+
+def test_verification_cache_is_bounded():
+    now = 0.0
+    for i in range(auth._VERIFIED_CACHE_MAX + 50):
+        auth._remember(f"token-{i}", now)
+
+    assert len(auth._VERIFIED_AT) <= auth._VERIFIED_CACHE_MAX
+    # Eviction is LRU, so the oldest inserts are the ones that went.
+    assert "token-0" not in auth._VERIFIED_AT
+    assert f"token-{auth._VERIFIED_CACHE_MAX + 49}" in auth._VERIFIED_AT