fix(router): constrain same-name deployment routing by access groups - #26161
Conversation
Filter router candidate deployments by caller-authorized model access groups when access is granted via group membership, preventing cross-group load balancing for shared public model names. Made-with: Cursor
Add a router utils unit test that directly exercises _filter_deployments_by_model_access_groups for access-group-only key permissions. Made-with: Cursor
Greptile SummaryThis PR tightens Confidence Score: 4/5Safe to merge with care — the main guard paths are correctly implemented, but No new P0/P1 issues found. Previously flagged P1s (stale
|
| Filename | Overview |
|---|---|
| litellm/router.py | Adds _filter_deployments_by_model_access_groups and _try_early_resolve_deployments_for_model_not_in_names; threads request_kwargs through sync routing path; guards litellm-model and default-fallback paths against access-group bypass. Core logic is sound but _try_early_resolve_deployments_for_model_not_in_names still returns team/pattern/default deployments without access-group filtering. |
| tests/test_litellm/router_utils/test_router_utils_common_utils.py | Adds unit test for _filter_deployments_by_model_access_groups using a Router with access-group-tagged deployments; no real network calls. |
| tests/test_litellm/test_router.py | Adds five regression tests covering access-group scoping, explicit-model bypass, litellm-model-fallback guard, and default-fallback guard; all use mock_response and make no real network calls. |
Reviews (8): Last reviewed commit: "Fix access-group bypass via litellm-mode..." | Re-trigger Greptile
Document why empty allowed_access_groups intentionally preserves unfiltered deployments to avoid breaking non-access-group authorization paths. Made-with: Cursor
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@Sameerlite Please see if the greptile comment is applicable here |
|
@Sameerlite Thanks for paying attention to this issue, but is there anything I can do to help move this along? |
|
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 e942ef6. Configure here.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
When _get_all_deployments returns 0 candidates and the litellm-model fallback branch (_get_deployment_by_litellm_model) finds deployments that the access-group filter then empties, _access_group_filter_emptied_candidates remained False (it was captured before that branch ran). The router would then proceed to default fallbacks; the fallback model could have no access_groups and short-circuit the filter, silently serving a caller blocked by access-group restrictions. Update the flag inside the litellm-model branch when filtering empties a non-empty candidate set so the default-fallback guard still triggers.
|
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 9d945fe. Configure here.
…ames
The router_code_coverage CI check requires every function in router.py to
be referenced by at least one test under tests/{local_testing,
router_unit_tests,test_litellm} in a file with "router" in its name.
The recently-extracted helper had no direct test, so the check failed
with "0.45% of functions in router.py are not tested".
Add a focused test that exercises the four return paths: model already
in self.model_names, no fallback applies, pattern-router match, and
default_deployment substitution (also asserting the stored default
isn't mutated).
https://claude.ai/code/session_019AVp1XL7RT9RxRe4qRLkay
|
|
…uting-fix fix(router): constrain same-name deployment routing by access groups
Summary
request_kwargsthroughget_available_deployment()so metadata-based filtering is applied on sync routing pathsTest plan
pytest tests/test_litellm/test_router.py -k \"access_group_scoped_key_filters_deployments_with_same_public_model or explicit_model_access_does_not_force_access_group_filtering\" -qFixes #18333
Note
Medium Risk
Changes core routing selection to enforce access-group-based authorization and alters fallback behavior, which could impact which deployments are chosen or when requests error. Risk is mitigated by added regression tests but affects permission-sensitive logic.
Overview
Router deployment resolution now applies access-group constraints when request metadata includes
user_api_key_authand the caller’s permissions for the requested public model are access-group-only, filtering candidate deployments (especially when multiple deployments share the samemodel_name).It also prevents unauthorized bypass via routing fallbacks by (a) skipping the
_get_deployment_by_litellm_modelbranch when access-group filtering emptied candidates and (b) blocking defaultfallbacksmodel substitution in the same scenario; syncget_available_deployment()now passesrequest_kwargsso this filtering runs on non-async paths.Adds targeted tests covering access-group-scoped routing, explicit model access behavior, and regression cases ensuring neither litellm-model lookup nor default fallbacks can circumvent access-group restrictions.
Reviewed by Cursor Bugbot for commit 9d945fe. Bugbot is set up for automated code reviews on this repo. Configure here.