fix(batches): attribute Vertex batch cost spend logs to key/team/tags - #33319
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
e47d7b5 to
dc83978
Compare
Greptile SummaryThis PR fixes Vertex batch jobs submitted through the passthrough endpoint producing no attributed spend row — the batch cost was silently dropped because the managed object was stored with a blank key, user, and team. The fix snapshots the authenticated identity and request tags onto the managed object's
Confidence Score: 5/5Safe to merge — the changes are well-scoped, all edge cases (legacy batches, None fields, malformed JSON, stash failures) are handled gracefully, and the scrubbing of internal data from API responses is consistently applied across all read paths. The snapshot/restore flow is correct end-to-end: identity re-assertion happens before the snapshot is taken, the snapshot survives status-update overwrites via explicit carry-forward, and the internal key is stripped before any batch object is returned to a caller. Test coverage is thorough, including the spoofing regression, DB-lookup elimination, legacy fallback, and the non-mutation of the raw DB row. No files require special attention.
|
| Filename | Overview |
|---|---|
| enterprise/litellm_enterprise/proxy/common_utils/check_batch_cost.py | Adds _get_batch_attribution, _get_batch_spend_attribution helpers; replaces ad-hoc user-table lookup with snapshot-based attribution, with clean legacy fallback. Logic is correct and handles all edge cases (None fields filtered, empty attribution dict, JSON/dict duality of file_object). |
| litellm/proxy/openai_files_endpoints/common_utils.py | Adds strip_internal_batch_attribution and read_stored_batch_attribution helpers, applies stripping in get_batch_from_database, and preserves the attribution snapshot across status updates in update_batch_in_database. Both string and dict forms of file_object are handled. |
| litellm/proxy/pass_through_endpoints/llm_provider_handlers/vertex_passthrough_logging_handler.py | Snapshots the authenticated identity + request tags onto batch_object.metadata before storage; stash failures are caught and logged so they never abort batch creation. Tag resolution priority (request → litellm_metadata → key auth metadata) is consistent with the passthrough pattern. |
| litellm/proxy/pass_through_endpoints/pass_through_endpoints.py | Re-asserts user_id, team_id, team_alias, key_alias, and user_email from the authenticated UserAPIKeyAuth after the client-metadata merge, preventing a request body from spoofing the batch-cost owner. Mirrors the existing api_key re-assertion pattern. |
| enterprise/litellm_enterprise/proxy/hooks/managed_files.py | Applies strip_internal_batch_attribution to the list-batches path, ensuring the attribution snapshot is scrubbed from the caller-facing response. One-liner change with correct placement. |
| tests/test_litellm/enterprise/proxy/test_batch_update_db_managed_output_file_id.py | Adds thorough regression tests: attribution preserved across status updates, stripping does not mutate the raw row, get_batch_from_database does not leak the snapshot, read_stored_batch_attribution handles all None/malformed inputs. Assertion polarity is correct. |
| tests/proxy_unit_tests/test_check_batch_cost.py | New TestBatchCostAttribution class tests snapshot recovery, zero-DB-lookup attribution, spoofing protection via snapshot > stored columns, missing-field filtering, and legacy fallback. All assertions are directionally correct. |
| tests/pass_through_unit_tests/test_pass_through_unit_tests.py | Adds spoofing-regression test that verifies client-supplied identity fields are overwritten by the authenticated key after the metadata merge. |
| tests/test_litellm/proxy/pass_through_endpoints/test_vertex_ai_batch_passthrough.py | Adds three tests for _store_batch_managed_object: key+tags stashed correctly, key-level tags used as fallback when no request tags, and stash failure is swallowed. Coverage is solid. |
Reviews (6): Last reviewed commit: "fix(batches): keep the batch attribution..." | Re-trigger Greptile
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 3 · PR risk: 0/10 |
dc83978 to
fa4ca91
Compare
fa4ca91 to
f404372
Compare
f404372 to
1be6493
Compare
… snapshot Vertex batches created through the passthrough logged no attributed cost row. The managed object was stored with no request identity, so at poll time CheckBatchCost had a blank key/user/team and the DB spend-logger dropped the row entirely; batch cost was silently lost and the key showed $0 spend. Snapshot the authenticated identity (key, owning user and team, their aliases, user email) and request tags onto the managed object file_object at create time, where the passthrough already resolved them in-memory, and read them back at poll time. CheckBatchCost no longer issues any per-batch key/team/user DB lookup; legacy batches with no snapshot fall back to the stored row. Request tags come from the request when present, otherwise the key's own tags (exposed in-memory via user_api_key_auth_metadata), so a tagged key still tags the batch cost. The passthrough re-asserts the authenticated identity fields in request metadata after the client-metadata merge, so a caller cannot spoof the owner used for spend attribution (previously only the key hash was protected).
…f re-assertion Regression tests for issue BerriAI#33316: attribution recovery, zero DB lookups on the snapshot path, ownership from the snapshot not the stored columns, legacy fallback, the create path stashing the authenticated identity plus request or key tags, and the passthrough re-asserting identity so client metadata cannot spoof the batch owner.
1be6493 to
9e42634
Compare
…overwrites A batch retrieve that detects a status change makes update_batch_in_database replace file_object with the raw provider response, which does not carry the create-time litellm_batch_attribution snapshot. Losing it made the later cost poll fall back to user/team attribution without the key hash, so batch spend skipped the caller's key-level spend and budget counters. Carry the stored snapshot forward from the existing row onto the response before serializing, so it survives every status update through completion.
|
@veria-ai this is ready for another look |
…ches The create-time litellm_batch_attribution snapshot (virtual-key hash, user email, key and team aliases) lives under the managed object's file_object metadata for cost attribution. Because Batch inherits pydantic extra="allow", that snapshot round-tripped into the LiteLLMBatch that get_batch_from_database and list_user_batches return, and the preserve-on-update path also injected it onto the response object. A caller with team-level access to a batch could read the creator's key hash, email, and alias. Strip the internal key from a copy at both stored-object return boundaries, and land the preserved snapshot only in the serialized DB file_object rather than on the returned response. The poller still reads the snapshot from the raw row, so attribution keeps working with no extra database calls.
|
@veria-ai this is ready for another look |
The ruff strict gate (UP045) flagged read_stored_batch_attribution for using Optional[dict] instead of dict | None, adding one new violation over the base and breaching the strict-rule ceiling. Switch the return annotation to the modern union so the gate no longer counts a new violation
|
Thanks for the contribution. i adopted your PR and merged in #34456 |
Relevant issues
Resolves #33316
Linear ticket
Pre-Submission checklist
Screenshots / Proof of Fix
Verified on a local proxy running the equivalent change. A Vertex batch submitted through the passthrough with a team-scoped, tagged virtual key now produces an attributed
aretrieve_batchcost row with real spend, key alias, team, and tags, where before no row was written at all and the key showed$0spendType
🐛 Bug Fix
Changes
Vertex batches created through the passthrough (
/vertex_ai/.../batchPredictionJobs) logged no attributed cost row. At create time the managed object was stored with no request identity (api_key="",user_api_key_user_idunset), so when theCheckBatchCostpoller later logged the cost it had a blank key, user, and team. In that state the DB spend-logger drops the row entirely rather than writing an unattributed one, so the batch cost was silently lost and the key showed$0spendThe identity needed for attribution is already resolved in memory by auth at create time and copied into the passthrough request metadata, so this snapshots it onto the managed object
file_objectjsonb right then (the key hash, the owning user and team, their aliases, user email, and the request tags, which fall back to the key's own tags when the request sent none) and reads it back when the batch completes.CheckBatchCostissues no per-batch key, team, or user database lookup; batches created before the snapshot existed fall back to the stored row. This needs no schema migrationTo make that snapshot safe to trust, the passthrough re-asserts the authenticated identity fields (
user_api_key_user_id,user_api_key_team_id, and the aliases and email) in the request metadata after the client-supplied metadata is merged, mirroring how the key hash was already protected. Previously a request body could setuser_api_key_user_idoruser_api_key_team_idand have the completed batch charged to another user or team's spendThe snapshot is stored only for internal cost attribution and must never reach a caller. Because
Batchinherits pydanticextra="allow", anything under the storedfile_objectround-trips into theLiteLLMBatchthat the retrieve and list endpoints return, which would expose the creator's key hash, email, and aliases to anyone with team-level access to the batch. The two places that build a batch from a stored row now strip the internal key from a copy before returning, and the preserve-on-update path writes the snapshot into the serialized DBfile_objectrather than onto the returned response object, so it stays server-sideThe snapshot also has to survive later writes to the same row. When a retrieve detects a status change,
update_batch_in_databaseoverwritesfile_objectwith the raw provider batch, which does not carrylitellm_batch_attribution, so without care the later cost poll would fall back to attribution with no key hash and the spend would skip the caller's key-level budget. The update path now reads the stored snapshot off the existing row and carries it onto the response before serializing, so it persists across every status update through completionIn
pass_through_endpoints.pythe identity re-assertion closes that spoofing gap. Invertex_passthrough_logging_handler.pythe create path stashes the authenticated identity plus tags ontofile_object. Incheck_batch_cost.pythe poll path builds the spend-log metadata from that snapshot with no DB call, keeping the pre-existing user-table enrichment only for the legacy no-snapshot fallbackQA runbook
vertex_aimodel withuse_in_pass_through: true, a Postgres-backed proxy, and an enterprise licenseproxy_batch_polling_intervalGET /spend/logs: acall_type=aretrieve_batchrow appears, attributed to the key alias, team alias, and tags, with non-zero spendFinal Attestation