From a738c45fc7223d08646e7d86e5617d2449e6fbf9 Mon Sep 17 00:00:00 2001 From: yucheng-berri Date: Mon, 17 Aug 2026 19:50:05 -0700 Subject: [PATCH] fix(proxy): strip callback credentials from the auth object stamped into request metadata (#37233) * fix(proxy): strip callback credentials from the auth object stamped into request metadata * style(proxy): drop the restating half of the stamp-site comment * test(proxy): pin that the stamped auth copy carries header-derived identity --- litellm/proxy/common_utils/callback_utils.py | 8 +- litellm/proxy/litellm_pre_call_utils.py | 20 ++- .../proxy/common_utils/test_callback_utils.py | 2 + .../proxy/test_litellm_pre_call_utils.py | 160 ++++++++++++++++++ 4 files changed, 180 insertions(+), 10 deletions(-) diff --git a/litellm/proxy/common_utils/callback_utils.py b/litellm/proxy/common_utils/callback_utils.py index 4afd7c76a35..9379a8577a3 100644 --- a/litellm/proxy/common_utils/callback_utils.py +++ b/litellm/proxy/common_utils/callback_utils.py @@ -39,10 +39,10 @@ _EXTRA_SENSITIVE_CALLBACK_KEYS: Final = {"gcs_path_service_account"} # already-encrypted input cheaply (no decrypt-attempt round trip) and # avoid double-encrypting if `LITELLM_SALT_KEY` is rotated between writes. _CALLBACK_VAR_ENCRYPTED_PREFIX: Final = "litellm_enc::" -# Metadata slots that hold operator-configured callback setup (and therefore -# integration credentials). Resolved from UserAPIKeyAuth during pre-call setup, -# never read back off the copies stamped into request metadata. -_CALLBACK_CONFIG_SLOTS: Final = frozenset({"logging", "callback_settings"}) +# Metadata slots that hold operator-configured callback and secret-manager setup +# (and therefore integration credentials). Resolved from UserAPIKeyAuth during +# pre-call setup, never read back off the copies stamped into request metadata. +_CALLBACK_CONFIG_SLOTS: Final = frozenset({"logging", "callback_settings", "secret_manager_settings"}) blue_color_code: Final = "\033[94m" reset_color_code: Final = "\033[0m" diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index 6172f3a9158..ef6e590ba26 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -1303,8 +1303,15 @@ class LiteLLMProxyRequestSetup: ) if user_api_key_dict.budget_reservation is not None: data[_metadata_variable_name]["user_api_key_budget_reservation"] = user_api_key_dict.budget_reservation - # Add the full UserAPIKeyAuth object for MCP server access control - data[_metadata_variable_name]["user_api_key_auth"] = user_api_key_dict + # UserAPIKeyAuth object for MCP server access control + data[_metadata_variable_name]["user_api_key_auth"] = user_api_key_dict.model_copy( + update={ + "metadata": strip_callback_config(user_api_key_dict.metadata), + "team_metadata": strip_callback_config(user_api_key_dict.team_metadata), + "project_metadata": strip_callback_config(user_api_key_dict.project_metadata), + "organization_metadata": strip_callback_config(user_api_key_dict.organization_metadata), + } + ) return data @staticmethod @@ -1326,10 +1333,11 @@ class LiteLLMProxyRequestSetup: ) # ignore any special fields - added_metadata: Final = {} - for k, v in management_endpoint_metadata.items(): - if k not in (LiteLLM_ManagementEndpoint_MetadataFields_Premium + LiteLLM_ManagementEndpoint_MetadataFields): - added_metadata[k] = v + added_metadata: Final = { + k: v + for k, v in (strip_callback_config(management_endpoint_metadata) or {}).items() + if k not in (LiteLLM_ManagementEndpoint_MetadataFields_Premium + LiteLLM_ManagementEndpoint_MetadataFields) + } if data[_metadata_variable_name].get("user_api_key_auth_metadata") is None: data[_metadata_variable_name]["user_api_key_auth_metadata"] = {} data[_metadata_variable_name]["user_api_key_auth_metadata"].update(added_metadata) diff --git a/tests/test_litellm/proxy/common_utils/test_callback_utils.py b/tests/test_litellm/proxy/common_utils/test_callback_utils.py index 59963bd3707..515a7b27c7b 100644 --- a/tests/test_litellm/proxy/common_utils/test_callback_utils.py +++ b/tests/test_litellm/proxy/common_utils/test_callback_utils.py @@ -492,6 +492,7 @@ def test_strip_callback_config_drops_credential_bearing_slots(): } ], "callback_settings": {"callback_vars": {"langfuse_secret_key": "litellm_enc::other"}}, + "secret_manager_settings": {"vault_token": "vt-secret"}, "priority": "high", "guardrails": ["presidio"], "langsmith_provisioning": {"api_key_id": "prov-1"}, @@ -501,6 +502,7 @@ def test_strip_callback_config_drops_credential_bearing_slots(): assert "logging" not in stripped assert "callback_settings" not in stripped + assert "secret_manager_settings" not in stripped assert stripped["priority"] == "high" assert stripped["guardrails"] == ["presidio"] assert stripped["langsmith_provisioning"] == {"api_key_id": "prov-1"} 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 d3b1e089489..0171e83be08 100644 --- a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py +++ b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py @@ -229,6 +229,43 @@ async def test_add_litellm_data_to_request_parses_string_metadata(): assert updated_data["metadata"]["generation_name"] == "gen123" +@pytest.mark.asyncio +async def test_stamped_auth_object_reflects_header_derived_identity(): + """ + Regression (LIT-5487): the stamped object is a copy taken partway through request setup, + so it only carries header-derived identity if the stamp still runs after those fields are + resolved. Moving the stamp earlier would silently misattribute spend. + """ + from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request + + request_mock = MagicMock(spec=Request) + request_mock.url = MagicMock() + request_mock.url.path = "/v1/chat/completions" + 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", "user": "end-user-from-header"} + request_mock.client = MagicMock() + request_mock.client.host = "127.0.0.1" + + user_api_key_dict = UserAPIKeyAuth(api_key="hashed-key", metadata={}, team_metadata={}) + + updated_data = await add_litellm_data_to_request( + data={"model": "gpt-3.5-turbo"}, + request=request_mock, + user_api_key_dict=user_api_key_dict, + proxy_config=MagicMock(), + general_settings={"user_header_name": "user"}, + version="test-version", + ) + + # precondition: the header was actually resolved onto the live object + assert user_api_key_dict.end_user_id == "end-user-from-header" + + stamped = updated_data["metadata"]["user_api_key_auth"] + assert stamped.end_user_id == "end-user-from-header" + + @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 / @@ -2306,6 +2343,129 @@ def test_add_user_api_key_auth_to_request_metadata(): assert result["messages"] == [{"role": "user", "content": "Hello"}] +def _auth_with_callback_credentials() -> UserAPIKeyAuth: + return UserAPIKeyAuth( + api_key="hashed-test-key-123", + key_alias="test-key-alias", + team_id="test-team-789", + team_alias="test-team-alias", + metadata={ + "logging": [{"callback_name": "langfuse", "callback_vars": {"langfuse_secret_key": "sk-KEY-CANARY"}}], + "rpm_limit_type": "guaranteed_throughput", + }, + team_metadata={ + "callback_settings": {"langfuse": {"callback_vars": {"langfuse_secret_key": "sk-TEAM-CANARY"}}}, + "secret_manager_settings": {"vault_token": "vt-TEAM-CANARY"}, + "model_rpm_limit": {"gpt-4": 10}, + }, + project_metadata={ + "logging": [{"callback_vars": {"langfuse_secret_key": "sk-PROJECT-CANARY"}}], + "project_tier": "gold", + }, + organization_metadata={ + "secret_manager_settings": {"vault_token": "vt-ORG-CANARY"}, + "org_tier": "platinum", + }, + ) + + +def test_stamped_auth_object_carries_no_callback_credentials(): + """ + Regression (LIT-5487): the UserAPIKeyAuth stamped into request metadata reaches every + raw-metadata logging integration, so it must not carry team/key callback credentials. + """ + user_api_key_dict = _auth_with_callback_credentials() + otel_span = object() + user_api_key_dict.parent_otel_span = otel_span + user_api_key_dict.budget_reservation = {"amount": 1.0} + user_api_key_dict.via_virtual_key = True + data = {"litellm_metadata": {}} + + result = LiteLLMProxyRequestSetup.add_user_api_key_auth_to_request_metadata( + data=data, + user_api_key_dict=user_api_key_dict, + _metadata_variable_name="litellm_metadata", + ) + + stamped = result["litellm_metadata"]["user_api_key_auth"] + emitted = json.dumps( + { + "metadata": stamped.metadata, + "team_metadata": stamped.team_metadata, + "project_metadata": stamped.project_metadata, + "organization_metadata": stamped.organization_metadata, + }, + default=str, + ) + assert "sk-KEY-CANARY" not in emitted + assert "sk-TEAM-CANARY" not in emitted + assert "vt-TEAM-CANARY" not in emitted + assert "sk-PROJECT-CANARY" not in emitted + assert "vt-ORG-CANARY" not in emitted + + # consumers keep the type and the non-credential slots they read + assert isinstance(stamped, UserAPIKeyAuth) + assert stamped.key_alias == "test-key-alias" + assert stamped.team_id == "test-team-789" + assert stamped.team_alias == "test-team-alias" + assert stamped.api_key == "hashed-test-key-123" + assert stamped.metadata["rpm_limit_type"] == "guaranteed_throughput" + assert stamped.team_metadata["model_rpm_limit"] == {"gpt-4": 10} + assert stamped.project_metadata["project_tier"] == "gold" + assert stamped.organization_metadata["org_tier"] == "platinum" + + # server-only markers are excluded from model_dump, so rebuilding the object + # instead of copying it would silently drop them + assert stamped.via_virtual_key is True + assert stamped.budget_reservation == {"amount": 1.0} + assert stamped.parent_otel_span is otel_span + + +def test_stamping_does_not_mutate_the_cached_auth_object(): + """ + Regression (LIT-5487): UserAPIKeyAuth is cached and model_copy is shallow, so stripping + in place would poison the shared dicts and silently kill team callbacks fleet-wide. + """ + user_api_key_dict = _auth_with_callback_credentials() + metadata_before = copy.deepcopy(user_api_key_dict.metadata) + team_metadata_before = copy.deepcopy(user_api_key_dict.team_metadata) + + LiteLLMProxyRequestSetup.add_user_api_key_auth_to_request_metadata( + data={"litellm_metadata": {}}, + user_api_key_dict=user_api_key_dict, + _metadata_variable_name="litellm_metadata", + ) + + assert user_api_key_dict.metadata == metadata_before + assert user_api_key_dict.team_metadata == team_metadata_before + + +def test_management_endpoint_metadata_drops_callback_credentials(): + """ + Regression (LIT-5487): user_api_key_auth_metadata is part of StandardLoggingPayload, so a + callback_settings-shaped team must not push credentials into it. + """ + data = {"litellm_metadata": {}} + + result = LiteLLMProxyRequestSetup.add_management_endpoint_metadata_to_request_metadata( + data=data, + management_endpoint_metadata={ + "callback_settings": {"langfuse": {"callback_vars": {"langfuse_secret_key": "sk-TEAM-CANARY"}}}, + "secret_manager_settings": {"vault_token": "vt-TEAM-CANARY"}, + "logging": [{"callback_vars": {"langfuse_secret_key": "sk-LOGGING-CANARY"}}], + "other_field": "value", + }, + _metadata_variable_name="litellm_metadata", + ) + + auth_metadata = result["litellm_metadata"]["user_api_key_auth_metadata"] + emitted = json.dumps(auth_metadata, default=str) + assert "sk-TEAM-CANARY" not in emitted + assert "vt-TEAM-CANARY" not in emitted + assert "sk-LOGGING-CANARY" not in emitted + assert auth_metadata["other_field"] == "value" + + @pytest.mark.parametrize( "data, model_group_settings, expected_headers_added", [