diff --git a/.github/workflows/test-unit.yml b/.github/workflows/test-unit.yml index c23678c51ae..b59e90cdfff 100644 --- a/.github/workflows/test-unit.yml +++ b/.github/workflows/test-unit.yml @@ -103,6 +103,7 @@ jobs: tests/test_litellm/completion_extras tests/test_litellm/compression tests/test_litellm/containers + tests/test_litellm/evals tests/test_litellm/experimental_mcp_client tests/test_litellm/models tests/test_litellm/repositories diff --git a/litellm/evals/main.py b/litellm/evals/main.py index 2f639d30ca0..a6e8d66f7c9 100644 --- a/litellm/evals/main.py +++ b/litellm/evals/main.py @@ -5,7 +5,7 @@ Provides create, list, get, update, delete, and cancel operations for evals import asyncio import contextvars -from collections.abc import Coroutine +from collections.abc import Coroutine, Mapping from functools import partial from typing import Final @@ -40,6 +40,48 @@ from litellm.utils import ProviderConfigManager, client base_llm_http_handler = BaseLLMHTTPHandler() DEFAULT_OPENAI_API_BASE: Final = "https://api.openai.com" +# Internal LiteLLM metadata keys that the proxy attaches to `metadata` and that +# must never be forwarded to the provider. +# +# The `user_api_key_` family is matched by prefix rather than by name, matching what +# the proxy already does to user-supplied metadata in +# `litellm/proxy/litellm_pre_call_utils.py` ("Strip spoofable auth metadata"). An +# exact-name list drifts: it currently misses 13 of the 33 `user_api_key_*` keys in +# the codebase, including `user_api_key_token`. +INTERNAL_METADATA_KEY_PREFIXES: Final = ("user_api_key_", "litellm_") + +INTERNAL_METADATA_KEYS: Final = frozenset( + { + "headers", + "requester_metadata", + "user_api_key", + "user_api_end_user_max_budget", + "global_max_parallel_requests", + "endpoint", + "requester_ip_address", + "user_agent", + "spend_logs_metadata", + "proxy_server_request", + "standard_logging_object", + } +) + + +def _is_internal_metadata_key(key: str) -> bool: + return key in INTERNAL_METADATA_KEYS or key.startswith(INTERNAL_METADATA_KEY_PREFIXES) + + +def _user_metadata( + metadata: Mapping[str, object] | None, +) -> dict[str, object] | None: # mutable-ok: stored as the request's metadata field, which the request types as a dict + """Return only the caller's metadata keys, or None if nothing is left.""" + if metadata is None: + return None + filtered: Final = { # mutable-ok: forwarded as the request's metadata field + k: v for k, v in metadata.items() if not _is_internal_metadata_key(k) + } + return filtered or None + @client async def acreate_eval( @@ -170,6 +212,11 @@ def create_eval( if name is not None: create_request["name"] = name + # Filter metadata to exclude internal LiteLLM fields + filtered_metadata: Final = _user_metadata(metadata) + if filtered_metadata is not None: + create_request["metadata"] = filtered_metadata + # Merge extra_body if provided if extra_body: create_request.update(extra_body) @@ -684,44 +731,9 @@ def update_eval( update_request["name"] = name # Filter metadata to exclude internal LiteLLM fields - if metadata is not None: - # List of internal LiteLLM metadata keys that should NOT be sent to OpenAI - internal_keys: Final = { - "headers", - "requester_metadata", - "user_api_key_hash", - "user_api_key_alias", - "user_api_key_spend", - "user_api_key_max_budget", - "user_api_key_team_id", - "user_api_key_user_id", - "user_api_key_org_id", - "user_api_key_team_alias", - "user_api_key_end_user_id", - "user_api_key_user_email", - "user_api_key_request_route", - "user_api_key_budget_reset_at", - "user_api_key_auth_metadata", - "user_api_key", - "user_api_end_user_max_budget", - "user_api_key_auth", - "litellm_api_version", - "global_max_parallel_requests", - "user_api_key_team_max_budget", - "user_api_key_team_spend", - "user_api_key_model_max_budget", - "user_api_key_user_spend", - "user_api_key_user_max_budget", - "user_api_key_metadata", - "endpoint", - "litellm_parent_otel_span", - "requester_ip_address", - "user_agent", - } - # Only include user-provided metadata keys - filtered_metadata: Final = {k: v for k, v in metadata.items() if k not in internal_keys} - if filtered_metadata: # Only add if there's user metadata - update_request["metadata"] = filtered_metadata + filtered_metadata: Final = _user_metadata(metadata) + if filtered_metadata is not None: + update_request["metadata"] = filtered_metadata # Merge extra_body if provided if extra_body: @@ -1218,8 +1230,11 @@ def create_run( } if name is not None: create_request["name"] = name - # if metadata is not None: - # create_request["metadata"] = metadata + + # Filter metadata to exclude internal LiteLLM fields + filtered_metadata: Final = _user_metadata(metadata) + if filtered_metadata is not None: + create_request["metadata"] = filtered_metadata # Merge extra_body if provided if extra_body: diff --git a/tests/test_litellm/evals/__init__.py b/tests/test_litellm/evals/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/test_litellm/evals/test_evals_metadata.py b/tests/test_litellm/evals/test_evals_metadata.py new file mode 100644 index 00000000000..8bb148c4e94 --- /dev/null +++ b/tests/test_litellm/evals/test_evals_metadata.py @@ -0,0 +1,228 @@ +"""Regression tests: create_eval / create_run must forward the caller's metadata. + +Both functions document a ``metadata`` parameter and both request TypedDicts +(``CreateEvalRequest``, ``CreateRunRequest``) declare a ``metadata`` field, but +neither builder ever set it, so the value was accepted and silently dropped. In +``create_run`` the assignment was present but commented out. + +``update_eval`` in the same module already did the right thing, filtering the +internal LiteLLM metadata keys the proxy attaches before forwarding the rest. +These tests pin that same behaviour onto the two create paths: the caller's keys +reach the request body, and the internal keys never do. +""" + +import pytest + +import litellm +from litellm.evals.main import INTERNAL_METADATA_KEYS, INTERNAL_METADATA_KEY_PREFIXES + +_DATA_SOURCE_CONFIG = {"type": "custom", "item_schema": {"type": "object"}} +_TESTING_CRITERIA = [{"type": "label_model", "name": "grader"}] +_DATA_SOURCE = {"type": "jsonl", "source": {"type": "file_id", "id": "file-123"}} + + +@pytest.fixture +def captured_body(monkeypatch): + """Capture the request body handed to the HTTP layer, without a network call.""" + seen = {} + + def _capture(handler_name, response): + def _handler(*args, **kwargs): + seen["request_body"] = kwargs["request_body"] + return response + + monkeypatch.setattr(litellm.evals.main.base_llm_http_handler, handler_name, _handler) + + _capture( + "create_eval_handler", + litellm.types.llms.openai_evals.Eval( + id="eval-123", + created_at=0, + data_source_config=_DATA_SOURCE_CONFIG, + testing_criteria=_TESTING_CRITERIA, + ), + ) + _capture( + "update_eval_handler", + litellm.types.llms.openai_evals.Eval( + id="eval-123", + created_at=0, + data_source_config=_DATA_SOURCE_CONFIG, + testing_criteria=_TESTING_CRITERIA, + ), + ) + _capture( + "create_run_handler", + litellm.types.llms.openai_evals.Run( + id="run-123", + eval_id="eval-123", + created_at=0, + status="queued", + model="gpt-4o", + data_source=_DATA_SOURCE, + ), + ) + return seen + + +def test_create_eval_forwards_caller_metadata(captured_body): + litellm.create_eval( + data_source_config=_DATA_SOURCE_CONFIG, + testing_criteria=_TESTING_CRITERIA, + metadata={"team": "search", "run_by": "nightly"}, + api_key="sk-test", + ) + assert captured_body["request_body"]["metadata"] == { + "team": "search", + "run_by": "nightly", + } + + +def test_create_run_forwards_caller_metadata(captured_body): + litellm.create_run( + eval_id="eval-123", + data_source=_DATA_SOURCE, + metadata={"team": "search"}, + api_key="sk-test", + ) + assert captured_body["request_body"]["metadata"] == {"team": "search"} + + +@pytest.mark.parametrize("internal_key", sorted(INTERNAL_METADATA_KEYS)) +def test_create_eval_strips_internal_metadata_keys(captured_body, internal_key): + # A dict value keeps every key valid for the logging path, which reads some + # of these (e.g. requester_metadata) as mappings on the way through. + litellm.create_eval( + data_source_config=_DATA_SOURCE_CONFIG, + testing_criteria=_TESTING_CRITERIA, + metadata={internal_key: {"leaked": True}, "team": "search"}, + api_key="sk-test", + ) + assert captured_body["request_body"]["metadata"] == {"team": "search"} + + +def test_create_eval_omits_metadata_when_only_internal_keys(captured_body): + litellm.create_eval( + data_source_config=_DATA_SOURCE_CONFIG, + testing_criteria=_TESTING_CRITERIA, + metadata={"user_api_key_hash": "leaked", "endpoint": "/v1/evals"}, + api_key="sk-test", + ) + assert "metadata" not in captured_body["request_body"] + + +def test_create_eval_omits_metadata_when_not_given(captured_body): + litellm.create_eval( + data_source_config=_DATA_SOURCE_CONFIG, + testing_criteria=_TESTING_CRITERIA, + api_key="sk-test", + ) + assert "metadata" not in captured_body["request_body"] + + +@pytest.mark.parametrize( + "internal_key", + [ + # the exact-name list used to miss every one of these + "user_api_key_token", + "user_api_key_team_metadata", + "user_api_key_object_permission_id", + "user_api_key_team_object_permission_id", + "user_api_key_budget_reservation", + "user_api_key_org_alias", + "user_api_key_project_id", + "litellm_parent_otel_span", + "litellm_api_version", + ], +) +def test_create_eval_strips_prefixed_internal_keys(captured_body, internal_key): + litellm.create_eval( + data_source_config=_DATA_SOURCE_CONFIG, + testing_criteria=_TESTING_CRITERIA, + metadata={internal_key: {"leaked": True}, "team": "search"}, + api_key="sk-test", + ) + assert captured_body["request_body"]["metadata"] == {"team": "search"} + + +def test_every_user_api_key_metadata_key_is_covered(): + """The proxy strips this family by prefix, so this filter must too. + + Pinned as a property rather than a list so a newly added user_api_key_* field + cannot start leaking just because nobody remembered to extend a denylist. + """ + from litellm.evals.main import _is_internal_metadata_key + + assert "user_api_key_" in INTERNAL_METADATA_KEY_PREFIXES + for key in ("user_api_key_token", "user_api_key_hash", "user_api_key_some_field_added_in_2027"): + assert _is_internal_metadata_key(key) + # a caller key that merely mentions the words is not internal + assert not _is_internal_metadata_key("my_user_api_key_note") + assert not _is_internal_metadata_key("team") + + +# The exact 30-name list that update_eval carried before this change. The prefix filter +# has to be a strict superset of it, or hoisting the filter would have quietly narrowed +# what update_eval strips. +_PREVIOUS_DENYLIST = { + "headers", + "requester_metadata", + "user_api_key_hash", + "user_api_key_alias", + "user_api_key_spend", + "user_api_key_max_budget", + "user_api_key_team_id", + "user_api_key_user_id", + "user_api_key_org_id", + "user_api_key_team_alias", + "user_api_key_end_user_id", + "user_api_key_user_email", + "user_api_key_request_route", + "user_api_key_budget_reset_at", + "user_api_key_auth_metadata", + "user_api_key", + "user_api_end_user_max_budget", + "user_api_key_auth", + "litellm_api_version", + "global_max_parallel_requests", + "user_api_key_team_max_budget", + "user_api_key_team_spend", + "user_api_key_model_max_budget", + "user_api_key_user_spend", + "user_api_key_user_max_budget", + "user_api_key_metadata", + "endpoint", + "litellm_parent_otel_span", + "requester_ip_address", + "user_agent", +} + + +def test_prefix_filter_covers_everything_the_old_denylist_did(): + from litellm.evals.main import _is_internal_metadata_key + + missed = sorted(k for k in _PREVIOUS_DENYLIST if not _is_internal_metadata_key(k)) + assert not missed, f"prefix filter no longer strips {missed}" + + +def test_update_eval_filtering_is_unchanged(captured_body): + """Hoisting the filter out of update_eval must not change what update_eval does. + + This is the path that already worked before the PR, so it is the one most at risk + from the refactor. + """ + litellm.update_eval( + eval_id="eval-123", + metadata={"team": "search", "user_api_key_hash": "leaked", "user_api_key_token": "leaked"}, + api_key="sk-test", + ) + assert captured_body["request_body"]["metadata"] == {"team": "search"} + + +def test_update_eval_omits_metadata_when_only_internal_keys(captured_body): + litellm.update_eval( + eval_id="eval-123", + metadata={"user_api_key_hash": "leaked"}, + api_key="sk-test", + ) + assert "metadata" not in captured_body["request_body"]