From a2d8a107c83ea47e999037ec549f1dd9a317d2b1 Mon Sep 17 00:00:00 2001 From: mateo <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 10 Nov 2026 16:39:25 +0000 Subject: [PATCH] fix: align file id, MCP toolset and JSON provider key handling Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- litellm/main.py | 5 ++ .../openai_files_endpoints/files_endpoints.py | 18 +++++++ .../mcp/litellm_proxy_mcp_handler.py | 23 ++++++++- .../llms/openai_like/test_aiand_provider.py | 19 +++++++ .../test_files_endpoint.py | 51 +++++++++++++++++++ .../mcp/test_litellm_proxy_mcp_handler.py | 13 +++++ 6 files changed, 128 insertions(+), 1 deletion(-) diff --git a/litellm/main.py b/litellm/main.py index febf9503a7e..d50eaeeab59 100644 --- a/litellm/main.py +++ b/litellm/main.py @@ -2799,6 +2799,11 @@ def _complete_custom_openai( raise litellm.AuthenticationError( message="Set BIGMODEL_API_KEY or pass api_key", llm_provider="bigmodel", model=model ) + json_provider: Final = JSONProviderRegistry.get(custom_llm_provider) + if json_provider is not None and client is None and not (api_key or litellm.api_key): + raise litellm.AuthenticationError( + message=f"Set {json_provider.api_key_env} or pass api_key", llm_provider=custom_llm_provider, model=model + ) organization = ( organization or (None if custom_llm_provider == "bigmodel" else litellm.organization or get_secret("OPENAI_ORGANIZATION")) diff --git a/litellm/proxy/openai_files_endpoints/files_endpoints.py b/litellm/proxy/openai_files_endpoints/files_endpoints.py index 5ce0ed8b493..82d8dae2f94 100644 --- a/litellm/proxy/openai_files_endpoints/files_endpoints.py +++ b/litellm/proxy/openai_files_endpoints/files_endpoints.py @@ -1127,6 +1127,15 @@ async def get_file_content( user_api_key_dict=user_api_key_dict, check_file_id_encoding=True, ) + if ( + original_file_id is not None + and is_managed_cloud_storage_uri(original_file_id) + and user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN + ): + raise HTTPException( + status_code=403, + detail="Raw cloud storage file ids can only be retrieved by a proxy admin key. Use the LiteLLM managed file id returned when the file was created.", + ) if not should_route: apply_team_provider_credentials( @@ -1557,6 +1566,15 @@ async def delete_file( user_api_key_dict=user_api_key_dict, check_file_id_encoding=True, ) + if ( + original_file_id is not None + and is_managed_cloud_storage_uri(original_file_id) + and user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN + ): + raise HTTPException( + status_code=403, + detail="Raw cloud storage file ids can only be deleted by a proxy admin key. Use the LiteLLM managed file id returned when the file was created.", + ) if should_route and credentials is not None: # Use model-based routing with credentials from config diff --git a/litellm/responses/mcp/litellm_proxy_mcp_handler.py b/litellm/responses/mcp/litellm_proxy_mcp_handler.py index d045689cb56..f640f1e8f64 100644 --- a/litellm/responses/mcp/litellm_proxy_mcp_handler.py +++ b/litellm/responses/mcp/litellm_proxy_mcp_handler.py @@ -267,6 +267,22 @@ class LiteLLM_Proxy_MCP_Handler: verbose_logger.debug("Key does not have access to toolset '%s', denying it.", name) return ("denied_toolset", name) + @staticmethod + async def _servers_granted_to_key( + server_names: Sequence[str], user_api_key_auth: "UserAPIKeyAuth" + ) -> tuple[str, ...]: + from litellm.proxy.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + + allowed_ids: Final = frozenset(await global_mcp_server_manager.get_allowed_mcp_servers(user_api_key_auth)) + return tuple( + name + for name in server_names + if (server := global_mcp_server_manager.get_mcp_server_by_name(name)) is not None + and server.server_id in allowed_ids + ) + @staticmethod async def _holds_access_group_named(name: str, user_api_key_auth: "UserAPIKeyAuth | None") -> bool: from litellm.proxy.mcp_server.auth.user_api_key_auth_mcp import ( @@ -332,8 +348,13 @@ class LiteLLM_Proxy_MCP_Handler: for name in mcp_servers ] ) - resolved_mcp_servers: Final = tuple(value for kind, value in resolutions if kind == "server") + named_mcp_servers: Final = tuple(value for kind, value in resolutions if kind == "server") resolved_toolset_ids: Final = tuple(value for kind, value in resolutions if kind == "toolset") + resolved_mcp_servers: Final = ( + await LiteLLM_Proxy_MCP_Handler._servers_granted_to_key(named_mcp_servers, user_api_key_auth) + if resolved_toolset_ids and user_api_key_auth is not None + else named_mcp_servers + ) denied_toolset_names: Final = tuple(value for kind, value in resolutions if kind == "denied_toolset") # Apply all resolved toolsets at once (union), avoiding permission overwrite. diff --git a/tests/unit/llms/openai_like/test_aiand_provider.py b/tests/unit/llms/openai_like/test_aiand_provider.py index 0274b58f19a..612303d4251 100644 --- a/tests/unit/llms/openai_like/test_aiand_provider.py +++ b/tests/unit/llms/openai_like/test_aiand_provider.py @@ -78,6 +78,25 @@ def test_aiand_completion_request(respx_mock: respx.MockRouter): assert request_body["messages"] == messages +def test_aiand_completion_does_not_send_the_openai_key(respx_mock: respx.MockRouter, monkeypatch: pytest.MonkeyPatch): + monkeypatch.delenv("AIAND_API_KEY", raising=False) + monkeypatch.setenv("OPENAI_API_KEY", "openai-key") + monkeypatch.setattr(litellm, "openai_key", "openai-global-key") + monkeypatch.setattr(litellm, "api_key", None) + route: Final = respx_mock.post(f"{AIAND_API_BASE}/chat/completions").mock( + return_value=httpx.Response(200, json={}) + ) + + with pytest.raises(litellm.AuthenticationError, match="AIAND_API_KEY"): + litellm.completion( + model="aiand/openai/gpt-oss-120b", + messages=[{"role": "user", "content": "Hello"}], + num_retries=0, + ) + + assert not route.called + + def test_aiand_responses_provider(): config = ProviderConfigManager.get_provider_responses_api_config( provider="aiand", diff --git a/tests/unit/proxy/openai_files_endpoint/test_files_endpoint.py b/tests/unit/proxy/openai_files_endpoint/test_files_endpoint.py index 27a65a91c41..aac8a82e62a 100644 --- a/tests/unit/proxy/openai_files_endpoint/test_files_endpoint.py +++ b/tests/unit/proxy/openai_files_endpoint/test_files_endpoint.py @@ -5642,6 +5642,57 @@ def test_delete_file_answers_403_for_a_raw_cloud_id_from_a_non_admin_key( afile_delete.assert_not_called() +@pytest.mark.parametrize( + ("method", "path_suffix", "sdk_call"), + (("get", "/content", "afile_content"), ("delete", "", "afile_delete")), +) +@pytest.mark.parametrize( + ("raw_file_id", "model_name"), + ((RAW_S3_FILE_ID, "bedrock-claude"), (RAW_GCS_FILE_ID, "vertex-gemini")), +) +def test_model_encoded_raw_cloud_id_answers_403_for_a_non_admin_key( + mocker: MockerFixture, + monkeypatch: pytest.MonkeyPatch, + method: str, + path_suffix: str, + sdk_call: str, + raw_file_id: str, + model_name: str, +): + import litellm.proxy.proxy_server as ps + from litellm.proxy.openai_files_endpoints.common_utils import encode_file_id_with_model + + cloud_router: Final = _cloud_files_router() + proxy_logging_obj: Final = setup_proxy_logging_object(monkeypatch, cloud_router) + monkeypatch.setattr("litellm.proxy.proxy_server.master_key", None) + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", None) + monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", cloud_router) + proxy_logging_obj.update_request_status = mocker.AsyncMock() + proxy_logging_obj.post_call_failure_hook = mocker.AsyncMock() + provider_call: Final = mocker.AsyncMock() + monkeypatch.setattr(litellm, sdk_call, provider_call) + app.dependency_overrides[ps.user_api_key_auth] = lambda: UserAPIKeyAuth( + api_key="test-key", + user_role=LitellmUserRoles.INTERNAL_USER, + user_id="test-user", + models=["bedrock-claude", "vertex-gemini"], + ) + encoded_file_id: Final = encode_file_id_with_model(raw_file_id, model_name) + + try: + response: Final = client.request( + method.upper(), + f"/v1/files/{encoded_file_id}{path_suffix}", + headers={"Authorization": "Bearer test-key"}, + ) + finally: + app.dependency_overrides.pop(ps.user_api_key_auth, None) + + assert response.status_code == 403, response.text + assert "proxy admin" in response.json()["error"]["message"] + provider_call.assert_not_called() + + def test_delete_file_forwards_a_raw_cloud_id_from_a_proxy_admin_key(mocker: MockerFixture, monkeypatch): from urllib.parse import quote diff --git a/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py b/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py index 44ba9007d92..07bb488fda0 100644 --- a/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py +++ b/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py @@ -1871,6 +1871,19 @@ async def test_a_denied_toolset_beside_a_granted_one_adds_nothing_to_the_scoped_ assert listed == ["calc-add"] +@pytest.mark.asyncio +async def test_a_server_the_key_does_not_hold_named_beside_a_granted_toolset_adds_nothing( + monkeypatch: pytest.MonkeyPatch, +): + listed: Final = await _tools_listed_through_the_gateway( + monkeypatch, + UserAPIKeyAuth(api_key="sk-team", team_id="team-1", user_role=LitellmUserRoles.INTERNAL_USER), + ("litellm_proxy/mcp/team-toolset", "litellm_proxy/mcp/docs"), + _team_grants("ts-granted"), + ) + assert listed == ["calc-add"] + + @pytest.mark.asyncio async def test_a_whole_gateway_entry_beside_an_ungranted_toolset_still_lists_the_keys_own_servers( monkeypatch: pytest.MonkeyPatch,