diff --git a/docs/my-website/docs/mcp_oauth.md b/docs/my-website/docs/mcp_oauth.md index f68c5e23aec..ed69408196f 100644 --- a/docs/my-website/docs/mcp_oauth.md +++ b/docs/my-website/docs/mcp_oauth.md @@ -189,4 +189,4 @@ curl http://localhost:4000/mcp-rest/tools/call \ | `client_id` | Yes | OAuth2 client ID. Supports `os.environ/VAR_NAME` | | `client_secret` | Yes | OAuth2 client secret. Supports `os.environ/VAR_NAME` | | `token_url` | Yes | Token endpoint URL | -| `scopes` | No | List of scopes to request | \ No newline at end of file +| `scopes` | No | List of scopes to request | diff --git a/litellm/integrations/prometheus.py b/litellm/integrations/prometheus.py index 0a61dab0680..1675201f1f1 100644 --- a/litellm/integrations/prometheus.py +++ b/litellm/integrations/prometheus.py @@ -29,7 +29,10 @@ from litellm.proxy._types import ( UserAPIKeyAuth, ) from litellm.types.integrations.prometheus import * -from litellm.types.integrations.prometheus import _sanitize_prometheus_label_name +from litellm.types.integrations.prometheus import ( + _sanitize_prometheus_label_name, + _sanitize_prometheus_label_value, +) from litellm.types.utils import StandardLoggingPayload if TYPE_CHECKING: @@ -1276,11 +1279,17 @@ class PrometheusLogger(CustomLogger): ) self.litellm_remaining_api_key_requests_for_model.labels( - user_api_key, user_api_key_alias, model_group, model_id + _sanitize_prometheus_label_value(user_api_key), + _sanitize_prometheus_label_value(user_api_key_alias), + _sanitize_prometheus_label_value(model_group), + _sanitize_prometheus_label_value(model_id), ).set(remaining_requests) self.litellm_remaining_api_key_tokens_for_model.labels( - user_api_key, user_api_key_alias, model_group, model_id + _sanitize_prometheus_label_value(user_api_key), + _sanitize_prometheus_label_value(user_api_key_alias), + _sanitize_prometheus_label_value(model_group), + _sanitize_prometheus_label_value(model_id), ).set(remaining_tokens) def _set_latency_metrics( @@ -1401,14 +1410,14 @@ class PrometheusLogger(CustomLogger): try: self.litellm_llm_api_failed_requests_metric.labels( - end_user_id, - user_api_key, - user_api_key_alias, - model, - user_api_team, - user_api_team_alias, - user_id, - standard_logging_payload.get("model_id", ""), + _sanitize_prometheus_label_value(end_user_id), + _sanitize_prometheus_label_value(user_api_key), + _sanitize_prometheus_label_value(user_api_key_alias), + _sanitize_prometheus_label_value(model), + _sanitize_prometheus_label_value(user_api_team), + _sanitize_prometheus_label_value(user_api_team_alias), + _sanitize_prometheus_label_value(user_id), + _sanitize_prometheus_label_value(standard_logging_payload.get("model_id", "")), ).inc() self.set_llm_deployment_failure_metrics(kwargs) except Exception as e: @@ -2354,7 +2363,11 @@ class PrometheusLogger(CustomLogger): increment metric when litellm.Router / load balancing logic places a deployment in cool down """ self.litellm_deployment_cooled_down.labels( - litellm_model_name, model_id, api_base, api_provider, exception_status + _sanitize_prometheus_label_value(litellm_model_name), + _sanitize_prometheus_label_value(model_id), + _sanitize_prometheus_label_value(api_base), + _sanitize_prometheus_label_value(api_provider), + _sanitize_prometheus_label_value(exception_status), ).inc() def increment_callback_logging_failure( @@ -3074,9 +3087,10 @@ def prometheus_label_factory( # Extract dictionary from Pydantic object enum_dict = enum_values.model_dump() - # Filter supported labels + # Filter supported labels and sanitize values to prevent breaking + # the Prometheus text format (e.g. U+2028 Line Separator in label values) filtered_labels = { - label: value + label: _sanitize_prometheus_label_value(value) for label, value in enum_dict.items() if label in supported_enum_labels } @@ -3094,14 +3108,14 @@ def prometheus_label_factory( # check sanitized key sanitized_key = _sanitize_prometheus_label_name(key) if sanitized_key in supported_enum_labels: - filtered_labels[sanitized_key] = value + filtered_labels[sanitized_key] = _sanitize_prometheus_label_value(value) # Add custom tags if configured if enum_values.tags is not None: custom_tag_labels = get_custom_labels_from_tags(enum_values.tags) for key, value in custom_tag_labels.items(): if key in supported_enum_labels: - filtered_labels[key] = value + filtered_labels[key] = _sanitize_prometheus_label_value(value) for label in supported_enum_labels: if label not in filtered_labels: diff --git a/litellm/proxy/_experimental/mcp_server/oauth2_token_cache.py b/litellm/proxy/_experimental/mcp_server/oauth2_token_cache.py index 6ed582c697a..0de381ee1df 100644 --- a/litellm/proxy/_experimental/mcp_server/oauth2_token_cache.py +++ b/litellm/proxy/_experimental/mcp_server/oauth2_token_cache.py @@ -8,6 +8,8 @@ with ``client_id``, ``client_secret``, and ``token_url``. import asyncio from typing import TYPE_CHECKING, Dict, Optional, Tuple, Union +import httpx + from litellm._logging import verbose_logger from litellm.caching.in_memory_cache import InMemoryCache from litellm.constants import ( @@ -39,9 +41,7 @@ class MCPOAuth2TokenCache(InMemoryCache): self._locks: Dict[str, asyncio.Lock] = {} def _get_lock(self, server_id: str) -> asyncio.Lock: - if server_id not in self._locks: - self._locks[server_id] = asyncio.Lock() - return self._locks[server_id] + return self._locks.setdefault(server_id, asyncio.Lock()) async def async_get_token(self, server: "MCPServer") -> Optional[str]: """Return a valid access token, fetching or refreshing as needed. @@ -76,8 +76,13 @@ class MCPOAuth2TokenCache(InMemoryCache): """ client = get_async_httpx_client(llm_provider=httpxSpecialProvider.MCP) - assert server.client_id is not None, "client_id must be set" - assert server.client_secret is not None, "client_secret must be set" + if not server.client_id or not server.client_secret or not server.token_url: + raise ValueError( + f"MCP server '{server.server_id}' missing required OAuth2 fields: " + f"client_id={bool(server.client_id)}, " + f"client_secret={bool(server.client_secret)}, " + f"token_url={bool(server.token_url)}" + ) data: Dict[str, str] = { "grant_type": "client_credentials", @@ -88,23 +93,41 @@ class MCPOAuth2TokenCache(InMemoryCache): data["scope"] = " ".join(server.scopes) verbose_logger.debug( - "Fetching OAuth2 client_credentials token for MCP server %s from %s", + "Fetching OAuth2 client_credentials token for MCP server %s", server.server_id, - server.token_url, ) - response = await client.post(server.token_url, data=data) # type: ignore[arg-type] - response.raise_for_status() + try: + response = await client.post(server.token_url, data=data) + response.raise_for_status() + except httpx.HTTPStatusError as exc: + raise ValueError( + f"OAuth2 token request for MCP server '{server.server_id}' " + f"failed with status {exc.response.status_code}" + ) from exc + body = response.json() + if not isinstance(body, dict): + raise ValueError( + f"OAuth2 token response for MCP server '{server.server_id}' " + f"returned non-object JSON (got {type(body).__name__})" + ) + access_token = body.get("access_token") if not access_token: raise ValueError( f"OAuth2 token response for MCP server '{server.server_id}' " - f"missing 'access_token': {body}" + f"missing 'access_token'" ) - expires_in = int(body.get("expires_in", 3600)) + # Safely parse expires_in — providers may return null or non-numeric values + raw_expires_in = body.get("expires_in") + try: + expires_in = int(raw_expires_in) if raw_expires_in is not None else MCP_OAUTH2_TOKEN_CACHE_DEFAULT_TTL + except (TypeError, ValueError): + expires_in = MCP_OAUTH2_TOKEN_CACHE_DEFAULT_TTL + ttl = max(expires_in - MCP_OAUTH2_TOKEN_EXPIRY_BUFFER_SECONDS, MCP_OAUTH2_TOKEN_CACHE_MIN_TTL) verbose_logger.info( diff --git a/litellm/types/integrations/prometheus.py b/litellm/types/integrations/prometheus.py index 146c390363b..fd788af9ac1 100644 --- a/litellm/types/integrations/prometheus.py +++ b/litellm/types/integrations/prometheus.py @@ -1,7 +1,7 @@ import re from dataclasses import dataclass from enum import Enum -from typing import Dict, List, Literal, Optional, Tuple +from typing import Any, Dict, List, Literal, Optional, Tuple from pydantic import BaseModel, Field from typing_extensions import Annotated @@ -41,6 +41,38 @@ def _sanitize_prometheus_label_name(label: str) -> str: return sanitized +def _sanitize_prometheus_label_value(value: Optional[Any]) -> Optional[str]: + """ + Sanitize a label value for Prometheus text format compatibility. + + Removes or replaces characters that break the Prometheus exposition format: + - U+2028 (Line Separator) and U+2029 (Paragraph Separator) are removed + - Carriage returns are removed + - Newlines are replaced with spaces + - Backslashes and double quotes are escaped per Prometheus spec + """ + if value is None: + return None + + # Coerce non-string values (int, bool, etc.) to str before sanitizing + if not isinstance(value, str): + value = str(value) + + # Remove Unicode line/paragraph separators that break text format + value = value.replace("\u2028", "").replace("\u2029", "") + + # Remove carriage returns + value = value.replace("\r", "") + + # Replace newlines with spaces + value = value.replace("\n", " ") + + # Escape backslashes and double quotes per Prometheus exposition format + value = value.replace("\\", "\\\\").replace('"', '\\"') + + return value + + @dataclass class MetricValidationError: """Error for invalid metric name""" diff --git a/tests/test_litellm/integrations/test_prometheus_labels.py b/tests/test_litellm/integrations/test_prometheus_labels.py index 9d4626b88a5..2553eb06271 100644 --- a/tests/test_litellm/integrations/test_prometheus_labels.py +++ b/tests/test_litellm/integrations/test_prometheus_labels.py @@ -292,6 +292,87 @@ def test_prometheus_metrics_use_normalized_routes(): print("✅ Prometheus metrics use normalized routes in labels") +def test_prometheus_label_value_sanitization(): + """ + Test that Prometheus label values are sanitized to prevent breaking + the Prometheus text format. + + Issue: Unicode Line Separator (U+2028) in label values (e.g. from a + malformed model name) breaks the Prometheus exposition format, causing + scrapers like Datadog to fail parsing the entire /metrics endpoint. + """ + from litellm.integrations.prometheus import ( + PrometheusLogger, + UserAPIKeyLabelValues, + prometheus_label_factory, + ) + from unittest.mock import MagicMock + + prometheus_logger = MagicMock() + prometheus_logger.get_labels_for_metric = ( + PrometheusLogger.get_labels_for_metric.__get__(prometheus_logger) + ) + + # Simulate a model name with U+2028 (Unicode Line Separator) appended + # and an api_key_alias with newlines and quotes + enum_values = UserAPIKeyLabelValues( + requested_model="claude-haiku-4-5-20251001\u2028", + api_key_alias='My Key "test"\nwith newline', + route="/v1/chat/completions", + status_code="400", + ) + + labels = prometheus_label_factory( + supported_enum_labels=prometheus_logger.get_labels_for_metric( + metric_name="litellm_proxy_total_requests_metric" + ), + enum_values=enum_values, + ) + + # U+2028 must be stripped + assert "\u2028" not in labels["requested_model"], ( + f"U+2028 should be removed from label value, got: {repr(labels['requested_model'])}" + ) + assert labels["requested_model"] == "claude-haiku-4-5-20251001" + + # Newlines must be replaced with spaces, quotes must be escaped + assert "\n" not in labels["api_key_alias"] + assert labels["api_key_alias"] == 'My Key \\"test\\" with newline' + + print("✅ Prometheus label values are properly sanitized") + + +def test_prometheus_label_value_sanitization_unicode_paragraph_separator(): + """Test that U+2029 (Paragraph Separator) is also stripped.""" + from litellm.types.integrations.prometheus import _sanitize_prometheus_label_value + + result = _sanitize_prometheus_label_value("model\u2029name") + assert result == "modelname" + assert "\u2029" not in result + + print("✅ U+2029 Paragraph Separator is stripped") + + +def test_prometheus_label_value_sanitization_none(): + """Test that None values pass through unchanged.""" + from litellm.types.integrations.prometheus import _sanitize_prometheus_label_value + + assert _sanitize_prometheus_label_value(None) is None + + print("✅ None values pass through unchanged") + + +def test_prometheus_label_value_sanitization_non_string_types(): + """Test that non-string values (int, bool, etc.) are coerced to str.""" + from litellm.types.integrations.prometheus import _sanitize_prometheus_label_value + + assert _sanitize_prometheus_label_value(200) == "200" + assert _sanitize_prometheus_label_value(True) == "True" + assert _sanitize_prometheus_label_value(3.14) == "3.14" + + print("✅ Non-string values are coerced to str") + + if __name__ == "__main__": test_user_email_in_required_metrics() test_user_email_label_exists() @@ -301,4 +382,8 @@ if __name__ == "__main__": test_route_normalization_preserves_static_routes() test_route_normalization_other_dynamic_apis() test_prometheus_metrics_use_normalized_routes() + test_prometheus_label_value_sanitization() + test_prometheus_label_value_sanitization_unicode_paragraph_separator() + test_prometheus_label_value_sanitization_none() + test_prometheus_label_value_sanitization_non_string_types() print("\n✅ All prometheus label tests passed!") diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_oauth2_token_cache.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_oauth2_token_cache.py index 2e0f78df605..55735dca98e 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_oauth2_token_cache.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_oauth2_token_cache.py @@ -5,10 +5,9 @@ Covers the critical path: resolve_mcp_auth(), token caching, auth priority, fallback to static token, and the skip-condition property. """ -import asyncio -import time from unittest.mock import AsyncMock, MagicMock, patch +import httpx import pytest from litellm.proxy._experimental.mcp_server.oauth2_token_cache import ( @@ -120,3 +119,39 @@ def test_needs_user_oauth_token_property(): # Non-OAuth2 → never needs user OAuth token assert _server(auth_type=MCPAuth.bearer_token).needs_user_oauth_token is False + + +@pytest.mark.asyncio +async def test_http_error_raises_value_error(): + """HTTP errors from the token endpoint are wrapped in a clear ValueError.""" + server = _server() + mock_response = MagicMock() + mock_response.status_code = 401 + mock_response.raise_for_status.side_effect = httpx.HTTPStatusError( + "Unauthorized", request=MagicMock(), response=mock_response, + ) + mock_client = AsyncMock() + mock_client.post.return_value = mock_response + + with patch( + "litellm.proxy._experimental.mcp_server.oauth2_token_cache.get_async_httpx_client", + return_value=mock_client, + ), pytest.raises(ValueError, match="failed with status 401"): + await resolve_mcp_auth(server) + + +@pytest.mark.asyncio +async def test_non_dict_response_raises_value_error(): + """A non-dict JSON response raises a clear ValueError.""" + server = _server() + resp = MagicMock() + resp.json.return_value = ["not", "a", "dict"] + resp.raise_for_status = MagicMock() + mock_client = AsyncMock() + mock_client.post.return_value = resp + + with patch( + "litellm.proxy._experimental.mcp_server.oauth2_token_cache.get_async_httpx_client", + return_value=mock_client, + ), pytest.raises(ValueError, match="non-object JSON"): + await resolve_mcp_auth(server) diff --git a/ui/litellm-dashboard/src/components/CreateUserButton.test.tsx b/ui/litellm-dashboard/src/components/CreateUserButton.test.tsx new file mode 100644 index 00000000000..829b73734dc --- /dev/null +++ b/ui/litellm-dashboard/src/components/CreateUserButton.test.tsx @@ -0,0 +1,264 @@ +import { render, screen, waitFor, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { CreateUserButton } from "./CreateUserButton"; +import * as networking from "./networking"; +import NotificationsManager from "./molecules/notifications_manager"; + +vi.mock("./networking", () => ({ + userCreateCall: vi.fn(), + modelAvailableCall: vi.fn().mockResolvedValue({ data: [] }), + invitationCreateCall: vi.fn(), + getProxyUISettings: vi.fn().mockResolvedValue({ + PROXY_BASE_URL: null, + PROXY_LOGOUT_URL: null, + DEFAULT_TEAM_DISABLED: false, + SSO_ENABLED: false, + }), + getProxyBaseUrl: vi.fn().mockReturnValue("http://localhost"), +})); + +vi.mock("./bulk_create_users_button", () => ({ + default: () =>