fix(responses): stop the chat completions bridge from leaking unknown request fields - #36268
Open
anchan77 wants to merge 5 commits into
Open
Conversation
…completions bridge
…ugh the chat completions bridge
Contributor
Greptile SummaryThe follow-up replaces the bridge filter’s broad
Confidence Score: 5/5The PR appears safe to merge with respect to the previously reported typing issue. No blocking failure remains; the bridge filter now uses the repository-preferred
|
| Filename | Overview |
|---|---|
| litellm/responses/litellm_completion_transformation/handler.py | The previously reported untyped filtering boundary now uses Mapping[str, object], resolving the prior Any concern. |
| litellm/responses/litellm_completion_transformation/transformation.py | Maps supported Responses API request fields into the chat-completions request. |
| litellm/main.py | Adds chat-completions optional-parameter processing for store and prompt-cache fields. |
| tests/test_litellm/responses/litellm_completion_transformation/test_handler.py | Adds bridge allowlist and unsupported-field regression coverage. |
| tests/test_litellm/responses/test_responses_api_request_body.py | Adds wire-level coverage for filtering and mapped Responses API parameters. |
| tests/test_litellm/test_main.py | Verifies store and prompt-cache parameters reach the chat-completions wire payload. |
| type-discipline-budget.json | Reduces the explicit-Any budget after removing the reported violations. |
Reviews (2): Last reviewed commit: "refactor(responses): type the bridge kwa..." | Re-trigger Greptile
Author
|
@greptileai typed the bridge filter with object instead of Any in ac82c77, please take another look |
…itellm_fix_responses_bridge_param_leak # Conflicts: # tests/test_litellm/test_main.py # type-discipline-budget.json
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
Existing bridge tests all short-circuited before litellm.completion/ acompletion returned, so the ModelResponse and CustomStreamWrapper handling branches (both sync and async) were never exercised.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
Problem this solves:
How it solves it:
User Flow
Before: a Codex CLI user pointing at a proxy whose model is a chat-completions-only backend gets a 400 on every turn
base_url = "http://localhost:4000/v1"andwire_api = "responses"for their provider in~/.codex/config.tomland runcodex exec "Create hello.txt containing 'bridge works'"client_metadatafield400: Invalid JSON payload received. Unknown name "client_metadata": Cannot find field.and Codex prints an ERROR instead of doing the taskAfter: the same Codex run completes because client-only fields never reach the backend
base_url = "http://localhost:4000/v1"andwire_api = "responses"for their provider in~/.codex/config.tomland runcodex exec "Create hello.txt containing 'bridge works'"client_metadatafieldbridge dropped unsupported params: ('client_metadata',)on each turn, andprompt_cache_keynow reaches backends that support prompt cachingRelevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@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
All runs hit a real Google backend over its OpenAI-compatible /chat/completions surface and cost real money. The proxy was run with
python litellm/proxy/proxy_cli.py --config /tmp/litellm_codex_bridge_config.yaml --detailed_debug --use_v2_migration_resolver. Note: current litellm_internal_staging HEAD cannot boot the proxy under the locked fastapi 0.141.1 (missingget_flat_dependant); both runs had the one-file boot fix from the unmerged branchlitellm_fix_management_v1_flat_dependantapplied locally, which is unrelated to request handlingConfig used (a chat-completions-only deployment,
drop_params: Trueon, exactly as an affected user would have):Before, at commit cb211b5 (base of this branch), the exact request the latest Codex (0.135.0) shape produces, against Google's strict Generative Language OpenAI-compat endpoint:
Same commit against the more tolerant Vertex AI OpenAI-compat endpoint returns 200 but the debug log proves the leak still goes over the wire:
After, at commit 31a4057, the identical curl returns 200 with the model reply:
and the debug log shows the guard working instead of the leak:
Real Codex CLI end to end, at commit 31a4057 (
CODEX_HOME=/tmp/codex-qa-home LITELLM_API_KEY=sk-1234 codex exec --skip-git-repo-check --sandbox workspace-write "Create a file named hello.txt containing the single line 'bridge works', then read it back and tell me its contents."):The proxy log for that session shows Codex really sent the field on every turn and the bridge dropped it every time:
One caveat surfaced during QA: Google's Generative Language compat endpoint also rejects the spec-valid
prompt_cache_keythis PR now forwards. Backends that reject valid OpenAI chat params can opt out per deployment withadditional_drop_params, as in the config above; tolerant spec-compliant backends need nothingSecond backend, at commit 369f059 (current tip), an OpenAI-compatible chat-completions-only deployment registered in the DB through the Admin UI (a customer's self-hosted vLLM gateway, credential stored in the DB), exercising the same Codex-shaped payload:
Routing prerequisite: the deployment needs
use_chat_completions_api: truein itslitellm_params, otherwise/v1/responsesis forwarded verbatim to a backend that has no such route:Without that flag the turn fails before reaching the bridge at all:
With the flag set, the same Codex-shaped request succeeds:
The guard fires on the way through, and the outgoing chat completions body carries
prompt_cache_keywith an emptyextra_body:Scope note on this backend: it reaches litellm through the stock
openaiprovider and the OpenAI Python SDK, which rejects unrecognized top-level kwargs on its own, so the pre-fixextra_bodyleak is not observable here the way it is on the raw-HTTP Vertex path above. What this backend does confirm is that the filter is live on a real deployment, that the drop is logged rather than silent, and thatprompt_cache_keysurvives the bridge.text.verbositydoes not reach this backend either way:verbosityis gated to the GPT-5 family ingpt_5_transformation.py, so a non-GPT-5 model on the generic openai provider drops it downstream of this PR, matching OpenAI's own model gatingType
🐛 Bug Fix
Changes
litellm/responses/litellm_completion_transformation/handler.py: residual kwargs are filtered against litellm params +OPENAI_CHAT_COMPLETION_PARAMS+ provider credential keys (plusextra_body,drop_params,additional_drop_paramsand friends) before being splatted intolitellm.completion; anything else is dropped with a debug log.allowed_openai_paramsentries always survive, andextra_bodyremains the verbatim escape hatch. Previously every unknown key was swept intoextra_bodyand merged into the upstream JSON, whichdrop_paramscould not preventlitellm/responses/litellm_completion_transformation/transformation.py: the bridge now mapssafety_identifier,prompt_cache_key,prompt_cache_retentionandtext.verbosity->verbosity, and sourcesmetadata/service_tierfrom the parsed responses request (they are namedaresponses()params, so the oldkwargs.get(...)reads could never see them)litellm/main.py:store,prompt_cache_keyandprompt_cache_retentionare fed intoget_optional_params; they are valid chat params excluded from provider-specific passthrough, so before this they silently never reached the wire on any chat completions callallowed_openai_paramshonored) and wire-level respx tests asserting the outgoing /chat/completions JSON has noclient_metadata/custom_metadataand does carryprompt_cache_key,safety_identifierandverbosity; plus a wire test forstore/prompt_cache_key/prompt_cache_retentionon plain chat completions. All five behavior tests fail at the base commit and pass heretype-discipline-budget.json: LIT002 ceiling lowered by the 2 violations this refactor removedFinal Attestation