scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-10 22:41:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 82

fix(proxy): close 6 more metadata/tag variant bypasses

Browse source 
Post-merge audit found 6 adjacent variants of the VERIA-28 class. All
fixed here with regression tests:

1. Strip widened from 3 named keys to the full user_api_key_* prefix.
   The proxy writes a dozen user_api_key_* fields (user_id, alias,
   spend, team_id, request_route, end_user_id, …) into
   data[_metadata_variable_name]; the 3-key strip left the rest
   exploitable for identity/spend forgery in audit logs and guardrails.

2. proxy_server_request['body'] snapshot moved to AFTER the strip.
   Was captured at line ~990 before the strip ran, so
   standard_logging_object, lago, and spend_tracking readers saw the
   attacker-forged payload even though the live data dict was clean.

3. get_tags_from_request_body (auth-time) now coerces JSON-string
   metadata via safe_json_loads. Previously crashed with
   AttributeError on string metadata (DoS; potential RBAC bypass if
   a caller swallowed the exception).

4. get_end_user_id_from_request_body coerces JSON-string
   metadata/litellm_metadata. Previously isinstance(dict) guard
   caused end-user budget attribution to be silently skipped when
   the caller sent metadata as a JSON string.

5. Four hand-rolled 'if data.get("metadata") is None: data["metadata"] = {}'
   blocks in proxy_server.py (7160, 7341, 7590, 11375) now guard on
   isinstance(dict). They crashed with TypeError when metadata was a
   JSON string (DoS).

6. _get_admin_metadata defensively guards with isinstance(dict);
   previously AttributeError'd on any leaked string metadata.

Also hoists the inline safe_json_loads import in _guardrail_modification_check
to module level per CLAUDE.md style.
This commit is contained in:
user 2026-04-17 00:08:40 +00:00
parent 76aa97f77b
commit b4e98d190a
No known key found for this signature in database
8 changed files with 281 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
litellm/integrations/custom_guardrail.py
View file

@ -268,7 +268,11 @@ class CustomGuardrail(CustomLogger):
team_meta: dict = {}
key_meta: dict = {}
for key in ("metadata", "litellm_metadata"):
meta = data.get(key) or {}
# Defensive: an unparsed JSON-string metadata could leak past the
# proxy's normal parse path; don't AttributeError on .get().
meta = data.get(key)
if not isinstance(meta, dict):
continue
team_meta = meta.get("user_api_key_team_metadata") or team_meta
key_meta = meta.get("user_api_key_metadata") or key_meta
return {**team_meta, **key_meta}

2
litellm/proxy/auth/auth_checks.py
View file

