From cf94f4d8b720e63beadd10a28cea8c1787830814 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?d=20=F0=9F=94=B9?= Date: Sun, 5 Apr 2026 09:23:32 +0800 Subject: [PATCH 1/9] fix(mcp): is_tool_name_prefixed validates against known server prefixes (#25085) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- .../mcp_server/mcp_server_manager.py | 7 +- .../proxy/_experimental/mcp_server/utils.py | 32 +++++-- .../mcp_server/test_is_tool_name_prefixed.py | 90 +++++++++++++++++++ 3 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 tests/test_litellm/proxy/_experimental/mcp_server/test_is_tool_name_prefixed.py diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index 7b87e7e7e61..5363e317ff7 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -2552,7 +2552,12 @@ class MCPServerManager: return server # If not found and tool name is prefixed, try extracting server name from prefix - if is_tool_name_prefixed(tool_name): + known_prefixes = { + normalize_server_name(get_server_prefix(s)) + for s in self.get_registry().values() + if get_server_prefix(s) + } + if is_tool_name_prefixed(tool_name, known_server_prefixes=known_prefixes): ( original_tool_name, server_name_from_prefix, diff --git a/litellm/proxy/_experimental/mcp_server/utils.py b/litellm/proxy/_experimental/mcp_server/utils.py index 8189f212bcb..79942eda54e 100644 --- a/litellm/proxy/_experimental/mcp_server/utils.py +++ b/litellm/proxy/_experimental/mcp_server/utils.py @@ -100,17 +100,39 @@ def split_server_prefix_from_name(prefixed_name: str) -> Tuple[str, str]: return prefixed_name, "" -def is_tool_name_prefixed(tool_name: str) -> bool: +def is_tool_name_prefixed( + tool_name: str, + known_server_prefixes: Optional[set] = None, +) -> bool: """ - Check if tool name has server prefix + Check if tool name has a known MCP server prefix. + + When ``known_server_prefixes`` is provided the function verifies that the + substring before the first separator is an actual registered server + prefix. Without it the check falls back to the legacy heuristic + (separator present anywhere in the name), which can produce false + positives for non-MCP tools whose names contain hyphens + (e.g. ``text-to-speech``, ``code-review``). Args: - tool_name: Tool name to check + tool_name: Tool name to check. + known_server_prefixes: Optional set of normalised server prefixes + currently registered in the MCP manager. Pass this whenever + the caller has access to the server registry so that the check + is accurate. Returns: - True if tool name is prefixed, False otherwise + True if tool name is prefixed, False otherwise. """ - return MCP_TOOL_PREFIX_SEPARATOR in tool_name + if MCP_TOOL_PREFIX_SEPARATOR not in tool_name: + return False + + if known_server_prefixes is not None: + candidate_prefix = tool_name.split(MCP_TOOL_PREFIX_SEPARATOR, 1)[0] + return normalize_server_name(candidate_prefix) in known_server_prefixes + + # Legacy fallback – separator present somewhere in the name. + return True def validate_mcp_server_name( diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_is_tool_name_prefixed.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_is_tool_name_prefixed.py new file mode 100644 index 00000000000..d761d9c54cc --- /dev/null +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_is_tool_name_prefixed.py @@ -0,0 +1,90 @@ +""" +Tests for is_tool_name_prefixed with known_server_prefixes parameter. + +Verifies fix for https://github.com/BerriAI/litellm/issues/25081 +""" + +import pytest + +from litellm.proxy._experimental.mcp_server.utils import is_tool_name_prefixed + + +# --------------------------------------------------------------------------- +# Legacy behaviour (no known_server_prefixes passed) +# --------------------------------------------------------------------------- + + +class TestLegacyBehaviour: + """Without known_server_prefixes the function falls back to heuristic.""" + + def test_plain_name_returns_false(self): + assert is_tool_name_prefixed("get_weather") is False + + def test_hyphenated_name_returns_true_legacy(self): + """Legacy heuristic: any hyphen → True (the bug this issue reports).""" + assert is_tool_name_prefixed("text-to-speech") is True + + def test_prefixed_name_returns_true_legacy(self): + assert is_tool_name_prefixed("myserver-get_weather") is True + + +# --------------------------------------------------------------------------- +# New behaviour (known_server_prefixes supplied) +# --------------------------------------------------------------------------- + + +class TestWithKnownPrefixes: + """When known_server_prefixes is supplied, only real prefixes match.""" + + PREFIXES = {"myserver", "weather_api", "code_tools"} + + def test_known_prefix_returns_true(self): + assert ( + is_tool_name_prefixed( + "myserver-get_weather", known_server_prefixes=self.PREFIXES + ) + is True + ) + + def test_hyphenated_non_mcp_tool_returns_false(self): + """This is the core fix: 'text-to-speech' is NOT an MCP-prefixed tool.""" + assert ( + is_tool_name_prefixed( + "text-to-speech", known_server_prefixes=self.PREFIXES + ) + is False + ) + + def test_code_review_not_misclassified(self): + assert ( + is_tool_name_prefixed( + "code-review", known_server_prefixes=self.PREFIXES + ) + is False + ) + + def test_no_separator_returns_false(self): + assert ( + is_tool_name_prefixed( + "simple_tool", known_server_prefixes=self.PREFIXES + ) + is False + ) + + def test_empty_prefixes_set_rejects_all(self): + """With an empty registry, nothing can be prefixed.""" + assert ( + is_tool_name_prefixed("myserver-get_weather", known_server_prefixes=set()) + is False + ) + + def test_prefix_normalisation(self): + """Server names with spaces are normalised to underscores.""" + prefixes = {"my_server"} + # add_server_prefix_to_name normalises spaces → underscores + assert ( + is_tool_name_prefixed( + "my_server-list_files", known_server_prefixes=prefixes + ) + is True + ) From d6351a3966e5cbbddee1bacd63020bb6aa857614 Mon Sep 17 00:00:00 2001 From: Neha Prasad Date: Sun, 5 Apr 2026 07:09:37 +0530 Subject: [PATCH 2/9] fix(s3_v2): use prepared URL for SigV4-signed S3 requests (#25074) --- litellm/integrations/s3_v2.py | 13 +++--- tests/test_litellm/integrations/test_s3_v2.py | 44 +++++++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/litellm/integrations/s3_v2.py b/litellm/integrations/s3_v2.py index 405bf9698cc..f767f61be87 100644 --- a/litellm/integrations/s3_v2.py +++ b/litellm/integrations/s3_v2.py @@ -403,9 +403,8 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): # Prepare the signed headers signed_headers = dict(aws_request.headers.items()) - # Make the request response = await self.async_httpx_client.put( - url, data=json_string, headers=signed_headers + prepped.url, data=json_string, headers=signed_headers ) response.raise_for_status() except Exception as e: @@ -582,8 +581,9 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): if self.s3_verify is not None else None ) - # Make the request - response = httpx_client.put(url, data=json_string, headers=signed_headers) + response = httpx_client.put( + prepped.url, data=json_string, headers=signed_headers + ) response.raise_for_status() except Exception as e: verbose_logger.exception(f"Error uploading to s3: {str(e)}") @@ -674,8 +674,9 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): # Prepare the signed headers signed_headers = dict(aws_request.headers.items()) - # Make the request - response = await self.async_httpx_client.get(url, headers=signed_headers) + response = await self.async_httpx_client.get( + prepped.url, headers=signed_headers + ) if response.status_code != 200: verbose_logger.exception( diff --git a/tests/test_litellm/integrations/test_s3_v2.py b/tests/test_litellm/integrations/test_s3_v2.py index b53c05fa241..943fe4ec37b 100644 --- a/tests/test_litellm/integrations/test_s3_v2.py +++ b/tests/test_litellm/integrations/test_s3_v2.py @@ -292,6 +292,50 @@ class TestS3V2UnitTests: assert result == {"downloaded": "data"} + @patch("asyncio.create_task") + @patch("litellm.integrations.s3_v2.CustomBatchLogger.periodic_flush") + def test_s3_v2_put_url_encodes_spaces_in_object_key( + self, mock_periodic_flush, mock_create_task + ): + import requests + from unittest.mock import AsyncMock + + from litellm.types.integrations.s3_v2 import s3BatchLoggingElement + + mock_periodic_flush.return_value = None + mock_create_task.return_value = None + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.raise_for_status = MagicMock() + + s3_object_key = "My Team/2025-09-14/test-key.json" + test_element = s3BatchLoggingElement( + s3_object_key=s3_object_key, + payload={"test": "data"}, + s3_object_download_filename="test-file.json", + ) + + s3_logger = S3Logger( + s3_bucket_name="test-bucket", + s3_endpoint_url="https://s3.amazonaws.com", + s3_aws_access_key_id="test-key", + s3_aws_secret_access_key="test-secret", + s3_region_name="us-east-1", + ) + s3_logger.async_httpx_client = AsyncMock() + s3_logger.async_httpx_client.put.return_value = mock_response + + asyncio.run(s3_logger.async_upload_data_to_s3(test_element)) + + call_args = s3_logger.async_httpx_client.put.call_args + assert call_args is not None + actual_url = call_args[0][0] + raw_url = f"https://s3.amazonaws.com/test-bucket/{s3_object_key}" + expected_url = requests.Request("PUT", raw_url).prepare().url + assert actual_url == expected_url + assert " " not in actual_url + @pytest.mark.asyncio async def test_async_log_event_skips_when_standard_logging_object_missing(): """ From e68cfaae0c1238d19a4944efb8af47c41dc949ce Mon Sep 17 00:00:00 2001 From: Christian Reynoso Hunter Date: Sat, 4 Apr 2026 22:40:56 -0300 Subject: [PATCH 3/9] fix(cache): Prevent "multiple values" error in get_cache_key (#20261) ## 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 --- litellm/caching/caching.py | 5 +- tests/local_testing/test_cache_preset_key.py | 87 ++++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 tests/local_testing/test_cache_preset_key.py diff --git a/litellm/caching/caching.py b/litellm/caching/caching.py index 406a4f8c98a..6a68ba8c4d1 100644 --- a/litellm/caching/caching.py +++ b/litellm/caching/caching.py @@ -312,8 +312,11 @@ class Cache: verbose_logger.debug("\nCreated cache key: %s", cache_key) hashed_cache_key = Cache._get_hashed_cache_key(cache_key) hashed_cache_key = self._add_namespace_to_cache_key(hashed_cache_key, **kwargs) + # Remove preset_cache_key from kwargs to avoid "got multiple values" TypeError + # when kwargs already contains preset_cache_key from upstream callers + 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 + preset_cache_key=hashed_cache_key, **kwargs_for_preset ) return hashed_cache_key diff --git a/tests/local_testing/test_cache_preset_key.py b/tests/local_testing/test_cache_preset_key.py new file mode 100644 index 00000000000..de0ec05603c --- /dev/null +++ b/tests/local_testing/test_cache_preset_key.py @@ -0,0 +1,87 @@ +""" +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' +""" + +import pytest +from unittest.mock import MagicMock, patch + + +class TestPresetCacheKeyFix: + """Tests for the preset_cache_key multiple values fix.""" + + def test_get_cache_key_with_preset_cache_key_in_kwargs(self): + """ + Test that get_cache_key handles kwargs that already contain preset_cache_key. + + This was causing: + TypeError: _set_preset_cache_key_in_kwargs() got multiple values + for keyword argument 'preset_cache_key' + """ + from litellm.caching.caching import Cache + + cache = Cache() + + # Simulate kwargs that already has preset_cache_key (as can happen + # when the cache key is recomputed in certain code paths) + kwargs_with_preset = { + "model": "gpt-4", + "messages": [{"role": "user", "content": "Hello"}], + "preset_cache_key": "existing_key_12345", # This caused the bug + "litellm_params": {}, + } + + # This should NOT raise TypeError + try: + result = cache.get_cache_key(**kwargs_with_preset) + assert result is not None + assert isinstance(result, str) + except TypeError as e: + if "multiple values for keyword argument" in str(e): + pytest.fail(f"Bug not fixed: {e}") + raise + + def test_get_cache_key_without_preset_cache_key(self): + """Test normal case without preset_cache_key in kwargs still works.""" + from litellm.caching.caching import Cache + + cache = Cache() + + kwargs_normal = { + "model": "gpt-4", + "messages": [{"role": "user", "content": "Hello"}], + "litellm_params": {}, + } + + result = cache.get_cache_key(**kwargs_normal) + assert result is not None + assert isinstance(result, str) + + def test_preset_cache_key_is_set_in_litellm_params(self): + """Verify that preset_cache_key is correctly set in litellm_params.""" + from litellm.caching.caching import Cache + + cache = Cache() + + litellm_params = {} + kwargs = { + "model": "gpt-4", + "messages": [{"role": "user", "content": "Hello"}], + "litellm_params": litellm_params, + } + + result = cache.get_cache_key(**kwargs) + + # The method should set preset_cache_key in litellm_params + assert "preset_cache_key" in litellm_params + assert litellm_params["preset_cache_key"] == result + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) From fc75380b88481bff17a0b25d6c7d7cf49e11c361 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sun, 5 Apr 2026 04:46:43 +0300 Subject: [PATCH 4/9] fix(presidio): use correct text positions in anonymize_text (#24998) * 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 ( first in text, not last) - Add assertion for token numbering order in test --- .../guardrails/guardrail_hooks/presidio.py | 106 ++++++----- .../guardrail_hooks/test_presidio.py | 168 ++++++++++++++++++ 2 files changed, 226 insertions(+), 48 deletions(-) diff --git a/litellm/proxy/guardrails/guardrail_hooks/presidio.py b/litellm/proxy/guardrails/guardrail_hooks/presidio.py index 0f4ebbd4880..e048ca21cba 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/presidio.py +++ b/litellm/proxy/guardrails/guardrail_hooks/presidio.py @@ -485,61 +485,71 @@ class _OPTIONAL_PresidioPIIMasking(CustomGuardrail): redacted_text = await response.json() - new_text = text if redacted_text is not None: verbose_proxy_logger.debug("redacted_text: %s", redacted_text) - # Process items in reverse order by start position so that - # replacing later spans first does not shift earlier coordinates. - for item in sorted( - redacted_text["items"], key=lambda x: x["start"], reverse=True - ): - start = item["start"] - end = item["end"] - replacement = item["text"] # replacement token - if item["operator"] == "replace" and output_parse_pii is True: - if request_data is None: - verbose_proxy_logger.warning( - "Presidio anonymize_text called without request_data — " - "PII tokens cannot be stored per-request. " - "This may indicate a missing caller update." + + if not output_parse_pii: + # No need to build numbered tokens — just use Presidio's + # already-anonymized text directly. The old code incorrectly + # applied anonymizer item positions (which reference the + # *output* text) to the *original* text, causing offset errors. + for item in redacted_text.get("items", []): + entity_type = item.get("entity_type", None) + if entity_type is not None: + masked_entity_count[entity_type] = ( + masked_entity_count.get(entity_type, 0) + 1 ) - request_data = {} - # Store pii_tokens in metadata to avoid leaking to LLM providers. - # Providers like Anthropic reject unknown top-level fields. - if not request_data.get("metadata"): - request_data["metadata"] = {} - if "pii_tokens" not in request_data["metadata"]: - request_data["metadata"]["pii_tokens"] = {} - pii_tokens = request_data["metadata"]["pii_tokens"] + return redacted_text["text"] - # Append a sequential number to make each token unique - # per request, so unmasking maps back to the correct - # original value. Format: , - # This is LLM-friendly and degrades gracefully if the - # LLM doesn't echo the token verbatim. - seq = len(pii_tokens) + 1 - if replacement.endswith(">"): - replacement = f"{replacement[:-1]}_{seq}>" - else: - replacement = f"{replacement}_{seq}" + # output_parse_pii is True — we need sequentially numbered + # tokens and a pii_tokens mapping for later unmasking. + # Use analyze_results positions (which reference the ORIGINAL + # text) instead of anonymizer items (which reference the output). + new_text = text + if request_data is None: + verbose_proxy_logger.warning( + "Presidio anonymize_text called without request_data — " + "PII tokens cannot be stored per-request. " + "This may indicate a missing caller update." + ) + request_data = {} + if not request_data.get("metadata"): + request_data["metadata"] = {} + if "pii_tokens" not in request_data["metadata"]: + request_data["metadata"]["pii_tokens"] = {} + pii_tokens = request_data["metadata"]["pii_tokens"] - # Use ORIGINAL text (not new_text) since start/end - # reference the original text's coordinates. - pii_tokens[replacement] = text[start:end] + # Assign sequence numbers in forward (left-to-right) order so + # that is the first entity in the text, etc. + sorted_forward = sorted( + analyze_results, key=lambda x: x["start"] + ) + seq_map = {} + for idx, ar in enumerate(sorted_forward, start=1): + seq_map[(ar["start"], ar["end"])] = idx + # Apply replacements in reverse order by start position so + # that replacing later spans first does not shift earlier + # coordinates in the original text. + for ar in reversed(sorted_forward): + start = ar["start"] + end = ar["end"] + entity_type = ar["entity_type"] + replacement = f"<{entity_type}>" + + seq = seq_map[(start, end)] + if replacement.endswith(">"): + replacement = f"{replacement[:-1]}_{seq}>" + else: + replacement = f"{replacement}_{seq}" + + pii_tokens[replacement] = text[start:end] new_text = new_text[:start] + replacement + new_text[end:] - entity_type = item.get("entity_type", None) - if entity_type is not None: - masked_entity_count[entity_type] = ( - masked_entity_count.get(entity_type, 0) + 1 - ) - # When output_parse_pii is True, new_text contains sequentially - # numbered tokens (e.g. ) that match the keys - # in pii_tokens. Returning redacted_text["text"] (Presidio's - # original output) would send un-numbered tokens to the LLM, - # making unmasking impossible. - # When output_parse_pii is False, new_text == redacted_text["text"] - # because no suffix is appended. + + masked_entity_count[entity_type] = ( + masked_entity_count.get(entity_type, 0) + 1 + ) + return new_text else: raise Exception("Invalid anonymizer response: received None") diff --git a/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_presidio.py b/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_presidio.py index 32a8c1b1070..38ea42285c1 100644 --- a/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_presidio.py +++ b/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_presidio.py @@ -2230,3 +2230,171 @@ async def test_apply_to_output_streaming_bytes_only_logs_warning(): mock_logger.warning.assert_called_once() warning_msg = mock_logger.warning.call_args[0][0] assert "Output PII masking was skipped" in warning_msg + + +@pytest.mark.asyncio +async def test_anonymize_text_uses_correct_positions_no_parse_pii(): + """ + Regression test for anonymizer offset bug (fixes #24160). + + The Presidio anonymizer returns items with start/end positions that + reference the *anonymized output* text, not the original input text. + When output_parse_pii is False, anonymize_text must return + redacted_text["text"] directly instead of manually splicing the + original text using those positions, which produces garbled output + with remnants of original PII data. + """ + original_text = ( + "My name is John Smith, my email is john@example.com, phone 555-867-5309" + ) + # Positions as returned by the analyzer (reference original text) + analyze_results = [ + {"end": 51, "entity_type": "EMAIL_ADDRESS", "score": 1.0, "start": 35}, + {"end": 21, "entity_type": "PERSON", "score": 0.85, "start": 11}, + {"end": 71, "entity_type": "PHONE_NUMBER", "score": 0.75, "start": 59}, + ] + # Anonymizer response — positions reference the *anonymized* text + anonymizer_response = { + "text": "My name is , my email is , phone ", + "items": [ + { + "start": 56, + "end": 70, + "entity_type": "PHONE_NUMBER", + "text": "", + "operator": "replace", + }, + { + "start": 33, + "end": 48, + "entity_type": "EMAIL_ADDRESS", + "text": "", + "operator": "replace", + }, + { + "start": 11, + "end": 19, + "entity_type": "PERSON", + "text": "", + "operator": "replace", + }, + ], + } + + guardrail = _OPTIONAL_PresidioPIIMasking( + presidio_analyzer_api_base="http://test-analyzer/", + presidio_anonymizer_api_base="http://test-anonymizer/", + mock_testing=False, + ) + + mock_iterator = _make_mock_session_iterator( + json_response=anonymizer_response, + ) + + masked_entity_count = {} + with patch.object(guardrail, "_get_session_iterator", mock_iterator): + result = await guardrail.anonymize_text( + text=original_text, + analyze_results=analyze_results, + output_parse_pii=False, + masked_entity_count=masked_entity_count, + ) + + expected = "My name is , my email is , phone " + assert result == expected, ( + f"anonymize_text produced garbled output with PII remnants.\n" + f"Expected: {expected!r}\n" + f"Got: {result!r}" + ) + assert masked_entity_count == { + "PERSON": 1, + "EMAIL_ADDRESS": 1, + "PHONE_NUMBER": 1, + } + + +@pytest.mark.asyncio +async def test_anonymize_text_uses_correct_positions_with_parse_pii(): + """ + Regression test for anonymizer offset bug with output_parse_pii=True + (fixes #24160). + + When output_parse_pii is True, anonymize_text must use positions from + analyze_results (which reference the original text) to build numbered + tokens and the pii_tokens mapping, not positions from anonymizer items + (which reference the anonymized output text). + """ + original_text = ( + "My name is John Smith, my email is john@example.com, phone 555-867-5309" + ) + analyze_results = [ + {"end": 51, "entity_type": "EMAIL_ADDRESS", "score": 1.0, "start": 35}, + {"end": 21, "entity_type": "PERSON", "score": 0.85, "start": 11}, + {"end": 71, "entity_type": "PHONE_NUMBER", "score": 0.75, "start": 59}, + ] + anonymizer_response = { + "text": "My name is , my email is , phone ", + "items": [ + { + "start": 56, + "end": 70, + "entity_type": "PHONE_NUMBER", + "text": "", + "operator": "replace", + }, + { + "start": 33, + "end": 48, + "entity_type": "EMAIL_ADDRESS", + "text": "", + "operator": "replace", + }, + { + "start": 11, + "end": 19, + "entity_type": "PERSON", + "text": "", + "operator": "replace", + }, + ], + } + + guardrail = _OPTIONAL_PresidioPIIMasking( + presidio_analyzer_api_base="http://test-analyzer/", + presidio_anonymizer_api_base="http://test-anonymizer/", + mock_testing=False, + output_parse_pii=True, + ) + + mock_iterator = _make_mock_session_iterator( + json_response=anonymizer_response, + ) + + masked_entity_count = {} + request_data = {"metadata": {}} + with patch.object(guardrail, "_get_session_iterator", mock_iterator): + result = await guardrail.anonymize_text( + text=original_text, + analyze_results=analyze_results, + output_parse_pii=True, + masked_entity_count=masked_entity_count, + request_data=request_data, + ) + + # Result must not contain any remnants of original PII + assert "John" not in result + assert "john@example.com" not in result + assert "555-867-5309" not in result + + # pii_tokens must map numbered tokens back to correct original values + pii_tokens = request_data["metadata"]["pii_tokens"] + token_values = set(pii_tokens.values()) + assert "John Smith" in token_values + assert "john@example.com" in token_values + assert "555-867-5309" in token_values + + # Tokens must be numbered in left-to-right order of appearance: + # PERSON (pos 11) → _1, EMAIL_ADDRESS (pos 35) → _2, PHONE_NUMBER (pos 59) → _3 + assert pii_tokens.get("") == "John Smith" + assert pii_tokens.get("") == "john@example.com" + assert pii_tokens.get("") == "555-867-5309" From 23e702dae609aa79682b971967dcf0bc05efab59 Mon Sep 17 00:00:00 2001 From: Bohdan Kulinich Date: Sun, 5 Apr 2026 04:48:39 +0300 Subject: [PATCH 5/9] feat(prometheus): add 7m and 10m latency histogram buckets (#25071) 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 --- litellm/types/integrations/prometheus.py | 2 ++ .../types/test_prometheus_latency_buckets.py | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 tests/test_litellm/types/test_prometheus_latency_buckets.py diff --git a/litellm/types/integrations/prometheus.py b/litellm/types/integrations/prometheus.py index 0d1501664b9..35b695cd054 100644 --- a/litellm/types/integrations/prometheus.py +++ b/litellm/types/integrations/prometheus.py @@ -156,6 +156,8 @@ LATENCY_BUCKETS = ( 180.0, 240.0, 300.0, + 420.0, # 7 minutes + 600.0, # 10 minutes (typical default LLM request timeout) float("inf"), ) diff --git a/tests/test_litellm/types/test_prometheus_latency_buckets.py b/tests/test_litellm/types/test_prometheus_latency_buckets.py new file mode 100644 index 00000000000..85670bb0b74 --- /dev/null +++ b/tests/test_litellm/types/test_prometheus_latency_buckets.py @@ -0,0 +1,17 @@ +"""LATENCY_BUCKETS covers long-running LLM calls (histograms are in seconds).""" + +import math + +from litellm.types.integrations.prometheus import LATENCY_BUCKETS + + +def test_latency_buckets_include_seven_and_ten_minutes(): + """Buckets beyond 5 min so histograms resolve requests up to default LLM timeouts.""" + assert 300.0 in LATENCY_BUCKETS + assert 420.0 in LATENCY_BUCKETS # 7 min + assert 600.0 in LATENCY_BUCKETS # 10 min + assert math.isinf(LATENCY_BUCKETS[-1]) + idx_300 = LATENCY_BUCKETS.index(300.0) + idx_420 = LATENCY_BUCKETS.index(420.0) + idx_600 = LATENCY_BUCKETS.index(600.0) + assert idx_300 < idx_420 < idx_600 From 4ca368923054ef1df73c1f3b815e39716ff2ce71 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Sun, 5 Apr 2026 01:15:08 -0700 Subject: [PATCH 6/9] chore: fixes --- .../workflows/run_llm_translation_tests.py | 0 .trivyignore | 12 - ci_cd/.grype.yaml | 36 --- ci_cd/security_scans.sh | 261 ------------------ docs/my-website/.trivyignore | 7 - ui/litellm-dashboard/.trivyignore | 7 - 6 files changed, 323 deletions(-) mode change 100755 => 100644 .github/workflows/run_llm_translation_tests.py delete mode 100644 .trivyignore delete mode 100644 ci_cd/.grype.yaml delete mode 100755 ci_cd/security_scans.sh delete mode 100644 docs/my-website/.trivyignore delete mode 100644 ui/litellm-dashboard/.trivyignore diff --git a/.github/workflows/run_llm_translation_tests.py b/.github/workflows/run_llm_translation_tests.py old mode 100755 new mode 100644 diff --git a/.trivyignore b/.trivyignore deleted file mode 100644 index 0d04ecacdb5..00000000000 --- a/.trivyignore +++ /dev/null @@ -1,12 +0,0 @@ -# LiteLLM Trivy Ignore File -# CVEs listed here are temporarily allowlisted pending fixes - -# Next.js vulnerabilities in UI dashboard (next@14.2.35) -# Allowlisted: 2026-01-31, 7-day fix timeline -# Fix: Upgrade to Next.js 15.5.10+ or 16.1.5+ - -# HIGH: DoS via request deserialization -GHSA-h25m-26qc-wcjf - -# MEDIUM: Image Optimizer DoS -CVE-2025-59471 diff --git a/ci_cd/.grype.yaml b/ci_cd/.grype.yaml deleted file mode 100644 index b9bc9db58f5..00000000000 --- a/ci_cd/.grype.yaml +++ /dev/null @@ -1,36 +0,0 @@ -ignore: - - vulnerability: CVE-2026-22184 - reason: no fixed zlib package is available yet in the Wolfi repositories, so this is ignored temporarily until an upstream release exists - # Wolfi base image: Python 3.13 and Node from apk have no fixed builds in Wolfi yet / not applicable - - vulnerability: CVE-2025-55130 - reason: Node in Wolfi apk; only used for Admin UI build/prisma - - vulnerability: CVE-2025-59465 - reason: Node in Wolfi apk; only used for Admin UI build/prisma - - vulnerability: CVE-2025-55131 - reason: Node in Wolfi apk; only used for Admin UI build/prisma - - vulnerability: CVE-2025-59466 - reason: Node in Wolfi apk; only used for Admin UI build/prisma - - vulnerability: CVE-2026-21637 - reason: Node in Wolfi apk; only used for Admin UI build/prisma - - vulnerability: CVE-2025-55132 - reason: Node in Wolfi apk; only used for Admin UI build/prisma - - vulnerability: GHSA-hx9q-6w63-j58v - reason: orjson dumps recursion; allowlisted - - vulnerability: GHSA-73rr-hh4g-fpgx - reason: diff npm transitive dep; override in package.json, allowlisted - - vulnerability: CVE-2026-0865 - reason: Python 3.13 in Wolfi base; no fixed apk build yet - - vulnerability: CVE-2025-15282 - reason: Python 3.13 in Wolfi base; no fixed apk build yet - - vulnerability: CVE-2026-0672 - reason: Python 3.13 in Wolfi base; no fixed apk build yet - - vulnerability: CVE-2025-15366 - reason: Python 3.13 in Wolfi base; no fixed apk build yet - - vulnerability: CVE-2025-15367 - reason: Python 3.13 in Wolfi base; no fixed apk build yet - - vulnerability: CVE-2025-11468 - reason: Python 3.13 in Wolfi base; no fixed apk build yet - - vulnerability: CVE-2025-12781 - reason: Python 3.13 in Wolfi base; no fixed apk build yet - - vulnerability: CVE-2026-1299 - reason: Python 3.13 in Wolfi base; no fixed apk build yet diff --git a/ci_cd/security_scans.sh b/ci_cd/security_scans.sh deleted file mode 100755 index 2138fca6cd5..00000000000 --- a/ci_cd/security_scans.sh +++ /dev/null @@ -1,261 +0,0 @@ -#!/bin/bash - -# Security Scans Script for LiteLLM -# This script runs comprehensive security scans including Trivy and Grype - -set -e - -echo "Starting security scans for LiteLLM..." - -# Function to install Trivy and required tools -install_trivy() { - echo "Installing Trivy and required tools..." - TRIVY_VERSION="0.35.0" - sudo apt-get update - sudo apt-get install -y wget jq curl bsdmainutils - wget -qO trivy.deb "https://github.com/aquasecurity/trivy/releases/download/v${TRIVY_VERSION}/trivy_${TRIVY_VERSION}_Linux-64bit.deb" - sudo dpkg -i trivy.deb - rm trivy.deb - echo "Trivy ${TRIVY_VERSION} installed successfully" -} - -# Function to install Grype -install_grype() { - echo "Installing Grype..." - curl -sSfL https://raw.githubusercontent.com/anchore/grype/main/install.sh | sudo sh -s -- -b /usr/local/bin - echo "Grype installed successfully" -} - -# Function to install ggshield -install_ggshield() { - echo "Installing ggshield..." - pip3 install --upgrade pip - pip3 install ggshield - echo "ggshield installed successfully" -} - -# # Function to run secret detection scans -# run_secret_detection() { -# echo "Running secret detection scans..." - -# if ! command -v ggshield &> /dev/null; then -# install_ggshield -# fi - -# # Check if GITGUARDIAN_API_KEY is set (required for CI/CD) -# if [ -z "$GITGUARDIAN_API_KEY" ]; then -# echo "Warning: GITGUARDIAN_API_KEY environment variable is not set." -# echo "ggshield requires a GitGuardian API key to scan for secrets." -# echo "Please set GITGUARDIAN_API_KEY in your CI/CD environment variables." -# exit 1 -# fi - -# echo "Scanning codebase for secrets..." -# echo "Note: Large codebases may take several minutes due to API rate limits (50 requests/minute on free plan)" -# echo "ggshield will automatically handle rate limits and retry as needed." -# echo "Binary files, cache files, and build artifacts are excluded via .gitguardian.yaml" - -# # Use --recursive for directory scanning and auto-confirm if prompted -# # .gitguardian.yaml will automatically exclude binary files, wheel files, etc. -# # GITGUARDIAN_API_KEY environment variable will be used for authentication -# echo y | ggshield secret scan path . --recursive || { -# echo "" -# echo "==========================================" -# echo "ERROR: Secret Detection Failed" -# echo "==========================================" -# echo "ggshield has detected secrets in the codebase." -# echo "Please review discovered secrets above, revoke any actively used secrets" -# echo "from underlying systems and make changes to inject secrets dynamically at runtime." -# echo "" -# echo "For more information, see: https://docs.gitguardian.com/secrets-detection/" -# echo "==========================================" -# echo "" -# exit 1 -# } - -# echo "Secret detection scans completed successfully" -# } - -# Function to run Trivy scans -run_trivy_scans() { - echo "Running Trivy scans..." - - echo "Scanning LiteLLM Docs..." - trivy fs --ignorefile .trivyignore --scanners vuln --dependency-tree --exit-code 1 --severity HIGH,CRITICAL,MEDIUM ./docs/ - - echo "Scanning LiteLLM UI..." - trivy fs --ignorefile .trivyignore --scanners vuln --dependency-tree --exit-code 1 --severity HIGH,CRITICAL,MEDIUM ./ui/ - - echo "Trivy scans completed successfully" -} - -# Function to build and scan Docker images with Grype -run_grype_scans() { - echo "Running Grype scans..." - - # Temporarily add wheel files to .dockerignore for security scans - echo "Temporarily modifying .dockerignore to exclude problematic wheel files..." - cp .dockerignore .dockerignore.backup 2>/dev/null || touch .dockerignore.backup - echo "/*.whl" >> .dockerignore - - # Build and scan Dockerfile.database - echo "Building and scanning Dockerfile.database..." - docker build --no-cache -t litellm-database:latest -f ./docker/Dockerfile.database . - grype litellm-database:latest --config ci_cd/.grype.yaml --fail-on critical - - # Build and scan main Dockerfile - echo "Building and scanning main Dockerfile..." - docker build --no-cache -t litellm:latest . - grype litellm:latest --config ci_cd/.grype.yaml --fail-on critical - - # Restore original .dockerignore - echo "Restoring original .dockerignore..." - mv .dockerignore.backup .dockerignore - - # Scan the locally built LiteLLM image for vulnerabilities with CVSS >= 4.0 - echo "Scanning locally built LiteLLM image for high-severity vulnerabilities..." - echo "Using locally built image: litellm:latest" - - # Allowlist of CVEs to be ignored in failure threshold/reporting - # - CVE-2025-8869: Not applicable on Python >=3.13 (PEP 706 implemented); pip fallback unused; no OS-level fix - # - GHSA-4xh5-x5gv-qwph: GitHub Security Advisory alias for CVE-2025-8869 - # - GHSA-5j98-mcp5-4vw2: glob CLI command injection via -c/--cmd; glob CLI is not used in the litellm runtime image, - # and the vulnerable versions are pulled in only via OS-level/node tooling outside of our application code - ALLOWED_CVES=( - "CVE-2025-8869" - "GHSA-4xh5-x5gv-qwph" - "CVE-2025-8291" # no fix available as of Oct 11, 2025 - "GHSA-5j98-mcp5-4vw2" - "CVE-2025-13836" # Python 3.13 HTTP response reading OOM/DoS - no fix available in base image - "CVE-2025-12084" # Python 3.13 xml.dom.minidom quadratic algorithm - no fix available in base image - "CVE-2025-60876" # BusyBox wget HTTP request splitting - no fix available in Chainguard Wolfi base image - "CVE-2026-0861" # Wolfi glibc still flagged even on 2.42-r5; upstream patched build unavailable yet - "CVE-2010-4756" # glibc glob DoS - awaiting patched Wolfi glibc build - "CVE-2019-1010022" # glibc stack guard bypass - awaiting patched Wolfi glibc build - "CVE-2019-1010023" # glibc ldd remap issue - awaiting patched Wolfi glibc build - "CVE-2019-1010024" # glibc ASLR mitigation bypass - awaiting patched Wolfi glibc build - "CVE-2019-1010025" # glibc pthread heap address leak - awaiting patched Wolfi glibc build - "CVE-2026-22184" # zlib untgz buffer overflow - untgz unused + no fixed Wolfi build yet - "GHSA-58pv-8j8x-9vj2" # jaraco.context path traversal - setuptools vendored only (v5.3.0), not used in application code (using v6.1.0+) - "GHSA-34x7-hfp2-rc4v" # node-tar hardlink path traversal - not applicable, tar CLI not exposed in application code - "GHSA-r6q2-hw4h-h46w" # node-tar not used by application runtime, Linux-only container, not affect by macOS APFS-specific exploit - "GHSA-8rrh-rw8j-w5fx" # wheel is from chainguard and will be handled by then TODO: Remove this after Chainguard updates the wheel - "CVE-2025-59465" # Node only used for Admin UI build/prisma - "CVE-2025-55131" # Node only used for Admin UI build/prisma - "CVE-2025-59466" # Node only used for Admin UI build/prisma - "CVE-2025-55130" # Node only used for Admin UI build/prisma - "CVE-2025-59467" # Node only used for Admin UI build/prisma - "CVE-2026-21637" # Node only used for Admin UI build/prisma - "CVE-2025-55132" # Node only used for Admin UI build/prisma - "GHSA-hx9q-6w63-j58v" # orjson dumps recursion; allowlisted - "CVE-2025-15281" # No fix available yet - "CVE-2026-0865" # No fix available yet - "CVE-2025-15282" # No fix available yet - "CVE-2026-0672" # No fix available yet - "CVE-2025-15366" # No fix available yet - "CVE-2025-15367" # No fix available yet - "CVE-2025-12781" # No fix available yet - "CVE-2025-11468" # No fix available yet - "CVE-2026-1299" # Python 3.13 email module header injection - not applicable, LiteLLM doesn't use BytesGenerator for email serialization - "CVE-2026-0775" # npm cli incorrect permission assignment - no fix available yet, npm is only used at build/prisma-generate time - "GHSA-3ppc-4f35-3m26" # minimatch ReDoS via repeated wildcards - from nodejs_wheel bundled npm, not used in application runtime code - "GHSA-83g3-92jg-28cx" # tar arbitrary file read/write via hardlink - from nodejs_wheel bundled npm, not used in application runtime code - "CVE-2026-2297" # Python 3.13 SourcelessFileLoader audit hook bypass - no fix available in base image - "GHSA-qffp-2rhf-9h96" # tar hardlink path traversal - from nodejs_wheel bundled npm, not used in application runtime code - "CVE-2026-2673" # OpenSSL 3.6.1 TLS 1.3 key exchange group negotiation issue - no fix available yet - "CVE-2026-3644" # Python 3.13 vulnerability - no fix available in base image - "CVE-2026-4224" # Python 3.13 Expat parser stack overflow in ElementDeclHandler - no fix available in base image - ) - - # Build JSON array of allowlisted CVE IDs for jq - ALLOWED_IDS_JSON=$(printf '%s\n' "${ALLOWED_CVES[@]}" | jq -R . | jq -s .) - - echo "Checking for vulnerabilities with CVSS score >= 4.0..." - echo "Allowlisted CVEs (ignored in threshold): ${ALLOWED_CVES[*]}" - echo "" - - # Show all high-severity vulnerabilities for transparency - TOTAL_HIGH_SEVERITY=$(grype litellm:latest -o json | jq -r ' - .matches[] - | select(.vulnerability.cvss[]?.metrics.baseScore >= 4.0) - | .vulnerability.id' | wc -l) - - if [ "$TOTAL_HIGH_SEVERITY" -gt 0 ]; then - echo "Total vulnerabilities found with CVSS >= 4.0: $TOTAL_HIGH_SEVERITY" - echo "" - echo "All high-severity vulnerabilities (including allowlisted):" - grype litellm:latest -o json | jq --argjson allow "$ALLOWED_IDS_JSON" -r ' - ["Package", "Version", "Vulnerability ID", "CVSS Score", "Allowlisted"], - (.matches[] - | select(.vulnerability.cvss[]?.metrics.baseScore >= 4.0) - | [.artifact.name, .artifact.version, .vulnerability.id, .vulnerability.cvss[0].metrics.baseScore, (if (.vulnerability.id as $id | $allow | index($id)) then "YES" else "NO" end)]) - | @tsv' | column -t -s $'\t' - echo "" - fi - - HIGH_SEVERITY_COUNT=$(grype litellm:latest -o json | jq --argjson allow "$ALLOWED_IDS_JSON" -r ' - .matches[] - | select(.vulnerability.cvss[]?.metrics.baseScore >= 4.0) - | select((.vulnerability.id as $id | $allow | index($id) | not)) - | .vulnerability.id' | wc -l) - - if [ "$HIGH_SEVERITY_COUNT" -gt 0 ]; then - echo "" - echo "==========================================" - echo "ERROR: Security Scan Failed" - echo "==========================================" - echo "Found $HIGH_SEVERITY_COUNT non-allowlisted vulnerabilities with CVSS score >= 4.0 in litellm:latest" - echo "" - echo "These vulnerabilities are NOT in the allowlist and must be addressed." - echo "Current allowlisted CVEs: ${ALLOWED_CVES[*]}" - echo "" - echo "Detailed vulnerability report:" - echo "" - grype litellm:latest -o json | jq --argjson allow "$ALLOWED_IDS_JSON" -r ' - ["Package", "Version", "Vulnerability ID", "CVSS Score", "Severity", "Fix Version", "Description"], - (.matches[] - | select(.vulnerability.cvss[]?.metrics.baseScore >= 4.0) - | select((.vulnerability.id as $id | $allow | index($id) | not)) - | [.artifact.name, .artifact.version, .vulnerability.id, .vulnerability.cvss[0].metrics.baseScore, .vulnerability.severity, (.vulnerability.fix.versions[0] // "No fix available"), .vulnerability.description]) - | @tsv' | column -t -s $'\t' - echo "" - echo "==========================================" - echo "Action Required:" - echo "==========================================" - echo "1. If a fix is available, update the package to the fixed version" - echo "2. If the vulnerability is not applicable or has no fix:" - echo " - Add the CVE/GHSA ID to ALLOWED_CVES array in ci_cd/security_scans.sh" - echo " - Add a comment explaining why it's safe to ignore" - echo "" - echo "Note: Some vulnerabilities may have multiple IDs (CVE-XXXX and GHSA-XXXX)." - echo "Add all relevant IDs to the allowlist if they refer to the same issue." - echo "==========================================" - echo "" - exit 1 - else - echo "No high-severity vulnerabilities (CVSS >= 4.0) found in litellm:latest" - fi - - echo "Grype scans completed successfully" -} - -# Main execution -main() { - echo "Installing security scanning tools..." - install_trivy - install_grype - - # echo "Running secret detection scans..." - # run_secret_detection - - echo "Running filesystem vulnerability scans..." - run_trivy_scans - - echo "Running Docker image vulnerability scans..." - run_grype_scans - - echo "All security scans completed successfully!" -} - -# Execute main function -main "$@" diff --git a/docs/my-website/.trivyignore b/docs/my-website/.trivyignore deleted file mode 100644 index 977504f2670..00000000000 --- a/docs/my-website/.trivyignore +++ /dev/null @@ -1,7 +0,0 @@ -# js-yaml CVE-2025-64718 -# This vulnerability is not applicable because we've forced js-yaml to version 4.1.1 -# via npm overrides in package.json. Trivy incorrectly reports this based on -# dependency requirements in the lockfile, but the actual installed version is 4.1.1. -# Verified with: npm list js-yaml -CVE-2025-64718 - diff --git a/ui/litellm-dashboard/.trivyignore b/ui/litellm-dashboard/.trivyignore deleted file mode 100644 index 977504f2670..00000000000 --- a/ui/litellm-dashboard/.trivyignore +++ /dev/null @@ -1,7 +0,0 @@ -# js-yaml CVE-2025-64718 -# This vulnerability is not applicable because we've forced js-yaml to version 4.1.1 -# via npm overrides in package.json. Trivy incorrectly reports this based on -# dependency requirements in the lockfile, but the actual installed version is 4.1.1. -# Verified with: npm list js-yaml -CVE-2025-64718 - From f233520c44061a07e3c43ba34ba6f79c73cc0bae Mon Sep 17 00:00:00 2001 From: Hendrik Jaks Date: Mon, 6 Apr 2026 21:13:06 +0300 Subject: [PATCH 7/9] fix(ui): resolve login redirect loop when reverse proxy adds HttpOnly to cookies (#23532) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 * 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 * 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 * 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 * 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 * 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 * fix(ui): guard storeLoginToken against empty/whitespace-only tokens Co-Authored-By: Claude Opus 4.6 * 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 * 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) * 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) * 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) * 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) * style: fix Black formatting in audit_logs.py Co-Authored-By: Claude Opus 4.6 (1M context) * fix CI: formatting, test params, remove token from login JSON Co-Authored-By: Claude Opus 4.6 (1M context) * fix: reformat with Black 23.x to match CI Co-Authored-By: Claude Opus 4.6 (1M context) * fix: keep token in login JSON body for UI storeLoginToken flow Co-Authored-By: Claude Opus 4.6 (1M context) * fix: use storeLoginToken in exchangeLoginCode, add credentials include Co-Authored-By: Claude Opus 4.6 (1M context) * 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) * Revert "revert: remove unrelated changes from HttpOnly cookie fix branch" This reverts commit 0684a1e27521ada35cf8a4afbdff7aeaa87ff41d. * Revert "fix: use storeLoginToken in exchangeLoginCode, add credentials include" This reverts commit 866405f443eb2004ee56ed2f52c9047593d8cc6e. * Revert "fix: keep token in login JSON body for UI storeLoginToken flow" This reverts commit 086c41640c5749399f11aba298321d1d16e10a92. * Revert "fix: reformat with Black 23.x to match CI" This reverts commit b2c3334c888a9dbb60fd94cce12f43e8f3aa7e82. * Revert "fix CI: formatting, test params, remove token from login JSON" This reverts commit 2905d47bd4013b81fbaa01cf2c7402d51360f1e6. --------- Co-authored-by: Claude Opus 4.6 Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --- litellm/proxy/proxy_server.py | 5 +- ui/litellm-dashboard/src/app/page.tsx | 16 +--- .../src/components/networking.test.ts | 33 ++++++++ .../src/components/networking.tsx | 6 +- .../src/components/user_dashboard.tsx | 12 ++- .../src/utils/cookieUtils.test.ts | 73 ++++++++++++++++- ui/litellm-dashboard/src/utils/cookieUtils.ts | 81 ++++++++++++++++++- 7 files changed, 198 insertions(+), 28 deletions(-) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 9738ae4f1a2..accf669f2e3 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -11506,8 +11506,11 @@ async def login_v2(request: Request): # noqa: PLR0915 litellm_dashboard_ui += "/ui/" litellm_dashboard_ui += "?login=success" + # Token is included in the response body so the UI can set a JS-accessible + # cookie even when a reverse proxy (e.g. nginx-ingress) adds HttpOnly to the + # server-set cookie, which would otherwise cause an infinite login redirect. json_response = JSONResponse( - content={"redirect_url": litellm_dashboard_ui}, + content={"redirect_url": litellm_dashboard_ui, "token": jwt_token}, status_code=status.HTTP_200_OK, ) json_response.set_cookie(key="token", value=jwt_token) diff --git a/ui/litellm-dashboard/src/app/page.tsx b/ui/litellm-dashboard/src/app/page.tsx index 44df1b5bd41..73c27c00724 100644 --- a/ui/litellm-dashboard/src/app/page.tsx +++ b/ui/litellm-dashboard/src/app/page.tsx @@ -42,6 +42,7 @@ import ToolPoliciesView from "@/components/ToolPoliciesView"; import SpendLogsTable from "@/components/view_logs"; import ViewUserDashboard from "@/components/view_users"; import { ThemeProvider } from "@/contexts/ThemeContext"; +import { clearTokenCookies, getCookie } from "@/utils/cookieUtils"; import { isJwtExpired } from "@/utils/jwtUtils"; import { buildLoginUrlWithReturn, consumeReturnUrl, isValidReturnUrl, normalizeUrlForCompare, storeReturnUrl } from "@/utils/returnUrlUtils"; import { formatUserRole, isAdminRole } from "@/utils/roles"; @@ -51,21 +52,12 @@ import { useRouter, useSearchParams } from "next/navigation"; import { Suspense, useEffect, useMemo, useRef, useState } from "react"; import { ConfigProvider, theme } from "antd"; -function getCookie(name: string) { - // Safer cookie read + decoding; handles '=' inside values - const match = document.cookie.split("; ").find((row) => row.startsWith(name + "=")); - if (!match) return null; - const value = match.slice(name.length + 1); - try { - return decodeURIComponent(value); - } catch { - return value; - } -} - function deleteCookie(name: string, path = "/") { // Best-effort client-side clear (works for non-HttpOnly cookies without Domain) document.cookie = `${name}=; Max-Age=0; Path=${path}`; + if (name === "token") { + clearTokenCookies(); + } } interface ProxySettings { diff --git a/ui/litellm-dashboard/src/components/networking.test.ts b/ui/litellm-dashboard/src/components/networking.test.ts index c57dcb97eb9..3c107fa586f 100644 --- a/ui/litellm-dashboard/src/components/networking.test.ts +++ b/ui/litellm-dashboard/src/components/networking.test.ts @@ -5,6 +5,7 @@ import * as Networking from "./networking"; vi.mock("@/utils/cookieUtils", () => ({ clearTokenCookies: vi.fn(), getCookie: vi.fn(), + storeLoginToken: vi.fn(), })); vi.mock("./molecules/notifications_manager", () => ({ @@ -79,6 +80,38 @@ describe("networking - expired session handling", () => { }); }); +describe("loginCall - storeLoginToken integration", () => { + const originalFetch = global.fetch; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + global.fetch = originalFetch; + }); + + it("calls storeLoginToken when response includes token", async () => { + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ redirect_url: "/ui/?login=success", token: "my-jwt" }), + }) as any; + const { storeLoginToken } = await import("@/utils/cookieUtils"); + await Networking.loginCall("admin", "pass"); + expect(storeLoginToken).toHaveBeenCalledWith("my-jwt"); + }); + + it("does not call storeLoginToken when response has no token", async () => { + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ redirect_url: "/ui/?login=success" }), + }) as any; + const { storeLoginToken } = await import("@/utils/cookieUtils"); + await Networking.loginCall("admin", "pass"); + expect(storeLoginToken).not.toHaveBeenCalled(); + }); +}); + describe("daily activity helpers", () => { const startTime = new Date("2025-02-12T00:00:00.000Z"); const endTime = new Date("2025-02-19T00:00:00.000Z"); diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 28f8d308de7..16f35605877 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -69,7 +69,7 @@ export const getInProductNudgesCall = async (accessToken: string) => { * Helper file for calls being made to proxy */ import MessageManager from "@/components/molecules/message_manager"; -import { clearTokenCookies } from "@/utils/cookieUtils"; +import { clearTokenCookies, storeLoginToken } from "@/utils/cookieUtils"; import { TagNewRequest, TagUpdateRequest, TagListResponse, TagInfoResponse } from "./tag_management/types"; import { Team } from "./key_team_helpers/key_list"; import { UserInfo } from "./view_users/types"; @@ -9255,14 +9255,14 @@ export const loginCall = async (username: string, password: string, useV3?: bool const exchangeData: LoginResponse = await exchangeResponse.json(); if (exchangeData.token) { - document.cookie = `token=${exchangeData.token}; path=/; SameSite=Lax`; + storeLoginToken(exchangeData.token); } return exchangeData; } // Backwards compatibility: v2 or old v3 returns token directly if (data.token) { - document.cookie = `token=${data.token}; path=/; SameSite=Lax`; + storeLoginToken(data.token); } return data; diff --git a/ui/litellm-dashboard/src/components/user_dashboard.tsx b/ui/litellm-dashboard/src/components/user_dashboard.tsx index f97d8ffab04..90eac56540d 100644 --- a/ui/litellm-dashboard/src/components/user_dashboard.tsx +++ b/ui/litellm-dashboard/src/components/user_dashboard.tsx @@ -1,5 +1,5 @@ "use client"; -import { clearTokenCookies } from "@/utils/cookieUtils"; +import { clearTokenCookies, getCookie } from "@/utils/cookieUtils"; import { Col, Grid } from "@tremor/react"; import { Typography } from "antd"; import { jwtDecode } from "jwt-decode"; @@ -35,12 +35,6 @@ export type UserInfo = { spend: number; }; -function getCookie(name: string) { - console.log("COOKIES", document.cookie); - const cookieValue = document.cookie.split("; ").find((row) => row.startsWith(name + "=")); - return cookieValue ? cookieValue.split("=")[1] : null; -} - interface UserDashboardProps { userID: string | null; userRole: string | null; @@ -103,7 +97,11 @@ const UserDashboard: React.FC = ({ // They are only cleared on logout useEffect(() => { const handleBeforeUnload = () => { + const token = sessionStorage.getItem("token"); sessionStorage.clear(); + if (token) { + sessionStorage.setItem("token", token); + } }; window.addEventListener("beforeunload", handleBeforeUnload); return () => window.removeEventListener("beforeunload", handleBeforeUnload); diff --git a/ui/litellm-dashboard/src/utils/cookieUtils.test.ts b/ui/litellm-dashboard/src/utils/cookieUtils.test.ts index 8b066e6a8ea..c7bd27a6a85 100644 --- a/ui/litellm-dashboard/src/utils/cookieUtils.test.ts +++ b/ui/litellm-dashboard/src/utils/cookieUtils.test.ts @@ -1,11 +1,12 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; -import { clearTokenCookies, getCookie } from "./cookieUtils"; +import { clearTokenCookies, getCookie, storeLoginToken } from "./cookieUtils"; describe("cookieUtils", () => { beforeEach(() => { document.cookie.split(";").forEach((c) => { document.cookie = c.replace(/^ +/, "").replace(/=.*/, "=;expires=" + new Date().toUTCString() + ";path=/"); }); + sessionStorage.clear(); vi.spyOn(console, "log").mockImplementation(() => {}); }); @@ -116,6 +117,55 @@ describe("cookieUtils", () => { vi.restoreAllMocks(); }); + + it("should clear sessionStorage token", () => { + sessionStorage.setItem("token", "stored-token"); + clearTokenCookies(); + expect(sessionStorage.getItem("token")).toBeNull(); + }); + }); + + describe("storeLoginToken", () => { + it("should store the token in sessionStorage", () => { + storeLoginToken("my-jwt-token"); + expect(sessionStorage.getItem("token")).toBe("my-jwt-token"); + }); + + it("should overwrite an existing token in sessionStorage", () => { + storeLoginToken("old-token"); + expect(sessionStorage.getItem("token")).toBe("old-token"); + + storeLoginToken("new-token"); + expect(sessionStorage.getItem("token")).toBe("new-token"); + }); + + it("should not throw when window is undefined (server-side rendering)", () => { + const originalWindow = global.window; + delete (global as any).window; + + expect(() => storeLoginToken("token")).not.toThrow(); + + global.window = originalWindow; + }); + + it("should not store empty string token", () => { + storeLoginToken(""); + expect(sessionStorage.getItem("token")).toBeNull(); + }); + + it("should not store whitespace-only token", () => { + storeLoginToken(" "); + expect(sessionStorage.getItem("token")).toBeNull(); + }); + + it("should set a JS-accessible cookie at /ui path", () => { + const cookieSpy = vi.spyOn(document, "cookie", "set"); + storeLoginToken("my-jwt-token"); + expect(cookieSpy).toHaveBeenCalledWith( + expect.stringContaining("path=/ui") + ); + vi.restoreAllMocks(); + }); }); describe("getCookie", () => { @@ -141,5 +191,26 @@ describe("cookieUtils", () => { expect(getCookie("token")).toBe("token-value"); expect(getCookie("other")).toBe("other-value"); }); + + it("should handle values containing '=' characters", () => { + document.cookie = "token=abc=def=ghi; path=/"; + expect(getCookie("token")).toBe("abc=def=ghi"); + }); + + it("should fall back to sessionStorage when cookie is not found", () => { + sessionStorage.setItem("token", "session-stored-jwt"); + expect(getCookie("token")).toBe("session-stored-jwt"); + }); + + it("should prefer cookie over sessionStorage", () => { + document.cookie = "token=cookie-value; path=/"; + sessionStorage.setItem("token", "session-value"); + expect(getCookie("token")).toBe("cookie-value"); + }); + + it("should not fall back to sessionStorage for non-token keys", () => { + sessionStorage.setItem("other", "other-value"); + expect(getCookie("other")).toBeNull(); + }); }); }); diff --git a/ui/litellm-dashboard/src/utils/cookieUtils.ts b/ui/litellm-dashboard/src/utils/cookieUtils.ts index 01add36542c..b4493744ad4 100644 --- a/ui/litellm-dashboard/src/utils/cookieUtils.ts +++ b/ui/litellm-dashboard/src/utils/cookieUtils.ts @@ -2,6 +2,23 @@ * Utility functions for managing cookies */ +/** + * Returns the cookie path for the UI. + * Derives the path from window.location.pathname so it works when + * LiteLLM is deployed behind a subpath (e.g. /myapp/ui instead of /ui). + * No imports from networking.tsx to avoid circular dependencies. + */ +function getUiCookiePath(): string { + if (typeof window === "undefined") return "/ui"; + // Match "/ui" only as a full path segment (followed by "/" or end of string) + // to avoid false matches like "/my-ui-tool/login" → "/my-ui". + const match = window.location.pathname.match(/\/ui(?=\/|$)/); + if (match && match.index !== undefined) { + return window.location.pathname.substring(0, match.index + 3); + } + return "/ui"; +} + /** * Clears the token cookie from both root and /ui paths */ @@ -16,7 +33,8 @@ export function clearTokenCookies() { // Clear with various combinations of path and SameSite // Include current path in case of custom server root path const currentPath = window.location.pathname; - const paths = ["/", "/ui"]; + const uiCookiePath = getUiCookiePath(); + const paths = ["/", uiCookiePath]; // Add the current path directory if it's different from root and /ui if (currentPath && currentPath !== "/" && !currentPath.startsWith("/ui")) { @@ -43,7 +61,45 @@ export function clearTokenCookies() { }); }); - console.log("After clearing cookies:", document.cookie); + try { + sessionStorage.removeItem("token"); + } catch { + // sessionStorage may be unavailable + } + +} + +/** + * Stores the login token so the UI can read it even when a reverse proxy + * (e.g. nginx-ingress) adds HttpOnly to the server-set cookie. + * + * Strategy: + * 1. Set a JS-accessible cookie at path "/ui". Because nginx only modifies + * server-set Set-Cookie headers, a cookie created via document.cookie will + * never carry HttpOnly. Using path "/ui" avoids colliding with the + * server-set HttpOnly cookie at path "/". + * 2. Also store in sessionStorage as a secondary fallback. + */ +export function storeLoginToken(token: string) { + if (typeof window === "undefined") return; + if (!token || !token.trim()) return; + + // 1. JS-accessible cookie at /ui — survives same-tab navigations and + // is readable by getCookie() via document.cookie. + try { + const secure = window.location.protocol === "https:" ? "; Secure" : ""; + const cookiePath = getUiCookiePath(); + document.cookie = `token=${encodeURIComponent(token)}; path=${cookiePath}; SameSite=Lax${secure}`; + } catch { + // cookie setting may fail in restrictive environments + } + + // 2. sessionStorage backup + try { + sessionStorage.setItem("token", token); + } catch { + // sessionStorage may be unavailable (e.g. private browsing quota exceeded) + } } /** @@ -53,6 +109,23 @@ export function clearTokenCookies() { */ export function getCookie(name: string) { if (typeof document === "undefined") return null; - const cookieValue = document.cookie.split("; ").find((row) => row.startsWith(name + "=")); - return cookieValue ? cookieValue.split("=")[1] : null; + const row = document.cookie.split("; ").find((r) => r.startsWith(name + "=")); + if (row) { + const raw = row.split("=").slice(1).join("="); + try { + return decodeURIComponent(raw); + } catch { + return raw; + } + } + // Fallback to sessionStorage — covers the case where a reverse proxy + // added HttpOnly to the server-set cookie, making it invisible to JS. + if (name === "token" && typeof window !== "undefined") { + try { + return sessionStorage.getItem(name); + } catch { + return null; + } + } + return null; } From ffb87dcac9a7f064b8c0bac32edfc4f4dad51fbf Mon Sep 17 00:00:00 2001 From: Sameer Kankute Date: Tue, 14 Apr 2026 20:53:17 +0530 Subject: [PATCH 8/9] Fix failing test and code qa + lint --- litellm/integrations/s3_v2.py | 3 +- .../guardrails/guardrail_hooks/presidio.py | 219 ++++++++++-------- tests/test_litellm/proxy/test_proxy_server.py | 5 +- 3 files changed, 122 insertions(+), 105 deletions(-) diff --git a/litellm/integrations/s3_v2.py b/litellm/integrations/s3_v2.py index f8d1710dfdc..a09a2afe26e 100644 --- a/litellm/integrations/s3_v2.py +++ b/litellm/integrations/s3_v2.py @@ -701,8 +701,9 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): # Prepare the signed headers signed_headers = dict(aws_request.headers.items()) + request_url = prepped.url or url response = await self.async_httpx_client.get( - prepped.url, headers=signed_headers + request_url, headers=signed_headers ) if response.status_code != 200: diff --git a/litellm/proxy/guardrails/guardrail_hooks/presidio.py b/litellm/proxy/guardrails/guardrail_hooks/presidio.py index e048ca21cba..67cb281029c 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/presidio.py +++ b/litellm/proxy/guardrails/guardrail_hooks/presidio.py @@ -433,6 +433,109 @@ class _OPTIONAL_PresidioPIIMasking(CustomGuardrail): # contain API keys or other secrets) in error responses. raise Exception(f"Presidio PII analysis failed: {type(e).__name__}") from e + async def _post_presidio_anonymize( + self, text: str, analyze_results: Any + ) -> Any: + """POST to Presidio anonymize; returns parsed JSON body.""" + # Use shared session to prevent memory leak (issue #14540) + async with self._get_session_iterator() as session: + anonymize_url = f"{self.presidio_anonymizer_api_base}anonymize" + verbose_proxy_logger.debug("Making request to: %s", anonymize_url) + anonymize_payload = { + "text": text, + "analyzer_results": analyze_results, + } + async with session.post( + anonymize_url, + json=anonymize_payload, + headers={"Accept": "application/json"}, + ) as response: + if response.status >= 400: + error_body = await response.text() + raise Exception( + f"Presidio anonymizer returned HTTP {response.status}: {error_body[:200]}" + ) + content_type = getattr( + response, + "content_type", + response.headers.get("Content-Type", ""), + ) + if "application/json" not in content_type: + error_body = await response.text() + raise Exception( + f"Presidio anonymizer returned non-JSON Content-Type '{content_type}'; body: '{error_body[:200]}'" + ) + return await response.json() + + def _finalize_presidio_anonymize_simple( + self, + redacted_text: Dict[str, Any], + masked_entity_count: Dict[str, int], + ) -> str: + # No need to build numbered tokens — just use Presidio's + # already-anonymized text directly. The old code incorrectly + # applied anonymizer item positions (which reference the + # *output* text) to the *original* text, causing offset errors. + for item in redacted_text.get("items", []): + entity_type = item.get("entity_type", None) + if entity_type is not None: + masked_entity_count[entity_type] = ( + masked_entity_count.get(entity_type, 0) + 1 + ) + return redacted_text["text"] + + def _finalize_presidio_anonymize_numbered_tokens( + self, + text: str, + analyze_results: Any, + request_data: Optional[Dict], + masked_entity_count: Dict[str, int], + ) -> str: + # output_parse_pii is True — we need sequentially numbered + # tokens and a pii_tokens mapping for later unmasking. + # Use analyze_results positions (which reference the ORIGINAL + # text) instead of anonymizer items (which reference the output). + new_text = text + if request_data is None: + verbose_proxy_logger.warning( + "Presidio anonymize_text called without request_data — " + "PII tokens cannot be stored per-request. " + "This may indicate a missing caller update." + ) + request_data = {} + if not request_data.get("metadata"): + request_data["metadata"] = {} + if "pii_tokens" not in request_data["metadata"]: + request_data["metadata"]["pii_tokens"] = {} + pii_tokens = request_data["metadata"]["pii_tokens"] + + # Assign sequence numbers in forward (left-to-right) order so + # that is the first entity in the text, etc. + sorted_forward = sorted(analyze_results, key=lambda x: x["start"]) + seq_map = {} + for idx, ar in enumerate(sorted_forward, start=1): + seq_map[(ar["start"], ar["end"])] = idx + + # Apply replacements in reverse order by start position so + # that replacing later spans first does not shift earlier + # coordinates in the original text. + for ar in reversed(sorted_forward): + start = ar["start"] + end = ar["end"] + entity_type = ar["entity_type"] + replacement = f"<{entity_type}>" + seq = seq_map[(start, end)] + if replacement.endswith(">"): + replacement = f"{replacement[:-1]}_{seq}>" + else: + replacement = f"{replacement}_{seq}" + pii_tokens[replacement] = text[start:end] + new_text = new_text[:start] + replacement + new_text[end:] + masked_entity_count[entity_type] = ( + masked_entity_count.get(entity_type, 0) + 1 + ) + return new_text + async def anonymize_text( self, text: str, @@ -449,110 +552,20 @@ class _OPTIONAL_PresidioPIIMasking(CustomGuardrail): if isinstance(analyze_results, list) and len(analyze_results) == 0: return text - # Use shared session to prevent memory leak (issue #14540) - async with self._get_session_iterator() as session: - # Make the request to /anonymize - anonymize_url = f"{self.presidio_anonymizer_api_base}anonymize" - verbose_proxy_logger.debug("Making request to: %s", anonymize_url) - anonymize_payload = { - "text": text, - "analyzer_results": analyze_results, - } - - async with session.post( - anonymize_url, - json=anonymize_payload, - headers={"Accept": "application/json"}, - ) as response: - # Validate HTTP status - if response.status >= 400: - error_body = await response.text() - raise Exception( - f"Presidio anonymizer returned HTTP {response.status}: {error_body[:200]}" - ) - - # Validate Content-Type is JSON - content_type = getattr( - response, - "content_type", - response.headers.get("Content-Type", ""), - ) - if "application/json" not in content_type: - error_body = await response.text() - raise Exception( - f"Presidio anonymizer returned non-JSON Content-Type '{content_type}'; body: '{error_body[:200]}'" - ) - - redacted_text = await response.json() - - if redacted_text is not None: - verbose_proxy_logger.debug("redacted_text: %s", redacted_text) - - if not output_parse_pii: - # No need to build numbered tokens — just use Presidio's - # already-anonymized text directly. The old code incorrectly - # applied anonymizer item positions (which reference the - # *output* text) to the *original* text, causing offset errors. - for item in redacted_text.get("items", []): - entity_type = item.get("entity_type", None) - if entity_type is not None: - masked_entity_count[entity_type] = ( - masked_entity_count.get(entity_type, 0) + 1 - ) - return redacted_text["text"] - - # output_parse_pii is True — we need sequentially numbered - # tokens and a pii_tokens mapping for later unmasking. - # Use analyze_results positions (which reference the ORIGINAL - # text) instead of anonymizer items (which reference the output). - new_text = text - if request_data is None: - verbose_proxy_logger.warning( - "Presidio anonymize_text called without request_data — " - "PII tokens cannot be stored per-request. " - "This may indicate a missing caller update." - ) - request_data = {} - if not request_data.get("metadata"): - request_data["metadata"] = {} - if "pii_tokens" not in request_data["metadata"]: - request_data["metadata"]["pii_tokens"] = {} - pii_tokens = request_data["metadata"]["pii_tokens"] - - # Assign sequence numbers in forward (left-to-right) order so - # that is the first entity in the text, etc. - sorted_forward = sorted( - analyze_results, key=lambda x: x["start"] - ) - seq_map = {} - for idx, ar in enumerate(sorted_forward, start=1): - seq_map[(ar["start"], ar["end"])] = idx - - # Apply replacements in reverse order by start position so - # that replacing later spans first does not shift earlier - # coordinates in the original text. - for ar in reversed(sorted_forward): - start = ar["start"] - end = ar["end"] - entity_type = ar["entity_type"] - replacement = f"<{entity_type}>" - - seq = seq_map[(start, end)] - if replacement.endswith(">"): - replacement = f"{replacement[:-1]}_{seq}>" - else: - replacement = f"{replacement}_{seq}" - - pii_tokens[replacement] = text[start:end] - new_text = new_text[:start] + replacement + new_text[end:] - - masked_entity_count[entity_type] = ( - masked_entity_count.get(entity_type, 0) + 1 - ) - - return new_text - else: + redacted_text = await self._post_presidio_anonymize(text, analyze_results) + if redacted_text is None: raise Exception("Invalid anonymizer response: received None") + + verbose_proxy_logger.debug("redacted_text: %s", redacted_text) + + if not output_parse_pii: + return self._finalize_presidio_anonymize_simple( + redacted_text, masked_entity_count + ) + + return self._finalize_presidio_anonymize_numbered_tokens( + text, analyze_results, request_data, masked_entity_count + ) except Exception as e: # Sanitize exception to avoid leaking the original text (which may # contain API keys or other secrets) in error responses. diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index daabed0def1..c32a1bdd463 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -104,7 +104,10 @@ def test_login_v2_returns_redirect_url_and_sets_cookie(monkeypatch): ) assert response.status_code == 200 - assert response.json() == {"redirect_url": "http://testserver/ui/?login=success"} + assert response.json() == { + "redirect_url": "http://testserver/ui/?login=success", + "token": "signed-token", + } assert response.cookies.get("token") == "signed-token" mock_authenticate_user.assert_awaited_once_with( From 69bf2bfb9ac785ce2c257ed817ff0a8d5887c6b9 Mon Sep 17 00:00:00 2001 From: Sameer Kankute Date: Tue, 14 Apr 2026 21:18:23 +0530 Subject: [PATCH 9/9] Fix tests --- litellm/integrations/s3_v2.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/litellm/integrations/s3_v2.py b/litellm/integrations/s3_v2.py index a09a2afe26e..7f7d47b3150 100644 --- a/litellm/integrations/s3_v2.py +++ b/litellm/integrations/s3_v2.py @@ -404,11 +404,14 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): # Prepare the signed headers signed_headers = dict(aws_request.headers.items()) + # Use prepared URL so path segments match SigV4 canonical request (e.g. %20 for spaces). + request_url = prepped.url or url + # Make the request with retry for transient S3 errors (500/503) max_retries = 3 for attempt in range(max_retries): response = await self.async_httpx_client.put( - url, data=json_string, headers=signed_headers + request_url, data=json_string, headers=signed_headers ) if response.status_code in (500, 503) and attempt < max_retries - 1: wait_time = 2**attempt # 1s, 2s @@ -590,6 +593,9 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): # Prepare the signed headers signed_headers = dict(aws_request.headers.items()) + # Use prepared URL so path segments match SigV4 canonical request (e.g. %20 for spaces). + request_url = prepped.url or url + httpx_client = _get_httpx_client( params={"ssl_verify": self.s3_verify} if self.s3_verify is not None @@ -599,7 +605,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): max_retries = 3 for attempt in range(max_retries): response = httpx_client.put( - url, data=json_string, headers=signed_headers + request_url, data=json_string, headers=signed_headers ) if response.status_code in (500, 503) and attempt < max_retries - 1: wait_time = 2**attempt # 1s, 2s