fix(mcp): reject fabricated DCR for providers without RFC 7591 registration (#78190)
Google's hosted Gmail/Drive MCP servers advertise no registration_endpoint
in their ASM, so the mcp SDK's create_client_registration_request falls
back to guessing POST {server_origin}/register. Those servers answer with
an opaque 404 (OAuthRegistrationError: Registration failed: 404) and the
gateway burns the reconnect ladder on an unrecoverable failure while the
CLI looks healthy because tools/list is unauthenticated on Google servers.
- Guard: HermesMCPOAuthProvider intercepts the fabricated registration
POST in the bidirectional auth_flow bridge and raises an actionable
OAuthRegistrationError (create an OAuth client, add oauth.client_id/
client_secret under mcp_servers.<name>.oauth in config.yaml) instead of
sending the request. Fires only when the URL is the exact SDK fallback
AND the ASM lacks a registration_endpoint, so servers advertising a
real RFC 7591 endpoint pass through untouched.
- Surfacing: needs_reauth tool errors now include the registration
guidance, and the generic 'MCP call failed' path unwraps TaskGroup
exception groups so the root cause reaches the model instead of
'unhandled errors in a TaskGroup (1 sub-exception)'.
- humanize_oauth_registration_error: 404 registration failures get the
same pre-registered-client guidance as 403 (CLI login path).
Tests: full discovery dance (401 -> PRM -> ASM without registration_endpoint
-> fabricated POST) drives the bridge and asserts the guard raises before
any network send; humanize 404 + passthrough cases; _auth_error_detail and
group unwrap surfacing.
Rebuilt on current main, where mcp_tool.py was decomposed and mcp 2.0
rides on httpx2: the surfacing helpers live in tools/mcp_tool_errors.py
and feed the shared _dispatch / _NEEDS_REAUTH_MSG paths in
tools/mcp_tool_handlers.py; the guard compares with the request's own URL
type and only fires once ASM was actually discovered (an undiscovered
ASM keeps main's discovery-context error, #113771), and closes the inner
SDK generator before raising so context.lock is released immediately.
This commit is contained in:
committed by
brooklyn!
parent
0e21933114
commit
7dab93b06e
@@ -14,7 +14,8 @@ import logging
|
||||
import pytest
|
||||
|
||||
from tools.mcp_tool_errors import (
|
||||
InvalidMcpUrlError, NonMcpEndpointError, _classify_mcp_failure, _unwrap_exception_group)
|
||||
InvalidMcpUrlError, NonMcpEndpointError, _auth_error_detail, _classify_mcp_failure, _mcp_call_failed_message,
|
||||
_unwrap_exception_group)
|
||||
from tools.mcp_tool import MCPServerTask
|
||||
|
||||
|
||||
@@ -66,6 +67,67 @@ class TestClassifyMcpFailure:
|
||||
assert _classify_mcp_failure(g) == "permanent"
|
||||
|
||||
|
||||
# ── _auth_error_detail / _mcp_call_failed_message ────────────────────────────
|
||||
|
||||
class TestAuthErrorSurfacing:
|
||||
def test_registration_detail_surfaces(self):
|
||||
"""The actionable registration guidance raised by the DCR guard must
|
||||
reach the needs_reauth tool error (GH#78190)."""
|
||||
pytest.importorskip("mcp.client.auth")
|
||||
from mcp.client.auth import OAuthRegistrationError
|
||||
|
||||
exc = OAuthRegistrationError(
|
||||
"MCP OAuth 'gmail': this provider does not support automatic "
|
||||
"client registration ... add it under config.yaml "
|
||||
"mcp_servers.<name>.oauth (client_id, client_secret), then run "
|
||||
"`hermes mcp login gmail`."
|
||||
)
|
||||
detail = _auth_error_detail(exc)
|
||||
assert detail.startswith(" ")
|
||||
assert "config.yaml" in detail
|
||||
assert "client_id" in detail
|
||||
|
||||
def test_registration_detail_unwraps_group(self):
|
||||
"""The transport raises the auth error inside a TaskGroup wrapper —
|
||||
the detail must still be found."""
|
||||
pytest.importorskip("mcp.client.auth")
|
||||
from mcp.client.auth import OAuthRegistrationError
|
||||
|
||||
exc = OAuthRegistrationError("config.yaml guidance")
|
||||
assert "config.yaml" in _auth_error_detail(_group(exc))
|
||||
|
||||
def test_non_registration_no_detail(self):
|
||||
assert _auth_error_detail(ValueError("nope")) == ""
|
||||
assert _auth_error_detail(_group(TimeoutError("t"))) == ""
|
||||
|
||||
def test_mcp_call_failed_message_unwraps_group(self):
|
||||
"""The generic error path must surface the group root cause instead
|
||||
of the opaque TaskGroup wrapper text."""
|
||||
pytest.importorskip("mcp.client.auth")
|
||||
from mcp.client.auth import OAuthRegistrationError
|
||||
|
||||
exc = OAuthRegistrationError("registration guidance text")
|
||||
msg = _mcp_call_failed_message(_group(exc))
|
||||
assert "OAuthRegistrationError" in msg
|
||||
assert "registration guidance text" in msg
|
||||
assert "TaskGroup" not in msg
|
||||
|
||||
def test_needs_reauth_tool_error_carries_guidance(self, monkeypatch):
|
||||
"""Through the real recovery handler: an unrecoverable registration
|
||||
failure returns needs_reauth WITH the guard's guidance."""
|
||||
pytest.importorskip("mcp.client.auth")
|
||||
import json
|
||||
from mcp.client.auth import OAuthRegistrationError
|
||||
from tools import mcp_tool_handlers as handlers
|
||||
|
||||
monkeypatch.setattr(handlers._loop, "_run_on_mcp_loop", lambda *_a, **_k: False)
|
||||
out = json.loads(handlers._handle_auth_error_and_retry(
|
||||
"gmail", OAuthRegistrationError("Create an OAuth client (config.yaml)."),
|
||||
lambda: None, "call_tool"))
|
||||
assert out["needs_reauth"] is True
|
||||
assert "requires re-authentication. Create an OAuth client (config.yaml). Run" in out["error"]
|
||||
|
||||
|
||||
# ── Keepalive failure log surfaces the root cause ────────────────────────────
|
||||
|
||||
@pytest.mark.no_isolate
|
||||
|
||||
@@ -1104,3 +1104,34 @@ def test_humanize_non_registration_403_passthrough():
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_humanize_404_registration_error_guidance():
|
||||
"""A 404 on the registration endpoint surfaces pre-registered-client
|
||||
guidance instead of the raw SDK traceback (GH#78190 — Google's hosted
|
||||
Gmail/Drive MCP servers answer the SDK's guessed /register with 404)."""
|
||||
from tools.mcp_oauth import humanize_oauth_registration_error
|
||||
|
||||
msg = humanize_oauth_registration_error(
|
||||
"gmail",
|
||||
"Registration failed: 404 <html>Error 404 (Not Found)!!1</html>",
|
||||
server_url="https://gmailmcp.googleapis.com/mcp/v1",
|
||||
)
|
||||
assert msg is not None
|
||||
assert "does not support automatic client registration" in msg
|
||||
assert "client_id" in msg
|
||||
assert "hermes mcp login gmail" in msg
|
||||
|
||||
|
||||
def test_humanize_404_non_registration_passthrough():
|
||||
"""A 404 that has nothing to do with registration stays raw."""
|
||||
from tools.mcp_oauth import humanize_oauth_registration_error
|
||||
|
||||
assert (
|
||||
humanize_oauth_registration_error(
|
||||
"srv",
|
||||
RuntimeError("HTTP 404: no such tool"),
|
||||
server_url="https://example.com/mcp",
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
@@ -234,3 +234,277 @@ async def _noop_callback() -> tuple[str, str | None]:
|
||||
raise AssertionError(
|
||||
"callback handler should not be invoked in bidirectional-generator tests"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fabricated_dcr_registration_rejected_before_network(tmp_path, monkeypatch):
|
||||
"""The bridge must raise instead of sending the SDK's *fabricated* DCR POST.
|
||||
|
||||
Google's hosted Gmail/Drive MCP servers advertise no RFC 7591
|
||||
``registration_endpoint`` in their ASM, so the SDK falls back to guessing
|
||||
``POST {server_origin}/register`` (``create_client_registration_request``,
|
||||
mcp/client/auth/utils.py) — a URL those servers answer with an opaque 404
|
||||
(GH#78190). The guard in ``HermesMCPOAuthProvider`` must intercept that
|
||||
guessed request and raise an actionable ``OAuthRegistrationError`` before
|
||||
any network call, instead of letting the gateway burn the reconnect
|
||||
ladder on an unrecoverable registration failure.
|
||||
|
||||
Drive the full discovery dance (401 -> PRM -> ASM without
|
||||
registration_endpoint -> fabricated registration POST) through the
|
||||
generator manually, exactly like the other tests in this file.
|
||||
"""
|
||||
from tools.mcp_tool import sdk_httpx
|
||||
httpx = sdk_httpx()
|
||||
from mcp.client.auth import OAuthRegistrationError
|
||||
from mcp.shared.auth import OAuthClientMetadata
|
||||
from pydantic import AnyUrl
|
||||
|
||||
from tools.mcp_oauth import HermesTokenStorage
|
||||
from tools.mcp_oauth_manager import _HERMES_PROVIDER_CLS, reset_manager_for_tests
|
||||
|
||||
assert _HERMES_PROVIDER_CLS is not None, "SDK OAuth types must be available"
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
reset_manager_for_tests()
|
||||
|
||||
# Empty storage: no tokens, no client_info — the SDK's 401 branch must
|
||||
# take the registration path (this is the cold gateway state for a
|
||||
# provider whose cached client is gone).
|
||||
storage = HermesTokenStorage("srv")
|
||||
|
||||
metadata = OAuthClientMetadata(
|
||||
redirect_uris=[AnyUrl("http://127.0.0.1:12345/callback")],
|
||||
client_name="Hermes Agent",
|
||||
)
|
||||
provider = _HERMES_PROVIDER_CLS(
|
||||
server_name="srv",
|
||||
server_url="https://example.com/mcp",
|
||||
client_metadata=metadata,
|
||||
storage=storage,
|
||||
redirect_handler=_noop_redirect,
|
||||
callback_handler=_noop_callback,
|
||||
)
|
||||
|
||||
req = httpx.Request("POST", "https://example.com/mcp")
|
||||
flow = provider.async_auth_flow(req)
|
||||
|
||||
# 1. Initial outbound MCP request.
|
||||
outbound = await flow.__anext__()
|
||||
assert outbound is not None
|
||||
|
||||
# 2. 401 with a resource_metadata pointer -> SDK starts PRM discovery.
|
||||
fake_401 = httpx.Response(
|
||||
401,
|
||||
request=outbound,
|
||||
headers={
|
||||
"www-authenticate": (
|
||||
'Bearer resource_metadata="https://example.com/.well-known/'
|
||||
'oauth-protected-resource"'
|
||||
)
|
||||
},
|
||||
)
|
||||
prm_request = await flow.asend(fake_401)
|
||||
assert isinstance(prm_request, httpx.Request)
|
||||
assert prm_request.method == "GET"
|
||||
|
||||
# 3. Serve PRM pointing at a fake authorization server.
|
||||
prm_response = httpx.Response(
|
||||
200,
|
||||
request=prm_request,
|
||||
headers={"Content-Type": "application/json"},
|
||||
json={
|
||||
"authorization_servers": ["https://auth.example.com"],
|
||||
"resource": "https://example.com/mcp",
|
||||
"bearer_methods_supported": ["header"],
|
||||
},
|
||||
)
|
||||
asm_request = await flow.asend(prm_response)
|
||||
assert isinstance(asm_request, httpx.Request)
|
||||
assert asm_request.method == "GET"
|
||||
|
||||
# 4. Serve ASM with NO registration_endpoint (the Google Gmail/Drive
|
||||
# MCP class of provider).
|
||||
asm_response = httpx.Response(
|
||||
200,
|
||||
request=asm_request,
|
||||
headers={"Content-Type": "application/json"},
|
||||
json={
|
||||
"issuer": "https://auth.example.com",
|
||||
"authorization_endpoint": "https://auth.example.com/authorize",
|
||||
"token_endpoint": "https://auth.example.com/token",
|
||||
"response_types_supported": ["code"],
|
||||
"code_challenge_methods_supported": ["S256"],
|
||||
},
|
||||
)
|
||||
|
||||
# 5. The SDK now wants to POST the fabricated registration URL — the
|
||||
# bridge must raise the actionable error instead of yielding it.
|
||||
with pytest.raises(OAuthRegistrationError) as excinfo:
|
||||
await flow.asend(asm_response)
|
||||
text = str(excinfo.value)
|
||||
assert "does not support automatic client registration" in text
|
||||
assert "https://example.com/register" in text
|
||||
assert "config.yaml" in text
|
||||
assert "hermes mcp login srv" in text
|
||||
await flow.aclose()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_advertised_registration_endpoint_passes_through(tmp_path, monkeypatch):
|
||||
"""A server that *advertises* a registration endpoint must NOT be blocked.
|
||||
|
||||
Real-world case: Linear's MCP ASM advertises
|
||||
``registration_endpoint: https://mcp.linear.app/register`` — the same
|
||||
origin+path the SDK would fabricate. The guard's AND condition (exact
|
||||
fabricated URL **and** no advertised registration_endpoint) must let
|
||||
such servers through; otherwise every legitimate DCR flow breaks.
|
||||
"""
|
||||
from tools.mcp_tool import sdk_httpx
|
||||
httpx = sdk_httpx()
|
||||
from mcp.shared.auth import OAuthClientMetadata
|
||||
from pydantic import AnyUrl
|
||||
|
||||
from tools.mcp_oauth import HermesTokenStorage
|
||||
from tools.mcp_oauth_manager import _HERMES_PROVIDER_CLS, reset_manager_for_tests
|
||||
|
||||
assert _HERMES_PROVIDER_CLS is not None, "SDK OAuth types must be available"
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
reset_manager_for_tests()
|
||||
|
||||
storage = HermesTokenStorage("srv")
|
||||
metadata = OAuthClientMetadata(
|
||||
redirect_uris=[AnyUrl("http://127.0.0.1:12345/callback")],
|
||||
client_name="Hermes Agent",
|
||||
)
|
||||
provider = _HERMES_PROVIDER_CLS(
|
||||
server_name="srv",
|
||||
server_url="https://example.com/mcp",
|
||||
client_metadata=metadata,
|
||||
storage=storage,
|
||||
redirect_handler=_noop_redirect,
|
||||
callback_handler=_noop_callback,
|
||||
)
|
||||
|
||||
req = httpx.Request("POST", "https://example.com/mcp")
|
||||
flow = provider.async_auth_flow(req)
|
||||
|
||||
outbound = await flow.__anext__()
|
||||
fake_401 = httpx.Response(
|
||||
401,
|
||||
request=outbound,
|
||||
headers={
|
||||
"www-authenticate": (
|
||||
'Bearer resource_metadata="https://example.com/.well-known/'
|
||||
'oauth-protected-resource"'
|
||||
)
|
||||
},
|
||||
)
|
||||
prm_request = await flow.asend(fake_401)
|
||||
prm_response = httpx.Response(
|
||||
200,
|
||||
request=prm_request,
|
||||
headers={"Content-Type": "application/json"},
|
||||
json={
|
||||
"authorization_servers": ["https://auth.example.com"],
|
||||
"resource": "https://example.com/mcp",
|
||||
"bearer_methods_supported": ["header"],
|
||||
},
|
||||
)
|
||||
asm_request = await flow.asend(prm_response)
|
||||
|
||||
# ASM DOES advertise a registration endpoint — at origin+/register,
|
||||
# exactly the URL the SDK would otherwise fabricate (Linear's ASM does
|
||||
# this: https://mcp.linear.app/register).
|
||||
asm_response = httpx.Response(
|
||||
200,
|
||||
request=asm_request,
|
||||
headers={"Content-Type": "application/json"},
|
||||
json={
|
||||
"issuer": "https://auth.example.com",
|
||||
"authorization_endpoint": "https://auth.example.com/authorize",
|
||||
"token_endpoint": "https://auth.example.com/token",
|
||||
"registration_endpoint": "https://example.com/register",
|
||||
"response_types_supported": ["code"],
|
||||
"code_challenge_methods_supported": ["S256"],
|
||||
},
|
||||
)
|
||||
|
||||
# The registration POST must be yielded (not blocked): the SDK uses the
|
||||
# advertised endpoint and the guard must pass it through.
|
||||
registration_request = await flow.asend(asm_response)
|
||||
assert isinstance(registration_request, httpx.Request)
|
||||
assert registration_request.method == "POST"
|
||||
assert str(registration_request.url) == "https://example.com/register"
|
||||
|
||||
# No OAuthRegistrationError may have been raised along the way. (The
|
||||
# dance ends here: a 200 registration response would hand control to the
|
||||
# browser-redirect flow, which is outside this test's scope.)
|
||||
await flow.aclose()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_guard_ignores_non_fabricated_post_paths(tmp_path, monkeypatch):
|
||||
"""A POST to a *different* path must never be blocked by the guard.
|
||||
|
||||
The guard is exact-URL matched: only the SDK's fabricated
|
||||
``{origin}/register`` POST is rejected when the ASM lacks a
|
||||
registration_endpoint. A server-side registration at ``/foo/register``
|
||||
or any other path passes through untouched. Undiscovered metadata (a
|
||||
blocked ASM fetch) never trips the guard: the SDK's guess may be a real
|
||||
endpoint, and the discovery-context error explains a failure there.
|
||||
"""
|
||||
from tools.mcp_tool import sdk_httpx
|
||||
httpx = sdk_httpx()
|
||||
from mcp.shared.auth import OAuthClientMetadata, OAuthMetadata
|
||||
from pydantic import AnyUrl
|
||||
|
||||
from tools.mcp_oauth import HermesTokenStorage
|
||||
from tools.mcp_oauth_manager import _HERMES_PROVIDER_CLS, reset_manager_for_tests
|
||||
|
||||
assert _HERMES_PROVIDER_CLS is not None, "SDK OAuth types must be available"
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
reset_manager_for_tests()
|
||||
|
||||
provider = _HERMES_PROVIDER_CLS(
|
||||
server_name="srv",
|
||||
server_url="https://example.com/mcp",
|
||||
client_metadata=OAuthClientMetadata(
|
||||
redirect_uris=[AnyUrl("http://127.0.0.1:12345/callback")],
|
||||
client_name="Hermes Agent",
|
||||
),
|
||||
storage=HermesTokenStorage("srv"),
|
||||
redirect_handler=_noop_redirect,
|
||||
callback_handler=_noop_callback,
|
||||
)
|
||||
|
||||
# No ASM discovered: even the exact guessed URL passes through.
|
||||
assert provider.context.oauth_metadata is None
|
||||
await provider._maybe_reject_fabricated_registration(
|
||||
httpx.Request("POST", "https://example.com/register")
|
||||
) # must not raise
|
||||
|
||||
# ASM discovered without registration_endpoint: only the exact
|
||||
# origin+/register POST is refused; everything else passes.
|
||||
provider.context.oauth_metadata = OAuthMetadata(
|
||||
issuer="https://auth.example.com",
|
||||
authorization_endpoint="https://auth.example.com/authorize",
|
||||
token_endpoint="https://auth.example.com/token",
|
||||
)
|
||||
for method, url in (
|
||||
("POST", "https://example.com/foo/register"),
|
||||
("POST", "https://example.com/register/"),
|
||||
("POST", "https://example.com/registers"),
|
||||
("POST", "https://other.example.com/register"),
|
||||
("POST", "https://example.com:8443/register"),
|
||||
("GET", "https://example.com/register"),
|
||||
):
|
||||
await provider._maybe_reject_fabricated_registration(
|
||||
httpx.Request(method, url)
|
||||
) # must not raise
|
||||
from mcp.client.auth import OAuthRegistrationError
|
||||
with pytest.raises(OAuthRegistrationError):
|
||||
await provider._maybe_reject_fabricated_registration(
|
||||
httpx.Request("POST", "https://example.com:443/register")
|
||||
)
|
||||
|
||||
@@ -1195,7 +1195,7 @@ def _maybe_preregister_client(storage: "HermesTokenStorage", cfg: dict, client_m
|
||||
|
||||
def humanize_oauth_registration_error(
|
||||
server_name: str, exc: BaseException | str, *, server_url: str | None = None) -> str | None:
|
||||
"""Turn a DCR 403/Forbidden into a useful next step; None for anything else so the caller keeps the
|
||||
"""Turn a DCR 403/Forbidden or 404 into a useful next step; None for anything else so the caller keeps the
|
||||
original text. Figma gates DCR on exact ``client_name`` (auto-set to ``Claude Code``), so this fires
|
||||
when the user overrode it or an older Hermes is running."""
|
||||
msg = str(exc)
|
||||
@@ -1203,6 +1203,15 @@ def humanize_oauth_registration_error(
|
||||
from tools.mcp_oauth_provider import _DISCOVERY_CONTEXT_LEAD
|
||||
if msg.startswith(_DISCOVERY_CONTEXT_LEAD): # the 403 there is the metadata fetch, not a DCR refusal
|
||||
return None
|
||||
# A 404 on registration (Google's hosted Gmail/Drive MCP servers answer the SDK's guessed /register with
|
||||
# one) is the same permanent "no RFC 7591 DCR" class as a 403 refusal (#78190).
|
||||
if ("404" in msg and ("not found" in lowered or "/register" in lowered)
|
||||
and any(k in lowered for k in ("regist", "dcr", "dynamic client"))):
|
||||
return (
|
||||
f"'{server_name}' does not support automatic client registration — the registration request returned "
|
||||
"404 (typical for Google's hosted Gmail/Drive MCP servers and other providers without RFC 7591 dynamic "
|
||||
"client registration). Create an OAuth client for this provider and add it under config.yaml as "
|
||||
f"`oauth: {{client_id: ..., client_secret: ...}}`, then re-run:\n hermes mcp login {server_name}")
|
||||
looks_like_registration = ("403" in msg or "forbidden" in lowered) and (
|
||||
any(k in lowered for k in ("regist", "dcr", "dynamic client"))
|
||||
or lowered.strip() in {"forbidden", "403 forbidden", "http 403: forbidden"}
|
||||
|
||||
@@ -206,6 +206,38 @@ class HermesMCPOAuthProvider(HermesProviderMixin, *_SDK_BASES):
|
||||
except Exception as exc: # pragma: no cover — must not throw
|
||||
self._log_nonfatal("invalid_client detection", exc)
|
||||
|
||||
async def _maybe_reject_fabricated_registration(self, outgoing: Any) -> None:
|
||||
"""Refuse to send the SDK's *fabricated* dynamic client registration (#78190).
|
||||
|
||||
When the discovered authorization-server metadata (RFC 8414) has no ``registration_endpoint``,
|
||||
``create_client_registration_request`` (mcp/client/auth/utils.py) guesses ``POST {origin}/register``.
|
||||
Providers without RFC 7591 DCR — Google's hosted Gmail/Drive MCP servers among them — answer with an
|
||||
opaque 404 and the gateway burns its reconnect ladder on an unrecoverable failure. Their supported
|
||||
path is a pre-registered client (``oauth.client_id`` / ``client_secret``), so raise that guidance
|
||||
instead of sending the request.
|
||||
|
||||
Fires only when the metadata WAS discovered and lacks the endpoint, and the request is a POST to the
|
||||
exact guessed URL. Undiscovered metadata (a WAF-blocked fetch) keeps the SDK's guess, so a real
|
||||
``/register`` still works and the discovery-context error (#113771) still explains a failure.
|
||||
"""
|
||||
meta = getattr(self.context, "oauth_metadata", None)
|
||||
if meta is None or getattr(meta, "registration_endpoint", None) or getattr(outgoing, "method", None) != "POST":
|
||||
return
|
||||
from urllib.parse import urljoin
|
||||
url = outgoing.url
|
||||
# Same URL type as the request (httpx or httpx2), so default-port handling matches on both sides.
|
||||
expected = type(url)(urljoin(self.context.get_authorization_base_url(self.context.server_url), "/register"))
|
||||
if (url.scheme, url.host, url.port, url.path) != (expected.scheme, expected.host, expected.port, expected.path):
|
||||
return
|
||||
from mcp.client.auth.oauth2 import OAuthRegistrationError
|
||||
name = self._hermes_server_name
|
||||
raise OAuthRegistrationError(
|
||||
f"MCP OAuth '{name}': this provider does not support automatic client registration (its authorization "
|
||||
f"server advertises no registration_endpoint), so the SDK's fallback would POST {expected} and fail "
|
||||
"(Google's hosted Gmail/Drive MCP servers and similar providers return 404 for it). Create an OAuth "
|
||||
"client for this provider and add it under config.yaml mcp_servers.<name>.oauth (client_id, "
|
||||
f"client_secret), then run `hermes mcp login {name}`.")
|
||||
|
||||
async def async_auth_flow(self, request): # type: ignore[override]
|
||||
try: # pre-flow hook: reload from disk if it changed (non-fatal on error)
|
||||
await get_manager().invalidate_if_disk_changed(self._hermes_server_name, hermes_home=self._hermes_home)
|
||||
@@ -234,6 +266,12 @@ class HermesMCPOAuthProvider(HermesProviderMixin, *_SDK_BASES):
|
||||
sent_access_token = tokens.access_token if tokens is not None else None
|
||||
self.context.lock.release()
|
||||
resource_lock_released = True
|
||||
else:
|
||||
try: # refuse the SDK's guessed DCR POST before it hits the network
|
||||
await self._maybe_reject_fabricated_registration(outgoing)
|
||||
except Exception:
|
||||
await inner.aclose() # release the SDK's context.lock now, not at GC
|
||||
raise
|
||||
incoming = yield outgoing
|
||||
if resource_lock_released:
|
||||
await self.context.lock.acquire()
|
||||
|
||||
@@ -11,7 +11,7 @@ import os
|
||||
import re
|
||||
from typing import Any, List, Optional
|
||||
from urllib.parse import urlparse
|
||||
from tools.mcp_tool_common import _sanitize_error, _core
|
||||
from tools.mcp_tool_common import _exc_str, _sanitize_error, _core
|
||||
from tools.mcp_tool_node_abi import NodeAbiMismatchError
|
||||
|
||||
logger = logging.getLogger("tools.mcp_tool")
|
||||
@@ -154,6 +154,26 @@ def _contains_only_cancellation(exc: BaseException) -> bool:
|
||||
return isinstance(exc, asyncio.CancelledError)
|
||||
|
||||
|
||||
def _auth_error_detail(exc: BaseException) -> str:
|
||||
"""`` <guidance>`` when *exc* (or its group root) is the SDK's ``OAuthRegistrationError``, else ``""``. The DCR
|
||||
guard in ``HermesMCPOAuthProvider`` raises one naming the next step (pre-register an OAuth client), and the
|
||||
generic needs-reauth text alone would hide it from the model (#78190)."""
|
||||
try:
|
||||
from mcp.client.auth import OAuthRegistrationError
|
||||
except ImportError: # pragma: no cover — SDK required in CI
|
||||
return ""
|
||||
root = _unwrap_exception_group(exc)
|
||||
text = str(root).strip() if isinstance(root, OAuthRegistrationError) else ""
|
||||
return f" {_sanitize_error(text)}" if text else ""
|
||||
|
||||
|
||||
def _mcp_call_failed_message(exc: BaseException) -> str:
|
||||
"""Generic tool-error text naming the group root cause: the transport's TaskGroup wrapper ``str()`` is
|
||||
"unhandled errors in a TaskGroup (1 sub-exception)", which tells the model nothing."""
|
||||
root = _unwrap_exception_group(exc)
|
||||
return _sanitize_error(f"MCP call failed: {type(root).__name__}: {_exc_str(root)}")
|
||||
|
||||
|
||||
def _classify_mcp_failure(exc: BaseException) -> str:
|
||||
"""``'permanent'`` (``run()`` parks instead of burning the retry ladder: auth 401/403,
|
||||
NonMcpEndpointError, InvalidMcpUrlError, missing stdio command, native addon built for another
|
||||
|
||||
@@ -21,7 +21,8 @@ from tools.mcp_tool_content import (
|
||||
_MCP_HARD_RESULT_CAP_CHARS, _cache_mcp_audio_block, _cache_mcp_image_block, _mcp_result_with_native_images,
|
||||
_render_mcp_dropped_block_notice, _render_mcp_resource_block, _strip_reserved_meta_keys,
|
||||
_truncate_mcp_text_result)
|
||||
from tools.mcp_tool_errors import _is_auth_error, _is_session_expired_error
|
||||
from tools.mcp_tool_errors import (
|
||||
_auth_error_detail, _is_auth_error, _is_session_expired_error, _mcp_call_failed_message)
|
||||
|
||||
logger = logging.getLogger("tools.mcp_tool")
|
||||
_MISSING = object()
|
||||
@@ -29,7 +30,7 @@ _MISSING = object()
|
||||
declaration.on_change = invalidate_check_fn_cache
|
||||
|
||||
_NEEDS_REAUTH_MSG = (
|
||||
"MCP server '{s}' requires re-authentication. Run `hermes mcp login {s}` (or delete the tokens file under "
|
||||
"MCP server '{s}' requires re-authentication.{detail} Run `hermes mcp login {s}` (or delete the tokens file under "
|
||||
"~/.hermes/mcp-tokens/ and restart). Do NOT retry this tool — ask the user to re-authenticate.")
|
||||
_STDIO_NO_RESPAWN_MSG = (
|
||||
"MCP server '{s}' stdio subprocess had exited (this is not a timeout — the call never reached the server). A "
|
||||
@@ -210,7 +211,8 @@ def _handle_auth_error_and_retry(server_name: str, exc: BaseException, retry_cal
|
||||
result = _retry_once(server_name, retry_call, op_description, "auth recovery")
|
||||
if result is not None:
|
||||
return result
|
||||
return _strike(server_name, _NEEDS_REAUTH_MSG.format(s=server_name), needs_reauth=True, server=server_name)
|
||||
return _strike(server_name, _NEEDS_REAUTH_MSG.format(s=server_name, detail=_auth_error_detail(exc)),
|
||||
needs_reauth=True, server=server_name)
|
||||
|
||||
|
||||
def _handle_session_expired_and_retry(server_name: str, exc: BaseException, retry_call, op_description: str,
|
||||
@@ -345,7 +347,7 @@ def _dispatch(server_name: str, server: Any, op: str, call, tool_timeout: float,
|
||||
if recovered is not None:
|
||||
return recovered
|
||||
on_final_failure(exc)
|
||||
return tool_error(_sanitize_error(f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}"))
|
||||
return tool_error(_mcp_call_failed_message(exc))
|
||||
|
||||
|
||||
@asynccontextmanager
|
||||
|
||||
Reference in New Issue
Block a user