From d0601692b8dc5fff8f31714372f65c584a2abf20 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Thu, 16 Apr 2026 21:48:36 +0000 Subject: [PATCH] fix(proxy): strip user_api_key_metadata injection slots from user input Expand the pre-call metadata strip to also remove user_api_key_metadata and user_api_key_team_metadata. The proxy writes these fields into data[_metadata_variable_name] with admin-authoritative values, but only into that one metadata key; the caller's value in the OTHER metadata key (metadata vs litellm_metadata) would otherwise persist and be picked up by _get_admin_metadata, letting a caller supply their own 'admin' config to disable guardrails, opt out of global policies, etc. VERIA-28 (High): Security Policy and Guardrail Bypass via Unsanitized Request Metadata. Add regression test at the proxy boundary verifying the strip, and extend the guardrail test to cover the post-strip admin-config path. --- litellm/proxy/litellm_pre_call_utils.py | 8 +- .../integrations/test_custom_guardrail.py | 16 ++ .../proxy/test_litellm_pre_call_utils.py | 225 +++++++++++++----- 3 files changed, 188 insertions(+), 61 deletions(-) diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index 95e1cbed44e..0b2c20b25a5 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -977,11 +977,17 @@ async def add_litellm_data_to_request( # noqa: PLR0915 "Setting client-provided x-api-key as api_key parameter (will override deployment key)" ) - # Strip internal pipeline state from user input + # Strip internal pipeline state and admin-injection slots from user input. + # The proxy writes user_api_key_metadata / user_api_key_team_metadata + # into data[_metadata_variable_name] below; if a caller pre-populates + # either key on the OTHER metadata field, _get_admin_metadata lookups + # would treat the caller's payload as admin-configured. for _meta_key in ("metadata", "litellm_metadata"): _user_meta = data.get(_meta_key) if isinstance(_user_meta, dict): _user_meta.pop("_pipeline_managed_guardrails", None) + _user_meta.pop("user_api_key_metadata", None) + _user_meta.pop("user_api_key_team_metadata", None) ########################################################## # Init - Proxy Server Request diff --git a/tests/test_litellm/integrations/test_custom_guardrail.py b/tests/test_litellm/integrations/test_custom_guardrail.py index b8eb0857364..0c904e9df50 100644 --- a/tests/test_litellm/integrations/test_custom_guardrail.py +++ b/tests/test_litellm/integrations/test_custom_guardrail.py @@ -241,6 +241,22 @@ class TestCustomGuardrailShouldRunGuardrail: result is False ), "Admin config in metadata must not be shadowed by user-supplied litellm_metadata" + # Test 6: After the pre-call strip runs, user-injected + # user_api_key_metadata in the non-authoritative metadata key is gone. + # _get_admin_metadata must then surface admin config unchanged. + data_post_strip = { + "model": "gpt-3.5-turbo", + "messages": [{"role": "user", "content": "test"}], + "metadata": {"user_api_key_metadata": {"disable_global_guardrails": True}}, + "litellm_metadata": {}, # post-strip: attacker payload removed + } + result = custom_guardrail.should_run_guardrail( + data=data_post_strip, event_type=GuardrailEventHooks.pre_call + ) + assert ( + result is False + ), "Admin config in metadata must be respected when other metadata key is empty" + def test_should_run_guardrail_with_opted_out_global_guardrails(self): """Test that per-guardrail opt-out only works from admin metadata""" from litellm.types.guardrails import GuardrailEventHooks diff --git a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py index cf7e71b14d4..e5adc673990 100644 --- a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py +++ b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py @@ -207,6 +207,79 @@ async def test_add_litellm_data_to_request_parses_string_metadata(): assert updated_data["metadata"]["generation_name"] == "gen123" +@pytest.mark.asyncio +async def test_add_litellm_data_to_request_strips_admin_injection_slots(): + """User-supplied user_api_key_metadata / user_api_key_team_metadata / + _pipeline_managed_guardrails must be stripped from both metadata keys + before the proxy writes its own admin-populated values. Otherwise a + caller can shadow admin config via the non-`_metadata_variable_name` + metadata key (e.g. litellm_metadata while the proxy writes to metadata). + """ + from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request + + request_mock = MagicMock(spec=Request) + request_mock.url.path = "/v1/chat/completions" + request_mock.url = MagicMock() + request_mock.url.__str__.return_value = "http://localhost/v1/chat/completions" + request_mock.method = "POST" + request_mock.query_params = {} + request_mock.headers = {"Content-Type": "application/json"} + request_mock.client = MagicMock() + request_mock.client.host = "127.0.0.1" + + # Caller tries to inject admin config into BOTH metadata keys + attacker_admin_payload = {"disable_global_guardrails": True} + data = { + "model": "gpt-3.5-turbo", + "metadata": { + "user_api_key_metadata": attacker_admin_payload, + "user_api_key_team_metadata": attacker_admin_payload, + "_pipeline_managed_guardrails": ["evaded"], + }, + "litellm_metadata": { + "user_api_key_metadata": attacker_admin_payload, + "user_api_key_team_metadata": attacker_admin_payload, + "_pipeline_managed_guardrails": ["evaded"], + }, + } + + real_admin_metadata = {"admin_flag": "from_proxy"} + user_api_key_dict = UserAPIKeyAuth( + api_key="hashed-key", + metadata=real_admin_metadata, + team_metadata=real_admin_metadata, + spend=0.0, + max_budget=100.0, + model_max_budget={}, + team_spend=0.0, + team_max_budget=200.0, + ) + + updated = await add_litellm_data_to_request( + data=data, + request=request_mock, + user_api_key_dict=user_api_key_dict, + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + + # The key that matches `_metadata_variable_name` gets proxy-populated + # with the real admin payload; the OTHER key must not retain the + # attacker's injection. + populated = updated["metadata"] + assert populated["user_api_key_metadata"] == real_admin_metadata + assert populated["user_api_key_team_metadata"] == real_admin_metadata + assert "_pipeline_managed_guardrails" not in populated or populated[ + "_pipeline_managed_guardrails" + ] != ["evaded"] + + other = updated.get("litellm_metadata") or {} + assert other.get("user_api_key_metadata") in (None, {}, real_admin_metadata) + assert other.get("user_api_key_team_metadata") in (None, {}, real_admin_metadata) + assert "_pipeline_managed_guardrails" not in other + + @pytest.mark.asyncio async def test_add_litellm_data_to_request_user_spend_and_budget(): from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request @@ -221,7 +294,10 @@ async def test_add_litellm_data_to_request_user_spend_and_budget(): request_mock.client = MagicMock() request_mock.client.host = "127.0.0.1" - data = {"model": "gpt-3.5-turbo", "messages": [{"role": "user", "content": "Hello"}]} + data = { + "model": "gpt-3.5-turbo", + "messages": [{"role": "user", "content": "Hello"}], + } user_api_key_dict = UserAPIKeyAuth( api_key="hashed-key", @@ -1023,6 +1099,7 @@ def test_add_headers_to_llm_call_by_model_group_existing_headers_in_data(): # Restore original model_group_settings litellm.model_group_settings = original_model_group_settings + import json import time from typing import Optional @@ -1040,15 +1117,16 @@ class TestCustomLogger(CustomLogger): def __init__(self): self.standard_logging_object: Optional[StandardLoggingPayload] = None super().__init__() - + async def async_log_success_event(self, kwargs, response_obj, start_time, end_time): print(f"SUCCESS CALLBACK CALLED! kwargs keys: {list(kwargs.keys())}") self.standard_logging_object = kwargs.get("standard_logging_object") print(f"Captured standard_logging_object: {self.standard_logging_object}") - + async def async_log_failure_event(self, kwargs, response_obj, start_time, end_time): print(f"FAILURE CALLBACK CALLED! kwargs keys: {list(kwargs.keys())}") + @pytest.mark.asyncio async def test_add_litellm_metadata_from_request_headers(): """ @@ -1065,8 +1143,16 @@ async def test_add_litellm_metadata_from_request_headers(): try: # Prepare test data (ensure no streaming, add mock_response and api_key to route to litellm.acompletion) - headers = {"x-litellm-spend-logs-metadata": '{"user_id": "12345", "project_id": "proj_abc", "request_type": "chat_completion", "timestamp": "2025-09-02T10:30:00Z"}'} - data = {"model": "gpt-4", "messages": [{"role": "user", "content": "Hello"}], "stream": False, "mock_response": "Hi", "api_key": "fake-key"} + headers = { + "x-litellm-spend-logs-metadata": '{"user_id": "12345", "project_id": "proj_abc", "request_type": "chat_completion", "timestamp": "2025-09-02T10:30:00Z"}' + } + data = { + "model": "gpt-4", + "messages": [{"role": "user", "content": "Hello"}], + "stream": False, + "mock_response": "Hi", + "api_key": "fake-key", + } # Create mock request with headers mock_request = MagicMock(spec=Request) @@ -1078,9 +1164,7 @@ async def test_add_litellm_metadata_from_request_headers(): # Create mock user API key dict mock_user_api_key_dict = UserAPIKeyAuth( - api_key="test-key", - user_id="test-user", - org_id="test-org" + api_key="test-key", user_id="test-user", org_id="test-org" ) # Create mock proxy logging object @@ -1095,7 +1179,7 @@ async def test_add_litellm_metadata_from_request_headers(): async def mock_post_call_success_hook(*args, **kwargs): # Return the response unchanged - return kwargs.get('response', args[2] if len(args) > 2 else None) + return kwargs.get("response", args[2] if len(args) > 2 else None) mock_proxy_logging_obj.during_call_hook = mock_during_call_hook mock_proxy_logging_obj.pre_call_hook = mock_pre_call_hook @@ -1108,10 +1192,15 @@ async def test_add_litellm_metadata_from_request_headers(): general_settings = {} # Create mock select_data_generator with correct signature - def mock_select_data_generator(response=None, user_api_key_dict=None, request_data=None): + def mock_select_data_generator( + response=None, user_api_key_dict=None, request_data=None + ): async def mock_generator(): - yield "data: " + json.dumps({"choices": [{"delta": {"content": "Hello"}}]}) + "\n\n" + yield "data: " + json.dumps( + {"choices": [{"delta": {"content": "Hello"}}]} + ) + "\n\n" yield "data: [DONE]\n\n" + return mock_generator() # Create the processor @@ -1129,22 +1218,28 @@ async def test_add_litellm_metadata_from_request_headers(): select_data_generator=mock_select_data_generator, llm_router=None, model="gpt-4", - is_streaming_request=False + is_streaming_request=False, ) # Sleep for 3 seconds to allow logging to complete await asyncio.sleep(3) # Check if standard_logging_object was set - assert test_logger.standard_logging_object is not None, "standard_logging_object should be populated after LLM request" + assert ( + test_logger.standard_logging_object is not None + ), "standard_logging_object should be populated after LLM request" # Verify the logging object contains expected metadata standard_logging_obj = test_logger.standard_logging_object - print(f"Standard logging object captured: {json.dumps(standard_logging_obj, indent=4, default=str)}") + print( + f"Standard logging object captured: {json.dumps(standard_logging_obj, indent=4, default=str)}" + ) SPEND_LOGS_METADATA = standard_logging_obj["metadata"]["spend_logs_metadata"] - assert SPEND_LOGS_METADATA == dict(json.loads(headers["x-litellm-spend-logs-metadata"])), "spend_logs_metadata should be the same as the headers" + assert SPEND_LOGS_METADATA == dict( + json.loads(headers["x-litellm-spend-logs-metadata"]) + ), "spend_logs_metadata should be the same as the headers" finally: litellm.callbacks = original_callbacks @@ -1197,7 +1292,9 @@ def test_get_internal_user_header_from_mapping_returns_expected_header(): {"header_name": "X-OpenWebUI-User-Email", "litellm_user_role": "customer"}, ] - header_name = LiteLLMProxyRequestSetup.get_internal_user_header_from_mapping(mappings) + header_name = LiteLLMProxyRequestSetup.get_internal_user_header_from_mapping( + mappings + ) assert header_name == "X-OpenWebUI-User-Id" @@ -1205,7 +1302,9 @@ def test_get_internal_user_header_from_mapping_none_when_absent(): mappings = [ {"header_name": "X-OpenWebUI-User-Email", "litellm_user_role": "customer"} ] - header_name = LiteLLMProxyRequestSetup.get_internal_user_header_from_mapping(mappings) + header_name = LiteLLMProxyRequestSetup.get_internal_user_header_from_mapping( + mappings + ) assert header_name is None single = {"header_name": "X-Only-Customer", "litellm_user_role": "customer"} @@ -1218,7 +1317,10 @@ def test_add_internal_user_from_user_mapping_sets_user_id_when_header_present(): headers = {"X-OpenWebUI-User-Id": "internal-user-123"} general_settings = { "user_header_mappings": [ - {"header_name": "X-OpenWebUI-User-Id", "litellm_user_role": "internal_user"}, + { + "header_name": "X-OpenWebUI-User-Id", + "litellm_user_role": "internal_user", + }, {"header_name": "X-OpenWebUI-User-Email", "litellm_user_role": "customer"}, ] } @@ -1312,7 +1414,7 @@ async def test_team_guardrails_append_to_key_guardrails(): metadata = updated_data.get("metadata", {}) guardrails = metadata.get("guardrails", []) - + assert "key-guardrail-1" in guardrails assert "key-guardrail-2" in guardrails assert "team-guardrail-1" in guardrails @@ -1341,7 +1443,7 @@ async def test_request_guardrails_do_not_override_key_guardrails(): metadata={"guardrails": ["key-guardrail-1"]}, team_metadata={}, ) - + # Test case: Request with empty guardrails should not result in empty guardrails data_with_empty = { "model": "gpt-3.5-turbo", @@ -1361,7 +1463,7 @@ async def test_request_guardrails_do_not_override_key_guardrails(): _metadata = updated_data_empty.get("metadata", {}) requested_guardrails = _metadata.get("guardrails", []) - + assert "guardrails" not in updated_data_empty assert "key-guardrail-1" in requested_guardrails assert len(requested_guardrails) == 1 @@ -1476,7 +1578,10 @@ def test_update_model_if_key_alias_exists(): assert data["model"] == "xai/grok-4-fast-non-reasoning" # Test case 2: Key alias doesn't exist - data = {"model": "unknown-model", "messages": [{"role": "user", "content": "Hello"}]} + data = { + "model": "unknown-model", + "messages": [{"role": "user", "content": "Hello"}], + } user_api_key_dict = UserAPIKeyAuth( api_key="test-key", aliases={"modelAlias": "xai/grok-4-fast-non-reasoning"}, @@ -1594,16 +1699,22 @@ async def test_embedding_header_forwarding_with_model_group(): # Verify that only x- prefixed headers (except x-stainless) were forwarded forwarded_headers = updated_data["headers"] - assert "X-Custom-Header" in forwarded_headers, "X-Custom-Header should be forwarded" + assert ( + "X-Custom-Header" in forwarded_headers + ), "X-Custom-Header should be forwarded" assert forwarded_headers["X-Custom-Header"] == "custom-value" assert "X-Request-ID" in forwarded_headers, "X-Request-ID should be forwarded" assert forwarded_headers["X-Request-ID"] == "test-request-123" # Verify that authorization header was NOT forwarded (sensitive header) - assert "Authorization" not in forwarded_headers, "Authorization header should not be forwarded" + assert ( + "Authorization" not in forwarded_headers + ), "Authorization header should not be forwarded" # Verify that Content-Type was NOT forwarded (doesn't start with x-) - assert "Content-Type" not in forwarded_headers, "Content-Type should not be forwarded" + assert ( + "Content-Type" not in forwarded_headers + ), "Content-Type should not be forwarded" # Verify original data fields are preserved assert updated_data["model"] == "local-openai/text-embedding-3-small" @@ -1659,8 +1770,9 @@ async def test_embedding_header_forwarding_without_model_group_config(): ) # Verify that headers were NOT added since model is not in forward list - assert "headers" not in updated_data or updated_data.get("headers") is None, \ - "Headers should not be forwarded for models not in forward_client_headers_to_llm_api list" + assert ( + "headers" not in updated_data or updated_data.get("headers") is None + ), "Headers should not be forwarded for models not in forward_client_headers_to_llm_api list" # Verify original data fields are preserved assert updated_data["model"] == "text-embedding-ada-002" @@ -1714,7 +1826,9 @@ async def test_add_guardrails_from_policy_engine(): attachment_registry = get_attachment_registry() attachment_registry._attachments = [ PolicyAttachment(policy="global-baseline", scope="*"), # applies to all - PolicyAttachment(policy="healthcare", teams=["healthcare-team"]), # applies to healthcare team + PolicyAttachment( + policy="healthcare", teams=["healthcare-team"] + ), # applies to healthcare team ] attachment_registry._initialized = True @@ -1757,7 +1871,10 @@ async def test_add_guardrails_from_policy_engine_accepts_dynamic_policies_and_po data = { "model": "gpt-4", "messages": [{"role": "user", "content": "Hello"}], - "policies": ["PII-POLICY-GLOBAL", "HIPAA-POLICY"], # Dynamic policies - should be accepted and removed + "policies": [ + "PII-POLICY-GLOBAL", + "HIPAA-POLICY", + ], # Dynamic policies - should be accepted and removed "metadata": {}, } @@ -1780,7 +1897,9 @@ async def test_add_guardrails_from_policy_engine_accepts_dynamic_policies_and_po ) # Verify that 'policies' was removed from the request body - assert "policies" not in data, "'policies' should be removed from request body to prevent forwarding to LLM provider" + assert ( + "policies" not in data + ), "'policies' should be removed from request body to prevent forwarding to LLM provider" # Verify that other fields are preserved assert "model" in data @@ -1869,7 +1988,9 @@ async def test_bearer_token_not_in_debug_logs(): from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request from litellm.proxy.proxy_server import ProxyConfig - secret_token = "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.fakesignature" + secret_token = ( + "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.fakesignature" + ) mock_request = MagicMock(spec=Request) mock_request.headers = { @@ -1898,8 +2019,10 @@ async def test_bearer_token_not_in_debug_logs(): logger.setLevel(logging.DEBUG) try: - with patch("litellm.proxy.proxy_server.llm_router", None), \ - patch("litellm.proxy.proxy_server.premium_user", True): + with ( + patch("litellm.proxy.proxy_server.llm_router", None), + patch("litellm.proxy.proxy_server.premium_user", True), + ): await add_litellm_data_to_request( data=data, request=mock_request, @@ -2020,9 +2143,7 @@ def test_resolve_project_model_specific_wins(): "gpt-4": {"azure": {"litellm_credentials": "team-gpt4"}}, "defaultconfig": {"azure": {"litellm_credentials": "team-default"}}, } - result = _resolve_credential_from_model_config( - "gpt-4", project_config, team_config - ) + result = _resolve_credential_from_model_config("gpt-4", project_config, team_config) assert result == "proj-gpt4" @@ -2034,9 +2155,7 @@ def test_resolve_project_default_wins_over_team(): "gpt-4": {"azure": {"litellm_credentials": "team-gpt4"}}, "defaultconfig": {"azure": {"litellm_credentials": "team-default"}}, } - result = _resolve_credential_from_model_config( - "gpt-4", project_config, team_config - ) + result = _resolve_credential_from_model_config("gpt-4", project_config, team_config) assert result == "proj-default" @@ -2091,12 +2210,8 @@ def test_apply_overrides_project_model_specific(setup_test_credentials): }, project_metadata={ "model_config": { - "defaultconfig": { - "azure": {"litellm_credentials": "hotel-rec-azure"} - }, - "gpt-4-vision": { - "azure": {"litellm_credentials": "hotel-rec-vision"} - }, + "defaultconfig": {"azure": {"litellm_credentials": "hotel-rec-azure"}}, + "gpt-4-vision": {"azure": {"litellm_credentials": "hotel-rec-vision"}}, } }, ) @@ -2123,12 +2238,8 @@ def test_apply_overrides_project_default(setup_test_credentials): }, project_metadata={ "model_config": { - "defaultconfig": { - "azure": {"litellm_credentials": "hotel-rec-azure"} - }, - "gpt-4-vision": { - "azure": {"litellm_credentials": "hotel-rec-vision"} - }, + "defaultconfig": {"azure": {"litellm_credentials": "hotel-rec-azure"}}, + "gpt-4-vision": {"azure": {"litellm_credentials": "hotel-rec-vision"}}, } }, ) @@ -2231,9 +2342,7 @@ def test_apply_overrides_missing_credential_name(setup_test_credentials): api_key="test-key", team_metadata={ "model_config": { - "gpt-4": { - "azure": {"litellm_credentials": "nonexistent-credential"} - } + "gpt-4": {"azure": {"litellm_credentials": "nonexistent-credential"}} } }, ) @@ -2272,9 +2381,7 @@ def test_apply_overrides_no_model_in_data(setup_test_credentials): api_key="test-key", team_metadata={ "model_config": { - "defaultconfig": { - "azure": {"litellm_credentials": "some-cred"} - } + "defaultconfig": {"azure": {"litellm_credentials": "some-cred"}} } }, ) @@ -2305,9 +2412,7 @@ def test_apply_overrides_clientside_api_version_preserved(setup_test_credentials api_key="test-key", team_metadata={ "model_config": { - "gpt-4-vision": { - "azure": {"litellm_credentials": "hotel-rec-vision"} - } + "gpt-4-vision": {"azure": {"litellm_credentials": "hotel-rec-vision"}} } }, )