Skip to content

fix(files): return hook-filtered page from GET /v1/files (LIT-4850) - #35424

Closed
devin-ai-integration[bot] wants to merge 2 commits into
litellm_internal_stagingfrom
devin_ai_fix_list_files_peruser_filtering_lit4850
Closed

fix(files): return hook-filtered page from GET /v1/files (LIT-4850)#35424
devin-ai-integration[bot] wants to merge 2 commits into
litellm_internal_stagingfrom
devin_ai_fix_list_files_peruser_filtering_lit4850

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • list_files dropped a filtered file page returned by the managed-files hook

How it solves it:

  • Accept the hook's AsyncCursorPage in the list_files post-call guard

Relevant issues

Fixes #28294

Linear ticket

Resolves LIT-4850

Pre-Submission checklist

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review

Screenshots / Proof of Fix

Verified against a real proxy on http://localhost:4000 running the managed-files hook with Postgres, routing to a live OpenAI account (real api.openai.com uploads, real managed-file rows), at commit 6ab5ab68dd. Two non-admin internal_user users (user-a, user-b) each uploaded one managed batch file, then GET /v1/files was exercised as each user and as admin, before and after the guard change on the same worktree.

Post-fix, per-user filtering is correct: user-a sees only their file, user-b only theirs, and the admin master key sees both (confirming both files exist provider-side).

# upload as user-a (form field target_model_names, a query param is ignored on upload)
curl -s http://localhost:4000/v1/files -H "Authorization: Bearer $KEY_A" \
  -F purpose=batch -F target_model_names=gpt-4o -F file=@a.jsonl     # -> id FILE_A
curl -s http://localhost:4000/v1/files -H "Authorization: Bearer $KEY_B" \
  -F purpose=batch -F target_model_names=gpt-4o -F file=@b.jsonl     # -> id FILE_B

curl -s http://localhost:4000/v1/files -H "Authorization: Bearer $KEY_A"  # -> only FILE_A
curl -s http://localhost:4000/v1/files -H "Authorization: Bearer $KEY_B"  # -> only FILE_B
curl -s http://localhost:4000/v1/files -H "Authorization: Bearer sk-1234" # -> FILE_A + FILE_B

Honest caveat on the reproduction: reverting the guard to the pre-fix isinstance(_response, OpenAIFileObject) form and restarting produced the exact same result, user-a still saw only their own file with no leak. At this HEAD the managed-files hook filters by mutating response.data in place and returning the same object, so post_call_success_hook hands the endpoint back an already-filtered object and the guard's reassignment is redundant. Every filtered response also came back with unified litellm_proxy:... ids rather than raw provider file-... ids, which further shows the filtered object is what is returned in both variants. So this change is a defensive correctness fix that makes the endpoint honor a hook returning a filtered AsyncCursorPage as a new object (a future hook refactor, or any custom hook that returns rather than mutates); it is behavior-neutral at the current HEAD rather than a live before/after leak fix.

# Check Result
1 user-a and user-b uploads create distinct managed files FILE_A != FILE_B, both 200
2 (fixed) user-a GET /v1/files returns only FILE_A pass
3 (fixed) user-b GET /v1/files returns only FILE_B pass
4 control: admin GET /v1/files returns both pass
5 (pre-fix guard) user-a GET /v1/files expected to leak FILE_B no leak; identical to fixed
6 source restored to committed fixed form clean git status

Type

Bug Fix

Changes

list_files runs the managed-files post_call_success_hook, which for list responses filters to the caller's own files and returns an AsyncCursorPage (enterprise/litellm_enterprise/proxy/hooks/managed_files.py). The endpoint then narrowed the hook return with isinstance(_response, OpenAIFileObject), so a filtered AsyncCursorPage returned as a distinct object would be dropped. The guard now also accepts AsyncCursorPage, matching the list-filtering return type and the parallel handling in create_file (which stays narrow on purpose since its hook branch returns a single OpenAIFileObject).

As the QA above documents, the shipped hook filters by in-place mutation today, so this is a defensive change rather than a fix for an observable runtime leak.

QA runbook

  • tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py::test_list_files_returns_hook_filtered_page_not_unfiltered_provider_response - the list_files endpoint returns the hook's filtered page rather than the provider's unfiltered page when the hook returns a distinct filtered object
    • Bring up the proxy with the managed-files hook and Postgres, and an OpenAI deployment gpt-4o
    • As two non-admin internal_user keys, upload one managed batch file each: curl -s http://localhost:4000/v1/files -H "Authorization: Bearer $KEY" -F purpose=batch -F target_model_names=gpt-4o -F file=@f.jsonl
    • GET /v1/files as each key and expect each to return only its own file id, and the master key to return both
    • Sanity check: this test drives the real endpoint and asserts the exact filtered id set; note it mocks the hook to return a distinct filtered page, whereas the shipped hook mutates in place, so it exercises the guard line rather than the live mutation flow

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

The list_files endpoint narrowed the managed-files post_call_success_hook
return to OpenAIFileObject. For list responses that hook returns an
AsyncCursorPage filtered to the caller's own files, so the isinstance guard
rejected it and the unfiltered provider response went back to the caller,
making per-user file filtering a silent no-op.

Widen the guard to also accept AsyncCursorPage so the filtered page is
returned. Add a regression asserting the endpoint returns the hook's
filtered page and another user's file is absent.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@greptile-apps

greptile-apps Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR updates the files-list endpoint to honor a page returned by its post-call hook.

  • Accepts AsyncCursorPage responses alongside individual file objects.
  • Adds an endpoint regression test confirming that the hook-processed page is returned.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
litellm/proxy/openai_files_endpoints/files_endpoints.py Broadens the accepted post-call hook response type so the processed files page reaches the client.
tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py Adds a regression test that distinguishes the provider page from the page returned by the hook.

Reviews (2): Last reviewed commit: "style(files): collapse list_files guard ..." | Re-trigger Greptile

@codecov

codecov Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@codspeed-hq

codspeed-hq Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing devin_ai_fix_list_files_peruser_filtering_lit4850 (6ab5ab6) with litellm_internal_staging (fa56283)1

Open in CodSpeed

Footnotes

  1. No successful run was found on litellm_internal_staging (b5cfc2c) during the generation of this report, so fa56283 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@mateo-berri

Copy link
Copy Markdown
Contributor

@greptileai

@mateo-berri

Copy link
Copy Markdown
Contributor

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 6ab5ab6. Configure here.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Closing this. A live e2e (real proxy + real OpenAI, two non-admin users) showed the per-user filtering leak this targeted is not reachable at HEAD: the managed-files hook filters by mutating response.data in place and returns the same object, so the endpoint already holds the filtered result and the isinstance guard reassignment is redundant. The check is technically incorrect but latent, and #28294's actual user-facing symptom was fixed by #28339. Not worth a standalone hardening PR right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] GET /v1/files returns raw provider IDs for batch output files — wrong created_by in CheckBatchCost

2 participants