@ -31,6 +31,7 @@ from litellm.constants import (
)
from litellm.litellm_core_utils.dd_tracing import tracer
from litellm.litellm_core_utils.get_llm_provider_logic import get_llm_provider
from litellm.litellm_core_utils.safe_json_loads import safe_json_loads
from litellm.proxy._types import (
RBAC_ROLES,
CallInfo,
@ -350,7 +351,6 @@ def _guardrail_modification_check(
failing loudly at the auth layer so operators see an explicit 403 instead
of a confusing silent-ignore.
"""
from litellm.litellm_core_utils.safe_json_loads import safe_json_loads
from litellm.proxy.guardrails.guardrail_helpers import can_modify_guardrails
def _coerce_to_dict(container: Any) -> Optional[dict]:

31
litellm/proxy/auth/auth_utils.py
View file

@ -823,19 +823,30 @@ def get_end_user_id_from_request_body(
user_from_body_user_field = request_body["user"]
return str(user_from_body_user_field)
def _as_dict(value: Any) -> dict:
# metadata / litellm_metadata can arrive as JSON strings from
# multipart/form-data or extra_body; coerce so string-encoded
# payloads can't evade end-user attribution.
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)
return parsed if isinstance(parsed, dict) else {}
return {}
# Check 4: 'litellm_metadata.user' in request_body (commonly Anthropic)
litellm_metadata = request_body.get("litellm_metadata")
if isinstance(litellm_metadata, dict):
user_from_litellm_metadata = litellm_metadata.get("user")
if user_from_litellm_metadata is not None:
return str(user_from_litellm_metadata)
litellm_metadata = _as_dict(request_body.get("litellm_metadata"))
user_from_litellm_metadata = litellm_metadata.get("user")
if user_from_litellm_metadata is not None:
return str(user_from_litellm_metadata)
# Check 5: 'metadata.user_id' in request_body (another common pattern)
metadata_dict = request_body.get("metadata")
if isinstance(metadata_dict, dict):
user_id_from_metadata_field = metadata_dict.get("user_id")
if user_id_from_metadata_field is not None:
return str(user_id_from_metadata_field)
metadata_dict = _as_dict(request_body.get("metadata"))
user_id_from_metadata_field = metadata_dict.get("user_id")
if user_id_from_metadata_field is not None:
return str(user_id_from_metadata_field)
# Check 6: 'safety_identifier' in request body (OpenAI Responses API parameter)
# SECURITY NOTE: safety_identifier can be set by any caller in the request body.

19
litellm/proxy/common_utils/http_parsing_utils.py
View file

@ -197,10 +197,10 @@ def check_file_size_under_limit(
if llm_router is not None and request_data["model"] in router_model_names:
try:
deployment: Optional[
Deployment
] = llm_router.get_deployment_by_model_group_name(
model_group_name=request_data["model"]
deployment: Optional[Deployment] = (
llm_router.get_deployment_by_model_group_name(
model_group_name=request_data["model"]
)
)
if (
deployment
@ -426,7 +426,16 @@ def get_tags_from_request_body(request_body: dict) -> List[str]:
List of tag names (strings), empty list if no valid tags found
"""
metadata_variable_name = get_metadata_variable_name_from_kwargs(request_body)
metadata = request_body.get(metadata_variable_name) or {}
metadata = request_body.get(metadata_variable_name)
# metadata can arrive as a JSON string from multipart/form-data or extra_body;
# coerce defensively so .get() below never raises AttributeError.
if isinstance(metadata, str):
from litellm.litellm_core_utils.safe_json_loads import safe_json_loads
parsed = safe_json_loads(metadata)
metadata = parsed if isinstance(parsed, dict) else {}
elif not isinstance(metadata, dict):
metadata = {}
tags_in_metadata: Any = metadata.get("tags", [])
tags_in_request_body: Any = request_body.get("tags", [])
combined_tags: List[str] = []

36
litellm/proxy/litellm_pre_call_utils.py
View file

@ -981,13 +981,16 @@ async def add_litellm_data_to_request( # noqa: PLR0915
# Init - Proxy Server Request
# we do this as soon as entering so we track the original request
##########################################################
# Track arrival time for queue time metric
# Track arrival time for queue time metric. The body snapshot is filled
# in after the admin-injection strip below so the audit / spend-tracking
# consumers of proxy_server_request["body"] see the cleaned metadata
# rather than attacker-forged user_api_key_* fields.
arrival_time = time.time()
data["proxy_server_request"] = {
"url": str(request.url),
"method": request.method,
"headers": _headers,
"body": copy.copy(data), # use copy instead of deepcopy
"body": None, # filled in post-strip; see below
"arrival_time": arrival_time, # Track when request arrived at proxy
}
@ -1087,19 +1090,24 @@ async def add_litellm_data_to_request( # noqa: PLR0915
# 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.
# via multipart/form-data or extra_body) cannot smuggle admin fields 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.
# The proxy populates a family of ``user_api_key_*`` fields below
# (user_api_key_metadata, user_api_key_user_id, user_api_key_alias,
# user_api_key_spend, user_api_key_team_metadata, …) into
# data[_metadata_variable_name]. Because the proxy only writes to ONE of
# the two metadata dicts, a caller pre-populating any of these keys on
# the OTHER metadata dict would have their forged values surface in
# guardrails, spend tracking, audit logs, and identity resolution. Strip
# by prefix so new ``user_api_key_*`` fields added in the future are
# covered without per-key maintenance.
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)
for _k in [k for k in _user_meta if k.startswith("user_api_key_")]:
_user_meta.pop(_k, 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
@ -1132,9 +1140,15 @@ async def add_litellm_data_to_request( # noqa: PLR0915
", ".join(_stripped_from),
)
# Fill in the proxy_server_request body snapshot now that metadata has
# been parsed and stripped. Consumers (standard_logging_payload, lago,
# spend_tracking_utils, streaming_iterator) read `body` to audit the
# request; taking the snapshot here ensures they see cleaned metadata.
data["proxy_server_request"]["body"] = copy.copy(data)
# Snapshot the (now-cleaned) requester-supplied metadata for downstream
# consumers. Taking the deepcopy AFTER the strip prevents attacker-
# injected admin slots (user_api_key_metadata, tags without opt-in,
# injected admin slots (user_api_key_*, tags without opt-in,
# _pipeline_managed_guardrails) from surviving in requester_metadata
# where guardrails and audit paths may read from it.
if "metadata" in data and isinstance(data["metadata"], dict):

15
litellm/proxy/proxy_server.py
View file

@ -2201,9 +2201,11 @@ def run_ollama_serve():
with open(os.devnull, "w") as devnull:
subprocess.Popen(command, stdout=devnull, stderr=devnull)
except Exception as e:
verbose_proxy_logger.debug(f"""
verbose_proxy_logger.debug(
f"""
LiteLLM Warning: proxy started with `ollama` model\n`ollama serve` failed with Exception{e}. \nEnsure you run `ollama serve`
""")
"""
)
def _get_process_rss_mb() -> Optional[float]:
@ -7157,7 +7159,10 @@ async def chat_completion( # noqa: PLR0915
global user_temperature, user_request_timeout, user_max_tokens, user_api_base
data = await _read_request_body(request=request)
if user_api_key_dict is not None:
if data.get("metadata") is None:
if not isinstance(data.get("metadata"), dict):
# Covers both missing and JSON-string metadata (multipart /
# extra_body); otherwise `data["metadata"][k] = v` below raises
# TypeError on a string value and 500s the request.
data["metadata"] = {}
if (
hasattr(user_api_key_dict, "user_id")
@ -11372,7 +11377,9 @@ async def async_queue_request(
# if users are using user_api_key_auth, set `user` in `data`
data["user"] = user_api_key_dict.user_id
if "metadata" not in data:
if not isinstance(data.get("metadata"), dict):
# Covers both missing and JSON-string metadata (multipart /
# extra_body); see above for the same guard upstream.
data["metadata"] = {}
data["metadata"]["user_api_key"] = user_api_key_dict.api_key
data["metadata"]["user_api_key_metadata"] = user_api_key_dict.metadata

38
tests/test_litellm/proxy/common_utils/test_http_parsing_utils.py
View file

@ -835,3 +835,41 @@ def test_safe_get_request_headers_state_unavailable():
result = _safe_get_request_headers(mock_request)
assert result == {"content-type": "application/json"}
class TestGetTagsFromRequestBodyStringCoerce:
"""Regression: the auth-time tag helper used `metadata.get("tags", ...)`
directly, which raised AttributeError when metadata arrived as a JSON
string (multipart/form-data or extra_body). That turned into a DoS at
auth time and potentially bypassed tag-based RBAC if the caller caught
the exception and fell through with empty tags.
"""
def test_json_string_metadata_is_coerced_to_dict(self):
from litellm.proxy.common_utils.http_parsing_utils import (
get_tags_from_request_body,
)
metadata_json = json.dumps({"tags": ["a", "b"]})
# Must not raise
tags = get_tags_from_request_body({"metadata": metadata_json})
assert tags == ["a", "b"]
def test_unparseable_string_metadata_is_ignored(self):
from litellm.proxy.common_utils.http_parsing_utils import (
get_tags_from_request_body,
)
# Must not raise; must yield no metadata tags but keep root tags
tags = get_tags_from_request_body(
{"metadata": "not-json", "tags": ["root-only"]}
)
assert tags == ["root-only"]
def test_dict_metadata_still_works(self):
from litellm.proxy.common_utils.http_parsing_utils import (
get_tags_from_request_body,
)
tags = get_tags_from_request_body({"metadata": {"tags": ["x"]}})
assert tags == ["x"]

166
tests/test_litellm/proxy/test_litellm_pre_call_utils.py
View file

@ -280,6 +280,172 @@ 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_all_user_api_key_prefix_keys():
"""Strip must cover the full user_api_key_* family, not a hand-maintained
list of 2-3 names. Proxy writes a dozen such fields (user_id, alias,
spend, team_id, request_route, …) and an attacker populating any of them
in the non-authoritative metadata key would otherwise forge identity /
spend in audit logs and guardrails."""
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": "application/json"}
request_mock.client = MagicMock()
request_mock.client.host = "127.0.0.1"
attacker_injected = {
"user_api_key_user_id": "victim",
"user_api_key_alias": "admin-key",
"user_api_key_spend": 0.0,
"user_api_key_team_id": "victim-team",
"user_api_key_end_user_id": "victim-user",
"user_api_key_request_route": "/fake/route",
"user_api_key_hash": "fake-hash",
}
data = {
"model": "gpt-3.5-turbo",
"metadata": {**attacker_injected},
"litellm_metadata": {**attacker_injected},
}
user_api_key_dict = UserAPIKeyAuth(
api_key="hashed-key",
user_id="real-user",
metadata={},
team_metadata={},
spend=42.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",
)
# The non-authoritative metadata dict must not retain ANY attacker-injected
# user_api_key_* key.
other = updated.get("litellm_metadata") or {}
attacker_leaks = [k for k in other if k.startswith("user_api_key_")]
assert attacker_leaks == [], f"Unexpected leaked keys: {attacker_leaks}"
@pytest.mark.asyncio
async def test_add_litellm_data_to_request_string_metadata_does_not_crash():
"""Regression: pre-strip code that pre-populated data['metadata'][k]=v
before the string-to-dict parse would crash on JSON-string metadata.
The snapshot / strip / admin-population pipeline must survive metadata
arriving as a string."""
import json as _json
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"
data = {
"model": "gpt-3.5-turbo",
"metadata": _json.dumps({"generation_name": "test"}),
}
user_api_key_dict = UserAPIKeyAuth(
api_key="hashed-key",
metadata={},
team_metadata={},
spend=0.0,
max_budget=100.0,
model_max_budget={},
team_spend=0.0,
team_max_budget=200.0,
)
# Must not raise TypeError / AttributeError.
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",
)
# The parsed metadata should be a dict and the proxy snapshot body
# should have been taken AFTER the strip (so no leaked user_api_key_*
# from a raw string snapshot).
assert isinstance(updated["metadata"], dict)
assert updated["metadata"].get("generation_name") == "test"
@pytest.mark.asyncio
async def test_add_litellm_data_to_request_proxy_server_request_body_is_post_strip():
"""Regression: proxy_server_request['body'] used to be snapshotted before
the admin-slot strip, so standard_logging_object and spend-tracking
readers saw attacker-injected payload. Snapshot must now be post-strip."""
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": "application/json"}
request_mock.client = MagicMock()
request_mock.client.host = "127.0.0.1"
data = {
"model": "gpt-3.5-turbo",
"metadata": {"user_api_key_user_id": "victim"},
}
user_api_key_dict = UserAPIKeyAuth(
api_key="hashed-key",
user_id="real-user",
metadata={},
team_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",
)
snapshot_body = updated["proxy_server_request"]["body"]
assert snapshot_body is not None
snapshot_metadata = snapshot_body.get("metadata") or {}
assert "user_api_key_user_id" not in snapshot_metadata or (
snapshot_metadata["user_api_key_user_id"] != "victim"
)
@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