Repository navigation
fix(params): keep _litellm_* kwargs out of provider request bodies by construction - #43221
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
|
4707651 to
fac0829
Compare
|
bugbot run |
fac0829 to
86892b7
Compare
|
Generated by Claude Code |
|
bugbot run Generated by Claude Code |
59656df to
ec48dc4
Compare
|
Generated by Claude Code |
|
bugbot run Generated by Claude Code |
… construction Kwargs LiteLLM code introduces for its own use were only kept out of provider bodies if someone also listed them in all_litellm_params. Undeclared ones went into extra_body or optional_params, reached the provider, and the provider rejected the request. is_litellm_owned_kwarg in types/utils.py now defines LiteLLM-owned once: a registered name, or any name starting with INTERNAL_KWARG_PREFIX from litellm/constants.py. Every filter that builds provider params from kwargs uses it: chat completion, transcription, embedding, image generation and edit, search and video, ElevenLabs text to speech, and the Bedrock batch mapper. The two untyped shared filters now take Mapping[str, object] The stream_chunk_size wire test becomes test_internal_params_wire.py. It also sends an undeclared _litellm_ kwarg and asserts that no _litellm_ key reaches any of the six provider bodies, while extra_body passthrough keeps working Refs LIT-8318, LIT-8319
ec48dc4 to
de2a4d1
Compare
|
Generated by Claude Code |
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit de2a4d1. Configure here.
TLDR
Problem this solves:
How it solves it:
_litellm_...now always counts as LiteLLM's ownIntentional product change: a request key starting with
_litellm_is now dropped before the provider call instead of being forwardedLiteLLM decides what to send a provider by removing the kwargs it owns. Until now, "owns" meant "appears in
all_litellm_params", a list someone has to update by hand. When a new internal setting was added without updating that list, it went into the provider's request body, and strict providers like OpenAI answeredUnknown parameterThis PR adds one function,
is_litellm_owned_kwarg, that owns anything in that list plus any name starting with_litellm_. The prefix lives inlitellm/constants.pyasINTERNAL_KWARG_PREFIX. Every place that strips LiteLLM's kwargs before a provider call now asks that function: chat completions, transcription, search, video generation, embeddings, image generation, image edit, ElevenLabs text to speech and Bedrock batches. A name with_litellm_only in the middle, likeprovider_litellm_knob, still goes to the providerThe tests send a
_litellm_key down each of those paths and check that the provider never sees it. One of them reads the raw HTTP body sent to six providers. If the function stops checking the prefix, 28 tests failUser Flow
Before: a request carrying a
_litellm_key fails at the provider"_litellm_sentinel": "x"in the bodyUnknown parameter: '_litellm_sentinel'After: the same request succeeds, and the key never leaves LiteLLM
"_litellm_sentinel": "x"Linear ticket
Resolves LIT-8318
Resolves LIT-8319
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
On main, only chat completions leaked the key. OpenAI traffic on
/v1/messagesand/v1/responsesgoes through the Responses API, which already dropped unknown keys, so those two cases show nothing changed there. Each response below is trimmed to its reply text and HTTP statusSetup: a proxy on localhost:4417 with
$LITELLM_MASTER_KEYset and this config:Before (8327cd6)
/v1/chat/completions
_litellm_key/v1/messages
/v1/messages/v1/responses
/v1/responsesAfter (fac0829)
/v1/chat/completions
/v1/messages
/v1/messages/v1/responses
/v1/responsesType
🐛 Bug Fix
Caveats (if any)
Low
_litellm_key is dropped silently, not rejectedFinal Attestation
Note
Medium Risk
Touches shared kwarg classification used on many API paths; behavior change drops client
_litellm_*keys silently, but scope is narrow and well-covered by tests.Overview
Introduces
is_litellm_owned_kwargandINTERNAL_KWARG_PREFIX(_litellm_) so LiteLLM-owned kwargs are recognized by name prefix as well as the existingall_litellm_paramslist. Undeclared_litellm_*keys are stripped before provider calls instead of being forwarded (fixing 400s from strict APIs).Provider-param filtering now uses this helper in completion/transcription/embeddings paths (
filter_out_litellm_params,get_non_default_*), image generation/edit, Bedrock batch JSONL mapping, and ElevenLabs TTS. Keys likeprovider_litellm_knob(prefix not at start) still pass through.Tests assert
_litellm_undeclared_sentinelnever appears in outbound HTTP bodies or provider payloads across those surfaces.Reviewed by Cursor Bugbot for commit de2a4d1. Bugbot is set up for automated code reviews on this repo. Configure here.