diff --git a/litellm/proxy/hooks/tag_rate_limits_shared.py b/litellm/proxy/hooks/tag_rate_limits_shared.py index 34989e5bf5a..c16003cd3bf 100644 --- a/litellm/proxy/hooks/tag_rate_limits_shared.py +++ b/litellm/proxy/hooks/tag_rate_limits_shared.py @@ -213,13 +213,19 @@ def resolve_success_event_metadata_variable_name( real, populated `metadata` dict for a standard (non LITELLM_METADATA_ROUTES) request, so the key-presence check always picks `litellm_metadata` there and silently reads no tags/identity at all. - Requiring the value to actually be a populated dict, matching - `_get_request_tags`'s own truthiness check in litellm_logging.py, only + + A plain truthiness check on `litellm_metadata` isn't enough either: a + caller can populate it with unrelated, non-empty content on a route where + `metadata` is the field the proxy actually wrote identity into, and + truthiness alone would still misresolve to the caller-controlled bucket. + `add_user_api_key_auth_to_request_metadata` (litellm_pre_call_utils.py) + unconditionally stamps a `user_api_key_auth` key into whichever bucket it + resolved as authoritative, overwriting anything a caller pre-populated + there -- so requiring that marker's presence, not mere truthiness, only ever prefers `litellm_metadata` when it is genuinely the field the proxy - wrote identity/tags into (LITELLM_METADATA_ROUTES pre-seed it before - admission runs, so it is always a populated dict by success time there).""" + wrote identity/tags into.""" litellm_metadata: Final = litellm_params_for_metadata.get("litellm_metadata") - if isinstance(litellm_metadata, Mapping) and litellm_metadata: + if isinstance(litellm_metadata, Mapping) and "user_api_key_auth" in litellm_metadata: return "litellm_metadata" return "metadata" diff --git a/tests/test_litellm/proxy/hooks/test_tag_rate_limits_shared.py b/tests/test_litellm/proxy/hooks/test_tag_rate_limits_shared.py index d645b09075c..7bb9d04fef0 100644 --- a/tests/test_litellm/proxy/hooks/test_tag_rate_limits_shared.py +++ b/tests/test_litellm/proxy/hooks/test_tag_rate_limits_shared.py @@ -16,6 +16,7 @@ from litellm.proxy.hooks.tag_rate_limits_shared import ( fixed_length_identity, order_tags_for_identity_resolution, partition_key, + resolve_success_event_metadata_variable_name, ) from litellm.types.router import TagRateLimitEntry, TagRateLimitScope @@ -124,6 +125,43 @@ def test_extract_key_hash_reads_metadata_when_it_is_the_authoritative_field(): assert extract_key_hash(request_kwargs, "metadata") == "real-hash" +# --------------------------------------------------------------------------- +# resolve_success_event_metadata_variable_name -- veria-ai finding surfaced on +# PR #38347: a caller-supplied, non-empty litellm_metadata must not be +# selected over the metadata the proxy actually wrote authenticated tags into +# --------------------------------------------------------------------------- + + +def test_resolve_success_event_metadata_variable_name_ignores_caller_forged_non_empty_litellm_metadata(): + """A caller-supplied litellm_metadata with unrelated content (no + user_api_key_auth marker) must not be picked over metadata, even though + it is a non-empty dict.""" + litellm_params_for_metadata = {"litellm_metadata": {"marker": True}} + assert resolve_success_event_metadata_variable_name(litellm_params_for_metadata) == "metadata" + + +def test_resolve_success_event_metadata_variable_name_selects_litellm_metadata_when_server_written(): + """A genuinely server-populated litellm_metadata (LITELLM_METADATA_ROUTES) + always carries the user_api_key_auth marker stamped by + add_user_api_key_auth_to_request_metadata.""" + litellm_params_for_metadata = { + "litellm_metadata": {"user_api_key_auth": object(), "tags": ["team_id:t1"]} + } + assert resolve_success_event_metadata_variable_name(litellm_params_for_metadata) == "litellm_metadata" + + +def test_resolve_success_event_metadata_variable_name_defaults_to_metadata_when_litellm_metadata_absent(): + assert resolve_success_event_metadata_variable_name({}) == "metadata" + + +def test_resolve_success_event_metadata_variable_name_defaults_to_metadata_when_litellm_metadata_none(): + assert resolve_success_event_metadata_variable_name({"litellm_metadata": None}) == "metadata" + + +def test_resolve_success_event_metadata_variable_name_defaults_to_metadata_when_litellm_metadata_empty(): + assert resolve_success_event_metadata_variable_name({"litellm_metadata": {}}) == "metadata" + + # --------------------------------------------------------------------------- # entry_applies -- enabled_for / disabled_for / apply_to_key_alias / apply_to_models # ---------------------------------------------------------------------------