mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
fix(batches): strip NUL bytes from passthrough batch tags before the managed object write (#36688)
PostgreSQL rejects NUL in jsonb with 22P05, and the tags go into the managed object's CREATE payload, so one poisoned tag aborts the whole row insert rather than just that column. With no LiteLLM_ManagedObjectTable row, CheckBatchCost never discovers the batch, so a batch that really ran and billed at the provider produces no spend at all. The create-time write is fire and forget, so nothing retries it. This regressed in #36468, which started passing request_tags and persist_attribution from the Anthropic passthrough; before that no caller-supplied string reached the column. Sanitize in the shared helper that builds the value, matching how spend_tracking_utils already handles LiteLLM_SpendLogs.request_tags. Both the Anthropic and the Vertex passthrough build tags through that one helper, so this covers both. Rename it to _sanitized_str_tuple since it no longer merely coerces.
This commit is contained in:
parent
2d12a3ea41
commit
0e9da56f89
4 changed files with 48 additions and 4 deletions
|
|
@ -10,17 +10,18 @@ from collections.abc import Mapping, Sequence
|
|||
from typing import Final
|
||||
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
from litellm.litellm_core_utils.safe_json_dumps import strip_null_bytes
|
||||
|
||||
|
||||
def optional_str(value: object) -> str | None:
|
||||
return value if isinstance(value, str) else None
|
||||
|
||||
|
||||
def _optional_str_tuple(value: object) -> tuple[str, ...] | None:
|
||||
def _sanitized_str_tuple(value: object) -> tuple[str, ...] | None:
|
||||
if not isinstance(value, list):
|
||||
return None
|
||||
items: Final[Sequence[object]] = value
|
||||
return tuple(tag for tag in items if isinstance(tag, str))
|
||||
return tuple(strip_null_bytes(tag) for tag in items if isinstance(tag, str))
|
||||
|
||||
|
||||
def is_collection_route(url_route: str, collection_suffix: str) -> bool:
|
||||
|
|
@ -37,12 +38,12 @@ def request_tags_from_metadata(request_metadata: Mapping[str, object]) -> tuple[
|
|||
tagged key does not put its tags in the top-level metadata "tags" on the
|
||||
passthrough path)
|
||||
"""
|
||||
tags: Final = _optional_str_tuple(request_metadata.get("tags"))
|
||||
tags: Final = _sanitized_str_tuple(request_metadata.get("tags"))
|
||||
if tags:
|
||||
return tags
|
||||
key_auth_metadata: Final = request_metadata.get("user_api_key_auth_metadata")
|
||||
if isinstance(key_auth_metadata, dict):
|
||||
return _optional_str_tuple(key_auth_metadata.get("tags"))
|
||||
return _sanitized_str_tuple(key_auth_metadata.get("tags"))
|
||||
return None
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -954,6 +954,16 @@ class TestAnthropicBatchPassthroughCostTracking:
|
|||
mock_managed_files_hook.store_unified_object_id.assert_awaited_once()
|
||||
return mock_managed_files_hook.store_unified_object_id.call_args[1]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_persisted_tags_are_db_safe(self, mock_logging_obj):
|
||||
"""Regression for PostgreSQL 22P05, asserted on the value that actually reaches
|
||||
store_unified_object_id so it stays pinned if the sanitation moves."""
|
||||
call_kwargs = await self._store_with_metadata(
|
||||
mock_logging_obj, {"user_api_key": "hashed-key-a", "tags": ["clean", "bad\x00tag"]}
|
||||
)
|
||||
|
||||
assert call_kwargs["request_tags"] == ("clean", "badtag")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_persists_key_hash_and_tags(self, mock_logging_obj):
|
||||
"""Regression (LIT-5288): the batch create must persist the creating key's hashed
|
||||
|
|
|
|||
|
|
@ -70,6 +70,28 @@ class TestRequestTagsFromMetadata:
|
|||
def test_malformed_key_auth_metadata_is_ignored(self):
|
||||
assert request_tags_from_metadata({"user_api_key_auth_metadata": "nope"}) is None
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"raw, expected",
|
||||
[
|
||||
(["bad\x00tag"], ("badtag",)),
|
||||
(["\x00leading"], ("leading",)),
|
||||
(["trailing\x00"], ("trailing",)),
|
||||
(["a\x00b\x00c"], ("abc",)),
|
||||
(["\x00"], ("",)),
|
||||
# every element, not just the first
|
||||
(["clean", "bad\x00tag"], ("clean", "badtag")),
|
||||
(["one\x00", "two\x00", "three\x00"], ("one", "two", "three")),
|
||||
],
|
||||
)
|
||||
def test_nul_bytes_are_stripped_from_request_tags(self, raw, expected):
|
||||
"""Regression for PostgreSQL 22P05: an unstripped NUL aborts the managed object row
|
||||
insert, so the batch is never cost tracked."""
|
||||
assert request_tags_from_metadata({"tags": raw}) == expected
|
||||
|
||||
def test_nul_bytes_are_stripped_from_key_tags_fallback(self):
|
||||
"""Regression for PostgreSQL 22P05: the key-tags fallback shares the same helper."""
|
||||
assert request_tags_from_metadata({"user_api_key_auth_metadata": {"tags": ["key\x00tag"]}}) == ("keytag",)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"url_route, suffix, expected",
|
||||
|
|
|
|||
|
|
@ -336,6 +336,17 @@ class TestVertexAIBatchPassthroughHandler:
|
|||
mock_managed_files_hook.store_unified_object_id.assert_called_once()
|
||||
return mock_managed_files_hook.store_unified_object_id.call_args[1]
|
||||
|
||||
def test_persisted_tags_are_db_safe(self, mock_logging_obj, mock_managed_files_hook):
|
||||
"""Regression for PostgreSQL 22P05, asserted for Vertex too so moving the
|
||||
sanitation somewhere that only covers Anthropic fails loudly."""
|
||||
call_kwargs = self._store_with_metadata(
|
||||
mock_logging_obj,
|
||||
mock_managed_files_hook,
|
||||
{"user_api_key": "hashed-key-a", "tags": ["clean", "bad\x00tag"]},
|
||||
)
|
||||
|
||||
assert call_kwargs["request_tags"] == ("clean", "badtag")
|
||||
|
||||
def test_create_persists_key_hash_and_tags(
|
||||
self, mock_logging_obj, mock_managed_files_hook
|
||||
):
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue