From 539a53c7934229a2c571b988c1a03e234a8ecc89 Mon Sep 17 00:00:00 2001 From: Vivek1898 <72497717+Vivek1898@users.noreply.github.com> Date: Fri, 9 Oct 2026 17:24:54 +0530 Subject: [PATCH 1/8] fix(guardrails): write key metadata into the request's resolved bucket A pre_call guardrail wrote `data["litellm_metadata"]` directly. Every reader of proxy-internal metadata resolves the bucket through `get_metadata_variable_name_from_kwargs`, which picks by presence: return "litellm_metadata" if "litellm_metadata" in kwargs else "metadata" So on a chat-completions request, whose bucket is `metadata`, creating that key moved every later read onto a dict holding only the `user_api_key_*` keys. The tags and router fields stayed behind in `metadata`, unread: - `get_deployments_for_tag` finds no tags, so a request tagged for one model is served by a deployment of another - SpendLogs lose `model_group` and `model_id` and nothing raises, because neither reader treats an empty bucket as an error. Reported as #45543 with all three cases. `BaseTranslation.merge_user_api_key_metadata_into_request` already exists for exactly this, and its docstring names this failure: "creating a `litellm_metadata` key on a route whose bucket is `metadata` (chat completions) flips the bucket for every later metadata write, and spend logging never sees those writes". It resolves the bucket via `get_or_create_metadata_bucket` and `setdefault`s each key, so a value already resolved upstream is never overwritten. The OpenAI chat guardrail handler calls it in three places. So this is not a new policy; it is the one already documented here, applied at the last site that bypassed it. Both hook call sites share the helper, so both are covered. Five tests. The third is the one that matters for review: a batch/file request that genuinely uses `litellm_metadata` must still write there, so the bucket stays resolved rather than hardcoded the other way round. Verified by reverting only the source file: 4 of the 5 fail, two on the assertion and two with `KeyError: 'user_api_key_team_id'`. The fifth, no `user_api_key_dict` writing no bucket at all, passes either way, which is correct. ruff check, ruff format and the test-quality gate are clean on both files. --- .../unified_guardrail/unified_guardrail.py | 29 ++++-- .../test_unified_guardrail_metadata_bucket.py | 96 +++++++++++++++++++ 2 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 tests/unit/proxy/guardrails/guardrail_hooks/test_unified_guardrail_metadata_bucket.py diff --git a/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py b/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py index 5ce2dfb3b87..cb8409ce13c 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py +++ b/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py @@ -223,15 +223,28 @@ def _a2a_jsonrpc_error_chunk(exc: HTTPException, request_id: str | None) -> Mapp def _ensure_litellm_metadata(data: dict, user_api_key_dict: UserAPIKeyAuth) -> None: - """Populate data['litellm_metadata'] from user_api_key_dict if absent.""" - if "litellm_metadata" not in data: - from litellm.llms.base_llm.guardrail_translation.base_translation import ( - BaseTranslation, - ) + """Merge the ``user_api_key_*`` metadata into the request's resolved bucket. - user_metadata: Final = BaseTranslation.transform_user_api_key_dict_to_metadata(user_api_key_dict) - if user_metadata: - data["litellm_metadata"] = user_metadata + Writing ``data["litellm_metadata"]`` directly creates a second bucket on a + route whose bucket is ``metadata`` (chat completions). Every reader resolves + the bucket through ``get_metadata_variable_name_from_kwargs``, which answers + ``litellm_metadata`` as soon as that key merely exists, so from then on they + all read the new dict -- which holds only the ``user_api_key_*`` keys. The + tags and router fields stay behind in ``metadata``, unread: + ``get_deployments_for_tag`` finds no tags and routes the request to a + deployment of another model, and SpendLogs lose ``model_group`` and + ``model_id``. Nothing raises (#45543). + + ``BaseTranslation.merge_user_api_key_metadata_into_request`` is the sanctioned + writer for exactly this -- it resolves the bucket via + ``get_or_create_metadata_bucket`` and ``setdefault``s each key, so an existing + value is never overwritten. The OpenAI chat guardrail handler already uses it. + """ + from litellm.llms.base_llm.guardrail_translation.base_translation import ( + BaseTranslation, + ) + + BaseTranslation.merge_user_api_key_metadata_into_request(data, user_api_key_dict) class UnifiedLLMGuardrails(CustomLogger): diff --git a/tests/unit/proxy/guardrails/guardrail_hooks/test_unified_guardrail_metadata_bucket.py b/tests/unit/proxy/guardrails/guardrail_hooks/test_unified_guardrail_metadata_bucket.py new file mode 100644 index 00000000000..7803d379d74 --- /dev/null +++ b/tests/unit/proxy/guardrails/guardrail_hooks/test_unified_guardrail_metadata_bucket.py @@ -0,0 +1,96 @@ +"""Regression tests for #45543. + +A pre_call guardrail used to write ``data["litellm_metadata"]`` directly. Every +reader resolves the proxy-internal bucket through +``get_metadata_variable_name_from_kwargs``, which answers ``litellm_metadata`` as +soon as that key merely exists, so creating it on a chat-completions request (whose +bucket is ``metadata``) moved every later read onto a dict holding only the +``user_api_key_*`` keys. Tags and router fields stayed behind in ``metadata``: +tag-based routing found no tags and picked a deployment of another model, and +SpendLogs lost ``model_group`` and ``model_id``. Nothing raised. +""" + +from typing import Final + +import pytest + +from litellm.litellm_core_utils.core_helpers import ( + get_metadata_variable_name_from_kwargs, +) +from litellm.proxy._types import UserAPIKeyAuth +from litellm.proxy.guardrails.guardrail_hooks.unified_guardrail.unified_guardrail import ( + _ensure_litellm_metadata, +) + + +@pytest.fixture +def user_api_key_dict() -> UserAPIKeyAuth: + return UserAPIKeyAuth(api_key="sk-test-45543", user_id="user-1", team_id="team-1") + + +def test_chat_completions_request_keeps_its_metadata_bucket(user_api_key_dict: UserAPIKeyAuth) -> None: + """The reported shape: proxy state already lives in ``metadata``.""" + data: Final = { + "model": "all-models", + "metadata": {"tags": ["model:glm-5"], "model_group": "all-models"}, + } + + _ensure_litellm_metadata(data, user_api_key_dict) + + # The bug was creating this key at all; its presence is what flips the readers. + assert "litellm_metadata" not in data + assert get_metadata_variable_name_from_kwargs(data) == "metadata" + # The fields the routing and spend-logging readers need are still where they look. + assert data["metadata"]["tags"] == ["model:glm-5"] + assert data["metadata"]["model_group"] == "all-models" + # And the guardrail still gets the key metadata it asked for. + assert data["metadata"]["user_api_key_team_id"] == "team-1" + assert data["metadata"]["user_api_key_user_id"] == "user-1" + + +def test_existing_metadata_values_are_not_overwritten(user_api_key_dict: UserAPIKeyAuth) -> None: + """Merging is ``setdefault``: a value already resolved upstream wins.""" + data: Final = { + "model": "all-models", + "metadata": {"user_api_key_user_id": "resolved-upstream"}, + } + + _ensure_litellm_metadata(data, user_api_key_dict) + + assert data["metadata"]["user_api_key_user_id"] == "resolved-upstream" + assert data["metadata"]["user_api_key_team_id"] == "team-1" + + +def test_litellm_metadata_route_still_uses_litellm_metadata(user_api_key_dict: UserAPIKeyAuth) -> None: + """Batch and file routes really do keep proxy state in ``litellm_metadata``. + + The fix must not flip those the other way, so the bucket is resolved rather + than hardcoded in either direction. + """ + data: Final = {"model": "m", "litellm_metadata": {"tags": ["keep-me"]}} + + _ensure_litellm_metadata(data, user_api_key_dict) + + assert "metadata" not in data + assert get_metadata_variable_name_from_kwargs(data) == "litellm_metadata" + assert data["litellm_metadata"]["tags"] == ["keep-me"] + assert data["litellm_metadata"]["user_api_key_team_id"] == "team-1" + + +def test_request_without_any_metadata_gets_the_default_bucket(user_api_key_dict: UserAPIKeyAuth) -> None: + data: Final = {"model": "m"} + + _ensure_litellm_metadata(data, user_api_key_dict) + + assert "litellm_metadata" not in data + assert data["metadata"]["user_api_key_team_id"] == "team-1" + + +def test_no_user_api_key_metadata_creates_no_bucket() -> None: + """``None`` yields no metadata, so nothing should be written at all.""" + data: Final = {"model": "m"} + + _ensure_litellm_metadata(data, None) # type: ignore[arg-type] # reason: the hook is reached with no key in tests and on unauthenticated routes + + assert "litellm_metadata" not in data + assert "metadata" not in data From 961cd44b00f33148b864eeb20e5e80ec49dabec5 Mon Sep 17 00:00:00 2001 From: Vivek1898 <72497717+Vivek1898@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:31:48 +0530 Subject: [PATCH 2/8] test(guardrails): update the two tests that pinned the old bucket behaviour CI surfaced two existing tests in test_grayswan.py that assert the implementation this PR changes. I had missed them: my search for existing coverage ran through `head -6` and truncated the list before this file. Both pinned the old implementation's *shape*, not its purpose, and both properties they protect still hold: test_ensure_litellm_metadata_populates_from_user_api_key_dict asserted the literal `litellm_metadata` key exists. Creating that key on a route whose bucket is `metadata` is the bug -- every reader resolves the bucket by presence, so a new key moves them all onto a near-empty dict. The property, that key metadata reaches the guardrail, is unchanged; it is now asserted on the resolved bucket. test_ensure_litellm_metadata_noop_when_already_present asserted the bucket stayed *exactly* `{"existing": "value"}`, because the old code returned early whenever `litellm_metadata` was present and so added nothing. The merge now runs via `setdefault`, so "an existing value is never overwritten" still holds -- asserted directly now, instead of through whole-dict equality. Renamed to say what it checks, and it seeds a `user_api_key_user_id` so overwriting would fail it. Both also keep asserting that a request genuinely using `litellm_metadata` stays on that bucket, so the fix cannot be read as flipping routes the other way. If either was protecting something I have misread, say so and I will restore the assertion and solve it differently. --- .../guardrail_hooks/test_grayswan.py | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/tests/unit/proxy/guardrails/guardrail_hooks/test_grayswan.py b/tests/unit/proxy/guardrails/guardrail_hooks/test_grayswan.py index a5ba9dab0bd..da3fb33bedd 100644 --- a/tests/unit/proxy/guardrails/guardrail_hooks/test_grayswan.py +++ b/tests/unit/proxy/guardrails/guardrail_hooks/test_grayswan.py @@ -570,7 +570,18 @@ def test_prepare_payload_includes_litellm_metadata( def test_ensure_litellm_metadata_populates_from_user_api_key_dict() -> None: - """Verify _ensure_litellm_metadata populates litellm_metadata.""" + """Verify _ensure_litellm_metadata populates the request's metadata bucket. + + Updated for #45543. This asserted the literal `litellm_metadata` key, which + is what the old implementation created unconditionally -- and creating it on + a route whose bucket is `metadata` is the bug: every reader resolves the + bucket by presence, so a new key moves them all onto a near-empty dict. The + property being protected, that the key metadata reaches the guardrail, is + unchanged; it is now asserted on the resolved bucket. + """ + from litellm.litellm_core_utils.core_helpers import ( + get_metadata_variable_name_from_kwargs, + ) from litellm.proxy.guardrails.guardrail_hooks.unified_guardrail.unified_guardrail import ( _ensure_litellm_metadata, ) @@ -580,23 +591,37 @@ def test_ensure_litellm_metadata_populates_from_user_api_key_dict() -> None: _ensure_litellm_metadata(data, user_auth) - assert "litellm_metadata" in data - assert data["litellm_metadata"]["user_api_key_user_id"] == "u1" - assert data["litellm_metadata"]["user_api_key_team_id"] == "t1" + bucket_name = get_metadata_variable_name_from_kwargs(data) + assert bucket_name == "metadata" + assert data[bucket_name]["user_api_key_user_id"] == "u1" + assert data[bucket_name]["user_api_key_team_id"] == "t1" -def test_ensure_litellm_metadata_noop_when_already_present() -> None: - """Verify _ensure_litellm_metadata does not overwrite existing litellm_metadata.""" +def test_ensure_litellm_metadata_does_not_overwrite_existing_values() -> None: + """Verify _ensure_litellm_metadata leaves values already in the bucket alone. + + Updated for #45543. This asserted the bucket stayed *exactly* + `{"existing": "value"}`, because the old implementation returned early + whenever `litellm_metadata` was present and so added nothing at all. The + merge now runs on the resolved bucket via `setdefault`, so the property + worth protecting -- an existing value is never overwritten -- still holds, + and is asserted directly rather than through whole-dict equality. The key + metadata the guardrail needs is added alongside it. + """ from litellm.proxy.guardrails.guardrail_hooks.unified_guardrail.unified_guardrail import ( _ensure_litellm_metadata, ) user_auth = UserAPIKeyAuth(user_id="should-not-appear") - data: dict = {"litellm_metadata": {"existing": "value"}} + data: dict = {"litellm_metadata": {"existing": "value", "user_api_key_user_id": "already-resolved"}} _ensure_litellm_metadata(data, user_auth) - assert data["litellm_metadata"] == {"existing": "value"} + # Still the `litellm_metadata` bucket: the fix resolves the bucket, it does + # not flip routes that genuinely use this one. + assert "metadata" not in data + assert data["litellm_metadata"]["existing"] == "value" + assert data["litellm_metadata"]["user_api_key_user_id"] == "already-resolved" class _CapturingClient: From 535460e32178af93f1965d1c925a22991ffc4a4a Mon Sep 17 00:00:00 2001 From: Vivek1898 <72497717+Vivek1898@users.noreply.github.com> Date: Mon, 12 Oct 2026 17:57:52 +0530 Subject: [PATCH 3/8] fix(guardrails): keep the forwarded request payload's type known The type gate failed with reportUnknownArgumentType 44745 over its 44744 ceiling (+1). The new call forwards `data` as an argument, where the old code only subscript-assigned it, and `data` is annotated here as a bare dict -- dict[Unknown, Unknown] under basedpyright -- while merge_user_api_key_metadata_into_request takes dict[str, Any]. Cast to _RequestData, this module's own alias (dict[str, object]). Measured per-rule on this file: reportUnknownArgumentType 34 -> 33 and no rule gets worse. Casting to dict[str, Any] instead also cleared it but traded the error for reportExplicitAny +1, and annotating the parameter would have moved the unknown to the two call sites rather than removing it. 39 tests still pass; ruff check and format clean. --- .../unified_guardrail/unified_guardrail.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py b/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py index cb8409ce13c..fa6d190c3c9 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py +++ b/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py @@ -244,7 +244,14 @@ def _ensure_litellm_metadata(data: dict, user_api_key_dict: UserAPIKeyAuth) -> N BaseTranslation, ) - BaseTranslation.merge_user_api_key_metadata_into_request(data, user_api_key_dict) + BaseTranslation.merge_user_api_key_metadata_into_request( + # cast-ok: `data` is the request payload, annotated here as a bare + # dict, so passing it on is an unknown argument type. _RequestData is + # this module's own alias for it and keeps the argument fully known + # without widening three signatures or naming Any again. + cast(_RequestData, data), + user_api_key_dict, + ) class UnifiedLLMGuardrails(CustomLogger): From ba2bc7c9fe7bbf55600dd6a6f106e8ec7099aa54 Mon Sep 17 00:00:00 2001 From: Vivek1898 <72497717+Vivek1898@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:10:16 +0530 Subject: [PATCH 4/8] fix(guardrails): put the cast-ok marker on the cast's own line The type-discipline gate then failed where the basedpyright one had passed: LIT006 wants `# cast-ok: ` on the cast call's first line, and my reason sat four lines above it, so LIT013 also fired -- a marker that suppresses nothing. Matches the existing cast in this file, which puts the marker as a trailing comment on `cast(`. Verified with the repo's own scripts/check_type_discipline.py against the gate's base SHA: LIT006 and LIT013 are back to the base count and LIT011 is one lower, so no rule grows. 39 tests pass; ruff clean. --- .../unified_guardrail/unified_guardrail.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py b/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py index fa6d190c3c9..03e86c346fc 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py +++ b/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py @@ -245,11 +245,9 @@ def _ensure_litellm_metadata(data: dict, user_api_key_dict: UserAPIKeyAuth) -> N ) BaseTranslation.merge_user_api_key_metadata_into_request( - # cast-ok: `data` is the request payload, annotated here as a bare - # dict, so passing it on is an unknown argument type. _RequestData is - # this module's own alias for it and keeps the argument fully known - # without widening three signatures or naming Any again. - cast(_RequestData, data), + cast( # cast-ok: _RequestData is this module's own alias for the bare-dict request payload + _RequestData, data + ), user_api_key_dict, ) From aa3faa947b2ff5e71f5113e6098e3b5c88fdbf79 Mon Sep 17 00:00:00 2001 From: Vivek1898 <72497717+Vivek1898@users.noreply.github.com> Date: Thu, 15 Oct 2026 12:55:52 +0530 Subject: [PATCH 5/8] test(guardrails): assert the wingback entry on the resolved metadata bucket test_proxy_pre_call_hook_records_one_guardrail_entry_per_check read data["litellm_metadata"] directly. That key existed only because the old _ensure_litellm_metadata created it unconditionally, and on a chat-completions request -- whose bucket is "metadata" -- creating it is the defect this PR fixes: readers resolve the bucket by presence, so the new key moved them all onto a near-empty dict. The property the test protects, one entry per check with its status, is unchanged; it now resolves the bucket with get_metadata_variable_name_from_kwargs and asserts there. Third test of this shape after the two in test_grayswan.py. 287 tests pass across the guardrail_hooks files that touch litellm_metadata. --- .../guardrail_hooks/test_wingback.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/unit/proxy/guardrails/guardrail_hooks/test_wingback.py b/tests/unit/proxy/guardrails/guardrail_hooks/test_wingback.py index c9e331de4a2..58a377cb779 100644 --- a/tests/unit/proxy/guardrails/guardrail_hooks/test_wingback.py +++ b/tests/unit/proxy/guardrails/guardrail_hooks/test_wingback.py @@ -252,13 +252,29 @@ async def test_proxy_post_call_hook_blocks_on_wingback_verdict(registered: _Wing @pytest.mark.asyncio @pytest.mark.parametrize("registered", [("pre_call", ALLOWED)], indirect=True) async def test_proxy_pre_call_hook_records_one_guardrail_entry_per_check(registered: _Wingback) -> None: + """One guardrail entry per check, recorded in the request's metadata bucket. + + Updated for #45543. This read the literal `litellm_metadata` key, which + existed only because the old `_ensure_litellm_metadata` created it + unconditionally -- and on a chat-completions request, whose bucket is + `metadata`, creating that key is the bug: every reader resolves the bucket + by presence, so a new key moves them all onto a near-empty dict. The + property being protected, one entry per check with its status, is + unchanged; it is now asserted on the resolved bucket. + """ + from litellm.litellm_core_utils.core_helpers import ( + get_metadata_variable_name_from_kwargs, + ) + data: Final = _chat_request("hello") await ProxyLogging(user_api_key_cache=DualCache()).pre_call_hook( user_api_key_dict=UserAPIKeyAuth(), data=data, call_type="completion" ) - entries: Final = data["litellm_metadata"]["standard_logging_guardrail_information"] + bucket_name = get_metadata_variable_name_from_kwargs(data) + assert bucket_name == "metadata" + entries: Final = data[bucket_name]["standard_logging_guardrail_information"] assert [(entry["guardrail_name"], entry["guardrail_status"]) for entry in entries] == [ ("wingback-runtime-security", "success") ] From f90ca3ff17fc579c44355e034a5070b93da2f800 Mon Sep 17 00:00:00 2001 From: Vivek1898 <72497717+Vivek1898@users.noreply.github.com> Date: Sun, 18 Oct 2026 14:39:52 +0530 Subject: [PATCH 6/8] test(guardrails): fold the #45543 cases into the mapped test file AGENTS.md asks bug fixes to extend the existing mapped test file rather than add one. The mapped file is also the better home on its merits: it already carries TestStreamingGuardrailInformationBucket, which pins the same bucket-flip shape from #38722, so these five cases belong beside it instead of in a file a reader would not think to open. Moved as TestEnsureLitellmMetadataBucket, following that file's conventions: the regression context in the class docstring, the user_api_key_dict fixture as a method, and the module's existing unified_module alias instead of a fresh private import. The `# type: ignore[arg-type]` on the None case is replaced with `# pyright: ignore[reportArgumentType]`, trailing on the call so it attaches to the right node. pyrightconfig.json sets enableTypeIgnoreComments to false, so the old form suppressed nothing and named no rule. --- .../test_unified_guardrail_metadata_bucket.py | 96 ------------------- .../test_unified_guardrail.py | 72 ++++++++++++++ 2 files changed, 72 insertions(+), 96 deletions(-) delete mode 100644 tests/unit/proxy/guardrails/guardrail_hooks/test_unified_guardrail_metadata_bucket.py diff --git a/tests/unit/proxy/guardrails/guardrail_hooks/test_unified_guardrail_metadata_bucket.py b/tests/unit/proxy/guardrails/guardrail_hooks/test_unified_guardrail_metadata_bucket.py deleted file mode 100644 index 7803d379d74..00000000000 --- a/tests/unit/proxy/guardrails/guardrail_hooks/test_unified_guardrail_metadata_bucket.py +++ /dev/null @@ -1,96 +0,0 @@ -"""Regression tests for #45543. - -A pre_call guardrail used to write ``data["litellm_metadata"]`` directly. Every -reader resolves the proxy-internal bucket through -``get_metadata_variable_name_from_kwargs``, which answers ``litellm_metadata`` as -soon as that key merely exists, so creating it on a chat-completions request (whose -bucket is ``metadata``) moved every later read onto a dict holding only the -``user_api_key_*`` keys. Tags and router fields stayed behind in ``metadata``: -tag-based routing found no tags and picked a deployment of another model, and -SpendLogs lost ``model_group`` and ``model_id``. Nothing raised. -""" - -from typing import Final - -import pytest - -from litellm.litellm_core_utils.core_helpers import ( - get_metadata_variable_name_from_kwargs, -) -from litellm.proxy._types import UserAPIKeyAuth -from litellm.proxy.guardrails.guardrail_hooks.unified_guardrail.unified_guardrail import ( - _ensure_litellm_metadata, -) - - -@pytest.fixture -def user_api_key_dict() -> UserAPIKeyAuth: - return UserAPIKeyAuth(api_key="sk-test-45543", user_id="user-1", team_id="team-1") - - -def test_chat_completions_request_keeps_its_metadata_bucket(user_api_key_dict: UserAPIKeyAuth) -> None: - """The reported shape: proxy state already lives in ``metadata``.""" - data: Final = { - "model": "all-models", - "metadata": {"tags": ["model:glm-5"], "model_group": "all-models"}, - } - - _ensure_litellm_metadata(data, user_api_key_dict) - - # The bug was creating this key at all; its presence is what flips the readers. - assert "litellm_metadata" not in data - assert get_metadata_variable_name_from_kwargs(data) == "metadata" - # The fields the routing and spend-logging readers need are still where they look. - assert data["metadata"]["tags"] == ["model:glm-5"] - assert data["metadata"]["model_group"] == "all-models" - # And the guardrail still gets the key metadata it asked for. - assert data["metadata"]["user_api_key_team_id"] == "team-1" - assert data["metadata"]["user_api_key_user_id"] == "user-1" - - -def test_existing_metadata_values_are_not_overwritten(user_api_key_dict: UserAPIKeyAuth) -> None: - """Merging is ``setdefault``: a value already resolved upstream wins.""" - data: Final = { - "model": "all-models", - "metadata": {"user_api_key_user_id": "resolved-upstream"}, - } - - _ensure_litellm_metadata(data, user_api_key_dict) - - assert data["metadata"]["user_api_key_user_id"] == "resolved-upstream" - assert data["metadata"]["user_api_key_team_id"] == "team-1" - - -def test_litellm_metadata_route_still_uses_litellm_metadata(user_api_key_dict: UserAPIKeyAuth) -> None: - """Batch and file routes really do keep proxy state in ``litellm_metadata``. - - The fix must not flip those the other way, so the bucket is resolved rather - than hardcoded in either direction. - """ - data: Final = {"model": "m", "litellm_metadata": {"tags": ["keep-me"]}} - - _ensure_litellm_metadata(data, user_api_key_dict) - - assert "metadata" not in data - assert get_metadata_variable_name_from_kwargs(data) == "litellm_metadata" - assert data["litellm_metadata"]["tags"] == ["keep-me"] - assert data["litellm_metadata"]["user_api_key_team_id"] == "team-1" - - -def test_request_without_any_metadata_gets_the_default_bucket(user_api_key_dict: UserAPIKeyAuth) -> None: - data: Final = {"model": "m"} - - _ensure_litellm_metadata(data, user_api_key_dict) - - assert "litellm_metadata" not in data - assert data["metadata"]["user_api_key_team_id"] == "team-1" - - -def test_no_user_api_key_metadata_creates_no_bucket() -> None: - """``None`` yields no metadata, so nothing should be written at all.""" - data: Final = {"model": "m"} - - _ensure_litellm_metadata(data, None) # type: ignore[arg-type] # reason: the hook is reached with no key in tests and on unauthenticated routes - - assert "litellm_metadata" not in data - assert "metadata" not in data diff --git a/tests/unit/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py b/tests/unit/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py index 84f4d7a4173..b1796330dd7 100644 --- a/tests/unit/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py +++ b/tests/unit/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py @@ -21,6 +21,9 @@ from litellm.integrations.custom_guardrail import ( log_guardrail_information, ) from litellm.litellm_core_utils.api_route_to_call_types import get_call_types_for_route +from litellm.litellm_core_utils.core_helpers import ( + get_metadata_variable_name_from_kwargs, +) from litellm.llms import discover_guardrail_translation_mappings, load_guardrail_translation_mappings from litellm.llms.base_llm.guardrail_translation.base_translation import BaseTranslation from litellm.llms.base_llm.guardrail_translation.utils import ( @@ -3291,3 +3294,72 @@ async def test_pass_through_guardrail_text_is_the_client_body_without_the_inject texts: Final = [call["inputs"]["texts"] for call in guardrail.apply_calls] assert [[json.loads(text) for text in call_texts] for call_texts in texts] == [[body]], texts assert data["litellm_metadata"]["user_api_key_user_email"] == "someone@example.com", data["litellm_metadata"] + + +class TestEnsureLitellmMetadataBucket: + """``_ensure_litellm_metadata`` must merge into the bucket the request + already resolves to, not plant ``litellm_metadata``. Creating that key on a + chat-completions request flips every reader onto a dict holding only the + ``user_api_key_*`` keys, so tags and router fields are left unread: tag + routing picks a deployment of another model and SpendLogs lose + ``model_group``/``model_id``, without raising (#45543). Same bucket-flip + shape as TestStreamingGuardrailInformationBucket above.""" + + @pytest.fixture + def user_api_key_dict(self) -> UserAPIKeyAuth: + return UserAPIKeyAuth(api_key="sk-test-45543", user_id="user-1", team_id="team-1") + + def test_chat_completions_request_keeps_its_metadata_bucket(self, user_api_key_dict: UserAPIKeyAuth) -> None: + data: Final = { + "model": "all-models", + "metadata": {"tags": ["model:glm-5"], "model_group": "all-models"}, + } + + unified_module._ensure_litellm_metadata(data, user_api_key_dict) + + # Creating the key at all is the bug; its presence is what flips the readers. + assert "litellm_metadata" not in data + assert get_metadata_variable_name_from_kwargs(data) == "metadata" + assert data["metadata"]["tags"] == ["model:glm-5"] + assert data["metadata"]["model_group"] == "all-models" + assert data["metadata"]["user_api_key_team_id"] == "team-1" + assert data["metadata"]["user_api_key_user_id"] == "user-1" + + def test_existing_metadata_values_are_not_overwritten(self, user_api_key_dict: UserAPIKeyAuth) -> None: + data: Final = { + "model": "all-models", + "metadata": {"user_api_key_user_id": "resolved-upstream"}, + } + + unified_module._ensure_litellm_metadata(data, user_api_key_dict) + + assert data["metadata"]["user_api_key_user_id"] == "resolved-upstream" + assert data["metadata"]["user_api_key_team_id"] == "team-1" + + def test_litellm_metadata_route_still_uses_litellm_metadata(self, user_api_key_dict: UserAPIKeyAuth) -> None: + """Batch and file routes do keep proxy state in ``litellm_metadata``, so + the bucket is resolved rather than hardcoded in either direction.""" + data: Final = {"model": "m", "litellm_metadata": {"tags": ["keep-me"]}} + + unified_module._ensure_litellm_metadata(data, user_api_key_dict) + + assert "metadata" not in data + assert get_metadata_variable_name_from_kwargs(data) == "litellm_metadata" + assert data["litellm_metadata"]["tags"] == ["keep-me"] + assert data["litellm_metadata"]["user_api_key_team_id"] == "team-1" + + def test_request_without_any_metadata_gets_the_default_bucket(self, user_api_key_dict: UserAPIKeyAuth) -> None: + data: Final = {"model": "m"} + + unified_module._ensure_litellm_metadata(data, user_api_key_dict) + + assert "litellm_metadata" not in data + assert data["metadata"]["user_api_key_team_id"] == "team-1" + + def test_no_user_api_key_metadata_creates_no_bucket(self) -> None: + data: Final = {"model": "m"} + + unified_module._ensure_litellm_metadata(data, None) # pyright: ignore[reportArgumentType] # the hook is reached with no key on unauthenticated routes + + assert "litellm_metadata" not in data + assert "metadata" not in data From 567fdd24ed7ab6689ba289f045ea9dd4e180fe6a Mon Sep 17 00:00:00 2001 From: Vivek1898 <72497717+Vivek1898@users.noreply.github.com> Date: Sun, 18 Oct 2026 14:39:52 +0530 Subject: [PATCH 7/8] docs(guardrails): cut the _ensure_litellm_metadata docstring to the trap It recounted the whole investigation; AGENTS.md asks explanations to stay concise. What a reader needs is why not to write data["litellm_metadata"] directly, which is seven lines. The failure history lives in #45543 and in the test class docstring, where it explains what the assertions are for. --- .../unified_guardrail/unified_guardrail.py | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py b/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py index 03e86c346fc..703942b3131 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py +++ b/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py @@ -225,20 +225,12 @@ def _a2a_jsonrpc_error_chunk(exc: HTTPException, request_id: str | None) -> Mapp def _ensure_litellm_metadata(data: dict, user_api_key_dict: UserAPIKeyAuth) -> None: """Merge the ``user_api_key_*`` metadata into the request's resolved bucket. - Writing ``data["litellm_metadata"]`` directly creates a second bucket on a - route whose bucket is ``metadata`` (chat completions). Every reader resolves - the bucket through ``get_metadata_variable_name_from_kwargs``, which answers - ``litellm_metadata`` as soon as that key merely exists, so from then on they - all read the new dict -- which holds only the ``user_api_key_*`` keys. The - tags and router fields stay behind in ``metadata``, unread: - ``get_deployments_for_tag`` finds no tags and routes the request to a - deployment of another model, and SpendLogs lose ``model_group`` and - ``model_id``. Nothing raises (#45543). - - ``BaseTranslation.merge_user_api_key_metadata_into_request`` is the sanctioned - writer for exactly this -- it resolves the bucket via - ``get_or_create_metadata_bucket`` and ``setdefault``s each key, so an existing - value is never overwritten. The OpenAI chat guardrail handler already uses it. + Not ``data["litellm_metadata"]``: creating that key on a route whose bucket + is ``metadata`` flips every reader of + ``get_metadata_variable_name_from_kwargs`` onto the new dict and silently + strands the tags and router fields (#45543). + ``merge_user_api_key_metadata_into_request`` resolves the bucket and + ``setdefault``s, so an upstream value is never overwritten. """ from litellm.llms.base_llm.guardrail_translation.base_translation import ( BaseTranslation, From 0142382f46957476e7676e5fa29dbdb867c9b956 Mon Sep 17 00:00:00 2001 From: Vivek1898 Date: Wed, 4 Nov 2026 08:34:08 +0530 Subject: [PATCH 8/8] test(guardrails): read the pass-through key metadata from the resolved bucket The pass-through test added upstream after this PR was opened asserted the injected key metadata under data["litellm_metadata"], which is the placement this PR removes. Read it from the bucket the request resolves to instead, so the test keeps checking what it was written for: the guardrail text is the client body without the injected key metadata. --- .../unified_guardrails/test_unified_guardrail.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py b/tests/unit/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py index b1796330dd7..89dd7a36766 100644 --- a/tests/unit/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py +++ b/tests/unit/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py @@ -3293,7 +3293,9 @@ async def test_pass_through_guardrail_text_is_the_client_body_without_the_inject texts: Final = [call["inputs"]["texts"] for call in guardrail.apply_calls] assert [[json.loads(text) for text in call_texts] for call_texts in texts] == [[body]], texts - assert data["litellm_metadata"]["user_api_key_user_email"] == "someone@example.com", data["litellm_metadata"] + # the key metadata lands in whichever bucket the request resolves to (#45543) + bucket: Final = data[get_metadata_variable_name_from_kwargs(data)] + assert bucket["user_api_key_user_email"] == "someone@example.com", bucket class TestEnsureLitellmMetadataBucket: