-
-
Notifications
You must be signed in to change notification settings - Fork 11.6k
fix(proxy): always merge caller-supplied tags into request metadata #27784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1437,46 +1437,8 @@ async def add_litellm_data_to_request( # noqa: PLR0915 | |
| if not _key_or_team_allows_client_pricing_override(user_api_key_dict): | ||
| _strip_client_pricing_overrides(data) | ||
|
|
||
| # 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) | ||
| # Also strip the root-level `tags` field. get_tags_from_request_body | ||
| # reads request_body["tags"] directly and feeds it to the policy | ||
| # engine, so leaving it in place here would let the strip-in-metadata | ||
| # above be trivially bypassed by moving the tags to the body root. | ||
| if "tags" in data: | ||
| data.pop("tags", None) | ||
| _stripped_from.append("tags (root)") | ||
| 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), | ||
| ) | ||
|
|
||
| # Fill in the proxy_server_request body snapshot now that metadata has | ||
| # been parsed and stripped. Consumers (standard_logging_payload, lago, | ||
| # been parsed. 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. | ||
| # | ||
|
|
@@ -1486,19 +1448,21 @@ async def add_litellm_data_to_request( # noqa: PLR0915 | |
| _body_snapshot = {k: v for k, v in data.items() if k != "secret_fields"} | ||
| data["proxy_server_request"]["body"] = _body_snapshot | ||
|
|
||
| # Snapshot the (now-cleaned) requester-supplied metadata for downstream | ||
| # consumers. Taking the deepcopy AFTER the strip prevents attacker- | ||
| # 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. | ||
| # Snapshot the requester-supplied metadata for downstream consumers. | ||
| # Taking the deepcopy after the user_api_key_* / _pipeline_managed_guardrails | ||
| # strip above prevents those proxy-internal slots — if a caller forged | ||
| # them — from leaking into requester_metadata where guardrails and audit | ||
| # paths may read from it. | ||
| if "metadata" in data and isinstance(data["metadata"], dict): | ||
| data[_metadata_variable_name]["requester_metadata"] = copy.deepcopy( | ||
| data["metadata"] | ||
| ) | ||
|
|
||
| # 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. | ||
| # Merge litellm_metadata into the metadata variable (preserving existing | ||
| # values). Runs after the user_api_key_* / _pipeline_managed_guardrails | ||
| # strip above so those proxy-internal slots — if a caller forged them | ||
| # into 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]: | ||
|
|
@@ -1665,27 +1629,19 @@ async def add_litellm_data_to_request( # noqa: PLR0915 | |
| user_agent = request.headers["user-agent"] | ||
| data[_metadata_variable_name]["user_agent"] = user_agent | ||
|
|
||
| # Check if using tag based routing. The helper reads caller-controlled | ||
| # sources (x-litellm-tags header, data["tags"] root-level), so its result | ||
| # is still gated by the same allow_client_tags flag that gated the | ||
| # body-metadata tag strip above. Otherwise the strip is trivially | ||
| # bypassed by sending tags via header or at the root of the body. | ||
| # Merge caller-supplied tags (x-litellm-tags header, data["tags"] root-level) | ||
| # into request metadata for tag-based routing and spend attribution. | ||
| tags = LiteLLMProxyRequestSetup.add_request_tag_to_metadata( | ||
| llm_router=llm_router, | ||
| headers=_headers, | ||
| data=data, | ||
| ) | ||
|
|
||
| if tags is not None and _admin_allow_client_tags: | ||
| if tags is not None: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High: Caller-controlled tag routing
|
||
| data[_metadata_variable_name]["tags"] = LiteLLMProxyRequestSetup._merge_tags( | ||
| request_tags=data[_metadata_variable_name].get("tags"), | ||
| tags_to_add=tags, | ||
| ) | ||
| elif tags is not None: | ||
| verbose_proxy_logger.warning( | ||
| "Ignored caller-supplied tags from header/root body: this " | ||
| "key/team does not have `allow_client_tags: true` in its metadata." | ||
| ) | ||
|
|
||
| # Team Callbacks controls | ||
| callback_settings_obj = _get_dynamic_logging_metadata( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High: Caller-controlled tag routing
A normal key holder can now send
x-litellm-tagsor root-leveltagsfor a tag assigned to a restricted deployment; this branch copies those values into request metadata, and router tag filtering later selects deployments whoselitellm_params.tagsmatch request metadata. Keep caller-supplied routing/budget tags behind an admin opt-in, or reject/ignore all client tag sources by default, includingmetadata.tags,litellm_metadata.tags, roottags, andx-litellm-tags.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deliberate change - The features this change breaks does not justify the gaps this closes.