Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions litellm/proxy/openai_files_endpoints/common_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ def prepare_data_with_credentials(
credentials: dict,
file_id: str | None = None,
include_internal_credentials: bool = False,
routing_model: str | None = None,
) -> None:
"""
Update data dictionary with model credentials (in-place).
Expand All @@ -463,8 +464,17 @@ def prepare_data_with_credentials(
file_id: Optional original file_id to set (for decoded file IDs)
include_internal_credentials: Preserve an immutable server-side snapshot
for code paths that must distinguish proxy config from request params.
routing_model: Model-group alias to keep in ``data["model"]`` after the
merge. Credentials carry the deployment's underlying
``litellm_params.model`` (which call sites dispatching straight to
the provider SDK need), but router-bound call sites must keep
routing by alias: the underlying model loses the model-group
identity (access groups, team scoping) when several aliases share
one provider model (#36103).
"""
data.update(credentials)
if routing_model is not None:
data["model"] = routing_model
if include_internal_credentials:
data["_litellm_internal_model_credentials"] = MappingProxyType(dict(credentials))
data.pop("custom_llm_provider", None)
Expand Down
6 changes: 5 additions & 1 deletion litellm/proxy/vector_store_files_endpoints/endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ def _update_request_data_with_managed_file_id(
data=data,
credentials=credentials,
file_id=llm_output_file_id, # Use the actual provider file ID
routing_model=routing_model,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Legacy IDs still lose aliases

When a vector-store operation uses a simple encoded file ID containing a model-group alias, the sibling branch merges credentials without passing model_used as routing_model. This replaces the alias with the underlying deployment model, causing incorrect group routing and attribution or a model-not-found response. How this was verified: The encoded-ID branch flows into router dispatch without any later assignment restoring model_used after the credential merge.

Rule Used: What: For security fix PRs, NEVER mention what sec... (source)

Knowledge Base Used: Proxy Server Request Flow

)
verbose_logger.info(
"Routing vector store file operation to model: %s, file_id: %s -> %s",
Expand Down Expand Up @@ -272,6 +273,7 @@ async def _update_request_data_with_model_routing_hint(
prepare_data_with_credentials(
data=data,
credentials=credentials,
routing_model=model_hint if isinstance(model_hint, str) else None,
)
return data

Expand All @@ -293,6 +295,7 @@ async def _update_request_data_with_model_routing_hint(
model_names_to_check.append(model_name)

openai_credentials = None
openai_model_name = None
for model_name in model_names_to_check:
credentials = llm_router.get_deployment_credentials_with_provider(model_id=model_name, team_id=caller_team_id)
if credentials is None:
Expand All @@ -313,9 +316,10 @@ async def _update_request_data_with_model_routing_hint(
if openai_credentials is not None:
return data
openai_credentials = credentials
openai_model_name = model_name

if openai_credentials is not None:
prepare_data_with_credentials(data=data, credentials=openai_credentials)
prepare_data_with_credentials(data=data, credentials=openai_credentials, routing_model=openai_model_name)
elif len(model_names_to_check) == 1:
await _authorize_model_routing_hint(
model=model_names_to_check[0],
Expand Down
34 changes: 34 additions & 0 deletions tests/test_litellm/proxy/test_model_based_routing_files_batches.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,40 @@ def test_does_not_add_internal_credentials_by_default(self):

assert "_litellm_internal_model_credentials" not in data

def test_routing_model_keeps_alias_over_credentials_model(self):
"""
Two model groups can share one litellm_params.model; router-bound call
sites pass routing_model so the group alias survives the merge and the
router can still apply per-group access controls (#36103).
"""
data = {"model": "team-a-gpt", "vector_store_id": "vs_1"}
credentials = {
"api_key": "sk-shared",
"custom_llm_provider": "openai",
"model": "openai/gpt-4o-mini",
}

prepare_data_with_credentials(
data=data, credentials=credentials, routing_model="team-a-gpt"
)

assert data["model"] == "team-a-gpt"
assert data["api_key"] == "sk-shared"
assert "custom_llm_provider" not in data

def test_credentials_model_lands_in_data_without_routing_model(self):
"""Direct-to-SDK call sites still need the deployment model (#25104)."""
data = {"model": "bedrock-batch-alias"}
credentials = {
"aws_region_name": "us-west-2",
"model": "bedrock/us.anthropic.claude-haiku-4-5-20251001-v1:0",
}

prepare_data_with_credentials(data=data, credentials=credentials)

assert data["model"] == "bedrock/us.anthropic.claude-haiku-4-5-20251001-v1:0"
assert data["aws_region_name"] == "us-west-2"


class TestRoundTrip:
"""Tests for encode -> decode round-trip integrity."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ async def test_vector_store_file_list_resolves_credentials_from_model_query_para

assert result["api_key"] == "sk-team-openai"
assert result["api_base"] == "https://api.openai.com/v1"
assert result["model"] == "openai/gpt-4o-mini"
# routing stays on the model-group alias, not the deployment's provider model (#36103)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 New comments violate repository guidance

This change adds explanatory inline comments here and at the other modified alias-routing assertions, despite the repository instruction prohibiting new comments unless explicitly requested. Remove the four redundant comments while retaining the assertions.

Context Used: CLAUDE.md (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!

assert result["model"] == "team-openai"
assert "custom_llm_provider" not in result
llm_router.get_deployment_credentials_with_provider.assert_called_once_with(
model_id="team-openai"
Expand Down Expand Up @@ -207,7 +208,8 @@ async def test_vector_store_file_list_resolves_single_openai_team_deployment():

assert result["api_key"] == "sk-team-openai"
assert result["api_base"] == "https://api.openai.com/v1"
assert result["model"] == "openai/gpt-4o-mini"
# routing stays on the model-group alias, not the deployment's provider model (#36103)
assert result["model"] == "team-openai"
assert "custom_llm_provider" not in result
llm_router.get_deployment_credentials_with_provider.assert_called_once_with(
model_id="team-openai", team_id=None
Expand Down Expand Up @@ -245,7 +247,8 @@ async def test_vector_store_file_list_wildcard_model_hint_falls_back_to_team_dep

assert result["api_key"] == "sk-team-openai"
assert result["api_base"] == "https://api.openai.com/v1"
assert result["model"] == "openai/gpt-4o-mini"
# routing stays on the matched team alias, not the deployment's provider model (#36103)
assert result["model"] == "team-openai"
assert "custom_llm_provider" not in result
assert llm_router.get_deployment_credentials_with_provider.call_count == 3

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,9 @@ def get_credentials(model_id):
assert response == {"ok": True}
assert captured_data["vector_store_id"] == "vs_provider_native"
assert captured_data["api_key"] == "sk-managed-deployment"
assert captured_data["model"] == "openai/managed-deployment"
# the alias survives the credential merge so router dispatch keeps the
# model-group identity (#36103)
assert captured_data["model"] == "managed-deployment"
llm_router.get_deployment_credentials_with_provider.assert_called_once_with(
model_id="managed-deployment"
)
Expand Down
Loading