fix(proxy): sort BYOK models by their displayed name in /v2/model/info#28079
Conversation
Team BYOK rows persist an internal `model_name` like
`model_name_{team_id}_{uuid}` and expose the user-facing name via
`model_info.team_public_model_name`. The UI's `getDisplayModelName`
and the search filter already fall back to that field, but
`_sort_models` was keying off the raw `model_name` — so BYOK rows
ranked by their opaque IDs and clumped at the end of the alphabetized
list instead of interleaving with non-BYOK rows.
Match the UI/search behavior: prefer `team_public_model_name` when
present, fall back to `model_name` otherwise.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR fixes several
Confidence Score: 5/5Safe to merge; the security fixes (teamId authorization, BYOK search scoping, unbounded-fetch cap) are correct and well-tested with no regressions introduced. All five new auth/scoping paths are covered by dedicated mock tests, the prisma_client None guard at the top of model_info_v2 ensures the function always has a live DB handle before reaching the new checks, and the sorting and search changes are straightforward key-function substitutions. The only notable gap is a redundant user-row lookup when both search and teamId are supplied together, which costs an extra DB round-trip but does not affect correctness. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/proxy_server.py | Adds BYOK search scoping, teamId authorization, display-name sorting, and a DoS cap on sorted DB fetches; logic is correct but non-admin requests with both search and teamId trigger two redundant DB lookups for the same user row. |
| tests/test_litellm/proxy/test_proxy_server.py | Adds thorough mock-only tests covering search-by-public-name, BYOK team scoping (router + DB side), DB-fetch pagination cap, team-filter direct_access removal, teamId 403 for non-members, and PROXY_ADMIN_VIEW_ONLY unscoped behavior. |
Reviews (6): Last reviewed commit: "style: apply black formatting to _fetch_..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
`_apply_search_filter_to_models` used Prisma's JSON path `string_contains` to match the BYOK `team_public_model_name` field, but that operator is case-sensitive in Postgres (no `mode: insensitive` flag like column-level string filters have). So a search for "claude" missed a stored "Claude Sonnet" via the DB branch even though the router-side path matched it case-insensitively. Widen the JSON branch to "row has a team_public_model_name set" and filter case-insensitively in Python so DB-only BYOK rows match the same terms users see in the UI. This also drops the now-unused DB-level page-size optimization and `sort_by` knob — the in-Python filter is the source of truth for `db_models_total_count` now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review again with new commit that resolves the p1 issue |
PR overview/v2 model info BYOK sorting and scopingThis PR updates model search/sort behavior for BYOK display names and adds team-aware filtering for Security review
Risk: 2/10 |
`_apply_search_filter_to_models` was widened to fetch every row with a `team_public_model_name` set so case-insensitive search could match mixed-case stored names. `/v2/model/info` is reachable by non-admin keys though, and the helper ran before `include_team_models` / `teamId` filtering — so a non-admin caller could search a common substring like "claude" and see BYOK rows belonging to teams they're not a member of. Resolve the caller's team membership once (admin → no scoping, else their `user_row.teams`) and drop BYOK rows (those with `model_info.team_id` set) outside that scope on both the router-side matches and the over-broad DB query, before display-name matching. Non-team rows are unaffected and remain gated by the existing `include_team_models` / `direct_access` paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review again with new commit that resolves veria's comment |
- /v2/model/info search now matches both `model_name` and
`model_info.team_public_model_name`, so team BYOK rows (which persist
an internal `model_name_{team_id}_{uuid}`) are findable by the public
name shown in the UI. DB query OR-includes a JSON-path match on
`team_public_model_name` for rows that exist only in the DB.
- `_filter_models_by_team_id` no longer short-circuits on the viewer's
`direct_access` flag — that describes the admin viewer's own
permissions and would leak every public model into a team-scoped view.
Models are kept only when they belong to the team (own BYOK, in
access_via_team_ids, or reachable via team.models / access groups).
- Added `_authorize_team_id_query`: the untrusted `teamId` query
parameter now requires the caller to be a proxy admin or a member of
the requested team, otherwise returns 403. Without this, any
authenticated user could enumerate another team's BYOK metadata by
guessing the team id.
- `_get_caller_byok_team_scope` now treats `PROXY_ADMIN_VIEW_ONLY` the
same as `PROXY_ADMIN` (both are admin roles); previously VIEW_ONLY
admins fell through to a user-id team lookup and saw only their own
teams' BYOK rows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review again with new commit that resolves raised issues |
Previously the DB-side search OR'd a JSON-path predicate
`{model_info: {path: [team_public_model_name], string_contains: ""}}`
to compensate for Prisma's case-sensitive JSON `string_contains` on
Postgres. That predicate matches every row that has any
`team_public_model_name` set, so any authenticated caller could force a
full BYOK-table read with `/v2/model/info?search=x` regardless of page
size.
Drop the JSON-path branch. The DB query now does a bounded
`model_name contains <search>` lookup. BYOK rows that are loaded into
the router are still searchable by their `team_public_model_name` via
the router-side filter; only the rare edge case of a BYOK row that
exists only in the DB (router sync failed) loses display-name search,
which is an acceptable trade-off given the DoS surface.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review again with new commit |
The previous bounding patch dropped the page-aware `take=N` on `find_many`, so a broad `?search=model` would load and decrypt every matching DB row on each request even though the response only returns one page. Restore bounded fetches in `_apply_search_filter_to_models`: * Unsorted searches use `take = max(0, page * size - router_count)`, i.e. exactly one page worth of remaining DB rows. * Sorted searches need ordering across the full match set, so they cap at `_SORTED_SEARCH_DB_FETCH_CAP = 500` instead of fetching everything. * Total count comes from a cheap `count(...)` query so pagination stays accurate without materializing every row. Wired `page`, `size`, and `sortBy` through from the endpoint and added a regression test covering both `take` values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
_apply_search_filter_to_models tripped Ruff's "too many statements" (51 > 50) after the bounded-fetch fix. Move the DB-side block into `_fetch_db_models_for_search`, which keeps the same behavior: * Bounded `take` via page math (unsorted) or `_SORTED_SEARCH_DB_FETCH_CAP` (sorted) * Cheap `count(...)` for accurate pagination totals * Caller-team scope applied to fetched rows before decrypt Pure refactor; no behavior change. All 8 BYOK/team tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI's "Check Black formatting" step flagged one line in the helper added in d55eecf. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile re-review |
Fixes LIT-2860
Description
The Model Management page calls /v2/model/info to list models. Team BYOK rows were
broken in several ways: they didn't sort or search by the public name shown in the
UI, the current-team filter still leaked unrelated models the admin had direct
access to, and the teamId query parameter let any authenticated caller enumerate
another team's BYOK metadata.
This PR fixes those issues end-to-end, plus the DoS surface introduced by the
search-fix patch along the way.
Cause
Team BYOK rows persist an internal model_name of the form
model_name_{team_id}_{uuid} and store the user-facing name in
model_info.team_public_model_name. The endpoint logic only operated on model_name,
so:
instead of alphabetical-by-public-name.
user, so BYOK rows silently dropped out of search results.
model_info.direct_access, which the upstream admin path sets on every non-team model
— selecting a team in the UI still showed every public model the viewer could call.
with no caller authorization, so a guessable team id was enough to dump that team's
BYOK metadata.
for PROXY_ADMIN; the view-only admin role fell through to a user-id team lookup and
saw only their own teams' BYOK.
"" predicate on model_info.team_public_model_name (Prisma's JSON string_contains is
case-sensitive on Postgres, so the workaround widened to "row has any
team_public_model_name set"). Any authenticated caller could force a full BYOK-table
read with /v2/model/info?search=x.
Fix
litellm/proxy/proxy_server.py:
model_info.team_public_model_name router-side; the DB-side query is a single bounded
model_name contains lookup (the over-broad JSON-path branch is gone). BYOK
rows in the router (the typical case) stay searchable by public name.
order.
team is in access_via_team_ids, or the id is reachable via team.models / access
groups. The viewer's direct_access flag no longer widens the result.
of the requested team before applying the teamId filter; otherwise 403.
(both unscoped).
Tests added/updated in tests/test_litellm/proxy/test_proxy_server.py cover: search
by team_public_model_name, non-admin team scoping, teamId authorization (admin /
member / non-member), view-only-admin unscoped behavior, and the team-id filter not
leaking on direct_access.
https://www.loom.com/share/5b064149b4d747f6b0012f57b3752d1b