fix(datadog): read team callback dd_* params from kwargs instead of blocked dynamic params (#35115) (#35687)

Team-scoped DD credentials (dd_api_key, dd_site) set via POST /team/{id}/callback were silently dropped because _request_blocked_callback_params blocks them from standard_callback_dynamic_params. The security block is correct for request-level injection, but team callback_vars are admin-configured and trusted.

Store the raw init kwargs on the Logging instance and read dd_* params from there in _process_dynamic_callback_list instead of from standard_callback_dynamic_params.

Adds an integration test that exercises the full Logging.__init__ flow with team callback_vars to prevent regression.

Co-authored-by: Aanchal Khandelwal <aan2210khandelwal@gmail.com>
This commit is contained in:
ryan-crabbe-berri 2026-08-03 16:19:56 -07:00 committed by GitHub
parent 8ad5d144a1
commit 7dab1ff75f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 354 additions and 18 deletions

View file

@ -1,7 +1,7 @@
from collections.abc import Iterator
from collections.abc import Iterator, Mapping
from typing import Any
from litellm.types.utils import StandardCallbackDynamicParams
from litellm.types.utils import TRUSTED_CALLBACK_VARS_FIELD, StandardCallbackDynamicParams
_CLIENT_CALLBACK_METADATA_SLOTS: tuple[str, ...] = ("litellm_metadata", "metadata")
@ -75,14 +75,32 @@ _supported_callback_params = [
"turn_off_message_logging",
]
_request_blocked_callback_params = {
"gcs_bucket_name",
"gcs_path_service_account",
"dd_api_key",
"dd_site",
"dd_agent_host",
"dd_agent_port",
}
_request_blocked_callback_params = frozenset(
{
"gcs_bucket_name",
"gcs_path_service_account",
"dd_api_key",
"dd_site",
"dd_agent_host",
"dd_agent_port",
}
)
def get_trusted_callback_params(kwargs: Mapping[str, Any] | None) -> tuple[tuple[str, str], ...]:
"""
Read callback params the proxy itself stamped from admin-configured team/key callback settings.
Request-body values never reach this field: the proxy strips it from client input before
setting it, so callbacks can consume credentials and destinations here without re-validating.
Returned as pairs rather than a mapping because the caller keeps this on the Logging object,
which the proxy deep-copies; a mappingproxy is not copyable and a dict would be mutable.
"""
trusted_vars = kwargs.get(TRUSTED_CALLBACK_VARS_FIELD) if kwargs else None
if not isinstance(trusted_vars, Mapping):
return ()
return tuple((key, str(value)) for key, value in trusted_vars.items() if isinstance(key, str))
def initialize_standard_callback_dynamic_params(

View file

@ -166,6 +166,9 @@ from ..integrations.s3_v2 import S3Logger as S3V2Logger
from ..integrations.supabase import Supabase
from ..integrations.traceloop import TraceloopLogger
from .exception_mapping_utils import _get_response_headers
from .initialize_dynamic_callback_params import (
get_trusted_callback_params,
)
from .initialize_dynamic_callback_params import (
initialize_standard_callback_dynamic_params as _initialize_standard_callback_dynamic_params,
)
@ -362,6 +365,7 @@ class Logging(LiteLLMLoggingBaseClass):
self.standard_callback_dynamic_params: StandardCallbackDynamicParams = (
self.initialize_standard_callback_dynamic_params(kwargs)
)
self._trusted_callback_vars: tuple[tuple[str, str], ...] = get_trusted_callback_params(kwargs)
# Process dynamic callbacks (after standard_callback_dynamic_params is initialized,
# so team-scoped credentials are available for callback initialization)
@ -459,9 +463,10 @@ class Logging(LiteLLMLoggingBaseClass):
# pass only the relevant dynamic params as custom_logger_init_args.
_custom_logger_init_args: dict | None = None
if callback == "datadog":
_custom_logger_init_args = {
k: v for k, v in self.standard_callback_dynamic_params.items() if k.startswith("dd_")
}
# dd_* params are blocked from standard_callback_dynamic_params
# (request-level security); only the proxy-stamped team/key
# callback vars are admin-configured and trusted.
_custom_logger_init_args = {k: v for k, v in self._trusted_callback_vars if k.startswith("dd_")}
callback_class = _init_custom_logger_compatible_class(
callback, # type: ignore[arg-type]

View file

@ -20,6 +20,8 @@ from litellm.constants import (
)
from litellm.litellm_core_utils.credential_accessor import CredentialAccessor
from litellm.litellm_core_utils.initialize_dynamic_callback_params import (
TRUSTED_CALLBACK_VARS_FIELD,
_request_blocked_callback_params,
iter_client_callback_metadata_dicts,
)
from litellm.litellm_core_utils.safe_json_loads import safe_json_loads
@ -356,6 +358,39 @@ def _strip_client_message_redaction_opt_out(data: dict[str, Any]) -> None:
)
def _strip_client_callback_credentials(
data: dict[str, Any], # mutable-ok: strips in place on the request body the pre-call pipeline threads through
) -> None:
"""Drop callback credentials and destinations supplied by the caller.
``_request_blocked_callback_params`` (Datadog + GCS credentials, sites and agent
hosts) are already ignored when building ``standard_callback_dynamic_params``.
Strip them from the body and every client metadata slot as well, so a caller
cannot pair its own ``dd_site``/``dd_agent_host`` with the team's admin-configured
``dd_api_key`` and have the resulting logs shipped to a host it controls.
``TRUSTED_CALLBACK_VARS_FIELD`` is proxy-owned; it is cleared here and repopulated
from team/key callback settings in ``add_litellm_data_to_request``.
"""
containers = (("body", data), *iter_client_callback_metadata_dicts(data))
stripped = tuple(
f"{label}.{field}"
for label, container in containers
for field in _request_blocked_callback_params
if field in container
)
for _, container in containers:
for field in _request_blocked_callback_params:
container.pop(field, None)
data.pop(TRUSTED_CALLBACK_VARS_FIELD, None)
if stripped:
verbose_proxy_logger.debug(
"Stripped client-supplied callback credentials from request: %s. "
"Configure these on the team or key callback settings instead.",
", ".join(sorted(stripped)),
)
def _strip_client_pricing_overrides(data: dict[str, Any]) -> None:
"""Drop pricing overrides from the request body and any metadata variant.
@ -524,7 +559,8 @@ def safe_add_api_version_from_query_params(data: dict, request: Request):
def convert_key_logging_metadata_to_callback(
data: AddTeamCallback, team_callback_settings_obj: TeamCallbackMetadata | None
data: AddTeamCallback,
team_callback_settings_obj: TeamCallbackMetadata | None,
) -> TeamCallbackMetadata:
if team_callback_settings_obj is None:
team_callback_settings_obj = TeamCallbackMetadata()
@ -1563,6 +1599,10 @@ async def add_litellm_data_to_request(
if not _key_or_team_allows_client_pricing_override(user_api_key_dict):
_strip_client_pricing_overrides(data)
# Same reason as the strips above: runs after the metadata string-to-dict parse
# so JSON-string metadata cannot smuggle callback credentials past the dict guard.
_strip_client_callback_credentials(data)
if not _allow_client_message_redaction_opt_out and litellm.turn_off_message_logging is True:
_strip_client_message_redaction_opt_out(data)
@ -1771,6 +1811,9 @@ async def add_litellm_data_to_request(
# unpack callback_vars in data
for k, v in callback_settings_obj.callback_vars.items():
data[k] = v
# Callbacks that must not honour request-supplied credentials read this
# proxy-owned field instead of the raw request kwargs.
data[TRUSTED_CALLBACK_VARS_FIELD] = callback_settings_obj.callback_vars
# Add disabled callbacks from key metadata
if user_api_key_dict.metadata and "litellm_disabled_callbacks" in user_api_key_dict.metadata:

View file

@ -3286,8 +3286,15 @@ agentic_loop_internal_litellm_params = [
"_code_interpreter_interception_converted_stream",
]
# Proxy-owned callback credentials, stamped from admin-configured team/key callback
# settings. Listed in all_litellm_params for the same reason as the agentic-loop
# fields above: an unrecognized top-level key is swept into extra_body and sent to
# the provider.
TRUSTED_CALLBACK_VARS_FIELD = "litellm_trusted_callback_vars"
all_litellm_params = (
agentic_loop_internal_litellm_params
+ [TRUSTED_CALLBACK_VARS_FIELD]
+ [
"metadata",
"litellm_metadata",

View file

@ -6,6 +6,7 @@ Verifies that DataDogLogger can be instantiated with per-team credentials
and that the DataDogHandler correctly resolves and caches per-team loggers.
"""
import copy
from unittest.mock import patch
import pytest
@ -13,7 +14,9 @@ import pytest
from litellm.integrations.datadog.datadog import DataDogLogger
from litellm.integrations.datadog.datadog_team_handler import (
DataDogHandler,
DatadogLoggingConfig,
)
from litellm.litellm_core_utils.initialize_dynamic_callback_params import (
TRUSTED_CALLBACK_VARS_FIELD,
)
from litellm.litellm_core_utils.specialty_caches.dynamic_logging_cache import (
DynamicLoggingCache,
@ -94,9 +97,7 @@ class TestDataDogLoggerCredentialKwargs:
assert logger.DD_API_KEY is None
assert "attacker.example.com" in logger.intake_url
def test_direct_api_mode_does_not_leak_env_api_key_when_disallowed(
self, datadog_env
):
def test_direct_api_mode_does_not_leak_env_api_key_when_disallowed(self, datadog_env):
"""With allow_env_credentials=False and no explicit key, init must fail rather than reuse env key."""
with pytest.raises(Exception, match="DD_API_KEY"):
with patch("asyncio.create_task"):
@ -261,3 +262,96 @@ class TestStandardCallbackDynamicParamsIncludesDatadog:
assert "dd_site" in annotations
assert "dd_agent_host" in annotations
assert "dd_agent_port" in annotations
def _build_logging_obj(kwargs: dict, *, with_datadog_callback: bool = True):
from litellm.litellm_core_utils.litellm_logging import Logging
with patch("asyncio.create_task"):
return Logging(
model="gpt-4",
messages=[{"role": "user", "content": "hi"}],
stream=False,
call_type="completion",
start_time="2026-01-01",
litellm_call_id="test-call-id",
function_id="test-func",
dynamic_success_callbacks=["datadog"] if with_datadog_callback else None,
kwargs=kwargs,
)
def _dd_loggers(logging_obj) -> list[DataDogLogger]:
return [cb for cb in (logging_obj.dynamic_success_callbacks or []) if isinstance(cb, DataDogLogger)]
class TestTeamCallbackFlowPassesDDCredentials:
"""
dd_* credentials reach DataDogHandler only from the proxy-stamped trusted field.
Team callback_vars are admin-configured, so they must survive
_request_blocked_callback_params; anything the caller put in the request body
must not, or a caller could pair its own dd_site with the team's dd_api_key.
"""
def test_trusted_callback_vars_reach_datadog_handler(self, datadog_env):
trusted_vars = {"dd_api_key": "team-dd-key-123", "dd_site": "us5.datadoghq.com"}
logging_obj = _build_logging_obj(
{
TRUSTED_CALLBACK_VARS_FIELD: trusted_vars,
"model": "gpt-4",
"litellm_params": {"metadata": {}},
}
)
dd_loggers = _dd_loggers(logging_obj)
assert len(dd_loggers) == 1, "DataDogLogger should be initialized from team callback_vars"
assert dd_loggers[0].DD_API_KEY == "team-dd-key-123"
assert "us5.datadoghq.com" in dd_loggers[0].intake_url
def test_request_kwargs_dd_params_are_ignored(self, datadog_env):
"""Top-level dd_* in the call kwargs are caller-controlled and must never be honoured."""
logging_obj = _build_logging_obj(
{
"dd_api_key": "caller-dd-key",
"dd_site": "attacker.example.com",
"dd_agent_host": "attacker.example.com",
"model": "gpt-4",
"litellm_params": {"metadata": {}},
}
)
dd_loggers = _dd_loggers(logging_obj)
assert len(dd_loggers) == 1
assert dd_loggers[0].DD_API_KEY == "global_api_key"
assert "attacker.example.com" not in dd_loggers[0].intake_url
assert "us1.datadoghq.com" in dd_loggers[0].intake_url
def test_logging_object_stays_deepcopyable(self):
"""The proxy deep-copies request data, and the Logging object rides along in it."""
logging_obj = _build_logging_obj(
{
TRUSTED_CALLBACK_VARS_FIELD: {"dd_api_key": "team-dd-key-123", "dd_site": "us5.datadoghq.com"},
"model": "gpt-4",
"litellm_params": {"metadata": {}},
},
with_datadog_callback=False,
)
assert copy.deepcopy(logging_obj)._trusted_callback_vars == logging_obj._trusted_callback_vars
def test_caller_cannot_redirect_team_credentials(self, datadog_env):
"""The exfil shape: caller's dd_site paired with the team's dd_api_key."""
logging_obj = _build_logging_obj(
{
TRUSTED_CALLBACK_VARS_FIELD: {"dd_api_key": "team-dd-key-123"},
"dd_site": "attacker.example.com",
"model": "gpt-4",
"litellm_params": {"metadata": {}},
}
)
dd_loggers = _dd_loggers(logging_obj)
assert len(dd_loggers) == 1
assert dd_loggers[0].DD_API_KEY == "team-dd-key-123"
assert "attacker.example.com" not in dd_loggers[0].intake_url

View file

@ -28,6 +28,9 @@ from litellm.proxy.litellm_pre_call_utils import (
check_if_token_is_service_account,
clean_headers,
)
from litellm.litellm_core_utils.initialize_dynamic_callback_params import (
TRUSTED_CALLBACK_VARS_FIELD,
)
from litellm.types.utils import CredentialItem
sys.path.insert(
@ -5554,3 +5557,169 @@ def test_warn_stale_team_alias_once_evicts_oldest_key_beyond_cap(monkeypatch):
pre_call_utils._warn_stale_team_alias_once("key-3", "stale alias")
assert list(pre_call_utils._STALE_TEAM_ALIAS_WARNING_KEYS) == ["key-2", "key-3"]
def _callback_credential_request_mock() -> MagicMock:
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"}
request_mock.client = MagicMock()
request_mock.client.host = "127.0.0.1"
return request_mock
_DATADOG_TEAM_KEY = UserAPIKeyAuth(
api_key="hashed-key",
team_id="team-1",
team_metadata={
"logging": [
{
"callback_name": "datadog",
"callback_type": "success",
"callback_vars": {"dd_api_key": "team-dd-key"},
}
]
},
)
@pytest.mark.asyncio
async def test_add_litellm_data_to_request_strips_caller_supplied_callback_credentials():
"""
The team admin sets dd_api_key only; a caller pairing its own dd_site with that key
would ship the team's Datadog credential to a host it controls.
"""
caller_destinations = {"dd_site": "attacker.example.com", "dd_agent_host": "attacker.example.com"}
data = {
"model": "gpt-3.5-turbo",
"messages": [{"role": "user", "content": "hello"}],
**caller_destinations,
"gcs_bucket_name": "attacker-bucket",
TRUSTED_CALLBACK_VARS_FIELD: {"dd_site": "smuggled.example.com"},
"metadata": {**caller_destinations, "safe_user_metadata": "kept"},
"litellm_metadata": dict(caller_destinations),
"litellm_params": {"metadata": dict(caller_destinations)},
}
updated = await add_litellm_data_to_request(
data=data,
request=_callback_credential_request_mock(),
user_api_key_dict=_DATADOG_TEAM_KEY,
proxy_config=MagicMock(),
general_settings={},
version="test-version",
)
assert "dd_site" not in updated
assert "dd_agent_host" not in updated
assert "gcs_bucket_name" not in updated
assert updated["dd_api_key"] == "team-dd-key"
assert updated[TRUSTED_CALLBACK_VARS_FIELD] == {"dd_api_key": "team-dd-key"}
for metadata_key in ("metadata", "litellm_metadata"):
assert "dd_site" not in updated[metadata_key]
assert "dd_agent_host" not in updated[metadata_key]
assert "dd_site" not in updated["litellm_params"]["metadata"]
assert updated["metadata"]["safe_user_metadata"] == "kept"
@pytest.mark.asyncio
async def test_add_litellm_data_to_request_strips_caller_supplied_callback_credentials_with_clientside_creds_allowed():
"""`allow_client_side_credentials` opens the auth-layer ban; the strip must still hold."""
data = {
"model": "gpt-3.5-turbo",
"messages": [{"role": "user", "content": "hello"}],
"dd_site": "attacker.example.com",
}
updated = await add_litellm_data_to_request(
data=data,
request=_callback_credential_request_mock(),
user_api_key_dict=_DATADOG_TEAM_KEY,
proxy_config=MagicMock(),
general_settings={"allow_client_side_credentials": True},
version="test-version",
)
assert "dd_site" not in updated
assert updated[TRUSTED_CALLBACK_VARS_FIELD] == {"dd_api_key": "team-dd-key"}
@pytest.mark.asyncio
async def test_add_litellm_data_to_request_omits_trusted_callback_vars_without_team_callbacks():
"""Without team/key callback settings the trusted field must not exist for a callback to read."""
data = {
"model": "gpt-3.5-turbo",
"messages": [{"role": "user", "content": "hello"}],
TRUSTED_CALLBACK_VARS_FIELD: {"dd_api_key": "caller-key", "dd_site": "attacker.example.com"},
}
updated = await add_litellm_data_to_request(
data=data,
request=_callback_credential_request_mock(),
user_api_key_dict=UserAPIKeyAuth(api_key="hashed-key"),
proxy_config=MagicMock(),
general_settings={},
version="test-version",
)
assert TRUSTED_CALLBACK_VARS_FIELD not in updated
def test_trusted_callback_vars_never_reach_the_provider():
"""
The stamped field rides the request body, so it has to be a recognised litellm param;
otherwise the OpenAI param builder sweeps it into extra_body and the provider 400s.
"""
from litellm.utils import get_non_default_completion_params
non_default = get_non_default_completion_params(
{
"model": "gpt-4",
TRUSTED_CALLBACK_VARS_FIELD: {"dd_api_key": "team-dd-key"},
"some_provider_param": "kept",
}
)
assert TRUSTED_CALLBACK_VARS_FIELD not in non_default
assert non_default["some_provider_param"] == "kept"
@pytest.mark.asyncio
async def test_key_level_callback_vars_survive_the_strip():
"""
Key-level callbacks configure their own destination and credentials, and they replace
team settings rather than merging with them, so only the request body is untrusted.
"""
key_with_datadog_callback = UserAPIKeyAuth(
api_key="hashed-key",
metadata={
"logging": [
{
"callback_name": "datadog",
"callback_type": "success",
"callback_vars": {"dd_api_key": "key-dd-key", "dd_site": "us5.datadoghq.com"},
}
]
},
)
data = {
"model": "gpt-3.5-turbo",
"messages": [{"role": "user", "content": "hello"}],
"dd_site": "attacker.example.com",
}
updated = await add_litellm_data_to_request(
data=data,
request=_callback_credential_request_mock(),
user_api_key_dict=key_with_datadog_callback,
proxy_config=MagicMock(),
general_settings={},
version="test-version",
)
assert updated[TRUSTED_CALLBACK_VARS_FIELD] == {"dd_api_key": "key-dd-key", "dd_site": "us5.datadoghq.com"}
assert updated["dd_site"] == "us5.datadoghq.com"