Skip to content

litellm_staging_04_04_2026 - #25192

Merged
Sameerlite merged 10 commits into
mainfrom
litellm_oss_staging_04_04_2026
Apr 14, 2026
Merged

litellm_staging_04_04_2026#25192
Sameerlite merged 10 commits into
mainfrom
litellm_oss_staging_04_04_2026

Conversation

@krrish-berri-2

Copy link
Copy Markdown
Contributor

Relevant issues

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Type

🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test

Changes

d 🔹 and others added 6 commits April 4, 2026 18:23
…es (#25085)

Fixes #25081.

is_tool_name_prefixed() checked for the presence of MCP_TOOL_PREFIX_SEPARATOR
(default '-') anywhere in the tool name.  Any non-MCP tool whose name
contains a hyphen (e.g. 'text-to-speech', 'code-review') was silently
misclassified as an MCP-prefixed tool.  When the semantic tool filter is
enabled, these tools would be routed through semantic matching and
potentially dropped.

Fix: accept an optional known_server_prefixes set.  When supplied, the
function extracts the candidate prefix (text before the first separator)
and checks it against the normalised set of registered server prefixes.
Only a genuine match returns True.  Without the set, legacy behaviour is
preserved for backward compatibility.

Updated _get_mcp_server_from_tool_name() to build the prefix set from
the live registry and pass it through.

9 new tests.

Co-authored-by: d 🔹 <258577966+voidborne-d@users.noreply.github.com>
## Problem

When `get_cache_key(**kwargs)` is called with kwargs that already
contains `preset_cache_key` (which can happen when cache key is
recomputed in certain code paths), the call to
`_set_preset_cache_key_in_kwargs()` fails with:

```
TypeError: _set_preset_cache_key_in_kwargs() got multiple values
for keyword argument 'preset_cache_key'
```

This is because `preset_cache_key` is passed both explicitly:
```python
self._set_preset_cache_key_in_kwargs(
    preset_cache_key=hashed_cache_key, **kwargs
)
```
And implicitly via `**kwargs` unpacking when `kwargs["preset_cache_key"]`
exists.

## Solution

Filter out `preset_cache_key` from kwargs before passing to
`_set_preset_cache_key_in_kwargs()`:

```python
kwargs_for_preset = {k: v for k, v in kwargs.items() if k != "preset_cache_key"}
self._set_preset_cache_key_in_kwargs(
    preset_cache_key=hashed_cache_key, **kwargs_for_preset
)
```

## Testing

Added unit tests covering:
- kwargs with existing preset_cache_key (the bug case)
- kwargs without preset_cache_key (regression test)
- Verification that preset_cache_key is correctly set in litellm_params
* fix(presidio): use correct text positions in anonymize_text (#24160)

The Presidio anonymizer endpoint returns items with start/end positions
that reference the *anonymized output* text, not the original input.
anonymize_text() was applying these positions to the original text,
causing garbled output with remnants of un-masked PII data.

When output_parse_pii is False, return redacted_text["text"] directly
from the anonymizer response instead of manually splicing.

When output_parse_pii is True, use analyze_results positions (which
correctly reference the original text) to build numbered replacement
tokens and the pii_tokens mapping.

* address review: remove dead code, fix token numbering order

- Remove unused `anon_item_by_entity` dict (Greptile P2)
- Number tokens left-to-right (<PERSON_1> first in text, not last)
- Add assertion for token numbering order in test
Extend LATENCY_BUCKETS beyond 5 minutes so request/LLM latency metrics
can distinguish long runs up to the typical default LLM request timeout.

Made-with: Cursor
@krrish-berri-2
krrish-berri-2 requested a review from a team April 5, 2026 16:38
@vercel

vercel Bot commented Apr 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Apr 14, 2026 3:50pm

Request Review

@CLAassistant

CLAassistant commented Apr 5, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
7 out of 8 committers have signed the CLA.

✅ nehaaprasad
✅ Dmitry-Kucher
✅ hunterchris
✅ jaxhend
✅ Sameerlite
✅ yuneng-berri
✅ kulia26
❌ d 🔹


d 🔹 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@greptile-apps

greptile-apps Bot commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This is a focused staging bundle of five targeted bug fixes: a TypeError crash in Cache.get_cache_key when preset_cache_key was already present in kwargs; SigV4 signature failures in S3Logger when object keys contain spaces; false-positive MCP prefix detection for hyphenated non-MCP tool names; garbled Presidio PII output caused by applying anonymizer output offsets to the original input text; and an infinite login-redirect loop when nginx-ingress adds HttpOnly to the server-set token cookie. Each fix is accompanied by new unit tests.

Confidence Score: 5/5

  • Safe to merge — all five bug fixes are correct, well-tested, and backward-compatible.
  • No P0 or P1 issues found. Every change has matching unit tests. The single P2 finding (inline imports in one test method) is a style nit that does not affect correctness or CI.
  • No files require special attention.

Important Files Changed

Filename Overview
litellm/caching/caching.py Correctly filters preset_cache_key from kwargs before forwarding to _set_preset_cache_key_in_kwargs to fix a TypeError: got multiple values crash when kwargs already contained that key.
litellm/integrations/s3_v2.py Uses the requests-prepared URL (prepped.url) for all S3 PUT/GET calls so that percent-encoding in object-key path segments matches the SigV4 canonical request — fixes signature failures for keys with spaces or special characters.
litellm/proxy/_experimental/mcp_server/utils.py Extends is_tool_name_prefixed to accept known_server_prefixes; when provided, only the substring before the first separator is matched against actual registered prefixes, eliminating false positives for non-MCP tools like text-to-speech.
litellm/proxy/guardrails/guardrail_hooks/presidio.py Refactors anonymize_text into three helpers: _post_presidio_anonymize, _finalize_presidio_anonymize_simple, and _finalize_presidio_anonymize_numbered_tokens. Correctly fixes the position-reference bug by using analyzer positions (original text) instead of anonymizer item positions (output text) when building numbered PII tokens.
ui/litellm-dashboard/src/utils/cookieUtils.ts Adds storeLoginToken (stores token in sessionStorage + JS cookie at /ui), updates getCookie to fall back to sessionStorage for the "token" key, and adds sessionStorage.removeItem("token") to clearTokenCookies. Correctly uses sessionStorage per CLAUDE.md guidance.
tests/test_litellm/integrations/test_s3_v2.py New test verifies that S3 PUT uses a percent-encoded URL; contains inline imports inside the test method body (violates CLAUDE.md style guide).

Sequence Diagram

sequenceDiagram
    participant Browser
    participant Proxy as LiteLLM Proxy
    participant nginx

    Browser->>Proxy: POST /login/v2
    Proxy-->>nginx: 200 OK + Set-Cookie header
    nginx-->>Browser: 200 OK, Set-Cookie with HttpOnly added by nginx
    Note over Browser: Server cookie is HttpOnly - invisible to JS

    Browser->>Browser: storeLoginToken(data.token from JSON body)
    Note over Browser: Sets JS cookie at path /ui and writes to sessionStorage

    Browser->>Browser: getCookie("token")
    alt JS cookie found at /ui
        Browser-->>Browser: returns cookie value
    else cookie blocked
        Browser-->>Browser: returns sessionStorage fallback
    end
Loading

Reviews (5): Last reviewed commit: "Fix tests" | Re-trigger Greptile

Comment on lines +1 to +10
"""
Test for preset_cache_key multiple values bug fix.

This test verifies that get_cache_key doesn't raise TypeError when kwargs
already contains preset_cache_key.

Issue: When get_cache_key(**kwargs) is called with kwargs containing
preset_cache_key, the call to _set_preset_cache_key_in_kwargs() would fail with:
TypeError: got multiple values for keyword argument 'preset_cache_key'
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Test placed in wrong directory

This file contains pure unit tests with no real network calls, so it belongs in tests/test_litellm/ (alongside the other new tests added by this PR) rather than tests/local_testing/. Tests under tests/local_testing/ are excluded from make test-unit, meaning this regression guard won't run in the standard CI pipeline.

Consider moving it to tests/test_litellm/caching/test_cache_preset_key.py.

… to cookies (#23532)

* fix(ui): resolve login redirect loop when reverse proxy adds HttpOnly to cookies

When LiteLLM is behind nginx-ingress or similar with security-hardened
configs, the reverse proxy adds HttpOnly to all Set-Cookie headers. This
makes the JWT token unreadable by JavaScript, causing an infinite login
redirect loop. Fix by returning the JWT token in the /v2/login response
body so the frontend can set a JS-accessible cookie directly.

Fixes #19663

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address Greptile review feedback

- Add window guard to setTokenCookie for SSR consistency with clearTokenCookies
- Add SSR test for window undefined case
- Add code comment explaining why JWT is included in response body

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address second round of Greptile review feedback

- Add loginCall integration tests verifying setTokenCookie is called with
  token and skipped when absent (backward-compatibility path)
- Use encodeURIComponent/decodeURIComponent in setTokenCookie/getCookie
  for defense-in-depth against non-standard token formats

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Update ui/litellm-dashboard/src/utils/cookieUtils.ts

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* Update ui/litellm-dashboard/src/utils/cookieUtils.ts

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* fix(ui): use sessionStorage instead of cookie for login token storage

Replace setTokenCookie (which is a no-op when reverse proxy adds HttpOnly)
with storeLoginToken using sessionStorage. Add sessionStorage fallback to
getCookie so the token is found even when the cookie is HttpOnly. Also handle
'=' in cookie values with .slice(1).join("=") and clear sessionStorage on
logout.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(ui): use shared getCookie in page.tsx and user_dashboard.tsx

Replace local getCookie functions in page.tsx and user_dashboard.tsx
with the shared one from cookieUtils that has the sessionStorage
fallback. Without this, the HttpOnly cookie fix was incomplete —
page.tsx (the dashboard entry point) could not read the token,
causing the redirect loop to persist.

Also scope the sessionStorage fallback to the "token" key only,
and clear sessionStorage in page.tsx deleteCookie.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(ui): scope deleteCookie sessionStorage cleanup to token key only

Also document the sessionStorage cross-tab trade-off: per-tab scope
means users behind an HttpOnly proxy must log in once per tab, but
this is intentional to avoid localStorage XSS exposure.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Update ui/litellm-dashboard/src/utils/cookieUtils.ts

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* Apply suggestion from @greptile-apps[bot]

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* style: remove stray double blank line in user_dashboard.tsx

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(ui): guard storeLoginToken against empty/whitespace-only tokens

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(ui): preserve sessionStorage token across beforeunload clear

The existing beforeunload handler calls sessionStorage.clear() to
flush cached UI data on page refresh. This also wiped the token
stored by storeLoginToken, re-introducing the redirect loop after
any page refresh in the HttpOnly proxy scenario. Now the token is
saved and restored across the clear.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(ui): set JS-accessible cookie at /ui path as HttpOnly workaround

sessionStorage alone is unreliable. Also set the token via
document.cookie at path=/ui — nginx only adds HttpOnly to server-set
Set-Cookie headers, so a JS-set cookie is always readable.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ui): use dynamic cookie path based on server_root_path

Hardcoded path=/ui breaks when LiteLLM is deployed with a custom
server_root_path. Now derives the cookie path from serverRootPath
so it works at /ui, /myapp/ui, etc.

Also reuse clearTokenCookies() in deleteCookie() to avoid duplication.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(ui): remove circular dependency in cookieUtils.ts

Derive the UI cookie path from window.location.pathname instead of
importing serverRootPath from networking.tsx. This breaks the
cookieUtils → networking → cookieUtils cycle that could cause
serverRootPath to be undefined under certain bundler configurations.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ui): harden getUiCookiePath regex and add missing tests

- Use regex /\/ui(?=\/|$)/ to match "/ui" only as a full path segment,
  preventing false matches on paths like "/my-ui-tool/login".
- Add unit tests for storeLoginToken empty/whitespace guard and
  cookie-at-/ui-path behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: fix Black formatting in audit_logs.py

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix CI: formatting, test params, remove token from login JSON

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: reformat with Black 23.x to match CI

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: keep token in login JSON body for UI storeLoginToken flow

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use storeLoginToken in exchangeLoginCode, add credentials include

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* revert: remove unrelated changes from HttpOnly cookie fix branch

Reset files not related to the login cookie fix back to main:
- prometheus.py, bedrock converse, guardrail handler
- auth_checks.py, reset_budget_job.py, audit_logs.py
- test_user_api_key_auth.py

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Revert "revert: remove unrelated changes from HttpOnly cookie fix branch"

This reverts commit 0684a1e.

* Revert "fix: use storeLoginToken in exchangeLoginCode, add credentials include"

This reverts commit 866405f.

* Revert "fix: keep token in login JSON body for UI storeLoginToken flow"

This reverts commit 086c416.

* Revert "fix: reformat with Black 23.x to match CI"

This reverts commit b2c3334.

* Revert "fix CI: formatting, test params, remove token from login JSON"

This reverts commit 2905d47.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@krrish-berri-2
krrish-berri-2 temporarily deployed to integration-redis-postgres April 6, 2026 18:13 — with GitHub Actions Inactive
@krrish-berri-2
krrish-berri-2 temporarily deployed to integration-postgres April 6, 2026 18:13 — with GitHub Actions Inactive
@krrish-berri-2
krrish-berri-2 temporarily deployed to integration-postgres April 6, 2026 18:13 — with GitHub Actions Inactive
@krrish-berri-2
krrish-berri-2 temporarily deployed to integration-postgres April 6, 2026 18:13 — with GitHub Actions Inactive
@krrish-berri-2
krrish-berri-2 temporarily deployed to integration-postgres April 6, 2026 18:13 — with GitHub Actions Inactive
@codspeed-hq

codspeed-hq Bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing litellm_oss_staging_04_04_2026 (69bf2bf) with main (e64d98f)

Open in CodSpeed

@codecov

codecov Bot commented Apr 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@krrish-berri-2 krrish-berri-2 changed the title chore: fixes litellm_staging_04_04_2026 Apr 13, 2026
@Sameerlite
Sameerlite temporarily deployed to integration-postgres April 14, 2026 14:53 — with GitHub Actions Inactive
@Sameerlite
Sameerlite temporarily deployed to integration-postgres April 14, 2026 14:53 — with GitHub Actions Inactive
@Sameerlite
Sameerlite temporarily deployed to integration-postgres April 14, 2026 14:53 — with GitHub Actions Inactive
@Sameerlite
Sameerlite temporarily deployed to integration-postgres April 14, 2026 14:53 — with GitHub Actions Inactive
@gitguardian

gitguardian Bot commented Apr 14, 2026

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
29203053 Triggered Generic Password 972e42c .circleci/config.yml View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@Sameerlite
Sameerlite merged commit b1c77d2 into main Apr 14, 2026
97 of 107 checks passed
@Sameerlite
Sameerlite deleted the litellm_oss_staging_04_04_2026 branch April 14, 2026 18:03
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants