-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
fix(proxy): hydrate wildcard discovery credentials #28284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6068,6 +6068,8 @@ async def get_available_models_for_user( | |
| include_model_access_groups=include_model_access_groups, | ||
| ) | ||
|
|
||
| effective_team_id = team_id or user_api_key_dict.team_id | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Medium: Unvalidated team_id used for wildcard discovery A caller can pass
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tl;dr - technically true, though actual severity is vastly overstated: AI analysis belowThe finding is technically valid but practically very low severity. Here's the breakdown: What's technically trueIn [utils.py:6051](litellm/proxy/utils.py:6051), the validation guard: if team_id and prisma_client and proxy_logging_obj and user_api_key_cache:skips membership validation when effective_team_id = team_id or user_api_key_dict.team_idThis unvalidated if team_id is not None and team_id in self.team_pattern_routers:
potential_team_only_wildcard_models = self.team_pattern_routers[team_id].route(...)...which could return another team's wildcard deployments. Why it's practically not exploitable
The one real (minor) side-effectIf someone did run this contrived configuration, the other team's hydrated credential would be used server-side in Verdict: Not a real vulnerability in any realistic deployment. You can safely dismiss or mark as "won't fix" with a note that the guard already works when a database is present, and team-scoped wildcards inherently require one. |
||
|
|
||
| # Get complete model list | ||
| all_models = get_complete_model_list( | ||
| key_models=key_models, | ||
|
|
@@ -6080,6 +6082,7 @@ async def get_available_models_for_user( | |
| model_access_groups=model_access_groups, | ||
| include_model_access_groups=include_model_access_groups, | ||
| only_model_access_groups=only_model_access_groups, | ||
| team_id=effective_team_id, | ||
| ) | ||
|
|
||
| return all_models | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setattrcould silently overwrite anyLiteLLM_Paramsfield. If a stored credential ever includes a"model"key (or other core field like"api_version"), it would quietly shadow the deployment's configured model name during wildcard discovery, producing an incorrect provider list with no error or warning. SinceLiteLLM_Paramsusesextra="allow", unexpected keys would also persist as phantom attributes.