mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-30 01:52:18 +00:00
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>
This commit is contained in:
parent
6fe2f2474a
commit
42fbc470b0
5 changed files with 71 additions and 29 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
"""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue