fix(anthropic): add reasoning_content when converting thinking blocks to OpenAI format - #27947
Biogod2020 wants to merge 6 commits into
Conversation
…I#27897) * fix: block NaN/Inf budget bypass and add missing non-admin guards Addresses three security issues: GHSA-wvg4-6222-3q4r: /user/update exposes max_budget, soft_budget, spend to self-editing non-admin users with no server-side guard. Non-admin callers now receive HTTP 403 if any of those fields appear in the update payload. GHSA-q775-qw9r-2r4g: _enforce_upperbound_key_params returned early (no-op) when upperbound_key_generate_params was absent from config, letting any authenticated user generate a key with unlimited max_budget. Fix adds a delegated-authority ceiling in _common_key_generation_helper: non-admins cannot grant a key more budget than their own key carries. GHSA-2rv4-xv66-fpjg: float('nan') passes every `value < 0` guard because nan < 0 is False in Python, and spend >= nan is always False, permanently disabling budget enforcement for any entity carrying a NaN max_budget. All write-time budget guards now use `not math.isfinite(v) or v < 0`. _enforce_upperbound_key_params validates finiteness unconditionally (before the early-return). All spend-enforcement comparisons in auth_checks.py are now guarded with math.isfinite(max_budget) as defense-in-depth. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix: close budget ceiling bypass for callers with no max_budget (GHSA-q775) Non-admin callers whose API key has no explicit max_budget (None) could bypass the delegated-authority ceiling and create keys with arbitrary budgets. Now blocks budget assignment when caller has no budget configured. Also removes redundant inline import of LitellmUserRoles. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: only apply budget ceiling to explicitly requested max_budget Capture the caller-supplied max_budget before _enforce_upperbound_key_params can fill it with a default, so auto-filled defaults don't trigger the ceiling guard for non-admin users with no budget on their own key. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: capture requested max_budget before any defaults are applied Move _requested_max_budget capture before both default_key_generate_params and upperbound_key_generate_params mutations, so auto-filled values don't trigger the ceiling check for non-admin users. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: allow unlimited-budget callers to delegate any budget Callers with max_budget=None (unlimited) can legitimately create budget-capped keys. Only block when caller has an explicit budget and the requested budget exceeds it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: yuneng-jiang <yuneng@berri.ai> Co-authored-by: ryan-crabbe-berri <ryan@berri.ai> Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
* feat(lasso): extend LassoGuardrail to support tool calling (RND-5748)
* fix(lasso): PR review followups for tool-calling guardrail (RND-5748)
* fix(lasso): handle object-style tool_calls in _update_tool_calls_from_masked (RND-5748)
* fix(lasso): use model role for tool_use blocks (RND-5748)
* test(lasso): add round-trip tests for message transformation (RND-5748)
* fix(lasso): remove unused imports, handle Responses-API input masking, flatten multimodal content (RND-5748)
* fix(lasso): inspect Responses-API input field (RND-5748)
* fix(lasso): guard text-cursor remap against Lasso count mismatch (RND-5748)
* fix(lasso): flatten list content in tool_result.content (RND-5748)
* fix(lasso): remap multimodal list content during masking (RND-5748)
Bug: _map_masked_messages_back counted list-content messages in
original_text_count but the remap loop only handled isinstance(str).
The positional text_cursor never advanced for list messages, causing
all subsequent masked texts to be written onto the wrong messages.
Fix: added elif isinstance(content, list) branch that replaces the
list with the masked text string and advances the cursor — mirrors
the existing string-content branch. Also handles the assistant +
tool_calls combo for list-content messages.
Test: test_map_masked_messages_back_list_content verifies a user
message with [text + image_url] followed by an assistant message
gets correct masked content on both (cursor stays aligned).
* refactor(lasso): extract _get_field and _extract_tool_call_fields helpers (RND-5748)
The dict-vs-object access pattern (x.get('y') if isinstance(x, dict)
else getattr(x, 'y', None)) was duplicated 14 times across 5 methods.
_get_field(obj, field) — single-point dict/Pydantic field access.
_extract_tool_call_fields(call) — returns (call_id, name, parsed_input)
with JSON argument parsing, replacing ~30 duplicate lines in both
async_post_call_success_hook and _expand_messages_for_classification.
Also simplified _update_tool_calls_from_masked, _prepare_payload tool
mapping, and _apply_masking_to_model_response call_id extraction.
Net ~60 lines removed. No behavior change — all 32 tests pass.
* fix(lasso): add count guard to _apply_masking_to_model_response (RND-5748)
_apply_masking_to_model_response used a bare text_cursor without
verifying 1:1 correspondence between text-bearing choices and masked
text entries. If Lasso returned a different number of text messages
than choices with content, masked text would be applied to the wrong
choice or silently skip choices.
Added the same count-mismatch guard pattern already used in
_map_masked_messages_back: count original text-bearing choices,
compare to masked_text length, skip text remap on mismatch with a
warning log. Tool_call masking via id-based lookup is unaffected.
Tests:
- test_apply_masking_to_model_response_multiple_choices: verifies
correct per-choice masked text with 2 choices
- test_apply_masking_to_model_response_count_mismatch: verifies
content is left unchanged when counts disagree
* fix(lasso): close two guardrail-bypass paths flagged in review (RND-5748)
* tool-call args: when function.arguments is malformed JSON or parses
to a non-object, preserve the raw string as {"arguments": <raw>} so
Lasso still inspects it instead of receiving input=None. Covers both
pre-call and post-call extraction (shared helper). Also resolves the
CodeQL empty-except warning since the except body now assigns parsed=None.
* Responses-API input: when a request carries both "messages" and
"input", inspect both. Previously a benign messages array let the
guardrail skip data["input"] entirely. The masking write-back is
split via a count boundary so masked messages flow back to
data["messages"] and masked input flows back to data["input"]
without cross-contamination.
Tests: malformed/non-object args round-trip, dual-field classification,
dual-field masking write-back split.
* chore(lasso): black formatting + comment on expand skip branch (RND-5748)
* black: wrap two long expressions in lasso.py and reformat dict
literals in test_lasso.py to satisfy CI lint.
* add a short comment in _expand_messages_for_classification
explaining why empty string and None content are intentionally
skipped (None is the OpenAI shape for a pure tool-call turn).
* fix(lasso): satisfy mypy in _handle_masking, _update_tool_calls_from_masked, _apply_masking_to_model_response (RND-5748)
* Narrow `response.get("messages")` into a local before slicing so
mypy doesn't see `Optional[List[Dict[str, str]]]` as non-indexable.
* Rename the two write-side `func` bindings in
`_update_tool_calls_from_masked` to `func_dict` / `func_obj` so
mypy doesn't unify the dict and Any|None branches.
* Rename the inner loop variable in `_apply_masking_to_model_response`
from `msg` to `masked_msg` to avoid clashing with the
`msg = choice.message` rebinding below.
No behavior change; resolves the 7 mypy errors from the CI lint job.
…iAI#27858) - Introduce `_CallbackCapabilities` dataclass and `ProxyLogging._callback_capabilities()` static method that inspects `litellm.callbacks` once and caches capability flags keyed on (list length, member ids); invalidates automatically when the callback list mutates without per-request iteration overhead - Replace O(n) `litellm.callbacks` walks in `async_pre_call_hook`, `during_call_hook`, `async_post_call_streaming_iterator_hook`, `async_post_call_streaming_hook`, and `post_call_response_headers_hook` with fast-path exits when no relevant callbacks are registered - Add `needs_iterator_wrap()` and `needs_per_chunk_streaming_hook()` instance methods to decouple iterator-level wrapping from per-chunk hook execution; avoids `get_response_string` materialization per chunk when no guardrail or chunk-hook callback is active - Introduce `_fast_serialize_simple_model_response_stream()` using `orjson` for common single-choice text streaming chunks, bypassing the full Pydantic serializer; falls back to `model_dump_json` for tool calls, logprobs, usage, and provider-specific fields - Add early-return in `_restamp_streaming_chunk_model` when downstream model already matches the requested model, avoiding unnecessary string comparisons on every chunk - Fix stale zero-cost cache bug in `_is_model_cost_zero`: move the per-router `_zero_cost_cache` dict onto the `Router` instance and clear it in `_invalidate_model_group_info_cache` so in-place pricing updates via `upsert_deployment` immediately resume budget enforcement - Add `scripts/benchmark_chat_completions_perf.py`: standalone async benchmarking tool with a mock OpenAI provider, LiteLLM proxy process management, non-streaming RPS, streaming TTFT, and full-stream latency measurements with repeat/median run support - Add comprehensive unit tests covering capability detection, cache invalidation, fast-path correctness, zero-cost cache regression, and the no-callback streaming fast path Co-authored-by: Yassin Kortam <yassinkortam@g.ucla.edu>
…riAI#27910) The mutation-test workflow timed out at the 350-minute job cap when running whole-folder mutation against litellm/proxy/management_endpoints/ (~30 files, ~1.5 MB of source). Every mutant was running the full test suite, and mutants were generated for lines no test covers — which would survive regardless, just wasting compute. mutmut 3.x's mutate_only_covered_lines setting runs the suite once up front to compute coverage, then skips mutating uncovered lines. This cuts the mutant count dramatically and is the right semantic for the score (no test → no kill possible → uncountable). Per-mutant test filtering by function name is already automatic in mutmut 3.x; no external coverage step is needed.
… to OpenAI format When converting Anthropic assistant messages with thinking blocks to OpenAI Chat Completions format, the reasoning_content field was missing. This caused multi-turn requests to DeepSeek reasoning models to fail with: 'The reasoning_content in the thinking mode must be passed back to the API.' The fix maps the first thinking block's text to the standard reasoning_content field on the assistant message dict, matching what OpenAI / DeepSeek reasoning APIs expect in conversation history.
|
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR bundles the advertised
Confidence Score: 4/5The security fixes and performance optimizations look correct; the advertised reasoning_content change is the weakest part of the PR and its known gaps are documented in existing review threads. The budget NaN/Inf fixes are comprehensive — input validation at every management endpoint and isfinite() guards at every comparison site. The _zero_cost_cache invalidation is correctly wired to _invalidate_model_group_info_cache and initialized before set_model_list. The _CallbackCapabilities caching preserves behavioral equivalence with the old per-request callback scan: only callbacks that directly define apply_guardrail or async_post_call_streaming_iterator_hook in their own class dict are added to the iterator chain, which matches the old type(callback).dict checks. The fast orjson serialization falls back to the Pydantic slow path for every non-trivial chunk. The transformation.py fix still has open issues noted in previous threads — first-block-only truncation and absence of an automated regression test. litellm/llms/anthropic/experimental_pass_through/adapters/transformation.py — the reasoning_content mapping still only uses the first thinking block (see existing thread); litellm/proxy/utils.py — _callback_capabilities_cache uses id()-based keys (see existing thread)
|
| Filename | Overview |
|---|---|
| litellm/llms/anthropic/experimental_pass_through/adapters/transformation.py | Core fix: adds reasoning_content when converting thinking blocks to OpenAI format; see previous thread comments for known gaps (single-block truncation, no automated test) |
| litellm/proxy/auth/auth_checks.py | Adds math.isfinite() guards on all budget comparisons (GHSA-2rv4-xv66-fpjg) and introduces a per-router _zero_cost_cache with correct invalidation on model changes |
| litellm/router.py | Initializes _zero_cost_cache and moves _access_groups_cache init before set_model_list to avoid AttributeError during construction; _invalidate_model_group_info_cache now clears both caches |
| litellm/proxy/utils.py | Adds _CallbackCapabilities cache and fast-path short-circuits throughout the streaming/guardrail pipeline; id()-based cache key has a stale-entry risk (already flagged in prior thread) |
| litellm/proxy/proxy_server.py | Introduces _fast_serialize_simple_model_response_stream (orjson fast path), splits needs_iterator_wrap from needs_per_chunk_streaming_hook, and fires deferred logging on both wrap and no-wrap paths |
| litellm/proxy/management_endpoints/key_management_endpoints.py | Adds NaN/Inf rejection for all budget numeric fields, delegated-authority ceiling check (GHSA-q775-qw9r-2r4g), and updates _validate_max_budget to cover non-finite values |
| litellm/proxy/management_endpoints/internal_user_endpoints.py | Adds self-escalation prevention (GHSA-wvg4-6222-3q4r) blocking non-admins from modifying their own max_budget/soft_budget/spend; refactors audit log into _schedule_user_update_audit_log |
Reviews (2): Last reviewed commit: "fix(anthropic): add reasoning_content wh..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| first_thinking = thinking_blocks[0] | ||
| assistant_message["reasoning_content"] = first_thinking.get( # type: ignore | ||
| "thinking", "" | ||
| ) |
There was a problem hiding this comment.
reasoning_content, but thinking_blocks can contain multiple ChatCompletionThinkingBlock entries (and ChatCompletionRedactedThinkingBlock entries that have no thinking field). If the first block is redacted, .get("thinking", "") returns "", and if there are several real thinking blocks, the subsequent reasoning text is silently dropped. DeepSeek expects reasoning_content to reflect the full reasoning chain, so this produces incorrect context in both cases.
| first_thinking = thinking_blocks[0] | |
| assistant_message["reasoning_content"] = first_thinking.get( # type: ignore | |
| "thinking", "" | |
| ) | |
| assistant_message["reasoning_content"] = "\n".join( # type: ignore | |
| block.get("thinking", "") | |
| for block in thinking_blocks | |
| if block.get("type") == "thinking" | |
| and block.get("thinking") | |
| ) |
| if len(thinking_blocks) > 0: | ||
| assistant_message["thinking_blocks"] = thinking_blocks # type: ignore | ||
| # DeepSeek / reasoning models require `reasoning_content` | ||
| # when assistant messages in conversation history contain | ||
| # thinking blocks. Without it, multi-turn requests fail with | ||
| # "The `reasoning_content` in the thinking mode must be | ||
| # passed back to the API". | ||
| first_thinking = thinking_blocks[0] | ||
| assistant_message["reasoning_content"] = first_thinking.get( # type: ignore | ||
| "thinking", "" | ||
| ) |
There was a problem hiding this comment.
The PR description says the fix was verified end-to-end, but there is no automated test that covers the reasoning_content mapping from thinking blocks. Per the project's custom rule for PRs claiming to fix a specific issue, evidence should be in the form of passing tests, not only a manual session description. A missing test means the next refactor of this code path has no regression guard.
Rule Used: What: Ensure that any PR claiming to fix an issue ... (source)
| scanning cost dominated the proxy overhead on low-config deployments. | ||
|
|
||
| Cache invalidates whenever the list length or member identities change. | ||
| """ | ||
| callbacks = litellm.callbacks | ||
| sig = (len(callbacks), tuple(id(c) for c in callbacks)) | ||
| cache = ProxyLogging._callback_capabilities_cache | ||
| cached = cache.get(sig) | ||
| if cached is not None: | ||
| return cached | ||
|
|
||
| has_post_call_response_headers = False |
There was a problem hiding this comment.
id()-based cache key is vulnerable to stale entries after GC
The cache signature is (len(callbacks), tuple(id(c) for c in callbacks)). Once a callback object is removed from litellm.callbacks and garbage-collected, CPython may reuse its memory address for a new, differently-configured callback. If that new callback is then added at the same list position, the signature matches the stale cache entry. The .clear() at 32 entries bounds the window but does not prevent it entirely. A safer key for object callbacks would incorporate type(c) alongside id(c), reducing the chance of a false hit between callbacks of different types that land at the same address.
|
CI requires targeting Patch also available on branch |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| # Callers with max_budget=None (unlimited) can delegate any budget. | ||
| if ( | ||
| user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN.value | ||
| and _requested_max_budget is not None |
There was a problem hiding this comment.
High: Key budget bypass by omitting max_budget
A non-admin caller with a finite user_api_key_dict.max_budget can omit max_budget on /key/generate, which leaves the generated key without key_max_budget after model_dump(exclude_none=True). Enforce the caller's ceiling after defaults are applied: if the caller has a finite key budget, either reject data.max_budget is None or cap/fill it to user_api_key_dict.max_budget, and reject any value above that ceiling.
High: Key budget window limits can still be disabledThis PR adds finite-number checks for key budgets, but the new validation only covers top-level fields. A non-admin key owner can still update Status: 1 new · 1 open |
| # budget enforcement for any key that carries it. | ||
| for elem in data: | ||
| key, value = elem | ||
| if key in _BUDGET_NUMERIC_KEYS and value is not None: |
There was a problem hiding this comment.
High: Nested key budget limits can be disabled
This finite check only covers top-level key fields, but budget_limits carries its own max_budget values. A non-admin key owner can call /key/update with budget_limits: [{"budget_duration":"1d","max_budget": NaN}]; _is_budget_change does not treat budget_limits as a budget change, this loop does not reject the nested NaN, and _virtual_key_multi_budget_check skips non-finite window limits, disabling that key's window budget. Validate every budget_limits[*].max_budget as finite and treat changes to budget_limits as budget changes requiring the same admin path as max_budget/spend.
The Anthropic /v1/messages -> OpenAI chat-completions pass-through adapter
(translate_anthropic_messages_to_openai) attaches the Anthropic-specific
thinking_blocks field to assistant messages unconditionally. Non-Anthropic
OpenAI-compatible backends reject it: on multi-turn conversations, models
like GLM behind an OpenAI-compatible endpoint fail with
400 invalid_request_error: Extra inputs are not permitted,
field: 'messages[1].thinking_blocks'
This breaks any multi-turn conversation once an earlier assistant turn
carried reasoning.
Verified directly against an OpenAI-compatible GLM endpoint (bypassing
litellm):
- assistant turn with thinking_blocks -> 400 (field rejected)
- assistant turn with reasoning_content -> 200 OK (the model consumes it and
reasons over the prior turn)
So the fix is to convert, not just drop: for non-Anthropic backends, strip the
raw thinking_blocks and set the OpenAI-style reasoning_content string
(concatenating the unredacted thinking blocks; redacted blocks carry no
readable text and are dropped).
Gate the thinking_blocks attachment on is_anthropic_claude_model or
is_bedrock_arn_model, the same pair of checks already used together elsewhere
in this file (e.g. for cache_control). Anthropic Claude backends (anthropic/*,
bedrock *anthropic*, vertex *claude*, and Bedrock ARNs such as Application
Inference Profiles that point at Claude) keep thinking_blocks and their signed
signatures unchanged. Everyone else gets reasoning_content instead. When the
target model is unknown (None) the prior behaviour is preserved (blocks
kept), so no existing caller changes.
This is the complete form of the half-fixes in BerriAI#27947 and BerriAI#28258, both of
which only add reasoning_content and leave thinking_blocks attached, so they
do not resolve the 400. Closes BerriAI#27946.
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
7 participants
Fixes #27946
Summary
When converting Anthropic assistant messages with thinking blocks to OpenAI Chat Completions format, the
reasoning_contentfield was missing. DeepSeek reasoning models (and OpenAI o-series) require this field on assistant messages in multi-turn conversation history.Changes
In
litellm/llms/anthropic/experimental_pass_through/adapters/transformation.py, whenthinking_blocksis populated, also setreasoning_contentfrom the first thinking block's text:Testing
Verified end-to-end with Claude Code → LiteLLM proxy → DeepSeek reasoning model via OpenAI-compatible endpoint. Multi-turn conversations now succeed where they previously failed with: