From e315984f54980d08802e3bf89066e6d5dda65488 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Tue, 21 Jul 2026 14:42:43 -0700 Subject: [PATCH] fix(datetime_utils): raise TypeError for non-str/datetime input Greptile flagged that narrowing get_mcp_oauth_user_credential_status from a bare except to except ValueError left a truthy non-string expires_at (possible in raw DB JSON) raising AttributeError through the endpoint as a 500. The same gap applied to every refactored site that catches (ValueError, TypeError): datetime.fromisoformat raised TypeError for non-strings, which those handlers were written against, but the helper's passthrough branch leaked AttributeError instead. parse_utc_datetime now validates its input and raises TypeError for anything that is not str or datetime, restoring the stdlib error contract at every call site. The MCP status endpoint catches it and also omits non-string expires_at/connected_at values from the response instead of failing response validation. --- litellm/litellm_core_utils/datetime_utils.py | 11 +++- .../mcp_management_endpoints.py | 7 +-- .../litellm_core_utils/test_datetime_utils.py | 12 +++-- .../test_mcp_management_endpoints.py | 51 ++++++++++++++++++- 4 files changed, 72 insertions(+), 9 deletions(-) diff --git a/litellm/litellm_core_utils/datetime_utils.py b/litellm/litellm_core_utils/datetime_utils.py index 1d4f615c2ce..c70d76e776e 100644 --- a/litellm/litellm_core_utils/datetime_utils.py +++ b/litellm/litellm_core_utils/datetime_utils.py @@ -17,8 +17,17 @@ def parse_utc_datetime(value: str | datetime) -> datetime: (key expiry checks, budget windows, spend reports). The "Z" suffix is handled explicitly because datetime.fromisoformat only accepts it from Python 3.11 and the project floor is 3.10. + + Raises ValueError for unparseable strings and TypeError for any other type, + mirroring datetime.fromisoformat's own contract so existing + ``except (ValueError, TypeError)`` handlers stay fail-closed. """ - parsed = datetime.fromisoformat(value.replace("Z", "+00:00")) if isinstance(value, str) else value + if isinstance(value, str): + parsed = datetime.fromisoformat(value.replace("Z", "+00:00")) + elif isinstance(value, datetime): + parsed = value + else: + raise TypeError(f"parse_utc_datetime expects str or datetime, got {type(value).__name__}") if parsed.tzinfo is None: return parsed.replace(tzinfo=timezone.utc) return parsed diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index 54129e88b45..554e09634fe 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -2005,14 +2005,15 @@ if MCP_AVAILABLE: try: exp = parse_utc_datetime(expires_at) is_expired = exp < datetime.now(timezone.utc) - except ValueError: + except (ValueError, TypeError): pass + connected_at = cred.get("connected_at") return MCPOAuthUserCredentialStatus( server_id=server_id, has_credential=True, - expires_at=expires_at, + expires_at=expires_at if isinstance(expires_at, str) else None, is_expired=is_expired, - connected_at=cred.get("connected_at"), + connected_at=connected_at if isinstance(connected_at, str) else None, ) @router.get( diff --git a/tests/test_litellm/litellm_core_utils/test_datetime_utils.py b/tests/test_litellm/litellm_core_utils/test_datetime_utils.py index 7d1d4cef8ec..c2278157744 100644 --- a/tests/test_litellm/litellm_core_utils/test_datetime_utils.py +++ b/tests/test_litellm/litellm_core_utils/test_datetime_utils.py @@ -39,11 +39,17 @@ def test_aware_datetime_passthrough_unchanged(): def test_result_always_comparable_to_utc_now(): for value in ("2026-01-20T00:00:00", "2026-01-20T00:00:00Z", datetime(2026, 1, 20)): - assert parse_utc_datetime(value) < datetime.now(timezone.utc) or parse_utc_datetime( - value - ) >= datetime.now(timezone.utc) + assert parse_utc_datetime(value) < datetime.now(timezone.utc) or parse_utc_datetime(value) >= datetime.now( + timezone.utc + ) def test_invalid_string_raises(): with pytest.raises(ValueError): parse_utc_datetime("not-a-date") + + +def test_non_str_non_datetime_raises_typeerror(): + for bad in (1755000000, 1755000000.0, None, {"expires_at": "2026-01-01"}): + with pytest.raises(TypeError): + parse_utc_datetime(bad) # pyright: ignore[reportArgumentType] # exercising the runtime guard diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index 3890318bd76..90972f5ccc8 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -3565,6 +3565,45 @@ async def test_get_mcp_oauth_user_credential_status_naive_past_expiry_is_expired assert result.expires_at == "2020-01-01T00:00:00" +@pytest.mark.asyncio +async def test_get_mcp_oauth_user_credential_status_non_string_expiry_degrades_gracefully(): + """A malformed non-string expires_at must report status instead of raising a 500.""" + if not mgmt_endpoints.MCP_AVAILABLE: + pytest.skip("MCP module not installed") + + from litellm.proxy.management_endpoints.mcp_management_endpoints import ( + get_mcp_oauth_user_credential_status, + ) + + server_id = "srv-1" + stored_payload = { + "type": "oauth2", + "access_token": "tok", + "expires_at": 1577836800, + "connected_at": "2019-01-01T00:00:00+00:00", + "server_id": server_id, + } + + with ( + patch( + "litellm.proxy.management_endpoints.mcp_management_endpoints.get_prisma_client_or_throw", + return_value=_make_prisma_client(), + ), + patch( + "litellm.proxy.management_endpoints.mcp_management_endpoints.get_user_oauth_credential", + new=AsyncMock(return_value=stored_payload), + ), + ): + result = await get_mcp_oauth_user_credential_status( + server_id=server_id, + user_api_key_dict=_make_user_auth("user-123"), + ) + + assert result.has_credential is True + assert result.is_expired is False + assert result.expires_at is None + + @pytest.mark.asyncio async def test_delete_mcp_oauth_user_credential_only_deletes_oauth(): """delete_mcp_oauth_user_credential only deletes OAuth2 credentials, not BYOK.""" @@ -5021,7 +5060,9 @@ def _edit_endpoint_patches(old_record, update_mock): ), patch( "litellm.proxy.management_endpoints.mcp_management_endpoints.get_mcp_server", - AsyncMock(side_effect=old_record) if isinstance(old_record, Exception) else AsyncMock(return_value=old_record), + AsyncMock(side_effect=old_record) + if isinstance(old_record, Exception) + else AsyncMock(return_value=old_record), ), patch( "litellm.proxy.management_endpoints.mcp_management_endpoints.update_mcp_server", @@ -5430,7 +5471,13 @@ def test_bundled_openapi_registry_parses_and_entries_are_well_formed(): registry_path = os.path.join( os.path.dirname(os.path.abspath(__file__)), - "..", "..", "..", "..", "litellm", "proxy", "openapi_registry.json", + "..", + "..", + "..", + "..", + "litellm", + "proxy", + "openapi_registry.json", ) with open(registry_path) as f: registry = json.load(f)