fix(proxy): enforce require_managed_files on every route that accepts a raw provider id - #35551
Conversation
…nd delete Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
|
Greptile SummaryThis PR adds opt-in managed-resource ownership validation before covered file, batch, fine-tuning, and vector-store-file operations reach provider routing
Confidence Score: 5/5The PR appears safe to merge The previously reported format-only acceptance path is closed by checking stored ownership before any managed ID is decoded or dispatched, and no blocking failure remains
|
| Filename | Overview |
|---|---|
| litellm/proxy/openai_files_endpoints/common_utils.py | Replaces format-only managed-ID acceptance with production ownership checks through the managed-files hook |
| litellm/proxy/openai_files_endpoints/files_endpoints.py | Validates caller-supplied file IDs before retrieval, content access, or deletion |
| litellm/proxy/batches_endpoints/endpoints.py | Validates batch input files and batch IDs before decoding, database lookup, credential resolution, or provider calls |
| litellm/proxy/fine_tuning_endpoints/endpoints.py | Validates training, validation, and fine-tuning job IDs before routing operations |
| litellm/proxy/vector_store_files_endpoints/endpoints.py | Centralizes managed file ownership validation in the resolver shared by all vector-store-file operations |
| tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py | Adds regression coverage for raw-ID rejection, owned managed IDs, and default-off behavior |
| tests/test_litellm/proxy/batches_endpoints/test_endpoints.py | Adds coverage that blocked batch and input-file IDs do not reach provider seams |
| tests/test_litellm/proxy/fine_tuning_endpoints/test_endpoints.py | Adds endpoint coverage for training, validation, and job ownership enforcement |
| tests/test_litellm/proxy/vector_store_files_endpoints/test_endpoints.py | Adds shared-resolver coverage for raw, missing, and inaccessible managed file IDs |
Reviews (3): Last reviewed commit: "fix(proxy): authenticate managed ids bef..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…_files_read_paths_35530
require_managed_files was only checked on upload, so raw provider ids still reached the batch, fine-tuning and vector store file routes. Ownership rows exist only for managed ids, so those requests were forwarded under shared credentials with no tenant check: knowing another tenant's id was enough to read, run against, cancel or delete their object. Generalise the file-id guard to validate_managed_id_requirement(resource_id, resource_kind) and call it on batch create/retrieve/cancel, fine-tuning create/retrieve/cancel (training_file and validation_file both) and the shared vector store file id resolver. Behaviour is unchanged when the setting is off.
|
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 8c0556a. Configure here.
TLDR
Problem this solves:
require_managed_filesonly guardedPOST /v1/filesHow it solves it:
User Flow
Before: team A opts into managed files, and another team can still use raw or forged ids to read, run against, cancel and destroy its objects
require_managed_files: trueinlitellm_settingsand restarts the proxyPOST /v1/fileswith a JSONL batch input and gets back a long scrambled gateway id,bGl0ZWxsbV9wcm94eTphcHBsaWNhdGlvbi9vY3RldC1zdHJlYW07dW5pZmll...file-MQfgEyYAXEtBzAyFxyEXg5. The same raw id is also visible in provider dashboards, ininput_file_idon any batch object, and in support ticketsPOST /v1/batcheswith the gateway id and gets a running batch backGET /v1/files/file-MQfgEyYAXEtBzAyFxyEXg5and gets200with team A's file metadataGET /v1/files/file-MQfgEyYAXEtBzAyFxyEXg5/contentand gets200with the file body in full, including the lineteam A confidential payroll row: alice 250000POST /v1/batcheswith"input_file_id": "file-MQfgEyYAXEtBzAyFxyEXg5"and gets200and a real batch id, a job now running on team A's data and billed to the shared accountGET /v1/batches/batch_6a7706968acc8190aec13a7768d55acafor a batch it does not own and gets200with its statusPOST /v1/fine_tuning/jobswith the same raw file astraining_file, andPOST /v1/fine_tuning/jobs/ftjob-abc123raw/cancel. Both come back with OpenAI's own error text, so the gateway relayed the raw ids upstream under the shared account instead of refusing themPOST /v1/vector_stores/<its own vector store id>/files. It gets200, so team A's file is attached to team B's vector storeGET /v1/files/<team A's gateway file id>andGET /v1/batches/<team A's gateway batch id>and gets403on both, so the only ids the gateway checks are the ones that look like its ownPOST /v1/batches/batch_6a7706968acc8190aec13a7768d55aca/canceland the status flips tocancellingDELETE /v1/files/file-MQfgEyYAXEtBzAyFxyEXg5and gets{"deleted": true}GET /v1/files/<its own gateway file id>/contentand now gets404 No such File object. Its data is gone, destroyed by a tenant it cannot see and has no way to identifyAfter: raw and forged ids are refused at the gateway, and team A's objects are untouched
require_managed_files: trueinlitellm_settingsand restarts the proxyPOST /v1/fileswith a JSONL batch input and gets back a long scrambled gateway id,bGl0ZWxsbV9wcm94eTphcHBsaWNhdGlvbi9vY3RldC1zdHJlYW07dW5pZmll...file-9szcvDSFsQpjhZUAFESRVgPOST /v1/batcheswith the gateway id and gets a running batch backGET /v1/files/file-9szcvDSFsQpjhZUAFESRVgand gets400 Raw provider file ids cannot be used when require_managed_files is enabled in litellm_settings. Use the LiteLLM managed file id returned when the file was created.GET /v1/files/file-9szcvDSFsQpjhZUAFESRVg/contentand gets the same400. No file body is returnedPOST /v1/batcheswith"input_file_id": "file-9szcvDSFsQpjhZUAFESRVg"and gets the same400. No batch is created and nothing is billedGET /v1/batches/batch_6a7709441a6c8190b8387188755ad369and gets400 Raw provider batch ids cannot be used when require_managed_files is enabled in litellm_settings...400 Raw provider file ids cannot be used...and400 Raw provider fine-tuning job ids cannot be used.... Neither reaches OpenAI, so no upstream error text comes backPOST /v1/vector_stores/<its own vector store id>/filesand gets404 File not found. The file is not attached403 The caller does not have access to this managed file id.and403 The caller does not have access to this managed batch id., now answered before the gateway picks credentials or contacts a providerPOST /v1/batches/batch_6a7709441a6c8190b8387188755ad369/canceland gets the same batch400. The batch keeps runningDELETE /v1/files/file-9szcvDSFsQpjhZUAFESRVgand gets the same file400. Nothing is deletedGET /v1/files/<its own gateway file id>/contentand gets200with the payroll line intact, andGET /v1/batches/<its own gateway batch id>still returns its batch. Team B can no longer read, attach, run against, cancel or delete anything belonging to team ARelevant issues
Fixes #35530
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Two teams on one proxy, one shared OpenAI account, hitting the real
api.openai.com(real files, real batch jobs, realspend). Team B replays team A's raw provider ids, a forged gateway-shaped id, and team A's real gateway ids at every
route that takes a caller-supplied id, then team A re-reads its own objects.
Both legs run the same script and differ only in the commit the proxy booted from: the before leg on the merge base
dee667edb7, the after leg on this PR's head8c0556abf6. Each leg had its own worktree, its own venv, its ownpostgres database and its own port. Response bodies are truncated at 600 characters, and long gateway ids are shown as
the shell variables holding them.
dee667edb78c0556abf6GET /v1/files/{raw id}GET /v1/files/{raw id}/contentPOST /v1/batcheswith rawinput_file_idGET /v1/batches/{raw id}POST /v1/fine_tuning/jobswith raw filePOST /v1/fine_tuning/jobs/{raw id}/cancelPOST /v1/vector_stores/{id}/files, forged idGET /v1/files/{team A gateway id}GET /v1/batches/{team A gateway id}POST /v1/batches/{raw id}/cancelcancellingDELETE /v1/files/{raw id}deleted: trueConfig, identical for both runs:
Two tenants, one key each:
Team A's legitimate setup, plus one batch created straight on the shared OpenAI account so the proxy never minted it
(the realistic case for a shared key: objects the gateway has no record of), plus a vector store team B owns and a
forged id wrapping team A's raw file id:
team_a_secret.jsonlis one batch line whose prompt isteam A confidential payroll row: alice 250000Before, at
dee667edb7(merge base, port 53417)Steps 5 and 6 answer with OpenAI's own error text, which is the tell: the raw ids were forwarded upstream under the
shared account rather than refused. Any raw id belonging to a real fine-tunable model or a real job would have gone
through the same way. Step 7 shows the gateway trusts the shape of a managed id rather than verifying it, so a
hand-forged one wrapping team A's raw file id attaches that file to team B's vector store
After, at
8c0556abf6(this PR's head, port 53219)No provider call is made for probes 1 to 6, 10 and 11, and the gateway's own resolution of team A's ids in 12, 13 and 14
is unaffected. Probe 14 is the guard letting team A's own id through: the 400 is OpenAI's, reached over the wire, and it
is byte-identical to the before leg's
The same sweep through the OpenAI Python SDK
The curl legs above are the routes; this is the client a customer actually holds. Same proxies, same objects,
OpenAI(api_key=<team key>, base_url="<proxy>/v1"):Default off, at
8c0556abf6(same head commit, port 58907)The gate is opt-in, so nobody who has not set
require_managed_files: truesees any change. Same head commit, samesweep, one line of config different:
Normalizing away the things that must differ per run (file, batch and vector-store ids, UUIDs, base64 gateway ids, model
hashes, unix timestamps, and the leg label), the sweep log from this run is line-for-line identical to the before leg's.
Status vectors for all three runs, probes 1 through 14 in order:
dee667edb78c0556abf6, off8c0556abf6, onThe default-off sweep log contains zero occurrences of
Raw provider, zero ofcannot be used, and zero HTTP 500s, andthe proxy log carries no traceback other than the upstream 400s and 404s and the two pre-existing 403s that the before
leg also produced
No 500s from the ownership check
The new guard resolves the enterprise hook at request time, so a proxy where that hook is missing would answer
500 Managed resource ownership validation is unavailablerather than falling open. Across all three runs, 42 probes, theproxy logs contain no 500 of any kind. Every refusal is a deliberate 400, 403 or 404
Type
🐛 Bug Fix
Changes
require_managed_fileswas enforced in exactly one place, on upload. Every other route that takes a caller-suppliedprovider id accepted a raw one and forwarded it to the provider under whatever credentials the calling team resolves
to. Ownership is only ever recorded for LiteLLM managed ids, so a raw id carries no tenant to check against, and on a
provider account shared by several teams, knowing another tenant's id is enough to read, reuse or destroy the object
behind it.
validate_managed_file_id_requirementinopenai_files_endpoints/common_utils.pyis generalised tovalidate_managed_id_requirement, which handles"file" | "batch" | "fine-tuning job". The validator checks eachmanaged id against the caller's stored ownership record before any route decodes or forwards it. Raw and model-encoded
ids raise 400, a forged managed-looking id without an ownership row raises 404, and another team's real id raises 403
Call sites invoke the ownership-aware guard before decoding managed ids, resolving credentials, or routing to a
provider:
openai_files_endpoints/files_endpoints.py:get_file,get_file_content,delete_filebatches_endpoints/endpoints.py:input_file_idon create,batch_idon retrieve and cancelfine_tuning_endpoints/endpoints.py:training_fileandvalidation_fileon create,fine_tuning_job_idonretrieve and cancel
vector_store_files_endpoints/endpoints.py: once in the shared id resolver that all five file routes funnel through(create, retrieve, content, update, delete)
validation_filematters on its own: it is uploaded and readable exactly like the training file, so a managedtraining_filemust not be able to smuggle a rawvalidation_filepast the guard. There is a test for that case.Tests cover raw ids, forged managed-looking ids and another team's real managed ids. They also assert every downstream
provider seam stays at zero calls, so a guard that runs after routing still fails
Known gaps, out of scope here
Worth separate issues rather than widening this PR:
/openai/v1/files/{id}and friends) accept raw ids by design and never reach this guard/v1/responses(response_id,previous_response_id), evals, videos, skills, threads, assistants and interactionstake caller-supplied ids and are not covered by
require_managed_filesat all?model=orx-litellm-modelwithout a key model-accesscheck
adding a separate isolation switch, is a product decision worth taking on its own
training_fileis still forwarded to the provider verbatimrather than translated back to the raw id, so a fine-tuning create fails upstream with
invalid training_file.That predates this PR and is unchanged by it
Final Attestation
Note
High Risk
This is a security fix for cross-tenant IDOR on shared provider credentials; behavior changes when require_managed_files is true, and misconfigured ownership hooks return 500.
Overview
When
require_managed_filesis enabled,validate_managed_id_requirementnow runs on every proxy route that accepts a caller-supplied file, batch, or fine-tuning job id—not only on file upload. Raw provider ids and model-encoded ids return 400; LiteLLM unified ids are checked against stored ownership (403 if another tenant’s id).The guard is wired into files (retrieve, content, delete), batches (create
input_file_id, retrieve, cancel), fine-tuning (create training/validation files, retrieve, cancel), and vector store files (via an async shared resolver used by all five file routes). AManagedResourceAccessCheckerprotocol types the managed-files hook.CI adds fine_tuning and vector_store_files test paths; new unit tests assert rejection happens before any provider/router call.
Reviewed by Cursor Bugbot for commit 8c0556a. Bugbot is set up for automated code reviews on this repo. Configure here.