fix(proxy): always merge caller-supplied tags into request metadata#27784
Conversation
Caller-supplied tags (`x-litellm-tags` header, body `tags`, `metadata.tags`) were silently dropped unless the key/team had `metadata.allow_client_tags: true` set. Restore the documented behavior: tags from the request always flow into `metadata.tags` and union with any admin-configured static tags from key/team/project metadata. Removes the `allow_client_tags` opt-in flag from the pre-call pipeline. The flag was only ever read here; it has no schema or endpoint footprint, so leftover values in existing key metadata are inert. Test cleanup mirrors the simplification: drop the three tests that verified the strip-when-not-opted-in path, drop the `allow_client_tags` fixture lines from the merge/union tests.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR removes the
Confidence Score: 3/5Merging restores documented tag-routing functionality but permanently removes the per-key/team tag restriction for all deployments, including those that may have been relying on the restriction as a security boundary. The core change removes the ability for admins to block caller-supplied routing tags on a per-key basis. Any deployment using tag-based routing to gate access to restricted model tiers (e.g., litellm/proxy/litellm_pre_call_utils.py — the tag propagation logic and the two now-stale security comments warrant close review before merging.
|
| Filename | Overview |
|---|---|
| litellm/proxy/litellm_pre_call_utils.py | Removes the allow_client_tags security gate unconditionally, meaning all authenticated callers can now supply routing/spend tags without admin opt-in; two comments referencing the removed strip are now stale and misleading. |
| tests/test_litellm/proxy/test_litellm_pre_call_utils.py | Drops three security-regression tests (strip-without-permission paths) and removes allow_client_tags: True fixtures from merge/union tests; remaining assertions are correct for the new unconditional behavior. |
| tests/proxy_unit_tests/test_proxy_utils.py | Removes allow_client_tags: True from two test fixtures; logic and assertions are otherwise unchanged and remain valid. |
Comments Outside Diff (3)
-
litellm/proxy/litellm_pre_call_utils.py, line 1437-1449 (link)Backwards-incompatible removal of
allow_client_tagssecurity gateAny deployment that relied on
allow_client_tags: false(the default before this change) to prevent authenticated callers from supplying arbitrary routing tags now silently accepts those tags. A caller can sendx-litellm-tags: premiumto reach a restrictedpremium-tagged deployment, or add body tags to misattribute spend to another team's tag budget — both with no admin consent. Per the project rule, removing a behavioral security gate without a user-controlled escape hatch risks breaking existing deployments that depend on the restriction. Consider keeping the flag but flipping its default totrueand adding adeny_client_tagsopt-out, rather than unconditionally removing the gate.Rule Used: What: avoid backwards-incompatible changes without... (source)
-
litellm/proxy/litellm_pre_call_utils.py, line 1451-1455 (link)Stale comment: the
allow_client_tagsstrip that this comment describes no longer exists. The phrase "tags without opt-in" is inaccurate now that tags are always accepted, and could mislead future maintainers into thinking the protection is still in place. -
litellm/proxy/litellm_pre_call_utils.py, line 1461-1463 (link)Stale comment: "runs AFTER the strip" no longer describes the real reason for ordering. The tag strip is gone, so the rationale should reflect the actual remaining guard (stripping
user_api_key_*and_UNTRUSTED_METADATA_CONTROL_FIELDSfromlitellm_metadata).
Reviews (1): Last reviewed commit: "fix(proxy): always merge caller-supplied..." | Re-trigger Greptile
| ) | ||
|
|
||
| if tags is not None and _admin_allow_client_tags: | ||
| if tags is not None: |
There was a problem hiding this comment.
High: Caller-controlled tag routing
A normal key holder can now send x-litellm-tags or root-level tags for a tag assigned to a restricted deployment; this branch copies those values into request metadata, and router tag filtering later selects deployments whose litellm_params.tags match request metadata. Keep caller-supplied routing/budget tags behind an admin opt-in, or reject/ignore all client tag sources by default, including metadata.tags, litellm_metadata.tags, root tags, and x-litellm-tags.
There was a problem hiding this comment.
Deliberate change - The features this change breaks does not justify the gaps this closes.
High: Caller-controlled tag routingThis PR makes caller-supplied tags flow into request metadata used by router tag filtering and spend attribution. A normal authenticated key holder can choose a tag assigned to a restricted deployment and have the router select that deployment. Status: 1 new · 2 open |
The tag-strip block was removed in the parent commit but two surrounding comments still referenced "tags without opt-in" and "runs AFTER the strip". Update them to describe the remaining user_api_key_* and _pipeline_managed_guardrails strip that the snapshot/merge ordering actually protects against.
| ) | ||
|
|
||
| if tags is not None and _admin_allow_client_tags: | ||
| if tags is not None: |
There was a problem hiding this comment.
High: Caller-controlled tag routing
x-litellm-tags and root-level tags are attacker-controlled request fields, but get_deployments_for_tag() later treats metadata.tags as routing input. A normal key holder can send the tag for a restricted deployment and route traffic there; keep these tags stripped or gate them on an admin-controlled key/team opt-in before merging them into metadata.
ad2a74c
into
litellm_internal_staging
Summary
Caller-supplied tags (
x-litellm-tagsheader, bodytags,metadata.tags) were silently dropped from request metadata unless the calling key/team hadmetadata.allow_client_tags: trueset. This broke two documented features:x-litellm-tags— the documented per-request header for routing to tagged deployments.request_tagsgoing into spend logs and/spend/tagsaggregations.This PR restores the previous behavior: tags from the request always flow into
metadata.tagsand union with any admin-configured static tags from key/team/project metadata.What changed
litellm/proxy/litellm_pre_call_utils.py— drop the conditional strip of callertagsfrom body /metadata/litellm_metadataand the conditional gate on thex-litellm-tagsheader merge. Both now run unconditionally.allow_client_tagsflag is removed from the pre-call pipeline. It was only ever read here — no schema, types, or endpoint footprint — so existing values in key/team metadata are inert.Test changes
"allow_client_tags": Truefixture lines from the merge/union/multipart tests; their assertions about union behavior continue to hold.Test plan
End-to-end verification against a running proxy:
x-litellm-tags: premiumroutes to thepremium-taggedtag-routeddeploymentx-litellm-tags: tenant:acmeroutes to thetenant:acme-tagged deploymentx-litellm-tags) appear inLiteLLM_SpendLogs.request_tags{"tags": [...]}) appear inLiteLLM_SpendLogs.request_tagsmetadata.tagsbody tags appear inLiteLLM_SpendLogs.request_tagsrequest_tags(no regression)Pytest:
uv run pytest tests/test_litellm/proxy/test_litellm_pre_call_utils.py— 125 passeduv run pytest tests/proxy_unit_tests/test_proxy_utils.py -k "spend_logs_metadata or duplicate_tags"— 69 passeduv run pytest tests/test_litellm/router_strategy/test_router_tag_routing.py— 21 passeduv run pytest tests/test_litellm/proxy/auth/test_auth_checks.py— 116 passed