diff --git a/litellm/proxy/common_request_processing.py b/litellm/proxy/common_request_processing.py index f5a50d1697a4..9ccaca760c73 100644 --- a/litellm/proxy/common_request_processing.py +++ b/litellm/proxy/common_request_processing.py @@ -1181,15 +1181,21 @@ async def common_processing_pre_call_logic( processing_start_time = time.time() queue_time_seconds = processing_start_time - arrival_time - # Store queue time in metadata after add_litellm_data_to_request to ensure it's preserved + # Initialize litellm_metadata for routes that use it (Responses, batches, files, etc.). + # This ensures rate limiters and other hooks stash proxy-internal state in the + # correct bucket instead of creating a provider-visible 'metadata' field. + # This must happen BEFORE pre-call processing (rate limits, guardrails, etc.) + # that may stash values into metadata. + from litellm.proxy.litellm_pre_call_utils import _get_metadata_variable_name + + _metadata_variable_name = _get_metadata_variable_name(request) + if _metadata_variable_name not in self.data: + self.data[_metadata_variable_name] = {} + if not isinstance(self.data[_metadata_variable_name], dict): + self.data[_metadata_variable_name] = {} + + # Store queue time in metadata if available if queue_time_seconds is not None: - from litellm.proxy.litellm_pre_call_utils import _get_metadata_variable_name - - _metadata_variable_name = _get_metadata_variable_name(request) - if _metadata_variable_name not in self.data: - self.data[_metadata_variable_name] = {} - if not isinstance(self.data[_metadata_variable_name], dict): - self.data[_metadata_variable_name] = {} self.data[_metadata_variable_name]["queue_time_seconds"] = queue_time_seconds self.data["model"] = ( diff --git a/tests/proxy_unit_tests/test_responses_metadata_leak_gh35197.py b/tests/proxy_unit_tests/test_responses_metadata_leak_gh35197.py new file mode 100644 index 000000000000..5e360f5076af --- /dev/null +++ b/tests/proxy_unit_tests/test_responses_metadata_leak_gh35197.py @@ -0,0 +1,98 @@ +""" +Regression test for GitHub issue #35197: /v1/responses leaks rate-limiter metadata to upstream. + +When a request to /v1/responses has rate limits enabled, the rate limiter should stash +internal state in litellm_metadata (proxy-internal), not metadata (provider-visible). +""" + +import os +import sys +from unittest.mock import MagicMock + +import pytest + +sys.path.insert(0, os.path.abspath("../..")) + + +class TestResponsesMetadataLeak: + """Test that /v1/responses does not leak rate-limiter metadata to upstream.""" + + def test_responses_route_uses_litellm_metadata(self): + """ + Verify that _get_metadata_variable_name correctly identifies + /v1/responses as a route that should use litellm_metadata. + """ + from litellm.proxy.litellm_pre_call_utils import _get_metadata_variable_name + + # Mock request for /v1/responses + mock_request = MagicMock() + mock_request.url.path = "/v1/responses" + + # This is what determines which metadata bucket to use + metadata_var_name = _get_metadata_variable_name(mock_request) + + assert ( + metadata_var_name == "litellm_metadata" + ), f"Expected 'litellm_metadata' for /v1/responses, got '{metadata_var_name}'" + + def test_batch_routes_use_litellm_metadata(self): + """ + Verify that /v1/batches also uses litellm_metadata. + """ + from litellm.proxy.litellm_pre_call_utils import _get_metadata_variable_name + + mock_request = MagicMock() + mock_request.url.path = "/v1/batches" + + metadata_var_name = _get_metadata_variable_name(mock_request) + + assert ( + metadata_var_name == "litellm_metadata" + ), f"Expected 'litellm_metadata' for /v1/batches, got '{metadata_var_name}'" + + def test_chat_completions_uses_metadata(self): + """ + Verify that /v1/chat/completions uses 'metadata' (backwards compatibility). + """ + from litellm.proxy.litellm_pre_call_utils import _get_metadata_variable_name + + mock_request = MagicMock() + mock_request.url.path = "/v1/chat/completions" + + metadata_var_name = _get_metadata_variable_name(mock_request) + + assert ( + metadata_var_name == "metadata" + ), f"Expected 'metadata' for /v1/chat/completions, got '{metadata_var_name}'" + + def test_litellm_metadata_bucket_selection(self): + """ + Test the core logic: get_or_create_metadata_bucket should use + the correct bucket name based on whether litellm_metadata exists. + """ + from litellm.litellm_core_utils.core_helpers import get_or_create_metadata_bucket + + # Test 1: When litellm_metadata is already in the data, use it + data_with_litellm_metadata = {"litellm_metadata": {"existing": "value"}} + bucket_name, bucket = get_or_create_metadata_bucket(data_with_litellm_metadata) + + assert bucket_name == "litellm_metadata" + assert bucket["existing"] == "value" + + # Test 2: When litellm_metadata doesn't exist, defaults to metadata + # (This is the old behavior that caused the bug) + data_without_litellm_metadata = {} + bucket_name, bucket = get_or_create_metadata_bucket(data_without_litellm_metadata) + + assert bucket_name == "metadata" + + # Test 3: The fix: if we pre-create litellm_metadata (as our fix does), + # then get_or_create_metadata_bucket will use it instead of metadata + data_prefilled = {"litellm_metadata": {}} + bucket_name, bucket = get_or_create_metadata_bucket(data_prefilled) + + assert bucket_name == "litellm_metadata" + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/tests/proxy_unit_tests/test_responses_rate_limiter_metadata_gh35197.py b/tests/proxy_unit_tests/test_responses_rate_limiter_metadata_gh35197.py new file mode 100644 index 000000000000..9d3ad863930a --- /dev/null +++ b/tests/proxy_unit_tests/test_responses_rate_limiter_metadata_gh35197.py @@ -0,0 +1,126 @@ +""" +End-to-end test for GitHub issue #35197: verify rate limiter uses correct metadata bucket. + +This test simulates what happens when: +1. A /v1/responses request comes in +2. pre-call processing initializes litellm_metadata +3. Rate limiter stashes rate-limit response +4. The stashed value ends up in litellm_metadata, NOT the provider-visible metadata +""" + +import sys +import os +from unittest.mock import MagicMock + +import pytest + +sys.path.insert(0, os.path.abspath("../..")) + + +def test_rate_limiter_uses_correct_metadata_bucket(): + """ + Simulate the rate limiter's metadata stashing behavior. + + Before the fix: + - litellm_metadata doesn't exist in data + - get_or_create_metadata_bucket() defaults to 'metadata' + - Rate limit response gets written to data["metadata"] + - This leaked metadata is sent to the provider + + After the fix: + - pre-call logic ensures litellm_metadata exists + - get_or_create_metadata_bucket() uses litellm_metadata + - Rate limit response gets written to data["litellm_metadata"] + - Metadata stays internal, not sent to provider + """ + from litellm.litellm_core_utils.core_helpers import get_or_create_metadata_bucket + + # Simulate the state AFTER our fix (litellm_metadata is pre-created) + data = { + "model": "test-model", + "input": "test input", + "litellm_metadata": {}, # This is initialized by our fix + } + + # Simulate what the rate limiter does + bucket_name, metadata_bucket = get_or_create_metadata_bucket(data) + + # Verify it uses litellm_metadata (internal), not metadata (provider-visible) + assert bucket_name == "litellm_metadata", f"Expected litellm_metadata, got {bucket_name}" + + # Simulate stashing the rate limit response (what the rate limiter does) + metadata_bucket["_litellm_proxy_rate_limit_response"] = { + "overall_code": "OK", + "statuses": [{"descriptor_key": "api_key", "rate_limit_type": "requests"}], + } + + # Verify it was written to the internal bucket, not creating a provider-visible one + assert "litellm_metadata" in data + assert "_litellm_proxy_rate_limit_response" in data["litellm_metadata"] + assert ( + "metadata" not in data + ), "Provider-visible metadata should NOT be created by rate limiter" + + +def test_rate_limiter_metadata_leak_without_fix(): + """ + This test demonstrates the bug: without litellm_metadata pre-initialized, + the rate limiter would create a provider-visible metadata field. + """ + from litellm.litellm_core_utils.core_helpers import get_or_create_metadata_bucket + + # Simulate the state WITHOUT our fix (no pre-created litellm_metadata) + data = { + "model": "test-model", + "input": "test input", + # litellm_metadata is NOT initialized - this was the bug + } + + # When rate limiter calls get_or_create_metadata_bucket + bucket_name, metadata_bucket = get_or_create_metadata_bucket(data) + + # Without the fix, it defaults to 'metadata' (not litellm_metadata) + # This is the OLD BUGGY BEHAVIOR - for demonstration only + assert bucket_name == "metadata", "BUG: defaults to 'metadata' without pre-initialization" + + # Simulate stashing rate limit response in the wrong bucket + metadata_bucket["_litellm_proxy_rate_limit_response"] = { + "overall_code": "OK", + "statuses": [{"descriptor_key": "api_key", "rate_limit_type": "requests"}], + } + + # The bug: metadata field exists and will be sent to provider + assert "metadata" in data, "BUG: metadata field was created" + assert "_litellm_proxy_rate_limit_response" in data["metadata"] + + # This metadata would be sent to the upstream provider, + # causing OpenAI-compatible backends to reject with "Unsupported parameter: metadata" + + +def test_responses_vs_chat_completions_metadata_usage(): + """ + Verify different routes use different metadata buckets as intended. + """ + from litellm.proxy.litellm_pre_call_utils import _get_metadata_variable_name + + # Responses API should use litellm_metadata + mock_responses_request = MagicMock() + mock_responses_request.url.path = "/v1/responses" + responses_bucket = _get_metadata_variable_name(mock_responses_request) + assert responses_bucket == "litellm_metadata", "Responses should use litellm_metadata" + + # Chat completions can use either (defaults to metadata for backwards compat) + mock_chat_request = MagicMock() + mock_chat_request.url.path = "/v1/chat/completions" + chat_bucket = _get_metadata_variable_name(mock_chat_request) + assert chat_bucket == "metadata", "Chat completions should use metadata" + + # Batches should use litellm_metadata + mock_batch_request = MagicMock() + mock_batch_request.url.path = "/v1/batches" + batch_bucket = _get_metadata_variable_name(mock_batch_request) + assert batch_bucket == "litellm_metadata", "Batches should use litellm_metadata" + + +if __name__ == "__main__": + pytest.main([__file__, "-v"])