feat(proxy): authorize caller-supplied tags against key/team metadata.tags - #27771
feat(proxy): authorize caller-supplied tags against key/team metadata.tags#27771yuneng-berri wants to merge 2 commits into
Conversation
Greptile SummaryThis PR removes the
Confidence Score: 3/5The change unconditionally opens tag injection to all callers, removing the only operator-side control over which keys/teams can supply routing and spend-attribution tags. The core change deletes a security gate that had explicit documentation of its attack vectors with no replacement mechanism for operators who want to preserve the restricted behavior. Two regression tests that directly guarded those scenarios are also deleted rather than updated. litellm/proxy/litellm_pre_call_utils.py and tests/test_litellm/proxy/test_litellm_pre_call_utils.py both need a second look — the former for the missing operator flag, the latter for the deleted regression guards.
|
| Filename | Overview |
|---|---|
| litellm/proxy/litellm_pre_call_utils.py | Removes the allow_client_tags gate entirely, making all caller-supplied tags unconditionally accepted into routing and spend metadata; a stale comment still references the removed "tags without opt-in" concept. |
| tests/test_litellm/proxy/test_litellm_pre_call_utils.py | Deletes two explicit tag-injection regression guards and two allow_client_tags=True opt-in tests rather than updating them to document the new unconditional behavior. |
| tests/proxy_unit_tests/test_proxy_utils.py | Removes allow_client_tags: True fixture lines from two spend-logs tests; union/merge assertions are unchanged and still pass. |
Comments Outside Diff (1)
-
litellm/proxy/litellm_pre_call_utils.py, line 1451-1455 (link)Stale comment still references "tags without opt-in" but the opt-in mechanism has been removed in this PR.
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/lit..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| _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 |
There was a problem hiding this comment.
The removed allow_client_tags gate was the only mechanism preventing callers from injecting tags to influence routing decisions or misattribute spend. With this change, any unauthenticated or low-privilege caller can supply x-litellm-tags: premium-tier or metadata.tags to reach tag-gated deployments and attribute spend to arbitrary tag buckets in /spend/tags aggregations. Custom rule b48b7341 requires that backwards-incompatible removals of behavior preserve a user-controlled flag for the old path. The prior allow_client_pricing_override gate (still present, a few lines above) demonstrates the expected pattern — consider keeping allow_client_tags as a per-key/team flag and defaulting it to true instead of removing it entirely, so operators who relied on the strip behavior can opt back in.
Rule Used: What: avoid backwards-incompatible changes without... (source)
| @@ -874,101 +874,8 @@ async def test_add_litellm_data_to_request_allows_redaction_opt_out_with_admin_o | |||
|
|
|||
There was a problem hiding this comment.
test_add_litellm_data_to_request_ignores_x_litellm_tags_header_without_permission and test_add_litellm_data_to_request_ignores_root_level_tags_without_permission were explicit regression tests for the path where an unprivileged caller injects tags via header or root body. Even if the behavior they test is intentionally being removed, deleting these rather than updating them to reflect the new expected behavior (e.g., asserting tags now flow through unconditionally) reduces the safety net for anyone who re-introduces the gate in the future.
Rule Used: What: Flag any modifications to existing tests and... (source)
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| 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: |
There was a problem hiding this comment.
High: Client-controlled routing tags
tags here comes directly from the request header or root body, and this PR also removed the earlier strip of metadata.tags / litellm_metadata.tags. Since router tag filtering uses metadata["tags"] to select deployments, a normal API caller can send a restricted deployment or team tag and have the request routed and accounted as if that tag was assigned by the key/team; keep caller-supplied tags behind an explicit admin opt-in and strip all tag sources by default.
High: Unauthorized tag spend attributionThis PR adds a privileged-tag filter for routing and tag-budget checks, but the filtered tags are not written back to request metadata. Downstream spend logging and spend counters consume the original caller-supplied metadata tags, so a normal API caller can charge spend to another tag bucket. Status: 1 new · 2 open |
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.
….tags
A caller-supplied tag is honored for tag-based routing and tag-budget
enforcement only when one of:
- the tag is not "privileged" (no deployment uses it, no budget
attached), or
- one of key.metadata.tags / team_metadata.tags carries a fnmatch glob
that matches the tag.
The privileged-tag set is auto-detected from existing router state and
LiteLLM_TagTable rows with a budget; cached in memory with a 30s TTL so
the hot path stays I/O-free. Reuses the metadata.tags field already used
for admin-injected static tags - no schema change, no new config.
Restores the per-tenant SaaS routing pattern via glob (e.g. a gateway key
with metadata.tags=["tenant:*"] can claim "tenant:acme") while preventing
unauthorized callers from reaching tag-gated deployments or polluting
tag-budget buckets that belong to other callers.
5d105a5 to
6b3880f
Compare
yuneng-berri
changed the title
|
Superseded by a fresh PR — scope shifted from 'restore caller-tag flow' to 'restore + add authorization gate', and the reviewer comments here were about the earlier scope. Re-opening with the combined two-commit shape for a clean review pass. |
| # privileged tag. Non-privileged tags pass through unchanged. | ||
| if request_tags: | ||
| await ensure_fresh_privileged_tags() | ||
| request_tags = filter_authorized_tags( |
There was a problem hiding this comment.
High: Unauthorized tag spend attribution
request_tags is filtered only for this router decision, but metadata["tags"] still contains the original caller-supplied tags. The cost callback later reads metadata["tags"] for request_tags and increments spend:tag:<tag>, so an API caller can include a victim budget tag and have their spend counted against that tag even though routing and the pre-call budget check ignored it. Filter the metadata tags in a central pre-call path, or write the authorized tag list back before any spend logging/counter code can read it.
1 participant
Summary
Restores tag-based routing and tag-based spend attribution for caller-supplied tags (
x-litellm-tagsheader, bodytags,metadata.tags), then gates the privileged paths so unauthorized callers cannot reach tag-routed deployments or pollute tag-budget buckets owned by other callers.Two commits, designed to read in order:
fix(proxy): always merge caller-supplied tags into request metadata— removes theallow_client_tagsstrip + merge gate added in fix(proxy): read guardrail config from admin metadata, fix tag routing consistency #25905. Caller tags flow unconditionally into request metadata. Documented routing and spend-attribution paths work again.feat(proxy): authorize caller-supplied tags against key/team metadata.tags— adds an authorization gate at the two privileged consumers (tag-based routing and tag-budget enforcement). Reuses the existingmetadata.tagsfield as the caller's authorized set, withfnmatchglob support.How the authorization gate works
A caller-supplied tag is honored for tag-based routing and tag-budget enforcement only when one of:
litellm_params.tags), no budget is attached to it (LiteLLM_TagTable.budget_id); ORkey.metadata.tags/team_metadata.tagscarries afnmatchglob pattern that matches the tag.The privileged-tag set is auto-detected from existing router state and tag-table rows. It is cached in memory with a 30-second TTL — zero per-request DB hits on the hot path.
Examples
{}x-litellm-tags: premium{"tags": ["premium"]}x-litellm-tags: premium{"tags": ["tenant:*"]}x-litellm-tags: tenant:acme{"tags": ["tenant:*"]}x-litellm-tags: premiumWhat changed
litellm/proxy/litellm_pre_call_utils.pyallow_client_tagsstrip block + the gated header mergelitellm/proxy/auth/tag_authorization.pycaller_authorized_for_tag,filter_authorized_tags, in-memory privileged-tag cachelitellm/router_strategy/tag_based_routing.pyrequest_tagsthrough the authorization gate before per-deployment matchinglitellm/proxy/auth/auth_checks.pytagsin_tag_max_budget_checkbefore tag-budget batch lookuptests/test_litellm/proxy/auth/test_tag_authorization.pytests/test_litellm/proxy/test_litellm_pre_call_utils.pytests/proxy_unit_tests/test_proxy_utils.pyMigration
No schema change. No new config field.
For customers using the existing tag features:
metadata.tags = ["pattern"]on keys that should claim privileged tags; one/key/updateper keyHot-path cost
fnmatch.fnmatchcasecallsllm_router.model_list(in-memory) + one indexed scan ofLiteLLM_TagTableTest plan
End-to-end verification against a running proxy with two
tag-routeddeployments (["premium"],["tenant:acme"]) and one default deployment:K_NO(nometadata.tags) sendsx-litellm-tags: premium→ falls back to default (was reaching premium before)K_NOsendsx-litellm-tags: tenant:acme→ falls back to default (was reaching acme before)K_NOsendsx-litellm-tags: workflow:report-gen→ non-privileged, recorded in spend logrequest_tagsK_PREMIUM(metadata.tags=["premium"]) sendsx-litellm-tags: premium→ routes to premium deploymentK_TENANT(metadata.tags=["tenant:*"]) sendsx-litellm-tags: tenant:acme→ glob authorizes → routes to acmeK_NOsends no tags → 200 + default model responsePytest:
tests/test_litellm/proxy/auth/test_tag_authorization.py— 18 passedtests/test_litellm/router_strategy/test_router_tag_routing.py— 21 passedtests/test_litellm/router_strategy/test_router_tag_regex_routing.py— 6 passedtests/test_litellm/proxy/auth/test_auth_checks.py— 116 passedtests/test_litellm/proxy/test_litellm_pre_call_utils.py— 125 passed