fix(otel v2): reject a langfuse_span_scope that conflicts with another callback entry on the same team or key

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
yucheng 2026-09-19 23:16:54 +00:00
parent 358e4ea27a
commit f63782f678
5 changed files with 141 additions and 15 deletions

View file

@ -157,6 +157,25 @@ def cross_entry_family_error(
)
def conflicting_span_scope_error(
callback_vars: Mapping[str, str] | None,
stored_vars_by_entry: Sequence[Mapping[str, str]],
) -> str | None:
"""Reject a ``langfuse_span_scope`` another entry already sets differently; the entries flatten last-wins."""
incoming: Final = None if callback_vars is None else callback_vars.get(_LANGFUSE_SPAN_SCOPE_VAR)
if incoming is None:
return None
return next(
(
f"{_LANGFUSE_SPAN_SCOPE_VAR} is already set to {stored!r} by another callback entry. "
f"Every entry shares one scope: remove that entry or send the same value."
for entry in stored_vars_by_entry
if (stored := entry.get(_LANGFUSE_SPAN_SCOPE_VAR)) not in (None, incoming)
),
None,
)
def logging_metadata_config_error(metadata: Mapping[str, object] | None) -> str | None:
"""Validate every ``logging`` entry of a team/key metadata payload."""
if not metadata:
@ -164,23 +183,34 @@ def logging_metadata_config_error(metadata: Mapping[str, object] | None) -> str
entries: Final = metadata.get("logging")
if not isinstance(entries, Sequence) or isinstance(entries, (str, bytes)):
return None
entry_vars: Final = tuple(_entry_callback_vars(entry) for entry in entries)
return next(
(error for error in (_logging_entry_error(entry) for entry in entries) if error is not None),
(
error
for error in (
*(_logging_entry_error(entry) for entry in entries),
*(conflicting_span_scope_error(entry_vars[i], entry_vars[:i]) for i in range(len(entry_vars))),
)
if error is not None
),
None,
)
def _entry_callback_vars(entry: object) -> Mapping[str, str]:
callback_vars: Final = entry.get("callback_vars") if isinstance(entry, Mapping) else None
if not isinstance(callback_vars, Mapping):
return MappingProxyType({})
return MappingProxyType({str(key): str(value) for key, value in callback_vars.items()})
def _logging_entry_error(entry: object) -> str | None:
if not isinstance(entry, Mapping):
return None
callback_name: Final = entry.get("callback_name")
callback_vars: Final = entry.get("callback_vars")
if not isinstance(callback_name, str) or not isinstance(callback_vars, Mapping):
if not isinstance(callback_name, str) or not isinstance(entry.get("callback_vars"), Mapping):
return None
return callback_config_error(
callback_name,
MappingProxyType({str(key): str(value) for key, value in callback_vars.items()}),
)
return callback_config_error(callback_name, _entry_callback_vars(entry))
def _newrelic_config_error(callback_vars: Mapping[str, str]) -> str | None:

View file

