mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-09 14:22:12 +00:00
fix(mcp): don't coerce ProxyException.code with int()
Greptile flagged a regression introduced in the previous commit's merged
exception handler: ``ProxyException.__init__`` normalizes ``code`` via
``str(code)``, so a ``code=None`` (valid per the type signature) becomes
the string ``"None"``. Coercing that with ``int(...)`` raises
``ValueError``, which propagates uncaught and rewrites the auth error as
an unhandled 500 — degrading security posture compared to the pre-merge
``str(e.code) in ("401", "403")`` shape.
Compare against both int and str forms of the auth-error codes instead
of coercing. Adds a regression test for the ``code=None`` case.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -142,12 +142,18 @@ class MCPRequestHandler:
|
||||
api_key=litellm_api_key, request=request
|
||||
)
|
||||
except (HTTPException, ProxyException) as e:
|
||||
# HTTPException.status_code is int; ProxyException.code is normalized
|
||||
# to str in its __init__ (proxy/_types.py).
|
||||
status = e.status_code if isinstance(e, HTTPException) else int(e.code)
|
||||
# HTTPException.status_code is int; ProxyException.code is
|
||||
# normalized to str in its __init__ but can be ``"None"`` or any
|
||||
# non-numeric string when the caller didn't supply a numeric
|
||||
# code, so we compare against both int and str forms rather
|
||||
# than coercing (``int("None")`` would raise ValueError and
|
||||
# rewrite the auth error as a 500).
|
||||
status = e.status_code if isinstance(e, HTTPException) else e.code
|
||||
if status in (
|
||||
401,
|
||||
403,
|
||||
"401",
|
||||
"403",
|
||||
) and MCPRequestHandler._target_servers_use_oauth2(
|
||||
path=request.url.path, mcp_servers=mcp_servers
|
||||
):
|
||||
|
||||
@@ -1028,6 +1028,38 @@ class TestMCPOAuth2FallbackTargetGating:
|
||||
await MCPRequestHandler.process_mcp_request(scope)
|
||||
assert exc_info.value.status_code == 401
|
||||
|
||||
async def test_proxy_exception_with_non_numeric_code_propagates(self):
|
||||
"""
|
||||
``ProxyException`` normalises ``code`` via ``str()`` in its __init__,
|
||||
so callers may produce ``"None"`` or any non-numeric string when no
|
||||
explicit code was supplied. The exception handler must not coerce
|
||||
with ``int(...)`` (which would raise ``ValueError`` and rewrite the
|
||||
auth error as an unhandled 500); it must simply re-raise.
|
||||
"""
|
||||
from litellm.proxy._types import ProxyException
|
||||
|
||||
scope = {
|
||||
"type": "http",
|
||||
"method": "POST",
|
||||
"path": "/mcp/atlassian_mcp",
|
||||
"headers": [(b"authorization", b"Bearer anything")],
|
||||
}
|
||||
|
||||
async def mock_user_api_key_auth_no_code(api_key, request):
|
||||
raise ProxyException(
|
||||
message="Authentication Error",
|
||||
type="auth_error",
|
||||
param="api_key",
|
||||
code=None,
|
||||
)
|
||||
|
||||
with patch(
|
||||
"litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.user_api_key_auth",
|
||||
side_effect=mock_user_api_key_auth_no_code,
|
||||
):
|
||||
with pytest.raises(ProxyException):
|
||||
await MCPRequestHandler.process_mcp_request(scope)
|
||||
|
||||
|
||||
class TestMCPCustomHeaderName:
|
||||
"""Test suite for custom MCP authentication header name functionality"""
|
||||
|
||||
Reference in New Issue
Block a user