fix: backport #27892 to litellm_1.84.0rc2 - #27903
Conversation
Authenticated clients could supply CustomPricingLiteLLMParams fields (input_cost_per_token, output_cost_per_token, etc.) in the request body. These were forwarded to register_model() in main.py, permanently mutating the shared global litellm.model_cost dict for all users on the instance. Adds all CustomPricingLiteLLMParams fields to _BANNED_REQUEST_BODY_PARAMS so is_request_body_safe() rejects them before they reach completion(). New pricing fields added to CustomPricingLiteLLMParams are auto-covered. Admin opt-in via allow_client_side_credentials or configurable_clientside_auth_params still works as before. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
aws_sts_endpoint, aws_web_identity_token, and aws_bedrock_runtime_endpoint in ingest_options.vector_store were passed directly to the Bedrock ingestion class, which reads them into boto3 STS client construction. Any authenticated caller could redirect AssumeRole calls to an attacker-controlled server, leaking the proxy's instance profile credentials. Calls is_request_body_safe() on ingest_options["vector_store"] before forwarding to litellm.aingest(). Same banned-params list and admin opt-in escape hatch (allow_client_side_credentials) as the /chat/completions path. ValueError from the safety check is caught and re-raised as HTTP 400. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis backport cherry-picks two security fixes from
Confidence Score: 3/5The RAG SSRF fix is clean, but the pricing-injection fix has a gap: the allow_client_side_credentials bypass silently disables the new ban for any deployment with credential passthrough enabled. The pricing field ban is added to the same list bypassed entirely when allow_client_side_credentials is True. An operator enabling that flag for credential passthrough in a multi-tenant deployment would unknowingly allow authenticated clients to overwrite the global litellm.model_cost registry, corrupting cost attribution for all users. litellm/proxy/auth/auth_utils.py — the interaction between the new CustomPricingLiteLLMParams ban and the allow_client_side_credentials bypass path warrants a closer look.
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/auth_utils.py | Adds CustomPricingLiteLLMParams fields to _BANNED_REQUEST_BODY_PARAMS; the ban is bypassed in full when allow_client_side_credentials=True, which conflates credential passthrough with global pricing registry writes. |
| litellm/proxy/rag_endpoints/endpoints.py | Adds is_request_body_safe check on vector_store config inside rag_ingest to block SSRF fields; correctly converts ValueError to HTTP 400. |
| tests/test_litellm/proxy/auth/test_auth_utils.py | New TestPricingInjectionBlocked class covers the core ban, the exhaustive field check, and the admin opt-in bypass; all tests use mocks only (no real network calls). |
| tests/test_litellm/proxy/rag_endpoints/test_rag_endpoints.py | TestRagIngestSSRFBlocked tests three SSRF fields and a clean-payload path; the happy-path assertion (status != 400) is weaker than it could be. |
Reviews (1): Last reviewed commit: "fix: block SSRF fields in RAG ingest vec..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| *sorted(_build_banned_observability_params()), | ||
| *sorted(CustomPricingLiteLLMParams.model_fields.keys()), | ||
| ) |
There was a problem hiding this comment.
allow_client_side_credentials
_check_banned_params returns early for every banned param when general_settings["allow_client_side_credentials"] is True, so the new CustomPricingLiteLLMParams entries are also fully bypassed. The allow_client_side_credentials flag was designed to allow trusted clients to supply their own API keys and endpoint overrides; it does not logically opt the admin into allowing clients to overwrite the global litellm.model_cost registry (which affects cost tracking for all users on the instance). An admin who enables credential passthrough for a multi-tenant deployment would unwittingly open global pricing writes to every authenticated caller. Consider checking the pricing fields with a separate, unconditional guard that is not gated on allow_client_side_credentials.
| assert response.status_code != 400, ( | ||
| f"Clean Bedrock ingest_options should not be rejected: {response.json()}" | ||
| ) |
There was a problem hiding this comment.
assert response.status_code != 400 passes even if the endpoint returns a 500 (e.g., if the aingest mock is not applied or the test client has auth issues). This means the test would silently "pass" on a server error rather than confirming the clean payload was actually accepted. Asserting response.status_code == 200 (or the expected 2xx code) would catch regressions properly.
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| # integrations are covered automatically. Sorted for stable iteration | ||
| # order and reviewable diffs. | ||
| *sorted(_build_banned_observability_params()), | ||
| *sorted(CustomPricingLiteLLMParams.model_fields.keys()), |
There was a problem hiding this comment.
High: Pricing overrides can be bypassed by the credentials opt-in
Adding custom pricing fields to _BANNED_REQUEST_BODY_PARAMS makes them subject to the existing allow_client_side_credentials early return in _check_banned_params. On deployments that enable client-side credentials, an authenticated client can send input_cost_per_token and output_cost_per_token in a completion request and update the shared model cost entry, letting them undercount or zero out spend for that model. Keep custom pricing in a separate deny list that is not bypassed by allow_client_side_credentials, or require a pricing-specific admin opt-in.
High: Pricing overrides can be bypassed by the client-side credentials opt-inThis PR adds custom pricing fields to the proxy request-body blocklist and adds RAG ingest validation. Because the pricing fields are added to the same blocklist that is bypassed when client-side credentials are allowed, deployments that enable that setting may still allow authenticated clients to change shared model pricing. Status: 1 new · 1 open |
3 participants
Backport of #27892 onto
litellm_1.84.0rc2.Cherry-picked commits
b95130eb32— fix: block client-side pricing injection via request bodyf1d07c13e5— fix: block SSRF fields in RAG ingest vector_store configConflict resolution
Cherry-pick applied cleanly with an auto-merge in
litellm/proxy/auth/auth_utils.py. No manual conflict resolution needed.Test plan
uv run pytest tests/test_litellm/proxy/auth/test_auth_utils.py -vuv run pytest tests/test_litellm/proxy/rag_endpoints/test_rag_endpoints.py -vmake test-unitpasses on the rc2 base