@ -31,6 +31,7 @@ from litellm.proxy._types import (
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
from litellm.proxy.common_utils.callback_config_validation import (
callback_config_error,
conflicting_span_scope_error,
cross_entry_family_error,
)
from litellm.proxy.common_utils.callback_utils import (
@ -344,6 +345,16 @@ async def add_team_callbacks(
if team_callback_settings is None or not isinstance(team_callback_settings, list):
team_callback_settings = []
# Decrypted, because the checks compare the incoming values against
# the stored ones and the credentials are encrypted at rest.
decrypted_logging: Final = decrypt_callback_vars(team_metadata).get("logging")
stored_entries: Final = decrypted_logging if isinstance(decrypted_logging, list) else ()
stored_entry_vars: Final = [ # mutable-ok: read-only input to the checks, never stored
entry.get("callback_vars") or {} for entry in stored_entries
]
scope_error: Final = conflicting_span_scope_error(data.callback_vars, stored_entry_vars)
if scope_error is not None:
raise _callback_config_error(scope_error)
# One entry has to own a credential family end to end. The entries are
# flattened into one dict before a request reads them, so an entry
# naming only a destination would pair with a key written on another
@ -352,13 +363,6 @@ async def add_team_callbacks(
# fine, which is how one integration covers both events. Proxy admins
# are exempt: they already hold every credential the proxy has.
if user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN:
# Decrypted, because the check compares the incoming values against
# the stored ones and the credentials are encrypted at rest.
decrypted_logging: Final = decrypt_callback_vars(team_metadata).get("logging")
stored_entries: Final = decrypted_logging if isinstance(decrypted_logging, list) else ()
stored_entry_vars: Final = [ # mutable-ok: read-only input to the check, never stored
entry.get("callback_vars") or {} for entry in stored_entries
]
family_error: Final = cross_entry_family_error(data.callback_vars, stored_entry_vars)
if family_error is not None:
raise HTTPException(

View file

@ -1,5 +1,9 @@
import pytest
from litellm.proxy.common_utils.callback_config_validation import (
callback_config_error,
conflicting_span_scope_error,
logging_metadata_config_error,
)
@ -37,3 +41,44 @@ def test_a_bad_span_scope_is_reported_even_when_the_environment_is_fine():
"langfuse_otel", {"langfuse_environment": "team-a-prod", "langfuse_span_scope": "everything"}
)
assert error is not None and "langfuse_span_scope" in error
@pytest.mark.parametrize(
"new_vars, stored, rejected",
[
({"langfuse_span_scope": "llm_only"}, [{"langfuse_span_scope": "full"}], True),
({"langfuse_span_scope": "full"}, [{"langfuse_public_key": "pk"}, {"langfuse_span_scope": "llm_only"}], True),
({"langfuse_span_scope": "llm_only"}, [{"langfuse_span_scope": "llm_only"}], False),
({"langfuse_span_scope": "llm_only"}, [{"langfuse_public_key": "pk"}], False),
({"langfuse_span_scope": "llm_only"}, [], False),
({"langfuse_public_key": "pk"}, [{"langfuse_span_scope": "llm_only"}], False),
(None, [{"langfuse_span_scope": "llm_only"}], False),
],
)
def test_one_span_scope_per_team(new_vars, stored, rejected):
"""The entries flatten last-wins, so a second scope would export whichever entry
was stored last. An entry that names no scope leaves the stored one in charge."""
error = conflicting_span_scope_error(new_vars, stored)
assert (error is not None) is rejected
if rejected:
assert "langfuse_span_scope" in error and stored[-1]["langfuse_span_scope"] in error
def test_key_logging_entries_may_not_disagree_on_the_span_scope():
disagreeing = {
"logging": [
{"callback_name": "langfuse_otel", "callback_type": "success", "callback_vars": {"langfuse_span_scope": "full"}},
{"callback_name": "langfuse_otel", "callback_type": "failure", "callback_vars": {"langfuse_span_scope": "llm_only"}},
]
}
error = logging_metadata_config_error(disagreeing)
assert error is not None and "langfuse_span_scope" in error and "'full'" in error
agreeing = {
"logging": [
{"callback_name": "langfuse_otel", "callback_type": "success", "callback_vars": {"langfuse_span_scope": "llm_only"}},
{"callback_name": "langfuse_otel", "callback_type": "failure", "callback_vars": {"langfuse_span_scope": "llm_only"}},
{"callback_name": "otel", "callback_type": "success", "callback_vars": {}},
]
}
assert logging_metadata_config_error(agreeing) is None

View file

@ -1565,3 +1565,48 @@ def test_one_entry_owns_a_credential_family(new_vars, stored, rejected):
"""
error = cross_entry_family_error(new_vars, stored)
assert (error is not None) is rejected
@pytest.mark.asyncio
@pytest.mark.parametrize("caller", [_admin_auth(), UserAPIKeyAuth(user_role=LitellmUserRoles.INTERNAL_USER, user_id="victim_admin", api_key="sk-team-admin")])
async def test_a_second_entry_may_not_flip_the_span_scope(patched_prisma, caller):
"""The entries flatten last-wins at request time, so a failure entry saying
llm_only next to a success entry saying full would export whichever is stored
last. Neither a proxy admin nor a team admin gets to store the disagreement."""
patched_prisma.get_data = AsyncMock(
return_value=_team_row(
metadata={
"logging": [
{
"callback_name": "langfuse_otel",
"callback_type": "success",
"callback_vars": {"langfuse_public_key": "pk", "langfuse_secret_key": "sk", "langfuse_span_scope": "full"},
}
]
}
)
)
data = AddTeamCallback(
callback_name="langfuse_otel",
callback_type="failure",
callback_vars={"langfuse_public_key": "pk", "langfuse_secret_key": "sk", "langfuse_span_scope": "llm_only"},
)
with pytest.raises(HTTPException) as exc:
await add_team_callbacks(
data=data,
http_request=Mock(spec=Request),
team_id="team-victim",
user_api_key_dict=caller,
)
assert exc.value.status_code == 400
assert "langfuse_span_scope" in str(exc.value.detail) and "'full'" in str(exc.value.detail)
patched_prisma.db.litellm_teamtable.update.assert_not_called()
data.callback_vars["langfuse_span_scope"] = "full"
await add_team_callbacks(
data=data,
http_request=Mock(spec=Request),
team_id="team-victim",
user_api_key_dict=caller,
)
patched_prisma.db.litellm_teamtable.update.assert_awaited_once()

View file

@ -170,7 +170,9 @@ const LoggingSettings: React.FC<LoggingSettingsProps> = ({
width={400}
placeholder={`os.environ/${paramName.toUpperCase()}`}
value={config.callback_vars[paramName] || ""}
onChange={(e: any) => updateCallbackVar(configIndex, paramName, e.target.value)}
onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
updateCallbackVar(configIndex, paramName, e.target.value)
}
/>
);
}