From db19f24d694ec82037f06361e4aa6ae10d194da5 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Thu, 16 Apr 2026 23:00:52 +0000 Subject: [PATCH] fix(proxy): move metadata strip after JSON-string parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Veria AI caught a bypass: metadata can arrive as a JSON string via multipart/form-data or extra_body, and the existing strip block ran before the string-to-dict parse. The isinstance(_user_meta, dict) guard returned False on the string, the strip was skipped, and then the parse turned the string into a dict — leaving user_api_key_metadata / user_api_key_team_metadata / _pipeline_managed_guardrails / tags intact in the parsed dict. Move the strip to run AFTER the parse and BEFORE the merge of litellm_metadata into data[_metadata_variable_name], closing the bypass for both raw-dict and string-encoded payloads. Regression test: test_add_litellm_data_to_request_strips_string_encoded_admin_injection. --- litellm/proxy/litellm_pre_call_utils.py | 103 ++++++++++-------- .../proxy/test_litellm_pre_call_utils.py | 68 +++++++++++- 2 files changed, 122 insertions(+), 49 deletions(-) diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index b1dae22c9fe..33affa5351c 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -977,49 +977,6 @@ async def add_litellm_data_to_request( # noqa: PLR0915 "Setting client-provided x-api-key as api_key parameter (will override deployment key)" ) - # Strip internal pipeline state and admin-injection slots from user input. - # The proxy writes user_api_key_metadata / user_api_key_team_metadata - # into data[_metadata_variable_name] below; if a caller pre-populates - # either key on the OTHER metadata field, _get_admin_metadata lookups - # would treat the caller's payload as admin-configured. - for _meta_key in ("metadata", "litellm_metadata"): - _user_meta = data.get(_meta_key) - if isinstance(_user_meta, dict): - _user_meta.pop("_pipeline_managed_guardrails", None) - _user_meta.pop("user_api_key_metadata", None) - _user_meta.pop("user_api_key_team_metadata", None) - - # Strip caller-supplied routing/budget tags unless the admin has opted - # this key or team in via metadata.allow_client_tags=True. Tags drive - # tag-based routing and tag budget attribution — accepting them from - # untrusted callers lets an attacker reach restricted deployments or - # misattribute spend to a victim team's tag. - _admin_allow_client_tags = False - for _admin_meta in ( - user_api_key_dict.metadata, - user_api_key_dict.team_metadata, - ): - if ( - isinstance(_admin_meta, dict) - and _admin_meta.get("allow_client_tags") is True - ): - _admin_allow_client_tags = True - break - if not _admin_allow_client_tags: - _stripped_from: List[str] = [] - for _meta_key in ("metadata", "litellm_metadata"): - _user_meta = data.get(_meta_key) - if isinstance(_user_meta, dict) and "tags" in _user_meta: - _user_meta.pop("tags", None) - _stripped_from.append(_meta_key) - if _stripped_from: - verbose_proxy_logger.warning( - "Stripped caller-supplied tags from %s: this key/team does " - "not have `allow_client_tags: true` in its metadata. Set it " - "to opt into client-supplied routing/budget tags.", - ", ".join(_stripped_from), - ) - ########################################################## # Init - Proxy Server Request # we do this as soon as entering so we track the original request @@ -1126,11 +1083,61 @@ async def add_litellm_data_to_request( # noqa: PLR0915 ) else: data["litellm_metadata"] = parsed_litellm_metadata - # Merge litellm_metadata into the metadata variable (preserving existing values) - if isinstance(data["litellm_metadata"], dict): - for key, value in data["litellm_metadata"].items(): - if key not in data[_metadata_variable_name]: - data[_metadata_variable_name][key] = value + + # Strip internal pipeline state and admin-injection slots from user input. + # Runs AFTER the string-to-dict parse above so JSON-string metadata (sent + # via multipart/form-data or extra_body) cannot smuggle `user_api_key_metadata` + # past the isinstance(dict) guard. + # + # The proxy writes user_api_key_metadata / user_api_key_team_metadata into + # data[_metadata_variable_name] below; if a caller pre-populates either + # key on the OTHER metadata field, _get_admin_metadata lookups would treat + # the caller's payload as admin-configured. + for _meta_key in ("metadata", "litellm_metadata"): + _user_meta = data.get(_meta_key) + if isinstance(_user_meta, dict): + _user_meta.pop("_pipeline_managed_guardrails", None) + _user_meta.pop("user_api_key_metadata", None) + _user_meta.pop("user_api_key_team_metadata", None) + + # Strip caller-supplied routing/budget tags unless the admin has opted + # this key or team in via metadata.allow_client_tags=True. Tags drive + # tag-based routing and tag budget attribution — accepting them from + # untrusted callers lets an attacker reach restricted deployments or + # misattribute spend to a victim team's tag. + _admin_allow_client_tags = False + for _admin_meta in ( + user_api_key_dict.metadata, + user_api_key_dict.team_metadata, + ): + if ( + isinstance(_admin_meta, dict) + and _admin_meta.get("allow_client_tags") is True + ): + _admin_allow_client_tags = True + break + if not _admin_allow_client_tags: + _stripped_from: List[str] = [] + for _meta_key in ("metadata", "litellm_metadata"): + _user_meta = data.get(_meta_key) + if isinstance(_user_meta, dict) and "tags" in _user_meta: + _user_meta.pop("tags", None) + _stripped_from.append(_meta_key) + if _stripped_from: + verbose_proxy_logger.warning( + "Stripped caller-supplied tags from %s: this key/team does " + "not have `allow_client_tags: true` in its metadata. Set it " + "to opt into client-supplied routing/budget tags.", + ", ".join(_stripped_from), + ) + + # Now merge litellm_metadata into the metadata variable (preserving existing + # values) — runs AFTER the strip so attacker injections in litellm_metadata + # cannot cross-contaminate the admin-authoritative metadata dict. + if "litellm_metadata" in data and isinstance(data["litellm_metadata"], dict): + for key, value in data["litellm_metadata"].items(): + if key not in data[_metadata_variable_name]: + data[_metadata_variable_name][key] = value data = LiteLLMProxyRequestSetup.add_user_api_key_auth_to_request_metadata( data=data, diff --git a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py index e433bebb3ac..351857b9b10 100644 --- a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py +++ b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py @@ -280,6 +280,70 @@ async def test_add_litellm_data_to_request_strips_admin_injection_slots(): assert "_pipeline_managed_guardrails" not in other +@pytest.mark.asyncio +async def test_add_litellm_data_to_request_strips_string_encoded_admin_injection(): + """Regression: metadata arriving as a JSON string (multipart/form-data or + extra_body) must not bypass the admin-injection strip. The parse happens + AFTER receipt, so the strip has to run after the parse, not before. + """ + from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request + + 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": "multipart/form-data"} + request_mock.client = MagicMock() + request_mock.client.host = "127.0.0.1" + + # Attacker encodes an admin-injection payload inside a JSON string. + attacker_payload = { + "user_api_key_metadata": {"disable_global_guardrails": True}, + "user_api_key_team_metadata": {"disable_global_guardrails": True}, + "_pipeline_managed_guardrails": ["evaded"], + } + data = { + "model": "gpt-3.5-turbo", + "metadata": json.dumps(attacker_payload), + "litellm_metadata": json.dumps(attacker_payload), + } + + real_admin_metadata = {"admin_flag": "from_proxy"} + user_api_key_dict = UserAPIKeyAuth( + api_key="hashed-key", + metadata=real_admin_metadata, + team_metadata=real_admin_metadata, + spend=0.0, + max_budget=100.0, + model_max_budget={}, + team_spend=0.0, + team_max_budget=200.0, + ) + + updated = await add_litellm_data_to_request( + data=data, + request=request_mock, + user_api_key_dict=user_api_key_dict, + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + + populated = updated["metadata"] + # The real admin payload from user_api_key_dict wins. + assert populated["user_api_key_metadata"] == real_admin_metadata + assert populated["user_api_key_team_metadata"] == real_admin_metadata + assert populated.get("_pipeline_managed_guardrails") != ["evaded"] + + other = updated.get("litellm_metadata") or {} + # After the strip, litellm_metadata has no admin-injection slots. + assert "user_api_key_metadata" not in other + assert "user_api_key_team_metadata" not in other + assert "_pipeline_managed_guardrails" not in other + + @pytest.mark.asyncio async def test_add_litellm_data_to_request_strips_user_tags_without_permission(): """Caller-supplied metadata.tags must be stripped when the key/team @@ -486,9 +550,11 @@ async def test_add_litellm_data_to_request_audio_transcription_multipart(): "file": b"Fake audio bytes", } + # Opt the key in to client-supplied tags so the parsed tags from the + # JSON-string multipart body aren't stripped by the admin-injection strip. user_api_key_dict = UserAPIKeyAuth( api_key="hashed-key", - metadata={}, + metadata={"allow_client_tags": True}, team_metadata={}, spend=0.0, max_budget=100.0,