Merge pull request #27081 from stuxf/fix/strip-callback-fields

chore(proxy): close callback-config and observability-credential side channels

litellm/proxy/auth/auth_utils.py

@@ -2,7 +2,7 @@ import os
 import re
 import sys
 from functools import lru_cache
-from typing import Any, Dict, List, Mapping, Optional, Tuple, Union
+from typing import Any, Dict, FrozenSet, List, Mapping, Optional, Tuple, Union
 
 from fastapi import HTTPException, Request, status
 
@@ -173,9 +173,67 @@ def _allow_model_level_clientside_configurable_parameters(
 # threat shape should be added here.
 _NESTED_CONFIG_KEYS: Tuple[str, ...] = ("litellm_embedding_config",)
 
-# Banned root-level params. Same list applies to every entry in
-# ``_NESTED_CONFIG_KEYS`` because those dicts get spread as ``**kwargs``
-# into the same outbound calls.
+# Metadata containers that carry per-request configuration consumed by the
+# observability callbacks. The same banned-param list applies — a value
+# under ``metadata.langfuse_host`` redirects the same Langfuse client and
+# leaks the same credentials as the root-level ``langfuse_host``, but the
+# original check only walked the request-body root, so the metadata path
+# was an unintentional bypass.
+_NESTED_METADATA_KEYS: Tuple[str, ...] = ("metadata", "litellm_metadata")
+
+# Banned request-body params. The same list applies to every entry in
+# ``_NESTED_CONFIG_KEYS`` (dicts spread as ``**kwargs`` into outbound
+# calls) and ``_NESTED_METADATA_KEYS`` (dicts read directly by integration
+# callbacks), so a single banned name is enforced wherever the field can
+# reach the call path from.
+# Per-request observability params that are SAFE to accept from clients.
+# These describe the request being logged (prompt version, sampling rate)
+# without choosing the destination or the credentials, so they don't
+# contribute to the data-exfil primitive that the rest of
+# ``_supported_callback_params`` does.
+_SAFE_CLIENT_CALLBACK_PARAMS: FrozenSet[str] = frozenset(
+    {
+        "langfuse_prompt_version",
+        "langsmith_sampling_rate",
+    }
+)
+
+# Observability fields that integrations read from the request body or
+# metadata but that are not (yet) listed in ``_supported_callback_params``.
+# Listed here so the proxy bans them today; the long-term cleanup is to
+# fold these into the canonical allowlist so they share one source of
+# truth with the rest.
+_EXTRA_BANNED_OBSERVABILITY_PARAMS: FrozenSet[str] = frozenset(
+    {
+        "posthog_api_url",
+        "phoenix_project_name",
+        "wandb_api_key",
+        "weave_project_id",
+    }
+)
+
+
+def _build_banned_observability_params() -> FrozenSet[str]:
+    """Derive the observability ban list from the canonical allowlist.
+
+    ``_supported_callback_params`` in
+    ``litellm/litellm_core_utils/initialize_dynamic_callback_params.py`` is
+    the single place that enumerates every observability field
+    integrations resolve from kwargs/metadata. Subtract the small set of
+    informational fields (``_SAFE_CLIENT_CALLBACK_PARAMS``) and union with
+    the extras the canonical allowlist hasn't caught up to yet. New
+    integrations added to the canonical allowlist are banned by default,
+    which is the safe failure mode.
+    """
+    from litellm.litellm_core_utils.initialize_dynamic_callback_params import (
+        _supported_callback_params,
+    )
+
+    return (
+        frozenset(_supported_callback_params) - _SAFE_CLIENT_CALLBACK_PARAMS
+    ) | _EXTRA_BANNED_OBSERVABILITY_PARAMS
+
+
 _BANNED_REQUEST_BODY_PARAMS: Tuple[str, ...] = (
     "api_base",
     "base_url",
@@ -190,11 +248,6 @@ _BANNED_REQUEST_BODY_PARAMS: Tuple[str, ...] = (
     # tokens) to the attacker's host, or coerces the proxy into
     # authenticating against the attacker's host with admin secrets.
     "aws_bedrock_runtime_endpoint",
-    "langsmith_base_url",
-    "langfuse_host",
-    "posthog_host",
-    "braintrust_host",
-    "slack_webhook_url",
     # Provider-specific endpoint overrides that flow into the outbound
     # request via ``optional_params``. Same threat as ``api_base``:
     # ``s3_endpoint_url`` redirects Bedrock file uploads to attacker
@@ -203,6 +256,11 @@ _BANNED_REQUEST_BODY_PARAMS: Tuple[str, ...] = (
     "s3_endpoint_url",
     "sagemaker_base_url",
     "deployment_url",
+    # Observability credentials, hosts, and project identifiers: derived
+    # from the canonical ``_supported_callback_params`` allowlist so new
+    # integrations are covered automatically. Sorted for stable iteration
+    # order and reviewable diffs.
+    *sorted(_build_banned_observability_params()),
 )
 
 
@@ -221,6 +279,8 @@ def _check_banned_params(
         if param not in body:
             continue
         if general_settings.get("allow_client_side_credentials") is True:
+            # Proxy-wide opt-in: every banned param is permitted, exit
+            # entirely so the rest of the loop doesn't waste work.
             return
         if (
             _allow_model_level_clientside_configurable_parameters(
@@ -231,7 +291,12 @@ def _check_banned_params(
             )
             is True
         ):
-            return
+            # Per-param opt-in: only THIS param is permitted by the
+            # deployment's ``configurable_clientside_auth_params``. Skip
+            # to the next banned param so a body that pairs an allowed
+            # ``api_base`` with an unallowed ``langfuse_host`` is still
+            # rejected for the second field.
+            continue
         raise ValueError(
             f"Rejected Request: {param} is not allowed in request body. "
             "Clientside passthrough requires explicit admin opt-in via "
@@ -275,9 +340,33 @@ def is_request_body_safe(
         nested = request_body.get(nested_key)
         if isinstance(nested, dict):
             _check_banned_params(nested, general_settings, llm_router, model)
+    for metadata_key in _NESTED_METADATA_KEYS:
+        metadata = _coerce_metadata_to_dict(request_body.get(metadata_key))
+        if metadata is not None:
+            _check_banned_params(metadata, general_settings, llm_router, model)
     return True
 
 
+def _coerce_metadata_to_dict(value: Any) -> Optional[Dict[str, Any]]:
+    """Return ``value`` as a dict, parsing it from JSON if delivered as a string.
+
+    Multipart/form-data and ``extra_body`` callers send ``litellm_metadata``
+    as a JSON-encoded string; the proxy parses it into a dict later in
+    ``add_litellm_data_to_request``, but the auth-time bouncer runs first
+    and would otherwise miss the banned-param check on a still-stringified
+    metadata blob.
+    """
+    if isinstance(value, dict):
+        return value
+    if isinstance(value, str):
+        from litellm.litellm_core_utils.safe_json_loads import safe_json_loads
+
+        parsed = safe_json_loads(value)
+        if isinstance(parsed, dict):
+            return parsed
+    return None
+
+
 async def pre_db_read_auth_checks(
     request: Request,
     request_data: dict,

litellm/proxy/litellm_pre_call_utils.py

@@ -121,6 +121,19 @@ _UNTRUSTED_ROOT_CONTROL_FIELDS = (
     "pillar_response_headers",
     "_guardrail_pipelines",
     "_pipeline_managed_guardrails",
+    # Callback-registration fields. ``callbacks``, ``service_callback``,
+    # and ``logger_fn`` are read by ``litellm.utils.function_setup`` and
+    # appended to process-wide ``litellm.{input,success,failure,_async_*,
+    # service}_callback`` lists / ``litellm.user_logger_fn`` — one request
+    # poisons the worker for every subsequent caller.
+    # ``litellm_disabled_callbacks`` is the inverse primitive: the
+    # legitimate path reads it from key/team metadata, the request-body
+    # version silently turns off admin-configured audit/observability
+    # for the caller's request.
+    "callbacks",
+    "service_callback",
+    "logger_fn",
+    "litellm_disabled_callbacks",
 )
 
 _UNTRUSTED_METADATA_CONTROL_FIELDS = (

tests/test_litellm/proxy/auth/test_auth_utils.py

@@ -1292,3 +1292,219 @@ class TestIsRequestBodySafeNestedConfig:
             )
             is True
         )
+
+
+# ── observability-callback ban (root + metadata) ───────────────────────────
+
+
+class TestObservabilityCallbackBans:
+    """The proxy must reject observability credentials, hosts, and project
+    identifiers regardless of whether they arrive at the request body root,
+    in ``metadata`` / ``litellm_metadata``, or in a JSON-string-encoded
+    metadata blob (multipart/``extra_body`` path).
+
+    The ban list is derived from
+    ``litellm.litellm_core_utils.initialize_dynamic_callback_params._supported_callback_params``
+    minus a small ``_SAFE_CLIENT_CALLBACK_PARAMS`` allow-list, plus
+    ``_EXTRA_BANNED_OBSERVABILITY_PARAMS`` for fields integrations read but
+    that are not yet in the canonical allow-list. The derivation keeps the
+    proxy in sync as new integrations are added.
+    """
+
+    @pytest.fixture(autouse=True)
+    def _disable_url_validation(self, monkeypatch):
+        import litellm
+
+        monkeypatch.setattr(litellm, "user_url_validation", False, raising=False)
+
+    @pytest.mark.parametrize(
+        "field",
+        [
+            "langfuse_public_key",
+            "langfuse_secret",
+            "langfuse_secret_key",
+            "langsmith_api_key",
+            "langsmith_project",
+            "langsmith_tenant_id",
+            "arize_api_key",
+            "arize_space_key",
+            "arize_space_id",
+            "posthog_api_key",
+            "posthog_api_url",
+            "braintrust_api_key",
+            "braintrust_project",
+            "phoenix_project_name",
+            "wandb_api_key",
+            "weave_project_id",
+            "gcs_bucket_name",
+            "gcs_path_service_account",
+            "humanloop_api_key",
+            "lunary_public_key",
+        ],
+    )
+    def test_observability_field_in_request_body_root_is_rejected(self, field):
+        with pytest.raises(ValueError) as exc:
+            is_request_body_safe(
+                request_body={"model": "gpt-4", field: "attacker-value"},
+                general_settings={},
+                llm_router=None,
+                model="gpt-4",
+            )
+        assert field in str(exc.value)
+
+    @pytest.mark.parametrize(
+        "metadata_key",
+        ["metadata", "litellm_metadata"],
+    )
+    @pytest.mark.parametrize(
+        "field",
+        [
+            "langfuse_host",
+            "langfuse_secret_key",
+            "langsmith_api_key",
+            "posthog_api_url",
+            "braintrust_project",
+            "phoenix_project_name",
+        ],
+    )
+    def test_observability_field_in_metadata_dict_is_rejected(
+        self, metadata_key, field
+    ):
+        # Verifies the metadata walk: a value smuggled inside ``metadata``
+        # or ``litellm_metadata`` is just as dangerous as the same field
+        # at the body root, and must hit the same gate.
+        with pytest.raises(ValueError) as exc:
+            is_request_body_safe(
+                request_body={
+                    "model": "gpt-4",
+                    metadata_key: {field: "attacker-value"},
+                },
+                general_settings={},
+                llm_router=None,
+                model="gpt-4",
+            )
+        assert field in str(exc.value)
+
+    @pytest.mark.parametrize(
+        "metadata_key",
+        ["metadata", "litellm_metadata"],
+    )
+    def test_observability_field_in_json_string_metadata_is_rejected(
+        self, metadata_key
+    ):
+        # Multipart/form-data and ``extra_body`` callers send metadata as a
+        # JSON-encoded string. The bouncer parses it before applying the
+        # banned-params check so the JSON-string path can't smuggle past
+        # the ``isinstance(dict)`` guard.
+        import json
+
+        with pytest.raises(ValueError) as exc:
+            is_request_body_safe(
+                request_body={
+                    "model": "gpt-4",
+                    metadata_key: json.dumps(
+                        {"langfuse_host": "https://attacker.example"}
+                    ),
+                },
+                general_settings={},
+                llm_router=None,
+                model="gpt-4",
+            )
+        assert "langfuse_host" in str(exc.value)
+
+    def test_admin_opt_in_allows_metadata_credential_passthrough(self):
+        # The opt-in gate covers the metadata path the same way it covers
+        # the root path — operators running BYO observability with
+        # clientside creds flip a single flag and both paths work.
+        assert (
+            is_request_body_safe(
+                request_body={
+                    "model": "gpt-4",
+                    "metadata": {
+                        "langfuse_host": "https://my-langfuse.example",
+                        "langfuse_public_key": "pk-mine",
+                        "langfuse_secret_key": "sk-mine",
+                    },
+                },
+                general_settings={"allow_client_side_credentials": True},
+                llm_router=None,
+                model="gpt-4",
+            )
+            is True
+        )
+
+    def test_safe_per_request_observability_metadata_is_allowed(self):
+        # Informational fields (sampling rate, prompt version) describe
+        # the request being logged — they don't choose the destination or
+        # credentials, so they must remain accepted from clients without
+        # the opt-in flag.
+        assert (
+            is_request_body_safe(
+                request_body={
+                    "model": "gpt-4",
+                    "metadata": {
+                        "langfuse_prompt_version": "v2",
+                        "langsmith_sampling_rate": 0.1,
+                    },
+                },
+                general_settings={},
+                llm_router=None,
+                model="gpt-4",
+            )
+            is True
+        )
+
+
+def test_model_level_allow_does_not_skip_subsequent_banned_params(monkeypatch):
+    """Greptile P1: ``_check_banned_params`` previously ``return``-ed when a
+    deployment's ``configurable_clientside_auth_params`` permitted one
+    banned field, exiting before any later banned field in the same body
+    was checked. The metadata walk this PR adds multiplies the surface
+    where that bypass matters: a body pairing a model-level-allowed
+    ``api_base`` with an observability credential like ``langfuse_host``
+    must still reject on the second field, not silently pass."""
+    from litellm.proxy.auth import auth_utils
+
+    monkeypatch.setattr(
+        auth_utils,
+        "_allow_model_level_clientside_configurable_parameters",
+        lambda model, param, request_body_value, llm_router: param == "api_base",
+    )
+
+    with pytest.raises(ValueError) as exc:
+        is_request_body_safe(
+            request_body={
+                "model": "gpt-4",
+                "api_base": "https://allowed-by-deployment.example",
+                "langfuse_host": "https://attacker.example",
+            },
+            general_settings={},
+            llm_router=None,
+            model="gpt-4",
+        )
+    assert "langfuse_host" in str(exc.value)
+
+
+def test_observability_ban_covers_canonical_supported_callback_params():
+    """Guard test: every entry in the canonical
+    ``_supported_callback_params`` allow-list must end up either banned by
+    the proxy or explicitly safe-listed. New integrations added to that
+    list are banned by default (the safe failure mode); flagging them as
+    safe is an explicit decision recorded in
+    ``_SAFE_CLIENT_CALLBACK_PARAMS``."""
+    from litellm.litellm_core_utils.initialize_dynamic_callback_params import (
+        _supported_callback_params,
+    )
+    from litellm.proxy.auth.auth_utils import (
+        _BANNED_REQUEST_BODY_PARAMS,
+        _SAFE_CLIENT_CALLBACK_PARAMS,
+    )
+
+    banned = set(_BANNED_REQUEST_BODY_PARAMS)
+    for param in _supported_callback_params:
+        assert param in banned or param in _SAFE_CLIENT_CALLBACK_PARAMS, (
+            f"{param} is in _supported_callback_params but neither banned nor "
+            f"safe-listed. Add it to _SAFE_CLIENT_CALLBACK_PARAMS if it is an "
+            f"informational per-request field; otherwise the derivation will "
+            f"ban it automatically."
+        )

tests/test_litellm/proxy/test_litellm_pre_call_utils.py

@@ -596,6 +596,59 @@ async def test_add_litellm_data_to_request_strips_user_control_fields():
     assert "pillar_response_headers" not in snapshot_body["metadata"]
 
 
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "control_field",
+    ["callbacks", "service_callback", "logger_fn", "litellm_disabled_callbacks"],
+)
+async def test_add_litellm_data_to_request_strips_callback_control_fields(
+    control_field,
+):
+    """``callbacks`` / ``service_callback`` / ``logger_fn`` get appended to
+    the worker-wide ``litellm.{input,success,failure,_async_*,service}_callback``
+    lists and ``litellm.user_logger_fn`` from inside ``function_setup`` —
+    one request poisons every subsequent caller in that worker.
+    ``litellm_disabled_callbacks`` is the inverse: a request-body value
+    silently disables admin-configured audit/observability for the call.
+    None has a documented per-request use, so all four are stripped at
+    the proxy boundary alongside the existing internal-only fields."""
+    request_mock = MagicMock(spec=Request)
+    request_mock.url.path = "/v1/chat/completions"
+    request_mock.url = MagicMock()
+    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"
+
+    sample_value = (
+        ["langfuse"]
+        if control_field
+        in ("callbacks", "service_callback", "litellm_disabled_callbacks")
+        else "module.func"
+    )
+
+    updated = await add_litellm_data_to_request(
+        data={
+            "model": "gpt-3.5-turbo",
+            "messages": [{"role": "user", "content": "hi"}],
+            control_field: sample_value,
+        },
+        request=request_mock,
+        user_api_key_dict=UserAPIKeyAuth(api_key="hashed-key"),
+        proxy_config=MagicMock(),
+        general_settings={},
+        version="test-version",
+    )
+
+    assert control_field not in updated
+    # The post-strip body snapshot used by audit/spend logging must also
+    # not retain the attacker-injected control field.
+    snapshot_body = updated["proxy_server_request"]["body"]
+    assert control_field not in snapshot_body
+
+
 @pytest.mark.asyncio
 async def test_add_litellm_data_to_request_allows_client_mock_response_with_admin_opt_in():
     request_mock = MagicMock(spec=Request)