feat(proxy): add credential overrides per team/project via model_config metadata - #24438
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds a Key changes:
Confidence Score: 5/5Safe to merge — all prior P0/P1 findings are resolved; the feature is opt-in (flag defaults to False) and the only remaining item is a minor P2 usability gap with provider-prefixed model names. All critical issues flagged in earlier review rounds have been addressed: credential secrets are no longer logged, the feature flag defaults to False, clientside precedence uses is-not-None guards, non-dict metadata values are type-guarded, the misleading log message is fixed, and tests correctly enable the feature flag via the fixture. The one remaining P2 concern (provider-prefixed model names without an alias not matching bare config keys) is a usability edge case already acknowledged in a test comment and does not affect correctness of the primary use cases. No files require special attention — all changed files are in good shape.
|
| Filename | Overview |
|---|---|
| litellm/proxy/litellm_pre_call_utils.py | Adds three well-guarded helper functions for credential override resolution; credential injection is correctly placed after the debug log; type guards, provider-hint logic, and per-key application (key not in data) are all present and correct. |
| litellm/init.py | Adds enable_model_config_credential_overrides: bool = False — correctly opt-in, consistent with the stability convention for new critical-path behaviors. |
| tests/test_litellm/proxy/test_litellm_pre_call_utils.py | 14 new tests covering all precedence scenarios, edge cases (non-dict config, missing credentials, alias resolution, provider hints), and the feature-flag default; setup_test_credentials fixture properly enables the flag and cleans up after each test. |
| docs/my-website/docs/proxy/credential_routing.md | Clear, accurate documentation; correctly states the feature is disabled by default and explains the full precedence chain, schema, and enabling steps. |
| docs/my-website/sidebars.js | Adds sidebar entry for the new credential_routing doc page; no issues. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming Request] --> B{Feature flag enabled?}
B -- No --> Z[No override — pass through]
B -- Yes --> C{Clientside api_base or api_key present?}
C -- Yes --> Z
C -- No --> D{model in data?}
D -- No --> Z
D -- Yes --> E{project_model_config or team_model_config in metadata?}
E -- Neither --> Z
E -- At least one --> F[_resolve_credential_from_model_config]
F --> G{Project: model-specific entry with credential?}
G -- Yes --> R[Return credential name]
G -- No --> H{Project: pre-alias name entry with credential?}
H -- Yes --> R
H -- No --> I{Project: defaultconfig with credential?}
I -- Yes --> R
I -- No --> J{Team: model-specific entry with credential?}
J -- Yes --> R
J -- No --> K{Team: pre-alias name entry with credential?}
K -- Yes --> R
K -- No --> L{Team: defaultconfig with credential?}
L -- Yes --> R
L -- No --> Z
R --> M[CredentialAccessor.get_credential_values]
M --> N{Values found?}
N -- No --> W[Log warning — skip]
N -- Yes --> O[Apply missing keys only: api_base / api_key / api_version if key not already in data]
O --> P[Log debug — applied credential]
P --> Q[Continue to enforced params check]
Reviews (10): Last reviewed commit: "fix test isolation: add fixture to guard..." | Re-trigger Greptile
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Respect clientside credentials — highest precedence | ||
| if data.get("api_base") or data.get("api_key"): | ||
| return |
There was a problem hiding this comment.
api_version not guarded by the early-return check
The guard only skips the override when api_base or api_key is present. If a caller explicitly passes api_version (but no api_base/api_key), the function continues, resolves a credential, and then silently overwrites their api_version via the loop on line 1395–1397. This breaks the contract that clientside values have highest precedence.
| # Respect clientside credentials — highest precedence | |
| if data.get("api_base") or data.get("api_key"): | |
| return | |
| if data.get("api_base") or data.get("api_key") or data.get("api_version"): | |
| return |
Alternatively, guard individual keys at application time so only absent keys are filled:
for key in ("api_base", "api_key", "api_version"):
if key in credential_values and key not in data:
data[key] = credential_values[key]| def _extract_credential_from_entry(entry: dict) -> Optional[str]: | ||
| """ | ||
| Extract litellm_credentials from a model_config entry. | ||
|
|
||
| Entry structure: {"azure": {"litellm_credentials": "name"}, ...} | ||
| Returns the first credential name found across all provider keys. | ||
| """ | ||
| for provider_config in entry.values(): | ||
| if isinstance(provider_config, dict): | ||
| credential_name = provider_config.get("litellm_credentials") | ||
| if credential_name: | ||
| return credential_name | ||
| return None |
There was a problem hiding this comment.
_extract_credential_from_entry iterates entry.values() and returns the credential belonging to whichever provider key happens to appear first in the dict. If a team stores both "openai" and "azure" keys in an entry, the credential returned depends solely on JSON key order — not on which provider the in-flight request is actually targeting. This can silently route a request to the wrong Azure endpoint.
The model name or custom_llm_provider from data is not threaded down into this helper, so it's impossible to pick the correct provider here. At minimum the caller should pass a provider hint and this function should attempt a provider-specific match before falling back to the first key.
def _extract_credential_from_entry(
entry: dict, provider: Optional[str] = None
) -> Optional[str]:
"""..."""
# Prefer an exact provider match when the provider is known
if provider and provider in entry:
provider_config = entry[provider]
if isinstance(provider_config, dict):
credential_name = provider_config.get("litellm_credentials")
if credential_name:
return credential_name
# Fall back to first available provider
for provider_config in entry.values():
if isinstance(provider_config, dict):
credential_name = provider_config.get("litellm_credentials")
if credential_name:
return credential_name
return None| for model_config in (project_model_config, team_model_config): | ||
| if not model_config: |
There was a problem hiding this comment.
model_config value causes unhandled AttributeError
_resolve_credential_from_model_config receives project_model_config and team_model_config straight from user-controlled metadata with no type assertion. The check if not model_config passes for any truthy value, including a non-empty string or list. Calling .get() on either of those raises AttributeError, surfacing as an unhandled 500 to callers.
Add an explicit type guard before the loop (or at extraction time in _apply_credential_overrides_from_model_config):
| for model_config in (project_model_config, team_model_config): | |
| if not model_config: | |
| for model_config in (project_model_config, team_model_config): | |
| if not model_config or not isinstance(model_config, dict): | |
| continue |
|
|
||
| import litellm | ||
| from litellm._logging import verbose_logger, verbose_proxy_logger | ||
| from litellm.litellm_core_utils.credential_accessor import CredentialAccessor |
There was a problem hiding this comment.
The new credential_accessor import is placed between two litellm._* (underscore-prefixed) imports, breaking the existing alphabetical grouping. Placing it after _service_logger keeps the block consistent with the surrounding style.
| import litellm | |
| from litellm._logging import verbose_logger, verbose_proxy_logger | |
| from litellm.litellm_core_utils.credential_accessor import CredentialAccessor | |
| from litellm._logging import verbose_logger, verbose_proxy_logger | |
| from litellm._service_logger import ServiceLogging | |
| from litellm.litellm_core_utils.credential_accessor import CredentialAccessor |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| def _extract_credential_from_entry(entry: dict) -> Optional[str]: | ||
| """ | ||
| Extract litellm_credentials from a model_config entry. | ||
|
|
||
| Entry structure: {"azure": {"litellm_credentials": "name"}, ...} | ||
| Returns the first credential name found across all provider keys. | ||
| """ | ||
| for provider_config in entry.values(): | ||
| if isinstance(provider_config, dict): | ||
| credential_name = provider_config.get("litellm_credentials") | ||
| if credential_name: | ||
| return credential_name | ||
| return None |
There was a problem hiding this comment.
entry.values() called without dict type guard — raises AttributeError on malformed metadata
_extract_credential_from_entry calls entry.values() unconditionally. Its callers in _resolve_credential_from_model_config only guard the outer model_config container with isinstance(model_config, dict) but do not check the inner values (model_entry, default_entry) before passing them to this function.
If an admin stores model_config like {"gpt-4": "my-cred-name"} (a string value), then model_entry = "my-cred-name", if model_entry: is True, and _extract_credential_from_entry("my-cred-name") calls "my-cred-name".values() → AttributeError → unhandled 500 on every request for that model.
The fix is to guard at the entry point of this function:
| def _extract_credential_from_entry(entry: dict) -> Optional[str]: | |
| """ | |
| Extract litellm_credentials from a model_config entry. | |
| Entry structure: {"azure": {"litellm_credentials": "name"}, ...} | |
| Returns the first credential name found across all provider keys. | |
| """ | |
| for provider_config in entry.values(): | |
| if isinstance(provider_config, dict): | |
| credential_name = provider_config.get("litellm_credentials") | |
| if credential_name: | |
| return credential_name | |
| return None | |
| def _extract_credential_from_entry(entry: dict) -> Optional[str]: | |
| """ | |
| Extract litellm_credentials from a model_config entry. | |
| Entry structure: {"azure": {"litellm_credentials": "name"}, ...} | |
| Returns the first credential name found across all provider keys. | |
| """ | |
| if not isinstance(entry, dict): | |
| return None | |
| for provider_config in entry.values(): | |
| if isinstance(provider_config, dict): | |
| credential_name = provider_config.get("litellm_credentials") | |
| if credential_name: | |
| return credential_name | |
| return None |
The existing tests cover non-dict provider_config (inner value) but there is no test for non-dict entry (outer value) — adding one would also catch this regression.
| # Apply credential overrides only for keys not already in the request | ||
| for key in ("api_base", "api_key", "api_version"): | ||
| if key in credential_values and key not in data: | ||
| data[key] = credential_values[key] |
There was a problem hiding this comment.
api_key written to data immediately before a debug log that dumps the entire data dict
After _apply_credential_overrides_from_model_config returns, the caller (add_litellm_data_to_request) executes:
verbose_proxy_logger.debug(
"[PROXY] returned data from litellm_pre_call_utils: %s", data
)At that point data["api_key"] contains the raw credential secret that was just injected here. This means any LITELLM_LOG=DEBUG session — even temporary troubleshooting — will print the plaintext Azure/OpenAI API key from the LiteLLM_CredentialsTable to stdout/stderr/log sinks.
Before this PR, api_key could only land in data when the caller explicitly sent it (and the early-return guard ensured the credential override was skipped entirely). This PR is the first code path that injects a credential secret into data without the user having provided it — making the debug-log exposure a new, systemic risk.
Consider masking the value before it reaches the log, for example storing a redacted copy or delaying injection until after the debug statement.
There was a problem hiding this comment.
@michelligabriele can we avoid this? don't want a key leak
| "model_config references credential '%s' but it was not found", | ||
| credential_name, | ||
| ) | ||
| return | ||
|
|
||
| # Apply credential overrides only for keys not already in the request |
There was a problem hiding this comment.
_apply_credential_overrides_from_model_config runs synchronously on every authenticated request
CredentialAccessor.get_credential_values performs an O(n) linear scan of litellm.credential_list inside this synchronous call in add_litellm_data_to_request (an async function). The early-return guards (if not project_model_config and not team_model_config) reduce call frequency to teams/projects that have opted in, but the project_metadata.get("model_config") dict access itself runs on every authenticated request, including high-traffic ones.
More importantly, litellm.credential_list is a module-level mutable list. Without a lock, concurrent writes during a credential refresh (see proxy_server.py:get_credentials) and concurrent reads here can cause a RuntimeError: list changed size during iteration or return a partial/stale credential silently — both of which happen in the hot path.
At minimum, iterate over a snapshot:
credential_values = CredentialAccessor.get_credential_values(credential_name)Inside CredentialAccessor.get_credential_values, change the loop to:
for credential in list(litellm.credential_list): # snapshot to avoid concurrent-modification race
if credential.credential_name == credential_name:
return credential.credential_values.copy()| team_metadata = user_api_key_dict.team_metadata or {} | ||
|
|
||
| project_model_config = project_metadata.get("model_config") | ||
| team_model_config = team_metadata.get("model_config") |
There was a problem hiding this comment.
The credential override runs after _update_model_if_key_alias_exists and _update_model_if_team_alias_exists, so data["model"] is already the resolved alias target when the lookup occurs. When a key or team alias maps a friendly name to a provider-prefixed deployment (e.g., "gpt-4" → "azure/gpt-4-0613", which is an explicit pattern shown in _update_model_if_key_alias_exists's own docstring), the model_config.get(model_name) call uses "azure/gpt-4-0613" while the team likely configured only "gpt-4" as the key in their model_config.
The result: model-specific overrides silently do nothing for aliased model names — only the "defaultconfig" fallback (if present) would match. No warning is emitted and no test covers this combination, making the failure completely silent.
Consider stripping any provider prefix before the lookup (so "azure/gpt-4-0613" is matched against "gpt-4" in model_config), and add a regression test for this scenario.
| def _apply_credential_overrides_from_model_config( | ||
| data: dict, | ||
| user_api_key_dict: UserAPIKeyAuth, | ||
| ) -> None: | ||
| """ | ||
| Walk the model_config precedence chain in team/project metadata. | ||
| If a matching credential is found, set api_base/api_key/api_version on data | ||
| so they override deployment defaults in the router. | ||
|
|
||
| Precedence (highest to lowest): | ||
| 1. Clientside credentials (already in data — skip if present) | ||
| 2. Project model-specific override | ||
| 3. Project default override (defaultconfig) | ||
| 4. Team model-specific override | ||
| 5. Team default override (defaultconfig) | ||
| 6. Deployment default (no action needed) | ||
| """ | ||
| # Respect clientside credentials — highest precedence | ||
| if data.get("api_base") or data.get("api_key"): | ||
| return | ||
|
|
||
| model_name = data.get("model") |
There was a problem hiding this comment.
Per the repository's stability conventions, new behavior affecting the critical request path should be gated by a user-controlled flag. While this feature is opt-in via the presence of "model_config" in metadata — so it does not break existing teams — any team that accidentally stores a "model_config" key in their metadata for a different purpose will silently trigger credential injection on every request.
Adding a global litellm flag (similar to other optional proxy behaviors) would give operators an explicit escape hatch if unexpected behavior is observed in production, and aligns with the pattern used for other new proxy features in this codebase.
Rule Used: What: avoid backwards-incompatible changes without... (source)
| result = _resolve_credential_from_model_config( | ||
| "azure/gpt-4-0613", team_config, None, pre_alias_model_name="gpt-4" | ||
| ) | ||
| assert result == "team-gpt4" | ||
|
|
||
|
|
||
| def test_resolve_post_alias_name_takes_priority(): | ||
| """Post-alias (resolved) name should be tried before pre-alias name.""" | ||
| team_config = { | ||
| "gpt-4": {"azure": {"litellm_credentials": "pre-alias-cred"}}, | ||
| "gpt-4o-team-1": {"azure": {"litellm_credentials": "post-alias-cred"}}, | ||
| } | ||
| result = _resolve_credential_from_model_config( | ||
| "gpt-4o-team-1", team_config, None, pre_alias_model_name="gpt-4" | ||
| ) | ||
| assert result == "post-alias-cred" |
There was a problem hiding this comment.
Both test_resolve_pre_alias_model_name_fallback (line 2251–2254) and test_resolve_post_alias_name_takes_priority (line 2263–2266) declare a variable named team_config but pass it as the second positional argument to _resolve_credential_from_model_config, which maps to project_model_config, not team_model_config.
The function signature is:
def _resolve_credential_from_model_config(
model_name: str,
project_model_config: Optional[dict], # ← position 2
team_model_config: Optional[dict], # ← position 3
pre_alias_model_name: Optional[str] = None,
)So team_config is silently tested in the project scope, while the team path (team_model_config) receives None. The assertions still pass because the project path is exercised successfully — but the team-scope pre-alias fallback (steps 5–6 in the precedence chain) has no unit-level coverage. If a regression were introduced specifically in the team branch of _resolve_credential_from_model_config, these tests would not catch it.
Fix by swapping the argument positions so that team_model_config is the one exercised:
# test_resolve_pre_alias_model_name_fallback
result = _resolve_credential_from_model_config(
"azure/gpt-4-0613", None, team_config, pre_alias_model_name="gpt-4"
)
# test_resolve_post_alias_name_takes_priority
result = _resolve_credential_from_model_config(
"gpt-4o-team-1", None, team_config, pre_alias_model_name="gpt-4"
)Rule Used: What: Flag any modifications to existing tests and... (source)
| def _apply_credential_overrides_from_model_config( | ||
| data: dict, | ||
| user_api_key_dict: UserAPIKeyAuth, | ||
| pre_alias_model_name: Optional[str] = None, | ||
| ) -> None: | ||
| """ | ||
| Walk the model_config precedence chain in team/project metadata. | ||
| If a matching credential is found, set api_base/api_key/api_version on data | ||
| so they override deployment defaults in the router. | ||
|
|
||
| Precedence (highest to lowest): | ||
| 1. Clientside credentials (already in data — skip if present) | ||
| 2. Project model-specific override | ||
| 3. Project default override (defaultconfig) | ||
| 4. Team model-specific override | ||
| 5. Team default override (defaultconfig) | ||
| 6. Deployment default (no action needed) | ||
| """ | ||
| # Respect clientside credentials — highest precedence | ||
| if data.get("api_base") or data.get("api_key"): | ||
| return |
There was a problem hiding this comment.
Per the project's stability convention (rule: avoid backwards-incompatible changes without user-controlled flags), new behaviour added to the critical request path should be gated by a litellm.* flag so operators have an explicit escape hatch.
While this feature is opt-in via the presence of "model_config" in team/project metadata, any team that already stores a "model_config" key for an unrelated purpose (e.g., storing display configuration) will silently trigger credential injection on every request. Without a global flag there is no way to disable the new path without removing the metadata key.
Consider adding a global guard similar to other optional proxy behaviours:
if not litellm.enable_model_config_credential_overrides:
returnand defaulting it to False until the feature is battle-tested.
Rule Used: What: avoid backwards-incompatible changes without... (source)
|
Re: feature flag (P2) — This feature is opt-in by metadata key presence, consistent with how Re: Re: provider selection order (P1) — This is a documented v0 limitation. The feature is scoped to Azure-only for the initial use case. Configs will have a single provider key per entry. Multi-provider support (with provider hint threading) is planned for a future iteration. All other P1s from previous review rounds have been addressed in subsequent commits. |
| credential_values = CredentialAccessor.get_credential_values(credential_name) | ||
| if not credential_values: | ||
| verbose_proxy_logger.warning( | ||
| "model_config references credential '%s' but it was not found", | ||
| credential_name, | ||
| ) | ||
| return |
There was a problem hiding this comment.
credential_values
CredentialAccessor.get_credential_values returns {} in two distinct cases: (1) the credential name is not in litellm.credential_list, and (2) the credential is found but its credential_values dict happens to be empty. Since not {} evaluates to True, both cases hit the same warning branch with the message "credential was not found", making it impossible to distinguish a typo in the credential name from a legitimate (but empty) credential entry.
If an operator creates a credential for future use but leaves credential_values blank, every request for that team will silently log a misleading warning claiming the credential doesn't exist.
Consider distinguishing the two cases in CredentialAccessor.get_credential_values (e.g., return None when not found vs {} when found-but-empty), or use a sentinel, and update the check accordingly:
credential_values = CredentialAccessor.get_credential_values(credential_name)
if credential_values is None:
verbose_proxy_logger.warning(
"model_config references credential '%s' but it was not found",
credential_name,
)
return
if not credential_values:
verbose_proxy_logger.debug(
"Credential '%s' found but has no values; skipping override",
credential_name,
)
return| if data.get("api_base") or data.get("api_key"): | ||
| return |
There was a problem hiding this comment.
api_base/api_key values bypass the clientside-credential guard
The guard uses Python truthiness (data.get("api_base") or data.get("api_key")), which means an explicitly set but empty-string value (e.g., api_base="") is treated the same as an absent key and the guard does not fire. If any upstream middleware normalizes a missing field to "" rather than leaving it absent, the credential override will unexpectedly run and overwrite the (empty) value — potentially injecting an entirely different Azure endpoint than intended.
A more robust check uses is not None to detect explicit presence regardless of truthiness:
| if data.get("api_base") or data.get("api_key"): | |
| return | |
| if data.get("api_base") is not None or data.get("api_key") is not None: |
| # Model-specific check (try resolved name, then pre-alias name) | ||
| for name in model_names_to_try: | ||
| model_entry = model_config.get(name) | ||
| if model_entry: | ||
| credential_name = _extract_credential_from_entry(model_entry) | ||
| if credential_name: | ||
| return credential_name |
There was a problem hiding this comment.
defaultconfig when model-specific entry lacks credentials
When a model-specific entry exists in model_config but contains no litellm_credentials, _extract_credential_from_entry returns None, and the loop silently falls through to try "defaultconfig". This means an operator who intentionally creates a model entry without a credential (e.g. "gpt-4": {"azure": {"deployment": "my-deployment"}}) expecting it to mean "no override for this model" will actually receive the defaultconfig credential applied instead.
Consider returning a sentinel "" (empty string) from _extract_credential_from_entry when an entry exists but has no litellm_credentials, and treating that as "explicitly no credential" to stop the chain for that model, rather than falling through to the default:
for name in model_names_to_try:
model_entry = model_config.get(name)
if model_entry:
credential_name = _extract_credential_from_entry(model_entry)
if credential_name:
return credential_name
# Entry exists but has no credential — stop searching this scope
# to avoid defaultconfig being applied against operator intent
breakIf the intended design IS to fall through to defaults when an entry has no credential, this should be documented explicitly in the docstring.
| def _apply_credential_overrides_from_model_config( | ||
| data: dict, | ||
| user_api_key_dict: UserAPIKeyAuth, | ||
| pre_alias_model_name: Optional[str] = None, | ||
| ) -> None: | ||
| """ | ||
| Walk the model_config precedence chain in team/project metadata. | ||
| If a matching credential is found, set api_base/api_key/api_version on data | ||
| so they override deployment defaults in the router. | ||
|
|
||
| Precedence (highest to lowest): | ||
| 1. Clientside credentials (already in data — skip if present) | ||
| 2. Project model-specific override | ||
| 3. Project default override (defaultconfig) | ||
| 4. Team model-specific override | ||
| 5. Team default override (defaultconfig) | ||
| 6. Deployment default (no action needed) | ||
| """ | ||
| # Respect clientside credentials — highest precedence | ||
| if data.get("api_base") is not None or data.get("api_key") is not None: | ||
| return |
There was a problem hiding this comment.
Per this project's stability convention (rule: avoid backwards-incompatible changes without user-controlled flags), new code added to the critical request path should be gated by a litellm.* flag. While this feature is opt-in via the presence of "model_config" in metadata, any team that already stores a "model_config" key in their metadata for a different purpose (e.g. display or application config) will silently trigger credential injection on every request. There is no way to disable this behavior at the operator level without removing the metadata key.
Consider adding a global guard:
if not getattr(litellm, "enable_model_config_credential_overrides", False):
returnand defaulting it to False in litellm/__init__.py until the feature is battle-tested.
Rule Used: What: avoid backwards-incompatible changes without... (source)
|
|
||
| <TabItem value="config" label="config.yaml"> | ||
|
|
||
| ```yaml |
There was a problem hiding this comment.
The doc says "The feature is enabled by default." This aligns with the current True default but conflicts with the stability convention that new critical-path behavior should be opt-in. If the flag default is changed to False in litellm/__init__.py, the documentation here should be updated to reflect that users need to enable it first.
| ```yaml | |
| The feature is **disabled by default**. To enable it globally: |
Rule Used: What: avoid backwards-incompatible changes without... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| def test_apply_overrides_no_model_in_data(): | ||
| """No model in request data -> skip override.""" | ||
| data = {"messages": [{"role": "user", "content": "hello"}]} | ||
| user_api_key_dict = UserAPIKeyAuth( | ||
| api_key="test-key", | ||
| team_metadata={ | ||
| "model_config": { | ||
| "defaultconfig": { | ||
| "azure": {"litellm_credentials": "some-cred"} | ||
| } | ||
| } | ||
| }, | ||
| ) | ||
| _apply_credential_overrides_from_model_config( | ||
| data=data, user_api_key_dict=user_api_key_dict | ||
| ) | ||
| assert "api_base" not in data | ||
|
|
||
|
|
||
| def test_apply_overrides_none_metadata(): | ||
| """None metadata on both team and project -> skip override.""" | ||
| data = {"model": "gpt-4"} | ||
| user_api_key_dict = UserAPIKeyAuth( | ||
| api_key="test-key", | ||
| team_metadata=None, | ||
| project_metadata=None, | ||
| ) | ||
| _apply_credential_overrides_from_model_config( | ||
| data=data, user_api_key_dict=user_api_key_dict | ||
| ) | ||
| assert "api_base" not in data |
There was a problem hiding this comment.
test_apply_overrides_no_model_in_data (line 2173) and test_apply_overrides_none_metadata (line 2192) both lack the setup_test_credentials fixture, which means litellm.enable_model_config_credential_overrides is False when they run.
The function's very first check is:
if not litellm.enable_model_config_credential_overrides:
returnSo both tests hit this early exit and their assertions ("api_base" not in data) pass trivially — not because the "no model" guard or the "None metadata" guard is exercised, but because the feature is disabled. If either of those guards were accidentally deleted, these tests would still pass.
Add the setup_test_credentials fixture to both tests (it sets enable_model_config_credential_overrides = True) so they actually validate the intended code paths:
def test_apply_overrides_no_model_in_data(setup_test_credentials):
...
def test_apply_overrides_none_metadata(setup_test_credentials):
...Rule Used: What: Flag any modifications to existing tests and... (source)
| verbose_proxy_logger.debug( | ||
| "model_config entry '%s' found but has no litellm_credentials, " | ||
| "falling through to defaultconfig", | ||
| _safe_name, | ||
| ) |
There was a problem hiding this comment.
When the inner for name in model_names_to_try loop finds an entry without credentials, the log says "falling through to defaultconfig". But the loop continues to the next name (i.e., the pre_alias_model_name) before reaching defaultconfig. If the pre-alias name has a credential, the function returns it — but the log implied it was about to skip straight to the default.
verbose_proxy_logger.debug(
"model_config entry '%s' found but has no litellm_credentials, "
"falling through to defaultconfig", # ← misleading if pre_alias_model_name is still pending
_safe_name,
)Consider a more accurate message:
| verbose_proxy_logger.debug( | |
| "model_config entry '%s' found but has no litellm_credentials, " | |
| "falling through to defaultconfig", | |
| _safe_name, | |
| ) | |
| verbose_proxy_logger.debug( | |
| "model_config entry '%s' found but has no litellm_credentials, " | |
| "trying next fallback", | |
| _safe_name, | |
| ) |
…e model_config type, fix import order
…edential injection after debug log
6c1b360 to
ce5716b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| import litellm | ||
| from litellm._logging import verbose_logger, verbose_proxy_logger | ||
| from litellm._service_logger import ServiceLogging | ||
| from litellm.litellm_core_utils.credential_accessor import CredentialAccessor |
| False # get response headers from LLM Api providers - example x-remaining-requests, | ||
| ) | ||
| enable_json_schema_validation: bool = False | ||
| enable_model_config_credential_overrides: bool = False |
|
Tip: Greploops — Automatically fix all review issues by running Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal. |
3 participants
Relevant issues
None — new feature request from enterprise customer needing per-team Azure endpoint routing.
Pre-Submission checklist
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:
Type
🆕 New Feature
Changes
Adds credential override resolution from
model_configin team/project metadata, enabling multi-team Azure setups where the same model name routes to different Azure endpoints per team/project.How it works
Teams/projects can set a
model_configkey in their metadata referencing credentials fromLiteLLM_CredentialsTable:{ "model_config": { "defaultconfig": { "azure": { "litellm_credentials": "hotel-team-azure-cred" } }, "gpt-4": { "azure": { "litellm_credentials": "hotel-team-gpt4-westus" } } } }The system walks a 4-level precedence chain (first match wins):
defaultconfig)defaultconfig)Clientside credentials (
api_base/api_keypassed in request body) take highest precedence above all of the above.Implementation
litellm/proxy/litellm_pre_call_utils.py:_apply_credential_overrides_from_model_config()— entry point, checks precedence, resolves credential, setsapi_base/api_key/api_versionon request data_resolve_credential_from_model_config()— walks the 4-level chain_extract_credential_from_entry()— extractslitellm_credentialsfrom a provider-scoped entryadd_litellm_data_to_request()What's NOT changed
model_configis a new key in existingmetadataJSON{**litellm_params, ..., **kwargs}) handles override automaticallyteam_metadata/project_metadataalready populated by auth pipelineTesting
tests/test_litellm/proxy/test_litellm_pre_call_utils.py