test(e2e): pin openai_passthrough routing, cost logging, and file list isolation - #37618
Conversation
…t isolation
Five e2e tests over routes a customer drives through the gateway, each one
pinning a fix that currently has no live coverage.
The dedicated /openai_passthrough prefix used to be swallowed by the
provider-scoped /{provider}/v1/files and /{provider}/v1/batches routes, which
bound "openai_passthrough" as a provider name and failed inside the gateway
before ever reaching OpenAI. Two tests now upload a file and list batches
through that prefix and assert OpenAI's own objects come back.
Streamed /openai_passthrough/v1/responses and /openai_passthrough/v1/embeddings
are relayed to OpenAI but still have to be costed, since the customer budgets
against this traffic. Both used to land a row the gateway could not use: the
streamed responses call logged a zero-cost row under a random id, and
embeddings wrote no row at all. Each test now reconciles the logged spend and
token counts against the response the caller was actually served.
GET /v1/files narrowed its data to the caller's own rows but left first_id and
last_id addressing the shared provider account's page, handing any caller raw
provider file ids belonging to other tenants. The new test asserts both cursors
address rows in the page the caller can see.
ResourceManager.defer now accepts any callable rather than one returning None,
so a delete that answers with a response model can be deferred as-is.
Greptile SummaryAdds tests-only end-to-end coverage for previously shipped OpenAI passthrough routing, spend logging, and file-list isolation behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| tests/e2e/llm_translation/test_passthrough_e2e.py | Adds live passthrough routing and spend-accounting tests without an eligible follow-up defect. |
| tests/e2e/llm_translation/passthrough_client.py | Adds typed client helpers and response models for OpenAI passthrough files, batches, Responses streams, and embeddings. |
| tests/e2e/batches/test_batches_e2e.py | Adds assertions that file-list cursors and pagination metadata agree with the caller-visible page. |
| tests/e2e/batches/batch_client.py | Extends the file-list response model with optional pagination fields. |
| tests/e2e/lifecycle.py | Broadens deferred cleanup callback return typing while teardown continues to discard callback results. |
| tests/e2e/coverage_registry/llm_conversational.yaml | Registers streamed OpenAI Responses passthrough cost coverage. |
| tests/e2e/coverage_registry/llm_nonconversational.yaml | Registers passthrough embeddings, batches, files, and file-list isolation coverage. |
Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/lit..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8b7c801. Configure here.
…tring The passthrough tests and their coverage registry rows pointed at the internal ticket id, which does not resolve for anyone following a link from status.litellm.ai. Each test docstring and registry rationale now names the GitHub issue it pins: #36086 for the two prefix routing cases, #36087 for the file list cursors, #36523 for streamed Responses cost, and #36646 for embeddings spend.
…itellm_lit_5870_passthrough_e2e_pins_wt
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d924226. Configure here.
|
Good coverage — these pin four real shipped fixes (tenant file-list isolation leaking other callers' raw provider file ids per #36087, the Holding off approval though: |
tin-berri
left a comment
There was a problem hiding this comment.
Approving per Tin — solid e2e coverage pinning four real shipped fixes (tenant file-list isolation per #36087, passthrough prefix routing per #36086, streamed-responses/embeddings cost logging per #36523/#36646), test-only. Note: buildkite/e2e-tests was still showing failing after a real 28-minute run as of my last check and I wasn't able to see which assertion failed — worth a follow-up look given this PR is specifically about tenant-isolation and billing correctness.
TLDR
Problem this solves:
/openai_passthroughprefix could silently start 500ing againHow it solves it:
User Flow
Before: a customer fronting OpenAI's own file, batch, and inference APIs through the gateway could not reach the file and batch routes at all, was handed other tenants' file IDs while listing their own, and was billed by OpenAI for inference the gateway never counted
500 {"error":{"message":"'openai_passthrough' is not a valid LlmProviders"}}, so the request dies inside the gateway and never reaches OpenAIdataarray correctly holds only their own files, butfirst_idandlast_idare OpenAI file IDs likefile-3LBv6nhR1frSAFNKYtc13Uthat belong to other tenants on the same provider account, andhas_moresays there is another page"stream": true, read back a completed response with"id": "resp_0f6eb485..."and 22 input plus 33 output tokens, then look it up at GET https://litellm-domain/spend/logs?request_id=resp_0f6eb485... and find nothing; the only nearby row is keyedchatcmpl-<random uuid>and reads $0Because the cursors in step 3 are real provider file IDs, any caller could take one and use it directly on the file routes to address a file another tenant uploaded
After: the same journey works end to end, the list page only ever names files the caller can see, and every relayed call is billed
200with OpenAI's own file object:"object": "file","purpose": "batch", and a"bytes"count matching what was uploaded200with OpenAI's own page:"object": "list"and their batches indatafirst_idandlast_idaddressing rows inside their owndata, bothnullwhen they own no files yet, and"has_more": falseA caller can no longer obtain another tenant's provider file ID from their own list page, so there is nothing to replay against the file routes
Relevant issues
Pins the fixes for #36086, #36087, #36523, and #36646
Linear ticket
Resolves LIT-5870
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
This PR adds tests only, so no product code differs between the two sides. Before is the merge base showing these behaviors have no coverage; After is the live run at the tip proving the responses and spend rows those tests assert are the ones a real proxy produces
Shared setup: a proxy from this branch on
http://localhost:33320against a local Postgres, two deployments registered from config (gpt-5.5andtext-embedding-3-small, bothopenai/), a virtual key fromPOST /key/generate, andPROXY_BATCH_WRITE_AT=3so spend rows land inside the poll window. No config flags beyond that; every route below is reachable on a stock proxy. All traffic hits real OpenAIBefore (02e67cd, the current merge base)
git show 02e67cd71595:tests/e2e/llm_translation/test_passthrough_e2e.py | grep -c OpenAIPassthrough->0, so nothing covers the dedicated prefix or the cost of what it relaysgit show 02e67cd71595:tests/e2e/batches/test_batches_e2e.py | grep -c test_list_page_cursors->0, so nothing covers the file-list cursors eithertests/e2e/coverage_registry/*.yamlat that commit for the five ids added here returns0for every one, so a regression in any of these ships with a green suiteAfter (8b7c801, still the commit the live run was driven from)
Case 1: the dedicated prefix reaches OpenAI's file API (#36086)
curl -s -X POST $P/openai_passthrough/v1/files -H "Authorization: Bearer $KEY" -F purpose=batch -F file=@qa-batch.jsonl->200{"object":"file","id":"file-PHRLJeXcFPBySUUA3uNHyy","purpose":"batch","filename":"qa-batch.jsonl","bytes":28,"created_at":1787219767,"expires_at":1789811767,"status":"processed"}curl -s -X DELETE $P/openai_passthrough/v1/files/file-PHRLJeXcFPBySUUA3uNHyy -H "Authorization: Bearer $KEY"->200{"object":"file","deleted":true,"id":"file-PHRLJeXcFPBySUUA3uNHyy"}, so the round trip cleans up after itselfCase 2: the dedicated prefix reaches OpenAI's batch API (#36086)
curl -s $P/openai_passthrough/v1/batches -H "Authorization: Bearer $KEY"->200{"object":"list","data":[{"id":"batch_6a86cb2412f481909c667f345e202adf","object":"batch","endpoint":"/v1/chat/completions","model":"gpt-4o-mini-2024-07-18","input_file_id":"file-3LBv6nhR1frSAFNKYtc13U","status":"cancelled"}]}, OpenAI's own page relayed verbatimCase 3: the file list's cursors name only the caller's own files (#36087)
curl -s -X POST $P/key/generate -H "Authorization: Bearer $MASTER" -d '{"user_id":"e2e-qa-lit5870-isolated"}'mints a key that owns nothingcurl -s $P/v1/files -H "Authorization: Bearer $KEY"->200{"data":[],"has_more":false,"object":"list","first_id":null,"last_id":null}. The provider account demonstrably holds files belonging to others, as Case 2's page showsfile-3LBv6nhR1frSAFNKYtc13U; before the fix those upstream IDs came back as this caller'sfirst_idandlast_idnext to an emptydata, which is exactly the leak. A fresh key is the strongest case becausedatais empty, so any non-null cursor here can only have come from someone elseCase 4: a streamed passthrough Responses call is billed under the ID the caller was served (#36523)
curl -sN -X POST $P/openai_passthrough/v1/responses -H "Authorization: Bearer $KEY" -d '{"model":"gpt-5.5","input":"Say hi in one word. e2e-qa-lit5870-c18","stream":true}'->200, 11 SSE events,x-litellm-call-id: bb105500-99cb-4db4-aac6-48d15b6b7e02, and theresponse.completedframe carries"id": "resp_0f6eb485f57e5868006a86cfa4f94087d094b03894149004f2"with"usage": {"input_tokens":22,"output_tokens":33,"output_tokens_details":{"reasoning_tokens":26},"total_tokens":55}curl -s "$P/spend/logs?request_id=resp_0f6eb485f57e5868006a86cfa4f94087d094b03894149004f2" -H "Authorization: Bearer $MASTER"->{"request_id":"resp_0f6eb485...","model":"gpt-5.5","spend":0.0011,"prompt_tokens":22,"completion_tokens":33,"call_type":"pass_through_endpoint","custom_llm_provider":"openai","status":"success"}. The row is keyed by the provider ID the caller actually read, and its 22 and 33 match the response exactlyCase 5: passthrough embeddings write a priced spend row (#36646)
curl -s -X POST $P/openai_passthrough/v1/embeddings -H "Authorization: Bearer $KEY" -d '{"model":"text-embedding-3-small","input":"cost this sentence e2e-qa-lit5870-c20"}'->200,x-litellm-call-id: b1eee2e9-a4f2-4525-86df-c30a0c9f7b9d,object=list,usage={"prompt_tokens":14,"total_tokens":14}, 1536 dimensionscurl -s "$P/spend/logs?request_id=b1eee2e9-a4f2-4525-86df-c30a0c9f7b9d" -H "Authorization: Bearer $MASTER"->{"request_id":"b1eee2e9-a4f2-4525-86df-c30a0c9f7b9d","model":"text-embedding-3-small","spend":2.8e-07,"prompt_tokens":14,"completion_tokens":0,"call_type":"pass_through_endpoint","custom_llm_provider":"openai","status":"success"}. The tests assertspend > 0rather than mere row existence, because an unmapped model logs a row at0.0All five tests also ran green against this same proxy at this commit:
4 passed in 13.94sfor the passthrough file and1 passed in 3.10sfor the file-list testType
✅ Test
Caveats (if any)
e2e_configEach test is proven by running green against a live proxy, real OpenAI, and a real database, not by reverting the four product fixes and watching it go red. Reverting them means unpicking four merged PRs on a base that has moved a long way since, so the evidence here is the assertions themselves plus the pasted responses they read. The sharpest of the four is the file-list case: the batch page in Case 2 shows the provider account really does hold
file-3LBv6nhR1frSAFNKYtc13U, which belongs to another caller, so a null cursor next to an emptydatais a narrowing the gateway had to do rather than an account that happened to be emptyThe list-page test mints a key on a fresh
user_id, sodatais empty and the cursors have to be null. That is on purpose: before the fix an emptydatastill came back with the upstream org'sfirst_idandlast_id. It does meanassert listed.has_more is not Trueis unconditional: it reads as "the proxy never forwards a cursor upstream" rather than "this page's cursors are consistent", so whoever implements real pagination pass-through will have to revisit that lineEMBEDDING_MODELsits intest_passthrough_e2e.pyas a literal, while its siblingCHEAP_OPENAI_MODELcomes frome2e_configwith anE2E_CHEAP_OPENAI_MODELoverride. Nothing breaks today, since the passthrough route relays the model name to OpenAI verbatim andtext-embedding-3-smallis what OpenAI calls it, but an environment that needs a different embeddings model has no way to say soThree items in the ticket's scope table have no new test here, for three different reasons:
#35551 (raw provider file IDs bypassed ownership checks) needs
litellm_settings.require_managed_files: true. That is a boot-time setting with no per-request, per-key, or management-route override, and the shared e2e stack does not run with it. Turning it on is a change to that stack's configuration, which lives outside this repo, so it has to be sequenced ahead of the test rather than requested by it#36151 (WebSocket passthrough not registered) is a websocket route, and the shared transport has no websocket seam. Worse, the fixed path closes with 1008 and 1011, which an HTTP client reads as a plain
403, the same status the unregistered route returned before the fix, so a test written over HTTP would pass identically on both sides and prove nothingThe over-correction guard on
/openai/v1/files, which must still reach the managed-file route rather than being swallowed by the new prefix, is already covered and needs nothing new.tests/e2e/batches/test_batches_e2e.py::test_batch_key_model_access_denieduploads through that exact route and unwraps the result, so it goes red the moment the route stops answering. A separate test for it was drafted and then dropped as a duplicate; its registry rowllm.files.openai.upload.nonstream.workswas already claimed by the batches suite, so no coverage was lost. Worth noting for anyone reproducing locally: that route answers500 {"error":{"message":"files_settings is not set, set it on your config.yaml file."}}unless your own config declaresfiles_settings, which the shared stack does and a minimal local config does notQA runbook
tests/e2e/llm_translation/test_passthrough_e2e.py::TestOpenAIPassthroughPrefix::test_passthrough_prefix_uploads_a_file_to_openai - the dedicated prefix relays a file upload to OpenAI instead of 500ing on a bad provider name
curl -X POST http://localhost:4000/key/generate -H "Authorization: Bearer sk-1234" -d '{}'curl -X POST http://localhost:4000/openai_passthrough/v1/files -H "Authorization: Bearer $KEY" -F purpose=batch -F file=@some.jsonl"object": "file","purpose": "batch", and"bytes"equal to the file's size. Assert onobject, not thefile-prefix: withgeneral_settings.passthrough_managed_object_ids: truethe ID is rewritten tolitellm_proxy:...curl -X DELETE http://localhost:4000/openai_passthrough/v1/files/$FILE_ID -H "Authorization: Bearer $KEY"tests/e2e/llm_translation/test_passthrough_e2e.py::TestOpenAIPassthroughPrefix::test_passthrough_prefix_lists_batches_from_openai - the dedicated prefix relays a batch listing to OpenAI
curl http://localhost:4000/openai_passthrough/v1/batches -H "Authorization: Bearer $KEY""object": "list". A body that is not an OpenAI list fails validation rather than passing vacuouslytests/e2e/batches/test_batches_e2e.py::TestOpenAIFiles::test_list_page_cursors_address_only_the_callers_own_files - a list page's cursors address rows in that page, never another tenant's files
user_idso it owns no filescurl http://localhost:4000/v1/files -H "Authorization: Bearer $KEY"first_idandlast_idto equal the first and last IDs indata, both null whendatais empty, andhas_morenot trueinput_file_idvalues that belong to other callerstests/e2e/llm_translation/test_passthrough_e2e.py::TestOpenAIPassthroughSpend::test_streamed_responses_call_logs_its_cost - a streamed passthrough Responses call is billed, keyed by the provider ID the caller read
curl -N -X POST http://localhost:4000/openai_passthrough/v1/responses -H "Authorization: Bearer $KEY" -d '{"model":"gpt-5.5","input":"Say hi in one word.","stream":true}'idandusageoff theresponse.completedSSE frame, not thex-litellm-call-idheader: this route never stamps the gateway's call ID onto the response, so polling by it finds nothingcurl "http://localhost:4000/spend/logs?request_id=$RESP_ID" -H "Authorization: Bearer sk-1234", retrying for up to two minutes since rows are written on a batch intervalspend > 0,call_typepass_through_endpoint, and prompt and completion tokens equal to the frame'sinput_tokensandoutput_tokenstests/e2e/llm_translation/test_passthrough_e2e.py::TestOpenAIPassthroughSpend::test_embeddings_call_logs_its_cost - a passthrough embeddings call is billed rather than writing no row at all
curl -X POST http://localhost:4000/openai_passthrough/v1/embeddings -H "Authorization: Bearer $KEY" -d '{"model":"text-embedding-3-small","input":"cost this sentence"}'curl "http://localhost:4000/spend/logs?request_id=$CALL_ID" -H "Authorization: Bearer sk-1234"with thex-litellm-call-idheader value. Here the call ID does work, because an embedding response carries no ID of its ownspend > 0andprompt_tokens > 0. Asserting only that a row exists is not enough: an unmapped model logs one at0.0Prerequisites for all five:
OPENAI_API_KEYin the environment, a Postgres-backed proxy, and a shortPROXY_BATCH_WRITE_ATif you do not want to wait the default ten seconds for spend rowsCI note, so the red gate is not mistaken for this diff: all three full-suite runs at this tip came back
3 failed, 643 passed, 58 skipped, and the three failures aretest_unflagged_model_converts_system_reminder_and_succeedson Bedrock Invoke, Vertex, and Azure Foundry, each stopping in the same place withprompt cache never became readable in full within 60.0s. Those cases live intest_messages_mid_conversation_system_e2e.pyandtest_messages_mid_conversation_system_native_providers_e2e.py, and this diff touches neither file. Both were rewritten on the base earlier today by 155ef8c and e11399f, and build 101 is the first full-suite run whose base carries that rewrite: the last run on a base without it passed the older version of the same three cases. All five tests added here passed in every run, and the staging breakage is tracked as LIT-5921. Separately,buildkite/e2e-testsis not one oflitellm_internal_staging's required checks, so the red X does not block mergingFinal Attestation
CHECKED: live-pr-risk at d924226. Two of the seven files are registry YAML and two are new cases appended to existing test files, which leaves three touching harness surface.
tests/e2e/lifecycle.pywidensResourceManager.deferfromCallable[[], None]toCallable[[], object], which is a contravariant widening, so all 289 pre-existingdefer(...)call sites still typecheck unchanged (290 on the head, the one new one being this PR's own), and the sole reader is the teardown loop that calls each entry and discards what it returns either way.tests/e2e/batches/batch_client.pyaddsfirst_id,last_id, andhas_moretoFileList, all optional and defaulting to None;FileListis referenced nowhere outside that file, andmodels.py's similarly namedFileListResponsebehindProxyClient.list_filesis a different model this diff never touches.passthrough_client.pyis additive only, with no existing method re-signed. The full e2e suite collects clean on the head (700 of 702, two deselected, 0 errors), the coverage collector runs--strictwithout a failure, and basedpyright overtests/e2ereports 0 errors. The five cases were driven live against a real proxy, database, and OpenAI at 8b7c801; the two commits since are a docstring-only edit and a merge of the base, and the live evidence at this tip is the full suite CI runs below3 failed, 643 passed, 58 skippedwith the same three mid-conversation-system cases as the only failures. Each run stamps a different priming marker, so those three reproduce rather than flake. Builds 101, 103, 104Note
Low Risk
Tests-only; no product code. Risk is limited to e2e harness typing (
deferreturn type) and live OpenAI traffic in CI.Overview
Adds live e2e pins for four already-shipped gateway bugs:
/openai_passthroughmust actually reach OpenAI (not bind as a provider name), streamed Responses and embeddings passthrough must write a priced spend row, andGET /v1/filescursors must name only the caller's files.Passthrough prefix (#36086). New cases upload a file and list batches under
/openai_passthrough/v1/...and assert OpenAI's own objects come back, so a regression that 500s on'openai_passthrough' is not a valid LlmProvidersfails the suite.Passthrough spend (#36523, #36646). Streamed Responses spend is polled by the
response.completedid and token counts must match that frame. Embeddings must logspend > 0and prompt tokens, not a missing or $0 row.File list isolation (#36087).
FileListnow modelsfirst_id/last_id/has_more. A fresh key's list page must have cursors that match its owndata(null when empty) and must not advertise another page.Coverage registry rows are added for each cell.
ResourceManager.defernow accepts cleanups that return a value so delete helpers can be registered directly.Reviewed by Cursor Bugbot for commit d924226. Bugbot is set up for automated code reviews on this repo. Configure here.