fix(proxy): hydrate wildcard discovery credentials (#28284) - CCI Run#28419
Conversation
* fix(proxy): hydrate wildcard discovery credentials * fix(proxy): constrain wildcard credential hydration
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes wildcard discovery for team-scoped BYOK deployments by threading
Confidence Score: 4/5Safe to merge; the change only affects wildcard model listing, not the request routing or auth hot-path, and all new behavior is covered by mock-only unit tests. The core fix is well-scoped and the router's
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/model_checks.py | Adds _hydrate_litellm_credential_name to resolve stored credentials before wildcard expansion, and threads team_id through _get_wildcard_models/get_complete_model_list so team-scoped BYOK router deployments are matched correctly. Logic is sound; minor edge noted around unconditional credential-name clearing. |
| litellm/proxy/utils.py | Computes effective_team_id = team_id or user_api_key_dict.team_id and passes it to get_complete_model_list. Minimal, targeted change; correct precedence. |
| tests/test_litellm/proxy/auth/test_model_checks.py | Adds four focused mock-only tests covering credential hydration, field preservation, missing-credential passthrough, and the end-to-end get_available_models_for_user path. No real network calls. |
Reviews (1): Last reviewed commit: "fix(proxy): hydrate wildcard discovery c..." | Re-trigger Greptile
| litellm_params = litellm_params.model_copy() | ||
| for key, value in credential_values.items(): | ||
| if ( | ||
| key in _CREDENTIAL_LITELLM_PARAM_FIELDS | ||
| and getattr(litellm_params, key, None) is None | ||
| ): | ||
| setattr(litellm_params, key, value) | ||
| litellm_params.litellm_credential_name = None | ||
| return litellm_params |
There was a problem hiding this comment.
litellm_credential_name is unconditionally cleared to None whenever the named credential exists, even if none of its keys match any CredentialLiteLLMParams field. In that edge case the credential lookup silently "consumes" the name and leaves every auth field still None, which would cause provider calls to fail without any indication that the credential name was resolved but produced no usable values. A guard or a debug log before clearing would make this easier to diagnose.
| litellm_params = litellm_params.model_copy() | |
| for key, value in credential_values.items(): | |
| if ( | |
| key in _CREDENTIAL_LITELLM_PARAM_FIELDS | |
| and getattr(litellm_params, key, None) is None | |
| ): | |
| setattr(litellm_params, key, value) | |
| litellm_params.litellm_credential_name = None | |
| return litellm_params | |
| litellm_params = litellm_params.model_copy() | |
| hydrated_any = False | |
| for key, value in credential_values.items(): | |
| if ( | |
| key in _CREDENTIAL_LITELLM_PARAM_FIELDS | |
| and getattr(litellm_params, key, None) is None | |
| ): | |
| setattr(litellm_params, key, value) | |
| hydrated_any = True | |
| if not hydrated_any: | |
| verbose_proxy_logger.debug( | |
| "litellm_credential_name=%s resolved but no matching CredentialLiteLLMParams fields found", | |
| litellm_params.litellm_credential_name, | |
| ) | |
| litellm_params.litellm_credential_name = None | |
| return litellm_params |
37ef8d9
into
litellm_internal_staging
PR overviewLow: Unvalidated team_id can steer wildcard discoveryThis PR hydrates stored credentials for wildcard model discovery and threads team_id into router lookups. The new effective team selection trusts a query-provided team_id even on paths where membership validation is skipped. Security review
Risk: 4/10 |
| include_model_access_groups=include_model_access_groups, | ||
| ) | ||
|
|
||
| effective_team_id = team_id or user_api_key_dict.team_id |
There was a problem hiding this comment.
Low: Unvalidated team_id controls wildcard expansion
When the DB/cache helpers are not initialized, the membership check above is skipped, but this line still trusts the query-string team_id. A caller can request /v1/models?team_id=<other-team> and have wildcard discovery use another team's router deployment/credential, exposing that team's discovered model IDs. Only pass the query team into get_complete_model_list after validate_membership has run; otherwise fall back to user_api_key_dict.team_id.
…erriAI#28419) * fix(proxy): hydrate wildcard discovery credentials * fix(proxy): constrain wildcard credential hydration Co-authored-by: Dibyo Mukherjee <dibyo@adobe.com>
* fix(proxy): hydrate wildcard discovery credentials (#28284) (#28419) * fix(proxy): hydrate wildcard discovery credentials * fix(proxy): constrain wildcard credential hydration Co-authored-by: Dibyo Mukherjee <dibyo@adobe.com> (cherry picked from commit 37ef8d9) * fix(team): refresh team cache on team_model_add/delete (LIT-3244) (#28683) * fix(team): refresh team cache on team_model_add/delete (LIT-3244) team_model_add and team_model_delete wrote to the DB but did not invalidate the in-memory LiteLLM_TeamTableCachedObj used by common_checks. After the v1.83.14 common_checks centralization made team.models authoritative on /v1/files and /v1/vector_stores/*, adding a Team-BYOK model silently failed to grant the new public model name to team members until the cache TTL expired (and a removed model kept working until then on the symmetric path). Extract the cache-refresh snippet from update_team into a small helper and apply it consistently at all three team-write sites. * test: also assert updated models in team-cache-refresh pin Strengthens the LIT-3244 regression test to also assert `call_kwargs["team_table"].models` matches the updated row, not just `team_id`. Both `existing_team` and `updated_team` share `team_id` in the test setup, so the previous assertion would have passed even if the implementation accidentally cached the pre-mutation row. Greptile review feedback. * fix(team): hydrate object_permission on cache-refreshing team updates The Prisma update calls in update_team, team_model_add, and team_model_delete returned a team row with object_permission_id set but object_permission=None (the relation was not requested via include=). _refresh_cached_team then wrote that to the in-memory LiteLLM_TeamTableCachedObj, and the cache-hit path in get_team_object returns the cached object without re-hydrating. Downstream consumers (validate_key_search_tools_against_team, the MCP/agent authz paths) treat a missing object_permission as no team-level restriction, so a team-write op silently dropped object-permission enforcement until the cache TTL expired or a DB-fetch path re-hydrated it. Add include={"object_permission": True} to all three updates so the refresh writes a complete cached team. Extend the LIT-3244 regression test to pin both the cached object_permission and the include shape on the Prisma call. Surfaced in PR review of LIT-3244. (cherry picked from commit 5f73ad4) --------- Co-authored-by: Dibyo Mukherjee <dibyo@adobe.com>
fix(proxy): hydrate wildcard discovery credentials
fix(proxy): constrain wildcard credential hydration
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes