mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-02 07:13:02 +00:00
fix(proxy): strip caller base_model from spend pricing and reject lone surrogates in multipart metadata (#47639)
* fix(proxy): strip caller base_model from spend pricing and reject lone surrogates in multipart metadata
* fix(proxy): type the parsed multipart metadata so basedpyright does not grow reportArgumentType
---------
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
(cherry picked from commit 2a268951c4)
This commit is contained in:
parent
bcfbeb86a3
commit
dee764834a
6 changed files with 111 additions and 7 deletions
|
|
@ -191,6 +191,23 @@ def is_otlp_trace_request(request: HTTPConnection) -> bool:
|
|||
return request.scope.get("method") == "POST" and get_route_path(request.scope) in {"/v1/traces", "/v1/logs"}
|
||||
|
||||
|
||||
def _parse_form_metadata(raw_metadata: str) -> object:
|
||||
"""The JSON body path rejects a lone surrogate escape (``"\\ud800"``) with a
|
||||
400 because no provider or database can encode it. ``json.loads`` accepts
|
||||
the escape, so the multipart ``metadata`` field gets the same check."""
|
||||
parsed: Final[object] = json.loads(raw_metadata)
|
||||
try:
|
||||
json.dumps(parsed, ensure_ascii=False).encode("utf-8")
|
||||
except UnicodeEncodeError as e:
|
||||
raise ProxyException(
|
||||
message=f"Invalid form payload: metadata {e}",
|
||||
type="invalid_request_error",
|
||||
param="metadata",
|
||||
code=status.HTTP_400_BAD_REQUEST,
|
||||
) from e
|
||||
return parsed
|
||||
|
||||
|
||||
async def read_request_body(request: Request | None) -> dict:
|
||||
"""
|
||||
Safely read the request body and parse it as JSON.
|
||||
|
|
@ -238,9 +255,13 @@ async def read_request_body(request: Request | None) -> dict:
|
|||
param="request_body",
|
||||
code=status.HTTP_400_BAD_REQUEST,
|
||||
)
|
||||
parsed_body = dict(form_data)
|
||||
if "metadata" in parsed_body and isinstance(parsed_body["metadata"], str):
|
||||
parsed_body["metadata"] = json.loads(parsed_body["metadata"])
|
||||
form_body: Final = dict(form_data)
|
||||
raw_metadata: Final = form_body.get("metadata")
|
||||
parsed_body = (
|
||||
{**form_body, "metadata": _parse_form_metadata(raw_metadata)}
|
||||
if isinstance(raw_metadata, str)
|
||||
else form_body
|
||||
)
|
||||
else:
|
||||
# Read the request body
|
||||
body: Final = await request.body()
|
||||
|
|
|
|||
|
|
@ -407,8 +407,9 @@ _ALLOW_CLIENT_MESSAGE_REDACTION_OPT_OUT_METADATA_KEY: Final = "allow_client_mess
|
|||
# ``litellm.model_cost`` map. Both effects belong to deployment configuration,
|
||||
# not to user-supplied request bodies, so the proxy strips them before they
|
||||
# reach the call path. Built from the Pydantic model so newly-added pricing
|
||||
# fields are covered automatically.
|
||||
_CLIENT_PRICING_CONTROL_FIELDS: Final = frozenset(CustomPricingLiteLLMParams.model_fields.keys())
|
||||
# fields are covered automatically. ``base_model`` selects which cost-map entry
|
||||
# prices the row, so a caller naming a cheaper model lowers their own spend.
|
||||
_CLIENT_PRICING_CONTROL_FIELDS: Final = frozenset(CustomPricingLiteLLMParams.model_fields.keys()) | {"base_model"}
|
||||
# ``model_info`` carries the same pricing fields when read by
|
||||
# ``use_custom_pricing_for_model``; strip from metadata for the same reason.
|
||||
# ``standard_logging_guardrail_information`` is proxy-written telemetry summed
|
||||
|
|
|
|||
|
|
@ -8525,7 +8525,7 @@ async def _create_spend_logs_with_poison_isolation(
|
|||
await repo.table.create_many(data=rows, skip_duplicates=True)
|
||||
return failure_budget
|
||||
except Exception as e:
|
||||
if not PrismaDBExceptionHandler.is_prisma_data_error(e):
|
||||
if not (PrismaDBExceptionHandler.is_prisma_data_error(e) or isinstance(e, UnicodeEncodeError)):
|
||||
raise
|
||||
if PrismaDBExceptionHandler.is_database_service_unavailable_error(e):
|
||||
raise
|
||||
|
|
|
|||
|
|
@ -1556,3 +1556,39 @@ def test_is_otlp_trace_request_matches_only_http_posts_to_the_otlp_routes(
|
|||
)
|
||||
|
||||
assert http_parsing_utils.is_otlp_trace_request(connection) is expected
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_form_data_metadata_with_lone_surrogate_escape_is_rejected_like_a_json_body():
|
||||
"""``json.loads`` accepts ``"\\ud800"`` and yields a lone surrogate that no
|
||||
provider or database can utf-8 encode. The JSON body path already answers
|
||||
400, so the multipart ``metadata`` field must too, instead of letting the
|
||||
value through to the spend log writers."""
|
||||
mock_request = MagicMock()
|
||||
mock_request.form = AsyncMock(
|
||||
return_value=FormData({"model": "whisper-1", "metadata": '{"tags": ["poison-\\ud800-tag"]}'})
|
||||
)
|
||||
mock_request.headers = {"content-type": "multipart/form-data"}
|
||||
mock_request.scope = {}
|
||||
mock_request.state._cached_headers = None
|
||||
|
||||
with pytest.raises(ProxyException) as exc_info:
|
||||
await read_request_body(mock_request)
|
||||
|
||||
assert exc_info.value.code == "400"
|
||||
assert "metadata" in exc_info.value.message
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_form_data_metadata_keeps_non_ascii_and_valid_surrogate_pairs():
|
||||
mock_request = MagicMock()
|
||||
mock_request.form = AsyncMock(
|
||||
return_value=FormData({"model": "whisper-1", "metadata": '{"tags": ["caf\\u00e9", "\\ud83d\\ude00"]}'})
|
||||
)
|
||||
mock_request.headers = {"content-type": "multipart/form-data"}
|
||||
mock_request.scope = {}
|
||||
mock_request.state._cached_headers = None
|
||||
|
||||
result = await read_request_body(mock_request)
|
||||
|
||||
assert result["metadata"]["tags"] == ["café", "😀"]
|
||||
|
|
|
|||
|
|
@ -57,7 +57,7 @@ class TestStripClientPricingOverrides:
|
|||
# The strip set is built from the model so additions are picked up
|
||||
# automatically — this test guards against the model and the strip
|
||||
# set drifting apart if someone replaces the auto-derivation later.
|
||||
assert _CLIENT_PRICING_CONTROL_FIELDS == frozenset(CustomPricingLiteLLMParams.model_fields.keys())
|
||||
assert _CLIENT_PRICING_CONTROL_FIELDS >= frozenset(CustomPricingLiteLLMParams.model_fields.keys())
|
||||
# Sanity: the obvious top-level pricing fields are in the set.
|
||||
for field in (
|
||||
"input_cost_per_token",
|
||||
|
|
@ -82,6 +82,22 @@ class TestStripClientPricingOverrides:
|
|||
"messages": [{"role": "user", "content": "hi"}],
|
||||
}
|
||||
|
||||
def test_root_base_model_dropped(self):
|
||||
# ``base_model`` picks the cost-map entry a spend row is priced with
|
||||
# (success rows via the standard logging payload, billed failure rows
|
||||
# via the failure lift), so a caller naming a cheaper model lowers
|
||||
# their own recorded spend.
|
||||
data = {
|
||||
"model": "gpt-4o",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
"base_model": "gpt-4o-mini",
|
||||
}
|
||||
_strip_client_pricing_overrides(data)
|
||||
assert data == {
|
||||
"model": "gpt-4o",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
}
|
||||
|
||||
def test_metadata_model_info_dropped(self):
|
||||
data = {
|
||||
"model": "gpt-4",
|
||||
|
|
@ -280,6 +296,7 @@ async def test_add_litellm_data_to_request_skips_strip_with_key_opt_in():
|
|||
"model": "gpt-4",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
"input_cost_per_token": 0.0001,
|
||||
"base_model": "gpt-4o-mini",
|
||||
"metadata": {"model_info": {"output_cost_per_token": 0.0002}},
|
||||
}
|
||||
|
||||
|
|
@ -295,6 +312,7 @@ async def test_add_litellm_data_to_request_skips_strip_with_key_opt_in():
|
|||
|
||||
assert updated["input_cost_per_token"] == 0.0001
|
||||
assert updated["metadata"]["model_info"] == {"output_cost_per_token": 0.0002}
|
||||
assert updated["base_model"] == "gpt-4o-mini"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -323,3 +323,31 @@ async def test_update_spend_logs_multiple_batches_with_failure():
|
|||
|
||||
# Verify all logs were cleared from transactions
|
||||
assert len(prisma_client.spend_log_transactions) == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_spend_logs_drops_row_prisma_cannot_encode_and_keeps_the_rest():
|
||||
"""A row the Prisma engine cannot utf-8 encode (a lone surrogate in a string
|
||||
column) is permanent, so it must be dropped like any other poison row instead
|
||||
of being requeued at the head of the queue where it would block every tenant."""
|
||||
prisma_client = MockPrismaClient()
|
||||
proxy_logging_obj = create_mock_proxy_logging()
|
||||
prisma_client.spend_log_transactions = [
|
||||
{"request_id": "good-1", "request_tags": '["a"]'},
|
||||
{"request_id": "poison", "request_tags": "\ud800"},
|
||||
{"request_id": "good-2", "request_tags": '["b"]'},
|
||||
]
|
||||
written = []
|
||||
|
||||
async def create_many(data, skip_duplicates):
|
||||
if any("\ud800" in row["request_tags"] for row in data):
|
||||
raise UnicodeEncodeError("utf-8", "\ud800", 0, 1, "surrogates not allowed")
|
||||
written.extend(row["request_id"] for row in data)
|
||||
|
||||
prisma_client.db.litellm_spendlogs.create_many = AsyncMock(side_effect=create_many)
|
||||
|
||||
await update_spend(prisma_client, None, proxy_logging_obj)
|
||||
|
||||
assert sorted(written) == ["good-1", "good-2"]
|
||||
assert prisma_client.spend_log_transactions == []
|
||||
proxy_logging_obj.failure_handler.assert_not_called()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue