fix(proxy): gate team allowed_passthrough_routes to proxy admins#28097
Conversation
allowed_passthrough_routes short-circuits the role-based route gate, so the keys endpoints already restrict it to proxy admins. The team writers (/team/new, /team/update) had no equivalent check, letting an org admin (a non-proxy-admin who clears the route gate and _verify_team_access) self-grant pass-through routes on their team. Lift the keys check into a shared helper and apply it to both team endpoints. Resolves LIT-3019
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR closes a privilege-escalation path where an org admin could call
Confidence Score: 5/5Safe to merge; the change is additive permission enforcement with no observable impact on correctly-permissioned callers. The refactor is a clean extract-and-reuse: the helper logic is identical to the key-endpoint version, the default entity=key keeps all existing call sites behaviorally unchanged, and both new call sites in team_endpoints are placed correctly in the execution order (post-access-verify, pre-DB-write). The three new tests cover admin pass-through, both non-admin block paths, and the edge case that an empty list is not treated as a setting attempt. No existing tests were altered. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/common_utils.py | Adds shared _check_passthrough_routes_caller_permission helper (moved from key_management_endpoints.py) with a new entity keyword param (default "key") for context-aware error messages; covers both top-level and metadata-smuggled allowed_passthrough_routes. |
| litellm/proxy/management_endpoints/key_management_endpoints.py | Removes the now-duplicated local _check_passthrough_routes_caller_permission and imports it from common_utils; all five existing call sites are unchanged and retain the default entity="key" behaviour. |
| litellm/proxy/management_endpoints/team_endpoints.py | Inserts the permission check in new_team (after member auto-enrolment, before any DB write) and in update_team (after _verify_team_access, before the kv update); both sites block non-proxy-admins from setting allowed_passthrough_routes on a team. |
| tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py | Adds three new unit/integration tests: unit test covers admin-allow, non-admin-block (top-level + metadata), and empty-list pass-through; integration tests exercise the full new_team and update_team code paths with mocked DB/access checks. |
Reviews (2): Last reviewed commit: "docs(proxy): note view-only admins are i..." | Re-trigger Greptile
…ssthrough gate Clarifies the proxy-admin guard per review feedback; no behavior change. Refs LIT-3019
|
@greptileai re review |
Summary
allowed_passthrough_routesshort-circuits the role-based route gate inRouteChecks.non_proxy_admin_allowed_routes_check, so the keys endpoints already restrict it to proxy admins. The team writers (/team/new,/team/update) had no equivalent check. An org admin — a non-proxy-admin who clears the route gate (org_admin_allowed_routes) and_verify_team_access(as org admin of the team's org) — could therefore self-grant arbitrary pass-through routes on a team in their org, an access-control field the keys path deliberately reserves for proxy admins.This lifts the keys check into a shared
_check_passthrough_routes_caller_permissionhelper (parametrized by entity for the error message) and applies it to both team endpoints. Behavior is now identical for keys and teams; the keys path is unchanged (same helper, defaultentity="key", all existing call sites intact). Both the top-level field and themetadata.allowed_passthrough_routessmuggling path are covered.Threat framing: for org admins this closes a real privilege escalation. For plain team admins it is defense-in-depth — the route gate already default-denies them at
/team/*, but the gate becomes load-bearing under configs that widen route access (e.g. a key minted with explicitallowed_routes).Refs LIT-1901
Before / After
before
after
Test plan
tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py::test_check_passthrough_routes_caller_permission_team— proxy admin allowed; non-admin blocked (top-level + metadata-smuggled); empty/absent allowed so clearing stays open...::test_new_team_blocks_non_admin_passthrough_routes—/team/newrejects non-proxy-admin (wrappedProxyExceptioncode 403)...::test_update_team_blocks_non_admin_passthrough_routes—/team/updaterejects non-proxy-admin after_verify_team_accesspassestest_key_management_endpoints.py -k passthroughpasses (helper moved, call sites unchanged)Refs LIT-3019