mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-29 06:33:28 +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>
Backport of #47639 to stable/1.105.x.
Cherry-picked from 2a268951c4 (main).
This commit is contained in:
parent
9fa48987fa
commit
e4c41e98cd
6 changed files with 111 additions and 7 deletions
|
|
@ -172,6 +172,23 @@ def is_otlp_trace_request(request: Request) -> bool:
|
|||
return request.method == "POST" and get_route_path(request.scope) == "/v1/traces"
|
||||
|
||||
|
||||
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.
|
||||
|
|
@ -216,9 +233,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()
|
||||
|
|
|
|||
|
|
@ -362,8 +362,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
|
||||
|
|
|
|||
|
|
@ -8100,7 +8100,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
|
||||
|
|
|
|||
|
|
@ -1366,3 +1366,39 @@ async def test_auth_body_read_and_trace_handler_leave_stream_for_receiver_limit(
|
|||
assert response.status_code == 413
|
||||
assert receive.await_count == 2
|
||||
store.insert_spans.assert_not_awaited()
|
||||
|
||||
|
||||
@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é", "😀"]
|
||||
|
|
|
|||
|
|
@ -58,7 +58,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(
|
||||
assert _CLIENT_PRICING_CONTROL_FIELDS >= frozenset(
|
||||
CustomPricingLiteLLMParams.model_fields.keys()
|
||||
)
|
||||
# Sanity: the obvious top-level pricing fields are in the set.
|
||||
|
|
@ -85,6 +85,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",
|
||||
|
|
@ -265,6 +281,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}},
|
||||
}
|
||||
|
||||
|
|
@ -280,6 +297,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