From 5e2df556d8065de5d8baf66e391d089570d88b05 Mon Sep 17 00:00:00 2001 From: shivam Date: Tue, 9 Jun 2026 17:10:15 -0700 Subject: [PATCH 001/168] fix(cost): store cost breakdown for /v1/realtime sessions Realtime cost calculation computed totals but never populated logging_obj.cost_breakdown, so spend logs and the UI Metrics/Cost Breakdown showed no input/output cost details. Co-authored-by: Cursor --- litellm/cost_calculator.py | 10 ++++ tests/test_litellm/test_cost_calculator.py | 60 +++++++++++++++++++++- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/litellm/cost_calculator.py b/litellm/cost_calculator.py index 88029615ba8..cf8fa602be5 100644 --- a/litellm/cost_calculator.py +++ b/litellm/cost_calculator.py @@ -1567,6 +1567,7 @@ def completion_cost( # noqa: PLR0915 custom_llm_provider=custom_llm_provider, litellm_model_name=model, data_residency=data_residency, + litellm_logging_obj=litellm_logging_obj, ) elif call_type == _MCP_CALL_TYPE: from litellm.proxy._experimental.mcp_server.cost_calculator import ( @@ -2494,6 +2495,7 @@ def handle_realtime_stream_cost_calculation( custom_llm_provider: str, litellm_model_name: str, data_residency: Optional[str] = None, + litellm_logging_obj: Optional[LitellmLoggingObject] = None, ) -> float: """ Handles the cost calculation for realtime stream responses. @@ -2533,4 +2535,12 @@ def handle_realtime_stream_cost_calculation( break # exit if we find a valid model total_cost = input_cost_per_token + output_cost_per_token + _store_cost_breakdown_in_logging_obj( + litellm_logging_obj=litellm_logging_obj, + prompt_tokens_cost_usd_dollar=input_cost_per_token, + completion_tokens_cost_usd_dollar=output_cost_per_token, + cost_for_built_in_tools_cost_usd_dollar=0.0, + total_cost_usd_dollar=total_cost, + ) + return total_cost diff --git a/tests/test_litellm/test_cost_calculator.py b/tests/test_litellm/test_cost_calculator.py index 82a4a60bf82..2b60cfc9ccd 100644 --- a/tests/test_litellm/test_cost_calculator.py +++ b/tests/test_litellm/test_cost_calculator.py @@ -385,7 +385,65 @@ def test_handle_realtime_stream_cost_calculation(): ) assert cost == 0.0 # No usage, no cost - + +def test_handle_realtime_stream_cost_calculation_stores_cost_breakdown(): + """Regression: realtime cost must populate logging_obj.cost_breakdown so the + spend logs / UI show input vs output cost (issue: cost_breakdown was None for + /v1/realtime even though a total spend was computed).""" + from datetime import datetime + + from litellm.litellm_core_utils.litellm_logging import Logging + + results: OpenAIRealtimeStreamList = [ + {"type": "session.created", "session": {"model": "gpt-4o-realtime-preview"}}, + { + "type": "response.done", + "response": { + "usage": { + "input_tokens": 100, + "output_tokens": 50, + "total_tokens": 150, + } + }, + }, + ] + combined_usage_object = RealtimeAPITokenUsageProcessor.collect_and_combine_usage_from_realtime_stream_results( + results=results, + ) + + logging_obj = Logging( + model="gpt-4o-realtime-preview", + messages=[], + stream=False, + call_type="_arealtime", + start_time=datetime.now(), + litellm_call_id="realtime-cost-breakdown-test", + function_id="realtime-cost-breakdown-test", + ) + + total_cost = handle_realtime_stream_cost_calculation( + results=results, + combined_usage_object=combined_usage_object, + custom_llm_provider="openai", + litellm_model_name="gpt-4o-realtime-preview", + litellm_logging_obj=logging_obj, + ) + + assert total_cost > 0 + assert logging_obj.cost_breakdown is not None + assert logging_obj.cost_breakdown["input_cost"] > 0 + assert logging_obj.cost_breakdown["output_cost"] > 0 + assert ( + abs( + logging_obj.cost_breakdown["input_cost"] + + logging_obj.cost_breakdown["output_cost"] + - total_cost + ) + < 1e-9 + ) + assert abs(logging_obj.cost_breakdown["total_cost"] - total_cost) < 1e-9 + + def test_realtime_stream_combines_text_and_audio_token_details(): """Realtime response.done usage with input_token_details / output_token_details.""" from litellm.cost_calculator import RealtimeAPITokenUsageProcessor From fa9664eced1b2ea5f0ce027f9fff83d28e2fb070 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 26 Jun 2026 09:58:12 -0700 Subject: [PATCH 002/168] fix(ci): exclude deleted files from ruff format check git diff --name-only includes deleted paths, so a PR that removes a litellm/**/*.py file feeds the gone path to ruff format --check, which exits 123 with 'No such file or directory'. Add --diff-filter=ACMR so only added/copied/modified/renamed files are checked, matching the pattern already used in test-litellm-ui-build.yml. --- .github/workflows/test-linting.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-linting.yml b/.github/workflows/test-linting.yml index ff6c40ac9ae..d0695e49268 100644 --- a/.github/workflows/test-linting.yml +++ b/.github/workflows/test-linting.yml @@ -54,7 +54,7 @@ jobs: env: BASE_SHA: ${{ github.event.pull_request.base.sha }} run: | - git diff --name-only "$BASE_SHA"...HEAD -- 'litellm/**/*.py' | grep -v '^litellm/enterprise/' > "$RUNNER_TEMP/ruff_format_files.txt" || true + git diff --name-only --diff-filter=ACMR "$BASE_SHA"...HEAD -- 'litellm/**/*.py' | grep -v '^litellm/enterprise/' > "$RUNNER_TEMP/ruff_format_files.txt" || true if [ ! -s "$RUNNER_TEMP/ruff_format_files.txt" ]; then echo "No changed litellm Python files to check with ruff format." exit 0 From 1d89e657317b14b2dbfffc66ffce4adab91480e5 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 1 Jul 2026 03:17:56 +0000 Subject: [PATCH 003/168] fix(proxy): trigger gateway fallbacks on local rate limit errors When pre-call hooks (parallel_request_limiter, dynamic_rate_limiter_v3) reject a request with ProxyRateLimitError, the router's fallback logic was never reached because the exception was raised before route_request was called. Add _pre_call_with_fallbacks that catches ProxyRateLimitError, resolves configured fallbacks (key-level router_settings -> router-level), and retries with each fallback model in order. If all fallbacks are also rate-limited, the original error is re-raised. --- litellm/proxy/common_request_processing.py | 114 +++++++- .../proxy/test_common_request_processing.py | 249 ++++++++++++++++++ 2 files changed, 362 insertions(+), 1 deletion(-) diff --git a/litellm/proxy/common_request_processing.py b/litellm/proxy/common_request_processing.py index 97f7d51970c..1acc0b6ebba 100644 --- a/litellm/proxy/common_request_processing.py +++ b/litellm/proxy/common_request_processing.py @@ -1174,6 +1174,118 @@ class ProxyBaseLLMRequestProcessing: return self.data, logging_obj + async def _pre_call_with_fallbacks( + self, + request: Request, + general_settings: dict, + proxy_logging_obj: ProxyLogging, + user_api_key_dict: UserAPIKeyAuth, + version: Optional[str], + proxy_config: ProxyConfig, + user_model: Optional[str], + user_temperature: Optional[float], + user_request_timeout: Optional[float], + user_max_tokens: Optional[int], + user_api_base: Optional[str], + model: Optional[str], + route_type: str, + llm_router: Optional[Router], + ) -> Tuple[dict, LiteLLMLoggingObj]: + from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError + + try: + return await self.common_processing_pre_call_logic( + request=request, + general_settings=general_settings, + proxy_logging_obj=proxy_logging_obj, + user_api_key_dict=user_api_key_dict, + version=version, + proxy_config=proxy_config, + user_model=user_model, + user_temperature=user_temperature, + user_request_timeout=user_request_timeout, + user_max_tokens=user_max_tokens, + user_api_base=user_api_base, + model=model, + route_type=route_type, + llm_router=llm_router, + ) + except ProxyRateLimitError as original_exc: + original_model = self.data.get("model") + if not original_model or not llm_router or self.data.get("disable_fallbacks"): + raise + + fallback_models = self._resolve_fallback_models( + model=original_model, + llm_router=llm_router, + proxy_config=proxy_config, + user_api_key_dict=user_api_key_dict, + ) + if not fallback_models: + raise + + verbose_proxy_logger.info( + "Local rate limit hit for model=%s, attempting fallbacks: %s", + original_model, + fallback_models, + ) + + for fallback_model in fallback_models: + if fallback_model == original_model: + continue + self.data["model"] = fallback_model + try: + return await self.common_processing_pre_call_logic( + request=request, + general_settings=general_settings, + proxy_logging_obj=proxy_logging_obj, + user_api_key_dict=user_api_key_dict, + version=version, + proxy_config=proxy_config, + user_model=user_model, + user_temperature=user_temperature, + user_request_timeout=user_request_timeout, + user_max_tokens=user_max_tokens, + user_api_base=user_api_base, + model=fallback_model, + route_type=route_type, + llm_router=llm_router, + ) + except ProxyRateLimitError: + continue + + self.data["model"] = original_model + raise original_exc + + def _resolve_fallback_models( + self, + model: str, + llm_router: Router, + proxy_config: ProxyConfig, + user_api_key_dict: UserAPIKeyAuth, + ) -> Optional[list]: + from litellm.router_utils.fallback_event_handlers import get_fallback_model_group + + fallbacks = None + + key_router_settings = getattr(user_api_key_dict, "router_settings", None) + if isinstance(key_router_settings, dict) and "fallbacks" in key_router_settings: + fallbacks = key_router_settings["fallbacks"] + + if fallbacks is None: + fallbacks = llm_router.fallbacks + + if not fallbacks: + return None + + fallback_model_group, generic_fallback_idx = get_fallback_model_group( + fallbacks=fallbacks, + model_group=model, + ) + if fallback_model_group is None and generic_fallback_idx is not None: + fallback_model_group = fallbacks[generic_fallback_idx]["*"] + return fallback_model_group + @staticmethod def _get_model_id_from_response(hidden_params: dict, data: dict) -> str: """Extract model_id from hidden_params with fallback to litellm_metadata.""" @@ -1349,7 +1461,7 @@ class ProxyBaseLLMRequestProcessing: "Ensure common_processing_pre_call_logic was called before using this parameter." ) else: - self.data, logging_obj = await self.common_processing_pre_call_logic( + self.data, logging_obj = await self._pre_call_with_fallbacks( request=request, general_settings=general_settings, proxy_logging_obj=proxy_logging_obj, diff --git a/tests/test_litellm/proxy/test_common_request_processing.py b/tests/test_litellm/proxy/test_common_request_processing.py index 1d0dafed171..d6527a4cb90 100644 --- a/tests/test_litellm/proxy/test_common_request_processing.py +++ b/tests/test_litellm/proxy/test_common_request_processing.py @@ -4352,3 +4352,252 @@ class TestResponseCostHeaderForTypedDictResponses: assert "x-litellm-response-cost" not in fastapi_response.headers recompute.assert_not_called() + + +class TestPreCallWithFallbacksOnLocalRateLimit: + """ + Regression tests for LIT-3890: proxy fallbacks must trigger when local rate + limits (key-level TPM/RPM or dynamic_rate_limiter_v3) reject a request. + """ + + @pytest.mark.asyncio + async def test_fallback_triggered_on_local_rate_limit(self): + """ + When the primary model is locally rate-limited, the request should + proceed with a configured fallback model. + """ + from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError + from litellm.proxy.common_request_processing import ProxyBaseLLMRequestProcessing + + primary_model = "gpt-4" + fallback_model = "gpt-3.5-turbo" + + processor = ProxyBaseLLMRequestProcessing(data={"model": primary_model}) + + call_count = 0 + + async def mock_pre_call_logic(**kwargs): + nonlocal call_count + call_count += 1 + model_in_data = processor.data.get("model") + if model_in_data == primary_model: + raise ProxyRateLimitError( + detail="TPM limit exceeded for gpt-4", + headers={"retry-after": "30"}, + ) + logging_obj = MagicMock() + return processor.data, logging_obj + + mock_router = MagicMock() + mock_router.fallbacks = [{"gpt-4": ["gpt-3.5-turbo"]}] + + with patch.object( + processor, + "common_processing_pre_call_logic", + side_effect=mock_pre_call_logic, + ): + data, logging_obj = await processor._pre_call_with_fallbacks( + request=MagicMock(), + general_settings={}, + proxy_logging_obj=MagicMock(), + user_api_key_dict=MagicMock(router_settings=None), + version=None, + proxy_config=MagicMock(), + user_model=None, + user_temperature=None, + user_request_timeout=None, + user_max_tokens=None, + user_api_base=None, + model=primary_model, + route_type="acompletion", + llm_router=mock_router, + ) + + assert processor.data["model"] == fallback_model + assert call_count == 2 + + @pytest.mark.asyncio + async def test_raises_when_no_fallbacks_configured(self): + """ + When no fallbacks are configured, the original rate limit error + should propagate unchanged. + """ + from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError + from litellm.proxy.common_request_processing import ProxyBaseLLMRequestProcessing + + processor = ProxyBaseLLMRequestProcessing(data={"model": "gpt-4"}) + + async def mock_pre_call_logic(**kwargs): + raise ProxyRateLimitError( + detail="TPM limit exceeded", + headers={"retry-after": "30"}, + ) + + mock_router = MagicMock() + mock_router.fallbacks = None + + with patch.object( + processor, + "common_processing_pre_call_logic", + side_effect=mock_pre_call_logic, + ): + with pytest.raises(ProxyRateLimitError): + await processor._pre_call_with_fallbacks( + request=MagicMock(), + general_settings={}, + proxy_logging_obj=MagicMock(), + user_api_key_dict=MagicMock(router_settings=None), + version=None, + proxy_config=MagicMock(), + user_model=None, + user_temperature=None, + user_request_timeout=None, + user_max_tokens=None, + user_api_base=None, + model="gpt-4", + route_type="acompletion", + llm_router=mock_router, + ) + + @pytest.mark.asyncio + async def test_raises_when_all_fallbacks_also_rate_limited(self): + """ + When all fallback models are also locally rate-limited, the original + error for the primary model should be re-raised. + """ + from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError + from litellm.proxy.common_request_processing import ProxyBaseLLMRequestProcessing + + processor = ProxyBaseLLMRequestProcessing(data={"model": "gpt-4"}) + + async def mock_pre_call_logic(**kwargs): + raise ProxyRateLimitError( + detail=f"TPM limit exceeded for {processor.data.get('model')}", + headers={"retry-after": "30"}, + ) + + mock_router = MagicMock() + mock_router.fallbacks = [{"gpt-4": ["gpt-3.5-turbo", "claude-3-haiku"]}] + + with patch.object( + processor, + "common_processing_pre_call_logic", + side_effect=mock_pre_call_logic, + ): + with pytest.raises(ProxyRateLimitError, match="gpt-4"): + await processor._pre_call_with_fallbacks( + request=MagicMock(), + general_settings={}, + proxy_logging_obj=MagicMock(), + user_api_key_dict=MagicMock(router_settings=None), + version=None, + proxy_config=MagicMock(), + user_model=None, + user_temperature=None, + user_request_timeout=None, + user_max_tokens=None, + user_api_base=None, + model="gpt-4", + route_type="acompletion", + llm_router=mock_router, + ) + + # Model should be restored to original + assert processor.data["model"] == "gpt-4" + + @pytest.mark.asyncio + async def test_fallback_uses_key_level_router_settings(self): + """ + Key-level router_settings fallbacks should take precedence over + router-level fallbacks. + """ + from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError + from litellm.proxy.common_request_processing import ProxyBaseLLMRequestProcessing + + processor = ProxyBaseLLMRequestProcessing(data={"model": "gpt-4"}) + + async def mock_pre_call_logic(**kwargs): + if processor.data.get("model") == "gpt-4": + raise ProxyRateLimitError( + detail="TPM limit exceeded", + headers={"retry-after": "30"}, + ) + return processor.data, MagicMock() + + mock_router = MagicMock() + mock_router.fallbacks = [{"gpt-4": ["gpt-3.5-turbo"]}] + + user_api_key_dict = MagicMock() + user_api_key_dict.router_settings = { + "fallbacks": [{"gpt-4": ["claude-3-haiku"]}] + } + + with patch.object( + processor, + "common_processing_pre_call_logic", + side_effect=mock_pre_call_logic, + ): + data, _ = await processor._pre_call_with_fallbacks( + request=MagicMock(), + general_settings={}, + proxy_logging_obj=MagicMock(), + user_api_key_dict=user_api_key_dict, + version=None, + proxy_config=MagicMock(), + user_model=None, + user_temperature=None, + user_request_timeout=None, + user_max_tokens=None, + user_api_base=None, + model="gpt-4", + route_type="acompletion", + llm_router=mock_router, + ) + + # Should use key-level fallback, not router-level + assert processor.data["model"] == "claude-3-haiku" + + @pytest.mark.asyncio + async def test_disable_fallbacks_flag_respected(self): + """ + When disable_fallbacks is set in request data, local rate limit + errors should not trigger fallback logic. + """ + from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError + from litellm.proxy.common_request_processing import ProxyBaseLLMRequestProcessing + + processor = ProxyBaseLLMRequestProcessing( + data={"model": "gpt-4", "disable_fallbacks": True} + ) + + async def mock_pre_call_logic(**kwargs): + raise ProxyRateLimitError( + detail="TPM limit exceeded", + headers={"retry-after": "30"}, + ) + + mock_router = MagicMock() + mock_router.fallbacks = [{"gpt-4": ["gpt-3.5-turbo"]}] + + with patch.object( + processor, + "common_processing_pre_call_logic", + side_effect=mock_pre_call_logic, + ): + with pytest.raises(ProxyRateLimitError): + await processor._pre_call_with_fallbacks( + request=MagicMock(), + general_settings={}, + proxy_logging_obj=MagicMock(), + user_api_key_dict=MagicMock(router_settings=None), + version=None, + proxy_config=MagicMock(), + user_model=None, + user_temperature=None, + user_request_timeout=None, + user_max_tokens=None, + user_api_base=None, + model="gpt-4", + route_type="acompletion", + llm_router=mock_router, + ) From 9ea149b49efbd13a94673870404fa25a32d8f70a Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 1 Jul 2026 04:13:36 +0000 Subject: [PATCH 004/168] refactor: remove getattr, unused param, and unnecessary comments --- litellm/proxy/common_request_processing.py | 4 +-- .../proxy/test_common_request_processing.py | 26 ------------------- 2 files changed, 1 insertion(+), 29 deletions(-) diff --git a/litellm/proxy/common_request_processing.py b/litellm/proxy/common_request_processing.py index 1acc0b6ebba..ddae83e50ae 100644 --- a/litellm/proxy/common_request_processing.py +++ b/litellm/proxy/common_request_processing.py @@ -1218,7 +1218,6 @@ class ProxyBaseLLMRequestProcessing: fallback_models = self._resolve_fallback_models( model=original_model, llm_router=llm_router, - proxy_config=proxy_config, user_api_key_dict=user_api_key_dict, ) if not fallback_models: @@ -1261,14 +1260,13 @@ class ProxyBaseLLMRequestProcessing: self, model: str, llm_router: Router, - proxy_config: ProxyConfig, user_api_key_dict: UserAPIKeyAuth, ) -> Optional[list]: from litellm.router_utils.fallback_event_handlers import get_fallback_model_group fallbacks = None - key_router_settings = getattr(user_api_key_dict, "router_settings", None) + key_router_settings = user_api_key_dict.router_settings if isinstance(key_router_settings, dict) and "fallbacks" in key_router_settings: fallbacks = key_router_settings["fallbacks"] diff --git a/tests/test_litellm/proxy/test_common_request_processing.py b/tests/test_litellm/proxy/test_common_request_processing.py index d6527a4cb90..a8a74200c25 100644 --- a/tests/test_litellm/proxy/test_common_request_processing.py +++ b/tests/test_litellm/proxy/test_common_request_processing.py @@ -4355,17 +4355,9 @@ class TestResponseCostHeaderForTypedDictResponses: class TestPreCallWithFallbacksOnLocalRateLimit: - """ - Regression tests for LIT-3890: proxy fallbacks must trigger when local rate - limits (key-level TPM/RPM or dynamic_rate_limiter_v3) reject a request. - """ @pytest.mark.asyncio async def test_fallback_triggered_on_local_rate_limit(self): - """ - When the primary model is locally rate-limited, the request should - proceed with a configured fallback model. - """ from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError from litellm.proxy.common_request_processing import ProxyBaseLLMRequestProcessing @@ -4418,10 +4410,6 @@ class TestPreCallWithFallbacksOnLocalRateLimit: @pytest.mark.asyncio async def test_raises_when_no_fallbacks_configured(self): - """ - When no fallbacks are configured, the original rate limit error - should propagate unchanged. - """ from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError from litellm.proxy.common_request_processing import ProxyBaseLLMRequestProcessing @@ -4461,10 +4449,6 @@ class TestPreCallWithFallbacksOnLocalRateLimit: @pytest.mark.asyncio async def test_raises_when_all_fallbacks_also_rate_limited(self): - """ - When all fallback models are also locally rate-limited, the original - error for the primary model should be re-raised. - """ from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError from litellm.proxy.common_request_processing import ProxyBaseLLMRequestProcessing @@ -4502,15 +4486,10 @@ class TestPreCallWithFallbacksOnLocalRateLimit: llm_router=mock_router, ) - # Model should be restored to original assert processor.data["model"] == "gpt-4" @pytest.mark.asyncio async def test_fallback_uses_key_level_router_settings(self): - """ - Key-level router_settings fallbacks should take precedence over - router-level fallbacks. - """ from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError from litellm.proxy.common_request_processing import ProxyBaseLLMRequestProcessing @@ -4554,15 +4533,10 @@ class TestPreCallWithFallbacksOnLocalRateLimit: llm_router=mock_router, ) - # Should use key-level fallback, not router-level assert processor.data["model"] == "claude-3-haiku" @pytest.mark.asyncio async def test_disable_fallbacks_flag_respected(self): - """ - When disable_fallbacks is set in request data, local rate limit - errors should not trigger fallback logic. - """ from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError from litellm.proxy.common_request_processing import ProxyBaseLLMRequestProcessing From b768b6206779a6dd5e2060be62fabfaad7e964ed Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 1 Jul 2026 06:42:51 +0000 Subject: [PATCH 005/168] fix(proxy): restore model state on non-rate-limit exceptions in fallback loop Addresses Greptile review feedback: wrap the fallback loop in try/except BaseException to always restore self.data['model'] to the original value when a non-ProxyRateLimitError exception escapes a fallback attempt. Add regression test for this edge case --- litellm/proxy/common_request_processing.py | 50 ++++++++++--------- .../proxy/test_common_request_processing.py | 46 +++++++++++++++++ 2 files changed, 73 insertions(+), 23 deletions(-) diff --git a/litellm/proxy/common_request_processing.py b/litellm/proxy/common_request_processing.py index ddae83e50ae..df2d1de48b6 100644 --- a/litellm/proxy/common_request_processing.py +++ b/litellm/proxy/common_request_processing.py @@ -1229,29 +1229,33 @@ class ProxyBaseLLMRequestProcessing: fallback_models, ) - for fallback_model in fallback_models: - if fallback_model == original_model: - continue - self.data["model"] = fallback_model - try: - return await self.common_processing_pre_call_logic( - request=request, - general_settings=general_settings, - proxy_logging_obj=proxy_logging_obj, - user_api_key_dict=user_api_key_dict, - version=version, - proxy_config=proxy_config, - user_model=user_model, - user_temperature=user_temperature, - user_request_timeout=user_request_timeout, - user_max_tokens=user_max_tokens, - user_api_base=user_api_base, - model=fallback_model, - route_type=route_type, - llm_router=llm_router, - ) - except ProxyRateLimitError: - continue + try: + for fallback_model in fallback_models: + if fallback_model == original_model: + continue + self.data["model"] = fallback_model + try: + return await self.common_processing_pre_call_logic( + request=request, + general_settings=general_settings, + proxy_logging_obj=proxy_logging_obj, + user_api_key_dict=user_api_key_dict, + version=version, + proxy_config=proxy_config, + user_model=user_model, + user_temperature=user_temperature, + user_request_timeout=user_request_timeout, + user_max_tokens=user_max_tokens, + user_api_base=user_api_base, + model=fallback_model, + route_type=route_type, + llm_router=llm_router, + ) + except ProxyRateLimitError: + continue + except BaseException: + self.data["model"] = original_model + raise self.data["model"] = original_model raise original_exc diff --git a/tests/test_litellm/proxy/test_common_request_processing.py b/tests/test_litellm/proxy/test_common_request_processing.py index a8a74200c25..ef26d9e3e87 100644 --- a/tests/test_litellm/proxy/test_common_request_processing.py +++ b/tests/test_litellm/proxy/test_common_request_processing.py @@ -4575,3 +4575,49 @@ class TestPreCallWithFallbacksOnLocalRateLimit: route_type="acompletion", llm_router=mock_router, ) + + @pytest.mark.asyncio + async def test_model_restored_on_non_rate_limit_exception(self): + from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError + from litellm.proxy.common_request_processing import ProxyBaseLLMRequestProcessing + + primary_model = "gpt-4" + + processor = ProxyBaseLLMRequestProcessing(data={"model": primary_model}) + + async def mock_pre_call_logic(**kwargs): + model_in_data = processor.data.get("model") + if model_in_data == primary_model: + raise ProxyRateLimitError( + detail="TPM limit exceeded for gpt-4", + headers={"retry-after": "30"}, + ) + raise ValueError("unexpected auth failure on fallback") + + mock_router = MagicMock() + mock_router.fallbacks = [{"gpt-4": ["gpt-3.5-turbo"]}] + + with patch.object( + processor, + "common_processing_pre_call_logic", + side_effect=mock_pre_call_logic, + ): + with pytest.raises(ValueError, match="unexpected auth failure"): + await processor._pre_call_with_fallbacks( + request=MagicMock(), + general_settings={}, + proxy_logging_obj=MagicMock(), + user_api_key_dict=MagicMock(router_settings=None), + version=None, + proxy_config=MagicMock(), + user_model=None, + user_temperature=None, + user_request_timeout=None, + user_max_tokens=None, + user_api_base=None, + model="gpt-4", + route_type="acompletion", + llm_router=mock_router, + ) + + assert processor.data["model"] == primary_model From 68a8fc2207d1ee24c6196317a01b8f5f7d685155 Mon Sep 17 00:00:00 2001 From: Mubashir Osmani Date: Thu, 2 Jul 2026 00:55:54 +0000 Subject: [PATCH 006/168] feat(s3_v2): send Content-MD5 on PUT and optional server-side encryption Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- litellm/integrations/s3_v2.py | 21 +++ tests/test_litellm/integrations/test_s3_v2.py | 136 ++++++++++++++++++ 2 files changed, 157 insertions(+) diff --git a/litellm/integrations/s3_v2.py b/litellm/integrations/s3_v2.py index 939289f96ea..db12d1d2c26 100644 --- a/litellm/integrations/s3_v2.py +++ b/litellm/integrations/s3_v2.py @@ -54,6 +54,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): s3_strip_base64_files: bool = False, s3_use_key_prefix: bool = False, s3_use_virtual_hosted_style: bool = False, + s3_server_side_encryption: Optional[str] = None, s3_callback_params_override: Optional[dict] = None, **kwargs, ): @@ -92,6 +93,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): s3_strip_base64_files=s3_strip_base64_files, s3_use_key_prefix=s3_use_key_prefix, s3_use_virtual_hosted_style=s3_use_virtual_hosted_style, + s3_server_side_encryption=s3_server_side_encryption, ) verbose_logger.debug(f"s3 logger using endpoint url {s3_endpoint_url}") @@ -145,6 +147,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): s3_strip_base64_files: bool = False, s3_use_key_prefix: bool = False, s3_use_virtual_hosted_style: bool = False, + s3_server_side_encryption: Optional[str] = None, params_source: Optional[dict] = None, ): """ @@ -194,6 +197,8 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): bool(params.get("s3_use_virtual_hosted_style", False)) or s3_use_virtual_hosted_style ) + self.s3_server_side_encryption = params.get("s3_server_side_encryption") or s3_server_side_encryption + return async def async_log_success_event(self, kwargs, response_obj, start_time, end_time): @@ -273,6 +278,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): async def async_upload_data_to_s3(self, batch_logging_element: s3BatchLoggingElement): try: + import base64 import hashlib import requests @@ -317,14 +323,21 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): # Calculate SHA256 hash of the content content_hash = hashlib.sha256(json_string.encode("utf-8")).hexdigest() + content_md5 = base64.b64encode(hashlib.md5(json_string.encode("utf-8")).digest()).decode() # Prepare the request headers = { "Content-Type": "application/json", + "Content-MD5": content_md5, "x-amz-content-sha256": content_hash, "Content-Language": "en", "Content-Disposition": f'inline; filename="{batch_logging_element.s3_object_download_filename}"', "Cache-Control": "private, immutable, max-age=31536000, s-maxage=0", + **( + {"x-amz-server-side-encryption": self.s3_server_side_encryption} + if self.s3_server_side_encryption + else {} + ), } req = requests.Request("PUT", url, data=json_string, headers=headers) prepped = req.prepare() @@ -447,6 +460,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): def upload_data_to_s3(self, batch_logging_element: s3BatchLoggingElement): try: + import base64 import hashlib import requests @@ -482,14 +496,21 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): # Calculate SHA256 hash of the content content_hash = hashlib.sha256(json_string.encode("utf-8")).hexdigest() + content_md5 = base64.b64encode(hashlib.md5(json_string.encode("utf-8")).digest()).decode() # Prepare the request headers = { "Content-Type": "application/json", + "Content-MD5": content_md5, "x-amz-content-sha256": content_hash, "Content-Language": "en", "Content-Disposition": f'inline; filename="{batch_logging_element.s3_object_download_filename}"', "Cache-Control": "private, immutable, max-age=31536000, s-maxage=0", + **( + {"x-amz-server-side-encryption": self.s3_server_side_encryption} + if self.s3_server_side_encryption + else {} + ), } req = requests.Request("PUT", url, data=json_string, headers=headers) prepped = req.prepare() diff --git a/tests/test_litellm/integrations/test_s3_v2.py b/tests/test_litellm/integrations/test_s3_v2.py index 3f21de41c53..3331f9f4f5e 100644 --- a/tests/test_litellm/integrations/test_s3_v2.py +++ b/tests/test_litellm/integrations/test_s3_v2.py @@ -1194,3 +1194,139 @@ def test_s3_callback_params_override_empty_dict_is_opt_in(): assert logger.s3_bucket_name is None finally: litellm.s3_callback_params = original + + +def _expected_content_md5(payload: dict) -> str: + import base64 + import hashlib + + from litellm.litellm_core_utils.safe_json_dumps import safe_dumps + + json_string = safe_dumps(payload) + return base64.b64encode(hashlib.md5(json_string.encode("utf-8")).digest()).decode() + + +@pytest.mark.asyncio +async def test_async_upload_sets_content_md5_header(): + """ + Object Lock buckets reject PUTs without a Content-MD5 header (AWS spec). + The async upload must send a base64 md5 of the exact signed body. + """ + from unittest.mock import AsyncMock, MagicMock + + from litellm.types.integrations.s3_v2 import s3BatchLoggingElement + + logger = S3Logger( + s3_bucket_name="test-bucket", + s3_aws_access_key_id="test-key", + s3_aws_secret_access_key="test-secret", + s3_region_name="us-east-1", + ) + + payload = {"test": "content-md5"} + test_element = s3BatchLoggingElement( + s3_object_key="2025-09-14/test-md5.json", + payload=payload, + s3_object_download_filename="test-md5.json", + ) + + response = MagicMock() + response.status_code = 200 + response.raise_for_status = MagicMock() + logger.async_httpx_client = AsyncMock() + logger.async_httpx_client.put.return_value = response + + await logger.async_upload_data_to_s3(test_element) + + headers = logger.async_httpx_client.put.call_args.kwargs["headers"] + assert headers["Content-MD5"] == _expected_content_md5(payload) + assert "x-amz-server-side-encryption" not in headers + + +def test_sync_upload_sets_content_md5_header(): + """The sync upload path must also send Content-MD5 for Object Lock buckets.""" + from unittest.mock import MagicMock + + from litellm.types.integrations.s3_v2 import s3BatchLoggingElement + + logger = S3Logger( + s3_bucket_name="test-bucket", + s3_aws_access_key_id="test-key", + s3_aws_secret_access_key="test-secret", + s3_region_name="us-east-1", + ) + + payload = {"test": "sync-content-md5"} + test_element = s3BatchLoggingElement( + s3_object_key="2025-09-14/test-sync-md5.json", + payload=payload, + s3_object_download_filename="test-sync-md5.json", + ) + + response = MagicMock() + response.status_code = 200 + response.raise_for_status = MagicMock() + mock_sync_client = MagicMock() + mock_sync_client.put.return_value = response + + with patch( + "litellm.integrations.s3_v2._get_httpx_client", + return_value=mock_sync_client, + ): + logger.upload_data_to_s3(test_element) + + headers = mock_sync_client.put.call_args.kwargs["headers"] + assert headers["Content-MD5"] == _expected_content_md5(payload) + assert "x-amz-server-side-encryption" not in headers + + +@pytest.mark.asyncio +async def test_async_upload_sets_server_side_encryption_header_when_configured(): + """ + When s3_server_side_encryption is set (e.g. buckets with a KMS default + encryption policy), the PUT must carry x-amz-server-side-encryption. + """ + from unittest.mock import AsyncMock, MagicMock + + from litellm.types.integrations.s3_v2 import s3BatchLoggingElement + + logger = S3Logger( + s3_bucket_name="test-bucket", + s3_aws_access_key_id="test-key", + s3_aws_secret_access_key="test-secret", + s3_region_name="us-east-1", + s3_server_side_encryption="aws:kms", + ) + + test_element = s3BatchLoggingElement( + s3_object_key="2025-09-14/test-sse.json", + payload={"test": "sse"}, + s3_object_download_filename="test-sse.json", + ) + + response = MagicMock() + response.status_code = 200 + response.raise_for_status = MagicMock() + logger.async_httpx_client = AsyncMock() + logger.async_httpx_client.put.return_value = response + + await logger.async_upload_data_to_s3(test_element) + + headers = logger.async_httpx_client.put.call_args.kwargs["headers"] + assert headers["x-amz-server-side-encryption"] == "aws:kms" + + +def test_s3_server_side_encryption_read_from_callback_params(): + """s3_server_side_encryption can be configured via s3_callback_params.""" + import litellm + + original = litellm.s3_callback_params + litellm.s3_callback_params = { + "s3_bucket_name": "from-global", + "s3_server_side_encryption": "aws:kms", + } + try: + logger = S3Logger() + assert logger.s3_server_side_encryption == "aws:kms" + finally: + litellm.s3_callback_params = original From e542be17addeb0b47d3cbaf18ebe50d0f3ceabad Mon Sep 17 00:00:00 2001 From: Mubashir Osmani Date: Thu, 2 Jul 2026 01:12:49 +0000 Subject: [PATCH 007/168] fix(s3_v2): pass usedforsecurity=False to hashlib.md5 for FIPS envs Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- litellm/integrations/s3_v2.py | 8 ++++++-- tests/test_litellm/integrations/test_s3_v2.py | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/litellm/integrations/s3_v2.py b/litellm/integrations/s3_v2.py index db12d1d2c26..5b953035cfd 100644 --- a/litellm/integrations/s3_v2.py +++ b/litellm/integrations/s3_v2.py @@ -323,7 +323,9 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): # Calculate SHA256 hash of the content content_hash = hashlib.sha256(json_string.encode("utf-8")).hexdigest() - content_md5 = base64.b64encode(hashlib.md5(json_string.encode("utf-8")).digest()).decode() + content_md5 = base64.b64encode( + hashlib.md5(json_string.encode("utf-8"), usedforsecurity=False).digest() + ).decode() # Prepare the request headers = { @@ -496,7 +498,9 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): # Calculate SHA256 hash of the content content_hash = hashlib.sha256(json_string.encode("utf-8")).hexdigest() - content_md5 = base64.b64encode(hashlib.md5(json_string.encode("utf-8")).digest()).decode() + content_md5 = base64.b64encode( + hashlib.md5(json_string.encode("utf-8"), usedforsecurity=False).digest() + ).decode() # Prepare the request headers = { diff --git a/tests/test_litellm/integrations/test_s3_v2.py b/tests/test_litellm/integrations/test_s3_v2.py index 3331f9f4f5e..070e278cdc0 100644 --- a/tests/test_litellm/integrations/test_s3_v2.py +++ b/tests/test_litellm/integrations/test_s3_v2.py @@ -1203,7 +1203,9 @@ def _expected_content_md5(payload: dict) -> str: from litellm.litellm_core_utils.safe_json_dumps import safe_dumps json_string = safe_dumps(payload) - return base64.b64encode(hashlib.md5(json_string.encode("utf-8")).digest()).decode() + return base64.b64encode( + hashlib.md5(json_string.encode("utf-8"), usedforsecurity=False).digest() + ).decode() @pytest.mark.asyncio From 29cc4e35d65a7d191e7c6d38b50d99f1315616d5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 2 Jul 2026 01:16:21 +0000 Subject: [PATCH 008/168] fix: allow S3 Content-MD5 on FIPS hosts --- tests/test_litellm/integrations/test_s3_v2.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tests/test_litellm/integrations/test_s3_v2.py b/tests/test_litellm/integrations/test_s3_v2.py index 070e278cdc0..246b378c982 100644 --- a/tests/test_litellm/integrations/test_s3_v2.py +++ b/tests/test_litellm/integrations/test_s3_v2.py @@ -1208,8 +1208,21 @@ def _expected_content_md5(payload: dict) -> str: ).decode() +def _require_non_security_md5(monkeypatch): + import hashlib + + original_md5 = hashlib.md5 + + def fips_md5(data=b"", *, usedforsecurity=True): + if usedforsecurity: + raise ValueError("MD5 blocked for security use") + return original_md5(data, usedforsecurity=usedforsecurity) + + monkeypatch.setattr(hashlib, "md5", fips_md5) + + @pytest.mark.asyncio -async def test_async_upload_sets_content_md5_header(): +async def test_async_upload_sets_content_md5_header(monkeypatch): """ Object Lock buckets reject PUTs without a Content-MD5 header (AWS spec). The async upload must send a base64 md5 of the exact signed body. @@ -1231,6 +1244,7 @@ async def test_async_upload_sets_content_md5_header(): payload=payload, s3_object_download_filename="test-md5.json", ) + _require_non_security_md5(monkeypatch) response = MagicMock() response.status_code = 200 @@ -1245,7 +1259,7 @@ async def test_async_upload_sets_content_md5_header(): assert "x-amz-server-side-encryption" not in headers -def test_sync_upload_sets_content_md5_header(): +def test_sync_upload_sets_content_md5_header(monkeypatch): """The sync upload path must also send Content-MD5 for Object Lock buckets.""" from unittest.mock import MagicMock @@ -1264,6 +1278,7 @@ def test_sync_upload_sets_content_md5_header(): payload=payload, s3_object_download_filename="test-sync-md5.json", ) + _require_non_security_md5(monkeypatch) response = MagicMock() response.status_code = 200 From f5f8ba93faf3ae6c50e51d035e09d7a0bb7a08c0 Mon Sep 17 00:00:00 2001 From: yucheng-berri Date: Thu, 2 Jul 2026 10:00:30 -0700 Subject: [PATCH 009/168] fix(mcp): tighten role-based visibility on /v1/mcp/server/submissions (#31932) Route non-full-admin callers through _sanitize_mcp_server_list_for_non_admin, matching the pattern the fetch and list handlers adopted. Replace the two regression tests that pinned the old partial-blank behavior with a sanitize/full-admin pair mirroring the fetch/list coverage. Resolves LIT-3929 --- .../mcp_management_endpoints.py | 17 +-- .../test_mcp_management_endpoints.py | 103 ++++++++++++------ 2 files changed, 71 insertions(+), 49 deletions(-) diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index 09d9809e4fb..1ee882920ff 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -148,7 +148,6 @@ if MCP_AVAILABLE: LitellmUserRoles, MakeMCPServersPublicRequest, MCPApprovalStatus, - MCPEnvVarScope, MCPOAuthUserCredentialRequest, MCPOAuthUserCredentialStatus, MCPSubmissionsSummary, @@ -460,18 +459,6 @@ if MCP_AVAILABLE: ) -> List[LiteLLM_MCPServerTable]: return [_redact_mcp_credentials(server) for server in mcp_servers] - def _redact_global_env_var_values(mcp_server: LiteLLM_MCPServerTable) -> None: - """Blank admin-supplied ``scope="global"`` env var secrets in place. - - Global entries hold the admin's plaintext credential (API key, - password, ...) and must never reach non-admin callers. Per-user - entries only carry a placeholder the user fills in themselves, so - their value is left intact. - """ - for env_var in mcp_server.env_vars or []: - if env_var.scope == MCPEnvVarScope.global_: - env_var.value = "" - def _user_is_full_admin(user_api_key_dict: UserAPIKeyAuth) -> bool: """True only for ``PROXY_ADMIN``; ``PROXY_ADMIN_VIEW_ONLY`` returns False. @@ -1114,9 +1101,9 @@ if MCP_AVAILABLE: prisma_client = get_prisma_client_or_throw("Database not connected. Connect a database to your proxy") submissions = await get_mcp_submissions(prisma_client) + submissions.items = _redact_mcp_credentials_list(submissions.items) if not _user_is_full_admin(user_api_key_dict): - for item in submissions.items: - _redact_global_env_var_values(item) + submissions.items = _sanitize_mcp_server_list_for_non_admin(submissions.items) return submissions @router.put( 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 ef8433fdaf0..e1ffcc58fce 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 @@ -3126,41 +3126,18 @@ class TestMCPApprovalWorkflow: assert result.pending_review == 1 @pytest.mark.asyncio - @pytest.mark.parametrize( - "user_role, expected_global_value", - [ - (LitellmUserRoles.PROXY_ADMIN, "super-secret"), - (LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY, ""), - ], - ) - async def test_get_submissions_redacts_global_env_for_view_only_admin( - self, user_role, expected_global_value - ): - """Read-only admins reviewing the submission queue must not receive the - submitter's global env var secrets; full admins still see them.""" + async def test_get_submissions_sanitizes_for_view_only_admin(self): + """PROXY_ADMIN_VIEW_ONLY reviewing the submission queue must go through + the non-admin sanitizer that fetch/list endpoints use: url, + static_headers, env, env_vars, and credentials are all dropped. A + mutation swapping the gate back to the old partial-blank pattern (which + left url/static_headers/env and env-var names intact) would fail this.""" from litellm.proxy._types import MCPSubmissionsSummary from litellm.proxy.management_endpoints.mcp_management_endpoints import ( get_mcp_server_submissions, ) - base = generate_mock_mcp_server_db_record(alias="Pending") - item = LiteLLM_MCPServerTable( - **{ - **base.model_dump(), - "env_vars": [ - { - "name": "ADMIN_API_KEY", - "value": "super-secret", - "scope": "global", - }, - { - "name": "USER_TOKEN", - "value": "placeholder-hint", - "scope": "user", - }, - ], - } - ) + item = _leaky_list_server() item.approval_status = "pending_review" summary = MCPSubmissionsSummary( total=1, pending_review=1, active=0, rejected=0, items=[item] @@ -3177,12 +3154,70 @@ class TestMCPApprovalWorkflow: ), ): result = await get_mcp_server_submissions( - user_api_key_dict=generate_mock_user_api_key_auth(user_role=user_role), + user_api_key_dict=generate_mock_user_api_key_auth( + user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY + ), ) - by_name = {ev.name: ev for ev in result.items[0].env_vars} - assert by_name["ADMIN_API_KEY"].value == expected_global_value - assert by_name["USER_TOKEN"].value == "placeholder-hint" + assert len(result.items) == 1 + sanitized = result.items[0] + assert sanitized.url is None + assert sanitized.static_headers is None + assert sanitized.env == {} + assert sanitized.env_vars is None + assert sanitized.credentials is None + + # The source record must not be mutated by sanitization. + assert item.url == "https://leaky.example.com/mcp?api_key=sk-embedded-in-url" + assert item.static_headers == {"Authorization": "Bearer sk-secret-header"} + + @pytest.mark.asyncio + async def test_get_submissions_full_admin_still_sees_secrets(self): + """The view-only redaction must not over-redact for a full PROXY_ADMIN, + who needs url/static_headers/env/env_vars to review the pending + submission. Only the explicit credentials field is cleared.""" + from litellm.proxy._types import MCPSubmissionsSummary + from litellm.proxy.management_endpoints.mcp_management_endpoints import ( + get_mcp_server_submissions, + ) + + item = _leaky_list_server() + item.approval_status = "pending_review" + summary = MCPSubmissionsSummary( + total=1, pending_review=1, active=0, rejected=0, items=[item] + ) + + with ( + patch( + "litellm.proxy.management_endpoints.mcp_management_endpoints.get_prisma_client_or_throw", + return_value=MagicMock(), + ), + patch( + "litellm.proxy.management_endpoints.mcp_management_endpoints.get_mcp_submissions", + AsyncMock(return_value=summary), + ), + ): + result = await get_mcp_server_submissions( + user_api_key_dict=generate_mock_user_api_key_auth( + user_role=LitellmUserRoles.PROXY_ADMIN + ), + ) + + assert len(result.items) == 1 + raw = result.items[0] + assert raw.url == "https://leaky.example.com/mcp?api_key=sk-embedded-in-url" + assert raw.static_headers == {"Authorization": "Bearer sk-secret-header"} + assert raw.env == {"UPSTREAM_TOKEN": "sk-secret-env"} + assert raw.credentials is None + assert raw.env_vars is not None + assert len(raw.env_vars) == 1 + # ``model_construct`` in ``_leaky_list_server`` skips validation, so + # env_vars stays as raw dicts; mirror the fixture shape here. + entry = raw.env_vars[0] + name = entry["name"] if isinstance(entry, dict) else entry.name + value = entry["value"] if isinstance(entry, dict) else entry.value + assert name == "GLOBAL_KEY" + assert value == "super-secret" @pytest.mark.asyncio async def test_approve_non_pending_server_raises_400(self): From 09f611e7b50d93ea224ab51053619ed67def4f7b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 2 Jul 2026 17:03:05 +0000 Subject: [PATCH 010/168] fix: include realtime transcription cost in breakdown --- litellm/cost_calculator.py | 18 ++++++++++-------- tests/test_litellm/test_cost_calculator.py | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/litellm/cost_calculator.py b/litellm/cost_calculator.py index bfdce95d89a..9d9564a7110 100644 --- a/litellm/cost_calculator.py +++ b/litellm/cost_calculator.py @@ -2564,7 +2564,16 @@ def handle_realtime_stream_cost_calculation( input_cost_per_token += _input_cost_per_token output_cost_per_token += _output_cost_per_token break # exit if we find a valid model - total_cost = input_cost_per_token + output_cost_per_token + transcription_cost = ( + handle_realtime_transcription_cost_calculation( + results=results, + custom_llm_provider=custom_llm_provider, + litellm_model_name=litellm_model_name, + ) + if any(r.get("type") == _TRANSCRIPTION_COMPLETED_EVENT_TYPE for r in results) + else 0.0 + ) + total_cost = input_cost_per_token + output_cost_per_token + transcription_cost _store_cost_breakdown_in_logging_obj( litellm_logging_obj=litellm_logging_obj, @@ -2574,13 +2583,6 @@ def handle_realtime_stream_cost_calculation( total_cost_usd_dollar=total_cost, ) - if any(r.get("type") == _TRANSCRIPTION_COMPLETED_EVENT_TYPE for r in results): - total_cost += handle_realtime_transcription_cost_calculation( - results=results, - custom_llm_provider=custom_llm_provider, - litellm_model_name=litellm_model_name, - ) - return total_cost diff --git a/tests/test_litellm/test_cost_calculator.py b/tests/test_litellm/test_cost_calculator.py index 4ba64f13b69..3a5d43b0300 100644 --- a/tests/test_litellm/test_cost_calculator.py +++ b/tests/test_litellm/test_cost_calculator.py @@ -637,6 +637,10 @@ def test_realtime_transcription_duration_cost(monkeypatch): ($0.017/min). The .completed events carry usage {type: duration, seconds: N}; cost must equal total_seconds * input_cost_per_second. """ + from datetime import datetime + + from litellm.litellm_core_utils.litellm_logging import Logging + monkeypatch.setenv("LITELLM_LOCAL_MODEL_COST_MAP", "True") monkeypatch.setattr(litellm, "model_cost", litellm.get_model_cost_map(url="")) @@ -667,17 +671,29 @@ def test_realtime_transcription_duration_cost(monkeypatch): combined = RealtimeAPITokenUsageProcessor.collect_and_combine_usage_from_realtime_stream_results( results=results ) + logging_obj = Logging( + model="gpt-realtime-whisper", + messages=[], + stream=False, + call_type="_arealtime", + start_time=datetime.now(), + litellm_call_id="realtime-transcription-cost-breakdown-test", + function_id="realtime-transcription-cost-breakdown-test", + ) cost = handle_realtime_stream_cost_calculation( results=results, combined_usage_object=combined, custom_llm_provider="openai", litellm_model_name="gpt-realtime-whisper", + litellm_logging_obj=logging_obj, ) # 90 seconds at $0.017/minute. expected = 90.0 * (0.017 / 60) assert abs(cost - expected) < 1e-9 assert cost > 0 # guards against the duration branch being dropped + assert logging_obj.cost_breakdown is not None + assert abs(logging_obj.cost_breakdown["total_cost"] - cost) < 1e-9 def test_realtime_transcription_duration_cost_resolves_model_from_litellm_name( From c370503091b27733faa0b19e5b3726a275931ea3 Mon Sep 17 00:00:00 2001 From: tin-berri Date: Thu, 2 Jul 2026 10:24:12 -0700 Subject: [PATCH 011/168] fix(mcp): gate OAuth authorize/token/register/discovery on auth_type=oauth2 (#31736) * fix(mcp): gate OAuth authorize/token/register/discovery on auth_type=oauth2 A non-oauth2 MCP server (notably auth_type=none, access-group gated) has no client_id and no authorization URL, yet the gateway OAuth endpoints did not check auth_type. authorize() raised "client_id is required" before the auth_type was ever examined, and the .well-known discovery builders always advertised authorization_servers / authorization_endpoint / token_endpoint / registration_endpoint, so spec-compliant MCP clients were pointed at an OAuth flow that can never succeed. Add an auth_type != oauth2 guard to the authorize, token, register, protected-resource and authorization-server paths (covering the internal UI OAuth endpoints too). The discovery guard sits after the OAuth pass-through branch so genuine pass-through servers keep proxying their upstream metadata. oauth2 servers are unaffected. * fix(mcp): accurate non-oauth2 message; 404 unknown discovery names to close enumeration oracle Address review feedback on the auth_type gate. The 400 message no longer claims access is governed by access groups, which is only true for auth_type=none; it now states that the gateway runs the OAuth client_id/authorize/token/register flow only for oauth2 servers and that the server is reached using its configured auth_type, which is accurate for every non-oauth2 type (api_key, oauth2_token_exchange, etc.). The discovery gate previously 404'd a named non-oauth2 server but still returned 200 metadata for an unknown name, which both serves a broken document for a typo and lets an unauthenticated caller enumerate non-OAuth server names by comparing 404 vs 200. A named discovery request now returns 200 only when it resolves to an oauth2 server; unknown (or hidden) and non-oauth2 names return the same 404. Root discovery and pass-through servers are unaffected. * Apply suggestions from code review Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --- .../mcp_server/discoverable_endpoints.py | 47 +++ .../mcp_management_endpoints.py | 3 + .../mcp_server/test_discoverable_endpoints.py | 317 ++++++++++++++++++ .../test_mcp_management_endpoints.py | 89 +++++ 4 files changed, 456 insertions(+) diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index 6933aa06b2d..d045d2a9e60 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -390,6 +390,46 @@ async def _store_per_user_token_server_side( ) +def _raise_if_not_oauth2(mcp_server: MCPServer) -> None: + """Reject a non-oauth2 server from the gateway's OAuth authorize/token/register flow.""" + if mcp_server.auth_type == MCPAuth.oauth2: + return + raise HTTPException( + status_code=400, + detail={ + "error": "server_not_oauth2", + "message": ( + f"MCP server '{mcp_server.server_name or mcp_server.name}' does not use OAuth " + f"(auth_type={mcp_server.auth_type}). This server does not support the authorization-code " + "flow; it has no client_id, authorize, token, or registration endpoint. " + "Access is controlled by the server's configured auth_type and access groups" + ), + }, + ) + + +def _raise_unless_oauth2_discovery_server( + mcp_server: Optional[MCPServer], + mcp_server_name: Optional[str], + description: str, +) -> None: + """404 a NAMED discovery request unless it resolves to an oauth2 server. + + A named server that is unknown (or hidden from the caller) and one that exists + but is non-oauth2 both return the same 404, so the well-known discovery paths + cannot be used to enumerate non-OAuth server names. Root discovery (no name) is + unaffected, and pass-through servers are resolved by the caller before this runs. + """ + if mcp_server_name is None: + return + if mcp_server is not None and mcp_server.auth_type == MCPAuth.oauth2: + return + raise HTTPException( + status_code=404, + detail=f"MCP server '{mcp_server_name}' is {description}", + ) + + async def authorize_with_server( request: Request, mcp_server: MCPServer, @@ -457,6 +497,7 @@ async def exchange_token_with_server( refresh_token: Optional[str] = None, scope: Optional[str] = None, ): + _raise_if_not_oauth2(mcp_server) if grant_type not in ("authorization_code", "refresh_token"): raise HTTPException(status_code=400, detail="Unsupported grant_type") @@ -582,6 +623,7 @@ async def register_client_with_server( token_endpoint_auth_method: Optional[str], fallback_client_id: Optional[str] = None, ): + _raise_if_not_oauth2(mcp_server) request_base_url = get_request_base_url(request) dummy_return = { "client_id": fallback_client_id or mcp_server.server_name, @@ -655,6 +697,7 @@ async def authorize( mcp_server = _resolve_oauth2_server_for_root_endpoints(client_ip=client_ip) if mcp_server is None: raise HTTPException(status_code=404, detail="MCP server not found") + _raise_if_not_oauth2(mcp_server) # Use server's stored client_id when caller doesn't supply one. # Raise a clear error instead of passing an empty string — an empty # client_id would silently produce a broken authorization URL. @@ -1063,6 +1106,8 @@ async def _build_oauth_protected_resource_response( detail=(f"Upstream oauth-protected-resource metadata unavailable for MCP server {mcp_server.name!r}"), ) + _raise_unless_oauth2_discovery_server(mcp_server, mcp_server_name, "not an OAuth-protected resource") + return { "authorization_servers": [ (f"{request_base_url}/{mcp_server_name}" if mcp_server_name else f"{request_base_url}") @@ -1149,6 +1194,8 @@ def _build_oauth_authorization_server_response( if mcp_server_name: mcp_server = global_mcp_server_manager.get_mcp_server_by_name(mcp_server_name, client_ip=client_ip) + _raise_unless_oauth2_discovery_server(mcp_server, mcp_server_name, "not an OAuth authorization server") + return { "issuer": request_base_url, # point to your proxy "authorization_endpoint": authorization_endpoint, diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index 1ee882920ff..9dab3498bc1 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -132,6 +132,7 @@ if MCP_AVAILABLE: update_mcp_server, ) from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + _raise_if_not_oauth2, authorize_with_server, exchange_token_with_server, get_request_base_url, @@ -1611,6 +1612,7 @@ if MCP_AVAILABLE: scope: Optional[str] = None, ): mcp_server = await _get_cached_temporary_mcp_server_or_404(server_id, user_api_key_dict, request=request) + _raise_if_not_oauth2(mcp_server) # Use the server's stored client_id when the caller doesn't supply one resolved_client_id = mcp_server.client_id or client_id or "" if not resolved_client_id: @@ -1655,6 +1657,7 @@ if MCP_AVAILABLE: scope: Optional[str] = Form(None), ): mcp_server = await _get_cached_temporary_mcp_server_or_404(server_id, user_api_key_dict, request=request) + _raise_if_not_oauth2(mcp_server) resolved_client_id = mcp_server.client_id or client_id or "" if not resolved_client_id: raise HTTPException( diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py index b4a26c911dc..bd94c84b951 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py @@ -3011,3 +3011,320 @@ async def test_token_endpoint_client_secret_basic_without_secret_returns_400(): code_verifier="verifier", ) assert exc_info.value.status_code == 400 + + +# ------------------------------------------------------------------- +# Non-oauth2 (auth_type=none, access-group gated) servers must not be +# driven through the gateway OAuth authorize/token/register/discovery +# flow, and must not be advertised as OAuth-protected in discovery docs. +# ------------------------------------------------------------------- + + +def _access_group_none_server(server_name="access_group_server"): + """A non-oauth2, access-group gated MCP server: no client_id, no OAuth.""" + from litellm.proxy._types import MCPTransport + from litellm.types.mcp import MCPAuth + from litellm.types.mcp_server.mcp_server_manager import MCPServer + + return MCPServer( + server_id=server_name, + name=server_name, + server_name=server_name, + alias=server_name, + transport=MCPTransport.http, + auth_type=MCPAuth.none, + access_groups=["eng"], + ) + + +@pytest.mark.asyncio +async def test_authorize_endpoint_rejects_non_oauth2_server(): + """authorize() against a none-auth server returns an accurate 'does not use OAuth' 400, + not the misleading 'client_id is required' that fired before the auth_type was checked.""" + try: + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + authorize, + ) + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + global_mcp_server_manager.registry.clear() + server = _access_group_none_server() + global_mcp_server_manager.registry[server.server_id] = server + + mock_request = MagicMock() + mock_request.base_url = "https://litellm.example.com/" + mock_request.headers = {} + + try: + with pytest.raises(HTTPException) as exc_info: + await authorize( + request=mock_request, + client_id=None, + mcp_server_name="access_group_server", + redirect_uri="http://127.0.0.1:60108/callback", + state="test_state", + ) + assert exc_info.value.status_code == 400 + detail_text = str(exc_info.value.detail) + assert "does not use OAuth" in detail_text + assert "client_id is required" not in detail_text + finally: + global_mcp_server_manager.registry.clear() + + +@pytest.mark.asyncio +async def test_token_endpoint_rejects_non_oauth2_server(): + """token_endpoint() against a none-auth server returns 'does not use OAuth' 400 instead + of the misleading 'token url is not set'.""" + try: + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + token_endpoint, + ) + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + global_mcp_server_manager.registry.clear() + server = _access_group_none_server() + global_mcp_server_manager.registry[server.server_id] = server + + mock_request = MagicMock() + mock_request.base_url = "https://litellm.example.com/" + mock_request.headers = {} + + try: + with pytest.raises(HTTPException) as exc_info: + await token_endpoint( + request=mock_request, + grant_type="authorization_code", + code="auth-code", + redirect_uri="http://localhost/callback", + client_id="some-client", + mcp_server_name="access_group_server", + client_secret=None, + code_verifier="verifier", + ) + assert exc_info.value.status_code == 400 + detail_text = str(exc_info.value.detail) + assert "does not use OAuth" in detail_text + assert "token url is not set" not in detail_text + finally: + global_mcp_server_manager.registry.clear() + + +@pytest.mark.asyncio +async def test_register_client_rejects_non_oauth2_server(): + """register_client() against a named none-auth server returns 'does not use OAuth' 400 + instead of the misleading 'authorization url is not set'.""" + try: + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + register_client, + ) + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + global_mcp_server_manager.registry.clear() + server = _access_group_none_server() + global_mcp_server_manager.registry[server.server_id] = server + + mock_request = MagicMock() + mock_request.base_url = "https://litellm.example.com/" + mock_request.headers = {} + + try: + with pytest.raises(HTTPException) as exc_info: + with patch( + "litellm.proxy._experimental.mcp_server.discoverable_endpoints._read_request_body", + new=AsyncMock(return_value={}), + ): + await register_client(request=mock_request, mcp_server_name="access_group_server") + assert exc_info.value.status_code == 400 + detail_text = str(exc_info.value.detail) + assert "does not use OAuth" in detail_text + assert "authorization url is not set" not in detail_text + finally: + global_mcp_server_manager.registry.clear() + + +@pytest.mark.asyncio +async def test_oauth_protected_resource_404_for_non_oauth2_server(): + """Discovery must not advertise a none-auth server as an OAuth-protected resource.""" + try: + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + _build_oauth_protected_resource_response, + ) + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + global_mcp_server_manager.registry.clear() + server = _access_group_none_server() + global_mcp_server_manager.registry[server.server_id] = server + + mock_request = MagicMock() + mock_request.base_url = "https://litellm.example.com/" + mock_request.headers = {} + + try: + with pytest.raises(HTTPException) as exc_info: + await _build_oauth_protected_resource_response( + request=mock_request, + mcp_server_name="access_group_server", + use_standard_pattern=False, + ) + assert exc_info.value.status_code == 404 + assert "not an OAuth-protected resource" in str(exc_info.value.detail) + finally: + global_mcp_server_manager.registry.clear() + + +@pytest.mark.asyncio +async def test_oauth_authorization_server_404_for_non_oauth2_server(): + """Discovery must not advertise a none-auth server as an OAuth authorization server.""" + try: + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + _build_oauth_authorization_server_response, + ) + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + global_mcp_server_manager.registry.clear() + server = _access_group_none_server() + global_mcp_server_manager.registry[server.server_id] = server + + mock_request = MagicMock() + mock_request.base_url = "https://litellm.example.com/" + mock_request.headers = {} + + try: + with pytest.raises(HTTPException) as exc_info: + _build_oauth_authorization_server_response( + request=mock_request, + mcp_server_name="access_group_server", + ) + assert exc_info.value.status_code == 404 + assert "not an OAuth authorization server" in str(exc_info.value.detail) + finally: + global_mcp_server_manager.registry.clear() + + +@pytest.mark.asyncio +async def test_oauth_protected_resource_passthrough_none_auth_not_404(): + """Regression guard for the protected-resource auth_type gate placement: a none-auth + server that opted into OAuth pass-through must still proxy upstream metadata, it must + NOT be 404'd. The gate has to sit after the pass-through branch.""" + try: + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + _build_oauth_protected_resource_response, + ) + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + from litellm.proxy._types import MCPTransport + from litellm.types.mcp import MCPAuth + from litellm.types.mcp_server.mcp_server_manager import MCPServer + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + global_mcp_server_manager.registry.clear() + passthrough_server = MCPServer( + server_id="passthrough_server", + name="passthrough_server", + server_name="passthrough_server", + alias="passthrough_server", + url="https://upstream.example.com/mcp", + transport=MCPTransport.http, + auth_type=MCPAuth.none, + oauth_passthrough=True, + extra_headers=["Authorization"], + ) + global_mcp_server_manager.registry[passthrough_server.server_id] = passthrough_server + + mock_request = MagicMock() + mock_request.base_url = "https://litellm.example.com/" + mock_request.headers = {} + + try: + with patch( + "litellm.proxy._experimental.mcp_server.discoverable_endpoints.fetch_upstream_oauth_protected_resource", + new=AsyncMock(return_value={"authorization_servers": ["https://upstream-idp.example.com"]}), + ): + response = await _build_oauth_protected_resource_response( + request=mock_request, + mcp_server_name="passthrough_server", + use_standard_pattern=False, + ) + assert response["authorization_servers"] == ["https://upstream-idp.example.com"] + assert response["resource"].endswith("/passthrough_server/mcp") + finally: + global_mcp_server_manager.registry.clear() + + +@pytest.mark.asyncio +async def test_oauth_protected_resource_404_for_unknown_server_name(): + """A discovery request for an unknown server name returns the same 404 as a non-oauth2 + server (not a 200 metadata doc with broken URLs), so the well-known paths cannot be used + to enumerate non-OAuth server names.""" + try: + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + _build_oauth_protected_resource_response, + ) + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + global_mcp_server_manager.registry.clear() + mock_request = MagicMock() + mock_request.base_url = "https://litellm.example.com/" + mock_request.headers = {} + + with pytest.raises(HTTPException) as exc_info: + await _build_oauth_protected_resource_response( + request=mock_request, + mcp_server_name="does_not_exist", + use_standard_pattern=True, + ) + assert exc_info.value.status_code == 404 + + +@pytest.mark.asyncio +async def test_oauth_authorization_server_404_for_unknown_server_name(): + """A named authorization-server discovery request for an unknown server returns 404, not a + 200 metadata document pointing at non-existent /{name}/authorize and /{name}/token.""" + try: + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + _build_oauth_authorization_server_response, + ) + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + except ImportError: + pytest.skip("MCP discoverable endpoints not available") + + global_mcp_server_manager.registry.clear() + mock_request = MagicMock() + mock_request.base_url = "https://litellm.example.com/" + mock_request.headers = {} + + with pytest.raises(HTTPException) as exc_info: + _build_oauth_authorization_server_response( + request=mock_request, + mcp_server_name="does_not_exist", + ) + assert exc_info.value.status_code == 404 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 e1ffcc58fce..39c4509c4d0 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 @@ -2068,6 +2068,7 @@ class TestTemporaryMCPSessionEndpoints: request = MagicMock() server = generate_mock_mcp_server_config_record(server_id="server-1") + server.auth_type = MCPAuth.oauth2 authorize_response = MagicMock() admin_auth = generate_mock_user_api_key_auth( user_role=LitellmUserRoles.PROXY_ADMIN, @@ -2110,6 +2111,91 @@ class TestTemporaryMCPSessionEndpoints: scope="scope1", ) + @pytest.mark.asyncio + async def test_mcp_authorize_rejects_non_oauth2_server(self): + """mcp_authorize must reject a none-auth server with an accurate 'does not use OAuth' + 400 before the client_id check, never delegating to authorize_with_server.""" + from litellm.proxy.management_endpoints.mcp_management_endpoints import ( + mcp_authorize, + ) + + server = generate_mock_mcp_server_config_record(server_id="none-server") + server.auth_type = MCPAuth.none + admin_auth = generate_mock_user_api_key_auth( + user_role=LitellmUserRoles.PROXY_ADMIN, + ) + + with ( + patch( + "litellm.proxy.management_endpoints.mcp_management_endpoints._get_cached_temporary_mcp_server_or_404", + return_value=server, + ), + patch( + "litellm.proxy.management_endpoints.mcp_management_endpoints.authorize_with_server", + AsyncMock(), + ) as authorize_mock, + ): + with pytest.raises(HTTPException) as exc_info: + await mcp_authorize( + request=MagicMock(), + server_id="none-server", + user_api_key_dict=admin_auth, + client_id=None, + redirect_uri="https://example.com/callback", + state="state123", + ) + + assert exc_info.value.status_code == 400 + detail_text = str(exc_info.value.detail) + assert "does not use OAuth" in detail_text + assert "missing_client_id" not in detail_text + authorize_mock.assert_not_awaited() + + @pytest.mark.asyncio + async def test_mcp_token_rejects_non_oauth2_server(self): + """mcp_token must reject a none-auth server with 'does not use OAuth' 400 before the + client_id check, never delegating to exchange_token_with_server.""" + from litellm.proxy.management_endpoints.mcp_management_endpoints import ( + mcp_token, + ) + + server = generate_mock_mcp_server_config_record(server_id="none-server") + server.auth_type = MCPAuth.none + admin_auth = generate_mock_user_api_key_auth( + user_role=LitellmUserRoles.PROXY_ADMIN, + ) + + with ( + patch( + "litellm.proxy.management_endpoints.mcp_management_endpoints._get_cached_temporary_mcp_server_or_404", + return_value=server, + ), + patch( + "litellm.proxy.management_endpoints.mcp_management_endpoints.exchange_token_with_server", + AsyncMock(), + ) as exchange_mock, + ): + with pytest.raises(HTTPException) as exc_info: + await mcp_token( + request=MagicMock(), + server_id="none-server", + user_api_key_dict=admin_auth, + grant_type="authorization_code", + code="code-123", + redirect_uri="https://example.com/callback", + client_id=None, + client_secret=None, + code_verifier="verifier", + refresh_token=None, + scope=None, + ) + + assert exc_info.value.status_code == 400 + detail_text = str(exc_info.value.detail) + assert "does not use OAuth" in detail_text + assert "missing_client_id" not in detail_text + exchange_mock.assert_not_awaited() + @pytest.mark.asyncio async def test_mcp_token_proxies_to_exchange_endpoint(self): from litellm.proxy.management_endpoints.mcp_management_endpoints import ( @@ -2118,6 +2204,7 @@ class TestTemporaryMCPSessionEndpoints: request = MagicMock() server = generate_mock_mcp_server_config_record(server_id="server-1") + server.auth_type = MCPAuth.oauth2 exchange_response = {"access_token": "token"} admin_auth = generate_mock_user_api_key_auth( user_role=LitellmUserRoles.PROXY_ADMIN, @@ -2170,6 +2257,7 @@ class TestTemporaryMCPSessionEndpoints: request = MagicMock() server = generate_mock_mcp_server_config_record(server_id="server-1") + server.auth_type = MCPAuth.oauth2 exchange_response = {"access_token": "new-token", "refresh_token": "new-rt"} admin_auth = generate_mock_user_api_key_auth( user_role=LitellmUserRoles.PROXY_ADMIN, @@ -2222,6 +2310,7 @@ class TestTemporaryMCPSessionEndpoints: request = MagicMock() server = generate_mock_mcp_server_config_record(server_id="server-1") + server.auth_type = MCPAuth.oauth2 register_response = {"client_id": "generated"} request_body = { "client_name": "LiteLLM", From bea8c9380bed3f12d6071ca9deae7f4eb80e5061 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Thu, 2 Jul 2026 14:09:39 -0700 Subject: [PATCH 012/168] refactor(ui): drive cache settings form from a typed frontend schema (#31939) * refactor(ui): drive cache settings form from a typed frontend schema The Cache Settings form was dynamically generated from field metadata shipped by the backend, and read its values back out of the DOM with document.querySelector. That loses type safety and makes client-side validation awkward, which is a poor fit for a form whose shape only changes when a developer edits code. Move the field definitions (name, label, type, default, help text, which redis type they apply to, section, and validation rules) into a typed frontend module and render them through antd Form with controlled state. The GET /cache/settings endpoint is still used to populate current values, and the save/test payload shape sent to POST /cache/settings and /cache/settings/test is unchanged. Per-field validation now lives on each field's antd rules, so an inline error can surface before and on submit; this is where the upcoming Redis URL validation will slot in. The backend's fields output in GET /cache/settings is no longer consumed by the UI, but is left in place since removing it is a separate backend change. * refactor(ui): validate list-field JSON inline so bad input blocks save sentinel_nodes and redis_startup_nodes had no validation rule, so malformed JSON passed validateFields, was caught while building the save payload, and the field was silently omitted; the user's cluster/sentinel config was discarded with no feedback. Add a jsonListRule (same shape as portRule) to both list fields so an invalid value surfaces inline and blocks save. * fix(ui): show valid-JSON examples for cache list fields and clarify the error The Startup Nodes and Sentinel Nodes help text showed Python-style single-quoted examples (e.g. [{'host': '127.0.0.1', 'port': '7001'}]), which the JSON validator correctly rejects, so pasting the example we display failed. Switch both examples to valid JSON with double quotes and change the parse-error message to "Must be a valid JSON array (use double quotes)" so the hint points at the fix. Also add a regression test asserting a numeric field (Database Index) is included in the save payload. * fix(ui): validate numeric cache fields as text so bad input blocks save Numeric fields (Database Index, TTL, Max Connections, Similarity Threshold) rendered as antd InputNumber, which silently coerces non-numeric input to empty. Because the fields are optional, an invalid entry like a full connection URL pasted into Database Index passed validation and was silently dropped from the save payload. Render numeric fields as text inputs with a validation rule (non-negative integer for Database Index and Max Connections, number for TTL and Similarity Threshold), mirroring how Port already works, so invalid input is preserved, flagged inline, and blocks submit instead of vanishing. The save payload still coerces these to real numbers. Adds a regression test for a non-numeric value entered into a numeric field. --- ui/litellm-dashboard/eslint-metrics.json | 4 +- ui/litellm-dashboard/eslint-suppressions.json | 5 - .../cache_settings/CacheFieldGroup.test.tsx | 138 --------- .../cache_settings/CacheFieldGroup.tsx | 47 ---- .../CacheFieldRenderer.test.tsx | 126 --------- .../cache_settings/CacheFieldRenderer.tsx | 149 ---------- .../cache_settings/CacheFieldSection.tsx | 42 +++ .../cache_settings/CacheFormField.tsx | 54 ++++ .../cache_settings/RedisTypeSelector.tsx | 2 +- .../cache_settings/cacheSettingsFields.ts | 261 ++++++++++++++++++ .../cache_settings/cacheSettingsUtils.test.ts | 86 ++++++ .../cache_settings/cacheSettingsUtils.ts | 169 +++++------- .../components/cache_settings/index.test.tsx | 147 ++++++++++ .../components/cache_settings/index.tsx | 246 ++++++++--------- 14 files changed, 780 insertions(+), 696 deletions(-) delete mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.test.tsx delete mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.tsx delete mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.test.tsx delete mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.tsx create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldSection.tsx create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFormField.tsx create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/cacheSettingsFields.ts create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/cacheSettingsUtils.test.ts create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/index.test.tsx diff --git a/ui/litellm-dashboard/eslint-metrics.json b/ui/litellm-dashboard/eslint-metrics.json index 92c5a991eb6..4475ce0e74b 100644 --- a/ui/litellm-dashboard/eslint-metrics.json +++ b/ui/litellm-dashboard/eslint-metrics.json @@ -1,5 +1,5 @@ { - "@typescript-eslint/no-explicit-any": 2013, + "@typescript-eslint/no-explicit-any": 1991, "complexity": 126, - "max-depth": 61 + "max-depth": 59 } diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index 7ea13deb934..44c0d8f55ef 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -834,11 +834,6 @@ "count": 1 } }, - "src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "src/app/(dashboard)/caching/components/cache_settings/RedisTypeSelector.tsx": { "no-restricted-imports": { "count": 1 diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.test.tsx deleted file mode 100644 index 6eba0718948..00000000000 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.test.tsx +++ /dev/null @@ -1,138 +0,0 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; -import { render, screen, cleanup } from "@testing-library/react"; -import CacheFieldGroup from "./CacheFieldGroup"; - -describe("CacheFieldGroup", () => { - vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ - default: () => ({ - token: "mock-token", - accessToken: "mock-access-token", - userId: "mock-user-id", - userEmail: "test@example.com", - userRole: "Admin", - premiumUser: false, - disabledPersonalKeyCreation: false, - showSSOBanner: false, - }), - })); - - beforeEach(() => { - cleanup(); - }); - - it("should filter and render fields based on redisType", () => { - /** - * Tests that CacheFieldGroup filters fields based on redis_type and redisType prop. - * This is the core functionality that shows/hides fields based on Redis deployment type. - */ - const fields = [ - { - field_name: "host", - field_type: "String", - ui_field_name: "Host", - redis_type: null, // Applies to all types - }, - { - field_name: "redis_startup_nodes", - field_type: "List", - ui_field_name: "Startup Nodes", - redis_type: "cluster", // Only for cluster - }, - { - field_name: "sentinel_nodes", - field_type: "List", - ui_field_name: "Sentinel Nodes", - redis_type: "sentinel", // Only for sentinel - }, - ]; - - const cacheSettings = { - host: "localhost", - redis_startup_nodes: [], - }; - - // Test with cluster type - should show host and redis_startup_nodes - const { rerender } = render( - , - ); - - expect(screen.getByText("Cluster Settings")).toBeInTheDocument(); - expect(screen.getAllByText("Host")).toHaveLength(1); - expect(screen.getByText("Startup Nodes")).toBeInTheDocument(); - expect(screen.queryByText("Sentinel Nodes")).not.toBeInTheDocument(); - - // Test with sentinel type - should show host and sentinel_nodes - rerender( - , - ); - - expect(screen.getByText("Sentinel Settings")).toBeInTheDocument(); - expect(screen.getAllByText("Host")).toHaveLength(1); - expect(screen.getByText("Sentinel Nodes")).toBeInTheDocument(); - expect(screen.queryByText("Startup Nodes")).not.toBeInTheDocument(); - - // Test with node type - should only show host - rerender(); - - expect(screen.getByText("Node Settings")).toBeInTheDocument(); - expect(screen.getAllByText("Host")).toHaveLength(1); - expect(screen.queryByText("Startup Nodes")).not.toBeInTheDocument(); - expect(screen.queryByText("Sentinel Nodes")).not.toBeInTheDocument(); - }); - - it("should return null when no fields are visible", () => { - /** - * Tests that CacheFieldGroup returns null when no fields match the redisType. - * This prevents rendering empty sections in the UI. - */ - const fields = [ - { - field_name: "redis_startup_nodes", - field_type: "List", - ui_field_name: "Startup Nodes", - redis_type: "cluster", // Only for cluster - }, - ]; - - const cacheSettings = {}; - - const { container } = render( - , - ); - - // Component should return null, so container should be empty - expect(container.firstChild).toBeNull(); - }); - - it("should use field_default when currentValue is not available", () => { - /** - * Tests that CacheFieldGroup falls back to field_default when currentValue is missing. - * This ensures fields display default values when cache settings are not set. - */ - const fields = [ - { - field_name: "port", - field_type: "Integer", - ui_field_name: "Port", - field_default: 6379, - redis_type: null, - }, - ]; - - const cacheSettings = {}; // No port value set - - render( - , - ); - - const input = screen.getByRole("spinbutton", { name: "" }); - expect(input).toBeInTheDocument(); - expect(input).toHaveAttribute("name", "port"); - expect(input).toHaveValue(6379); - }); -}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.tsx deleted file mode 100644 index 21eb0199853..00000000000 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.tsx +++ /dev/null @@ -1,47 +0,0 @@ -import React from "react"; -import CacheFieldRenderer from "./CacheFieldRenderer"; - -interface CacheFieldGroupProps { - title: string; - fields: any[]; - cacheSettings: { [key: string]: any }; - redisType: string; - gridCols?: string; -} - -const CacheFieldGroup: React.FC = ({ - title, - fields, - cacheSettings, - redisType, - gridCols = "grid-cols-1 gap-6 sm:grid-cols-2", -}) => { - const shouldShowField = (field: any): boolean => { - // Show field if it applies to all types (redis_type is null/undefined) or to current selected type - if (field.redis_type === null || field.redis_type === undefined) { - return true; - } - - return field.redis_type === redisType; - }; - - const visibleFields = fields.filter(shouldShowField); - - if (visibleFields.length === 0) { - return null; - } - - return ( -
-

{title}

-
- {visibleFields.map((field) => { - const currentValue = cacheSettings[field.field_name] ?? field.field_default ?? ""; - return ; - })} -
-
- ); -}; - -export default CacheFieldGroup; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.test.tsx deleted file mode 100644 index 3cfacac7fda..00000000000 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.test.tsx +++ /dev/null @@ -1,126 +0,0 @@ -import { describe, it, expect, vi } from "vitest"; -import { render, screen } from "@testing-library/react"; -import CacheFieldRenderer from "./CacheFieldRenderer"; - -// Mock the useAuthorized hook to avoid Next.js router dependency -vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ - default: () => ({ - token: "mock-token", - accessToken: "mock-access-token", - userId: "mock-user-id", - userEmail: "test@example.com", - userRole: "Admin", - premiumUser: false, - disabledPersonalKeyCreation: false, - showSSOBanner: false, - }), -})); - -describe("CacheFieldRenderer", () => { - it("should render a checkbox for Boolean field type", () => { - /** - * Tests that Boolean fields render as checkboxes with proper defaultChecked value. - * This is the core functionality for boolean cache settings. - */ - const field = { - field_name: "ssl", - field_type: "Boolean", - ui_field_name: "Enable SSL", - field_description: "Enable SSL encryption", - }; - - render(); - - const checkbox = screen.getByRole("checkbox", { name: "" }); - expect(checkbox).toBeInTheDocument(); - expect(checkbox).toBeChecked(); - expect(screen.getByText("Enable SSL")).toBeInTheDocument(); - expect(screen.getByText("Enable SSL encryption")).toBeInTheDocument(); - }); - - it("should render a textarea for List field type", () => { - /** - * Tests that List fields render as textareas with JSON stringified values. - * This handles array/list cache settings like redis_startup_nodes. - */ - const field = { - field_name: "redis_startup_nodes", - field_type: "List", - ui_field_name: "Redis Startup Nodes", - field_description: "List of Redis cluster nodes", - }; - - const currentValue = [ - { host: "localhost", port: 6379 }, - { host: "localhost", port: 6380 }, - ]; - - render(); - - const textarea = screen.getByRole("textbox"); - expect(textarea).toBeInTheDocument(); - expect(textarea.tagName).toBe("TEXTAREA"); - expect(textarea).toHaveValue(JSON.stringify(currentValue, null, 2)); - expect(screen.getByText("Redis Startup Nodes")).toBeInTheDocument(); - }); - - it("should render a password input for password field", () => { - /** - * Tests that password fields render as password inputs. - * This ensures sensitive data is masked in the UI. - */ - const field = { - field_name: "password", - field_type: "String", - ui_field_name: "Password", - field_description: "Redis password", - }; - - render(); - - const input = screen.getByPlaceholderText("Redis password"); - expect(input).toBeInTheDocument(); - expect(input).toHaveAttribute("type", "password"); - expect(input).toHaveValue("secret123"); - }); - - it("should render a number input for Integer field type", () => { - /** - * Tests that Integer fields render as number inputs. - * This ensures proper validation for numeric cache settings. - */ - const field = { - field_name: "port", - field_type: "Integer", - ui_field_name: "Port", - field_description: "Redis port number", - }; - - render(); - - const input = screen.getByPlaceholderText("Redis port number"); - expect(input).toBeInTheDocument(); - expect(input).toHaveAttribute("type", "number"); - expect(input).toHaveValue(6379); - }); - - it("should render a text input for String field type", () => { - /** - * Tests that String fields render as text inputs. - * This is the default rendering for text-based cache settings. - */ - const field = { - field_name: "host", - field_type: "String", - ui_field_name: "Host", - field_description: "Redis host address", - }; - - render(); - - const input = screen.getByPlaceholderText("Redis host address"); - expect(input).toBeInTheDocument(); - expect(input).toHaveAttribute("type", "text"); - expect(input).toHaveValue("localhost"); - }); -}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.tsx deleted file mode 100644 index 27d9fc57200..00000000000 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.tsx +++ /dev/null @@ -1,149 +0,0 @@ -"use client"; - -import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; -import { NumberInput, TextInput } from "@tremor/react"; -import { Select } from "antd"; -import React, { useEffect, useState } from "react"; -import { fetchAvailableModels, ModelGroup } from "@/components/llm_calls/fetch_models"; -import NumericalInput from "@/components/shared/numerical_input"; - -interface CacheFieldRendererProps { - field: any; - currentValue: any; -} - -const CacheFieldRenderer: React.FC = ({ field, currentValue }) => { - const [modelInfo, setModelInfo] = useState([]); - const [selectedModel, setSelectedModel] = useState(currentValue || ""); - const { accessToken } = useAuthorized(); - - useEffect(() => { - if (!accessToken) return; - - const loadModels = async () => { - try { - const uniqueModels = await fetchAvailableModels(accessToken); - console.log("Fetched models for selector:", uniqueModels); - - if (uniqueModels.length > 0) { - setModelInfo(uniqueModels); - } - } catch (error) { - console.error("Error fetching model info:", error); - } - }; - - loadModels(); - }, [accessToken]); - - if (field.field_type === "Boolean") { - return ( -
- -
- - {field.field_description} -
-
- ); - } - - if (field.field_type === "Integer" || field.field_type === "Float") { - return ( -
- - -

{field.field_description}

-
- ); - } - - if (field.field_type === "List") { - return ( -
- -