From 42fbc470b09a017cd534fc66f7c86f00e9d2818e Mon Sep 17 00:00:00 2001 From: yucheng Date: Wed, 23 Sep 2026 22:30:53 +0000 Subject: [PATCH] fix(proxy): reject invalid otel_internal_spans in team metadata.logging Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../callback_config_validation.py | 13 ++++++ .../key_management_endpoints.py | 17 ++------ .../management_endpoints/team_endpoints.py | 3 ++ .../test_callback_management_endpoints.py | 26 +++++------- .../test_team_endpoints.py | 41 +++++++++++++++++++ 5 files changed, 71 insertions(+), 29 deletions(-) diff --git a/litellm/proxy/common_utils/callback_config_validation.py b/litellm/proxy/common_utils/callback_config_validation.py index d2071ef83e3..6c60b4bff7c 100644 --- a/litellm/proxy/common_utils/callback_config_validation.py +++ b/litellm/proxy/common_utils/callback_config_validation.py @@ -10,6 +10,8 @@ from collections.abc import Mapping, Sequence from types import MappingProxyType from typing import Final +from fastapi import HTTPException + _NEWRELIC_CALLBACK: Final = "newrelic" _NEWRELIC_VAR_PREFIX: Final = "newrelic_" _LANGFUSE_OTEL_CALLBACK: Final = "langfuse_otel" @@ -254,6 +256,17 @@ def logging_metadata_config_error(metadata: Mapping[str, object] | None) -> str ) +def raise_on_invalid_logging_metadata(metadata: Mapping[str, object] | None) -> None: + """Reject a metadata payload whose ``logging`` entries the runtime cannot honor. + + Without this the same config the management surface rejects would be + accepted here and then silently ignored or misrouted at request time. + """ + error: Final = logging_metadata_config_error(metadata) + if error is not None: + raise HTTPException(status_code=400, detail={"error": error}) # mutable-ok: FastAPI detail contract + + def _entry_callback_name(entry: object) -> str | None: callback_name: Final = entry.get("callback_name") if isinstance(entry, Mapping) else None return callback_name if isinstance(callback_name, str) else None diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index 1845f11c0fa..69bfbfe3408 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -70,7 +70,7 @@ from litellm.proxy.common_utils.auth_cache_invalidation_pubsub import ( evict_and_broadcast, publish_auth_cache_invalidation, ) -from litellm.proxy.common_utils.callback_config_validation import logging_metadata_config_error +from litellm.proxy.common_utils.callback_config_validation import raise_on_invalid_logging_metadata from litellm.proxy.common_utils.callback_utils import ( decrypt_callback_vars, encrypt_callback_vars, @@ -792,17 +792,6 @@ def key_generation_check( return _personal_key_generation_check(user_api_key_dict=user_api_key_dict, data=data) -def raise_on_invalid_key_logging_config(metadata: Mapping[str, object] | None) -> None: - """Key-level logging writes go through key metadata, not /team/callback. - - Without this the same New Relic config the team endpoint rejects would be - accepted here and then silently ignored or misrouted at request time. - """ - error: Final = logging_metadata_config_error(metadata) - if error is not None: - raise HTTPException(status_code=400, detail={"error": error}) # mutable-ok: FastAPI detail contract - - def common_key_access_checks( user_api_key_dict: UserAPIKeyAuth, data: GenerateKeyRequest | UpdateKeyRequest, @@ -1183,7 +1172,7 @@ async def _common_key_generation_helper( ) validate_budget_duration(data.budget_duration) - raise_on_invalid_key_logging_config(data.metadata) + raise_on_invalid_logging_metadata(data.metadata) if data.throttle_on_budget_exceeded is True and user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN.value: raise HTTPException( @@ -2312,7 +2301,7 @@ def prepare_metadata_fields(data: BaseModel, non_default_values: dict, existing_ """ Check LiteLLM_ManagementEndpoint_MetadataFields (proxy/_types.py) for fields that are allowed to be updated """ - raise_on_invalid_key_logging_config(non_default_values.get("metadata")) + raise_on_invalid_logging_metadata(non_default_values.get("metadata")) if "metadata" not in non_default_values: # allow user to set metadata to none non_default_values["metadata"] = existing_metadata.copy() diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index d092fc2fbb7..4bdd0ff4fbd 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -115,6 +115,7 @@ from litellm.proxy.auth.auth_utils import ( ) from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.common_utils.auth_cache_invalidation_pubsub import evict_and_broadcast +from litellm.proxy.common_utils.callback_config_validation import raise_on_invalid_logging_metadata from litellm.proxy.common_utils.callback_utils import encrypt_callback_vars from litellm.proxy.common_utils.json_merge_patch import apply_json_merge_patch from litellm.proxy.common_utils.user_api_key_cache import UserApiKeyCache @@ -1507,6 +1508,7 @@ async def new_team( validate_budget_duration(data.budget_duration) validate_budget_duration(data.team_member_budget_duration) validate_team_model_max_budget(model_max_budget=data.model_max_budget, premium_user=premium_user) + raise_on_invalid_logging_metadata(data.metadata) if data.soft_budget is not None: if data.max_budget is not None: @@ -2235,6 +2237,7 @@ async def update_team( if data.team_id is None: raise HTTPException(status_code=400, detail={"error": "No team id passed in"}) + raise_on_invalid_logging_metadata(data.metadata) verbose_proxy_logger.debug("/team/update - %s", data) # Validate budget values are not negative diff --git a/tests/test_litellm/proxy/management_endpoints/test_callback_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_callback_management_endpoints.py index 6fd9b6f9b34..414c7d50480 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_callback_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_callback_management_endpoints.py @@ -390,46 +390,42 @@ class TestNewRelicKeyLoggingValidation: from fastapi import HTTPException from litellm.integrations.otel.model.config import is_otel_v2_enabled - from litellm.proxy.management_endpoints.key_management_endpoints import ( - raise_on_invalid_key_logging_config, - ) + from litellm.proxy.common_utils.callback_config_validation import raise_on_invalid_logging_metadata monkeypatch.delenv("LITELLM_OTEL_V2", raising=False) is_otel_v2_enabled.cache_clear() try: with pytest.raises(HTTPException) as exc: - raise_on_invalid_key_logging_config(self._metadata({"newrelic_api_key": "k"})) + raise_on_invalid_logging_metadata(self._metadata({"newrelic_api_key": "k"})) assert "LITELLM_OTEL_V2" in str(exc.value.detail) monkeypatch.setenv("LITELLM_OTEL_V2", "true") is_otel_v2_enabled.cache_clear() with pytest.raises(HTTPException) as exc: - raise_on_invalid_key_logging_config( + raise_on_invalid_logging_metadata( self._metadata({"newrelic_api_key": "k", "newrelic_region": "mars"}) ) assert "Unknown newrelic_region" in str(exc.value.detail) with pytest.raises(HTTPException) as exc: - raise_on_invalid_key_logging_config(self._metadata({"newrelic_region": "eu"})) + raise_on_invalid_logging_metadata(self._metadata({"newrelic_region": "eu"})) assert "requires newrelic_api_key" in str(exc.value.detail) - raise_on_invalid_key_logging_config(self._metadata({"newrelic_api_key": "k", "newrelic_region": "EU"})) + raise_on_invalid_logging_metadata(self._metadata({"newrelic_api_key": "k", "newrelic_region": "EU"})) finally: is_otel_v2_enabled.cache_clear() def test_ignores_metadata_without_newrelic_logging(self, monkeypatch): from litellm.integrations.otel.model.config import is_otel_v2_enabled - from litellm.proxy.management_endpoints.key_management_endpoints import ( - raise_on_invalid_key_logging_config, - ) + from litellm.proxy.common_utils.callback_config_validation import raise_on_invalid_logging_metadata monkeypatch.delenv("LITELLM_OTEL_V2", raising=False) is_otel_v2_enabled.cache_clear() try: - assert raise_on_invalid_key_logging_config(None) is None - assert raise_on_invalid_key_logging_config({"logging": "not-a-list"}) is None - assert raise_on_invalid_key_logging_config({"tags": ["a"]}) is None - assert raise_on_invalid_key_logging_config(self._metadata({})) is None + assert raise_on_invalid_logging_metadata(None) is None + assert raise_on_invalid_logging_metadata({"logging": "not-a-list"}) is None + assert raise_on_invalid_logging_metadata({"tags": ["a"]}) is None + assert raise_on_invalid_logging_metadata(self._metadata({})) is None assert ( - raise_on_invalid_key_logging_config( + raise_on_invalid_logging_metadata( {"logging": [{"callback_name": "langfuse", "callback_vars": {"langfuse_public_key": "pk"}}]} ) is None diff --git a/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py index 7a9b6b66946..28df88db03e 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py @@ -628,6 +628,47 @@ async def test_update_team_rejects_a_duration_that_never_advances( mock_find_unique.assert_not_awaited() +@pytest.mark.asyncio +@pytest.mark.parametrize("endpoint", ["new_team", "update_team"]) +async def test_team_endpoints_reject_bogus_logging_metadata(endpoint, mock_db_client, mock_admin_auth): + """metadata.logging entries the runtime cannot honor must 400 instead of + being persisted, the same contract /key/generate and /key/update enforce.""" + from fastapi import Request + + from litellm.proxy._types import NewTeamRequest, UpdateTeamRequest + from litellm.proxy.management_endpoints.team_endpoints import new_team, update_team + + mock_db_client.db = MagicMock() + mock_db_client.db.litellm_teamtable = MagicMock() + mock_db_client.db.litellm_teamtable.create = AsyncMock() + mock_db_client.db.litellm_teamtable.update = AsyncMock() + _wire_team_create_tx(mock_db_client) + + metadata: Final = { + "logging": [ + {"callback_name": "langfuse_otel", "callback_vars": {"otel_internal_spans": "sometimes"}} + ] + } + data: Final = ( + NewTeamRequest(team_alias="my-team", metadata=metadata) + if endpoint == "new_team" + else UpdateTeamRequest(team_id="team-1", metadata=metadata) + ) + handler: Final = new_team if endpoint == "new_team" else update_team + + with pytest.raises(ProxyException) as exc_info: + await handler( + data=data, + http_request=MagicMock(spec=Request), + user_api_key_dict=mock_admin_auth, + ) + + assert str(exc_info.value.code) == "400" + assert "otel_internal_spans" in str(exc_info.value.message) + mock_db_client.db.litellm_teamtable.create.assert_not_awaited() + mock_db_client.db.litellm_teamtable.update.assert_not_awaited() + + @pytest.mark.asyncio async def test_new_team_with_object_permission(mock_db_client, mock_admin_auth): """