diff --git a/litellm/llms/github_copilot/authenticator.py b/litellm/llms/github_copilot/authenticator.py index 59e1804f4532..28cd6cf5e683 100644 --- a/litellm/llms/github_copilot/authenticator.py +++ b/litellm/llms/github_copilot/authenticator.py @@ -1,6 +1,8 @@ import json import os +import threading import time +from collections.abc import Callable from datetime import datetime from typing import Any, Final @@ -25,6 +27,16 @@ DEFAULT_GITHUB_DEVICE_CODE_URL: Final = "https://github.com/login/device/code" DEFAULT_GITHUB_ACCESS_TOKEN_URL: Final = "https://github.com/login/oauth/access_token" DEFAULT_GITHUB_API_KEY_URL: Final = "https://api.github.com/copilot_internal/v2/token" +_SHARED_LOGIN_ALLOWED: Final = threading.Event() +_SHARED_LOGIN_ALLOWED.set() +_SHARED_LOGIN_DISABLED_MESSAGE: Final = ( + "GitHub Copilot shared device login is disabled on the LiteLLM proxy. Create an LLM credential with Auth Type " + "'Per-user GitHub OAuth' and reference it from the deployment with litellm_credential_name" +) + + +def disable_github_copilot_shared_login() -> None: + _SHARED_LOGIN_ALLOWED.clear() @with_config(ConfigDict(extra="allow", strict=True)) @@ -62,8 +74,9 @@ def github_api_headers( class Authenticator: - def __init__(self) -> None: + def __init__(self, shared_login_allowed: Callable[[], bool] = _SHARED_LOGIN_ALLOWED.is_set) -> None: """Initialize the GitHub Copilot authenticator with configurable token paths.""" + self._shared_login_allowed = shared_login_allowed # Token storage paths self.token_dir = os.getenv( "GITHUB_COPILOT_TOKEN_DIR", @@ -74,7 +87,15 @@ def __init__(self) -> None: os.getenv("GITHUB_COPILOT_ACCESS_TOKEN_FILE", "access-token"), ) self.api_key_file = os.path.join(self.token_dir, os.getenv("GITHUB_COPILOT_API_KEY_FILE", "api-key.json")) - self._ensure_token_dir() + if self._shared_login_allowed(): + self._ensure_token_dir() + + def _raise_if_shared_login_disabled(self) -> None: + if not self.is_shared_login_allowed(): + raise GetAPIKeyError(message=_SHARED_LOGIN_DISABLED_MESSAGE, status_code=401) + + def is_shared_login_allowed(self) -> bool: + return self._shared_login_allowed() def get_access_token(self) -> str: """ @@ -86,6 +107,7 @@ def get_access_token(self) -> str: Raises: GetAccessTokenError: If unable to obtain an access token after retries. """ + self._raise_if_shared_login_disabled() try: with open(self.access_token_file, "r") as f: access_token = f.read().strip() @@ -100,8 +122,7 @@ def get_access_token(self) -> str: "GitHub Copilot device-code login needs a human and cannot run inside a running event loop " "or a worker thread (for example the LiteLLM proxy). Log in once outside the proxy with " '`python -c "from litellm.llms.github_copilot.authenticator import Authenticator; ' - 'Authenticator().get_access_token()"` and mount the resulting access-token file into ' - "the proxy, or set GITHUB_COPILOT_TOKEN_DIR to a directory that already holds it." + 'Authenticator().get_access_token()"`.' ), status_code=401, ) @@ -135,6 +156,7 @@ def get_api_key(self) -> str: Raises: GetAPIKeyError: If unable to obtain an API key. """ + self._raise_if_shared_login_disabled() try: with open(self.api_key_file, "r") as f: api_key_info = json.load(f) @@ -184,6 +206,8 @@ def get_api_base(self) -> str | None: Returns: Optional[str]: The GitHub Copilot API endpoint, or None if not found. """ + if not self._shared_login_allowed(): + return None try: with open(self.api_key_file, "r") as f: api_key_info: Final = json.load(f) diff --git a/litellm/llms/github_copilot/chat/transformation.py b/litellm/llms/github_copilot/chat/transformation.py index 9553327db970..feb6492ef4d2 100644 --- a/litellm/llms/github_copilot/chat/transformation.py +++ b/litellm/llms/github_copilot/chat/transformation.py @@ -50,6 +50,8 @@ def _get_openai_compatible_provider_info( custom_llm_provider: str, ) -> tuple[str | None, str | None, str]: dynamic_api_base: Final = self.api_base_without_login(api_base) + if not self.authenticator.is_shared_login_allowed(): + return dynamic_api_base, api_key, custom_llm_provider try: dynamic_api_key: Final = self.authenticator.get_api_key() except GetAPIKeyError as e: @@ -95,13 +97,17 @@ def _transform_messages( return transformed_messages - def _copilot_headers(self, session_token: str | None) -> Mapping[str, str]: + def _copilot_headers(self, session_token: str | None, model: str) -> Mapping[str, str]: if session_token is not None: return get_copilot_default_headers(session_token) try: return get_copilot_default_headers(self.authenticator.get_api_key()) - except GetAPIKeyError: - return {} + except GetAPIKeyError as exc: + raise AuthenticationError( + model=model, + llm_provider="github_copilot", + message=str(exc), + ) def validate_environment( self, @@ -121,7 +127,7 @@ def validate_environment( cast("dict[str, object]", litellm_params) # cast-ok: litellm_params arrives as an untyped request dict ) session_token: Final = user_session.token if user_session is not None else None - validated_headers: Final = {**self._copilot_headers(session_token), **parent_headers} + validated_headers: Final = {**self._copilot_headers(session_token, model), **parent_headers} if session_token is not None: # the caller's stored session token wins unconditionally, even over a # caller-supplied api_key or the parent's placeholder Authorization diff --git a/litellm/main.py b/litellm/main.py index b80819354f00..9b2e7dd5123a 100644 --- a/litellm/main.py +++ b/litellm/main.py @@ -69,7 +69,7 @@ NADIR_DEFAULT_API_BASE, OPENAI_AUDIO_TRANSCRIPTION_PROVIDERS, ) -from litellm.exceptions import LiteLLMUnknownProvider +from litellm.exceptions import AuthenticationError, LiteLLMUnknownProvider from litellm.integrations.custom_logger import CustomLogger from litellm.litellm_core_utils.asyncify import asyncify, run_async_function from litellm.litellm_core_utils.audio_utils.utils import ( @@ -2602,6 +2602,23 @@ def _complete_hosted_vllm(ctx: CompletionDispatchContext) -> _CompletionDispatch return response +def _get_github_copilot_shared_api_key( + get_api_key: Callable[[], str], + model: str, + custom_llm_provider: str, +) -> str: + from litellm.llms.github_copilot.common_utils import GetAPIKeyError + + try: + return get_api_key() + except GetAPIKeyError as exc: + raise AuthenticationError( + model=model, + llm_provider=custom_llm_provider, + message=str(exc), + ) + + def _complete_custom_openai( ctx: CompletionDispatchContext, ) -> _CompletionDispatchResult: @@ -2654,18 +2671,22 @@ def _complete_custom_openai( # Add GitHub Copilot headers (same as /responses endpoint does) if custom_llm_provider == "github_copilot": from litellm.llms.github_copilot.authenticator import Authenticator - from litellm.llms.github_copilot.common_utils import ( - get_copilot_default_headers, - pin_session_authorization, - ) + from litellm.llms.github_copilot.common_utils import get_copilot_default_headers, pin_session_authorization from litellm.llms.github_copilot.per_user_auth import ( require_github_copilot_user_session, ) user_session: Final = require_github_copilot_user_session(litellm_params) - copilot_headers: Final = get_copilot_default_headers( - user_session.token if user_session is not None else Authenticator().get_api_key() + copilot_token: Final = ( + user_session.token + if user_session is not None + else _get_github_copilot_shared_api_key( + get_api_key=Authenticator().get_api_key, + model=model, + custom_llm_provider=custom_llm_provider, + ) ) + copilot_headers: Final = get_copilot_default_headers(copilot_token) if extra_headers: copilot_headers.update( cast("dict[str, str]", extra_headers) # cast-ok: extra_headers is a str-valued request dict diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index e7226f62757c..26ed39698063 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -1337,6 +1337,9 @@ async def proxy_startup_event(app: FastAPI) -> AsyncGenerator[ProxyLifespanState shared_aiohttp_session import json + from litellm.llms.github_copilot.authenticator import disable_github_copilot_shared_login + + disable_github_copilot_shared_login() init_verbose_loggers() prometheus_multiproc_dir: Final = os.environ.get("PROMETHEUS_MULTIPROC_DIR") diff --git a/tests/unit/llms/github_copilot/embedding/test_github_copilot_embedding_transformation.py b/tests/unit/llms/github_copilot/embedding/test_github_copilot_embedding_transformation.py index 88fc8ba6e6ae..9c5fac25a033 100644 --- a/tests/unit/llms/github_copilot/embedding/test_github_copilot_embedding_transformation.py +++ b/tests/unit/llms/github_copilot/embedding/test_github_copilot_embedding_transformation.py @@ -6,6 +6,7 @@ from litellm.exceptions import AuthenticationError +from litellm.llms.github_copilot.authenticator import Authenticator from litellm.llms.github_copilot.embedding.transformation import ( GithubCopilotEmbeddingConfig, ) @@ -264,8 +265,7 @@ def test_validate_environment_uses_per_user_session_and_skips_authenticator(): from litellm.llms.github_copilot.per_user_auth import GithubCopilotUserSession config = GithubCopilotEmbeddingConfig() - config.authenticator = MagicMock() - config.authenticator.get_api_key.side_effect = AssertionError("shared authenticator must not run") + config.authenticator = Authenticator(shared_login_allowed=lambda: False) session = GithubCopilotUserSession(token="user-copilot-token", api_base="https://api.githubcopilot.com") headers = config.validate_environment( @@ -276,7 +276,23 @@ def test_validate_environment_uses_per_user_session_and_skips_authenticator(): litellm_params={"github_copilot_user_session": session}, ) assert headers["Authorization"] == "Bearer user-copilot-token" - config.authenticator.get_api_key.assert_not_called() + + +def test_shared_authentication_error_is_raised_when_login_is_disabled(): + config = GithubCopilotEmbeddingConfig() + config.authenticator = Authenticator(shared_login_allowed=lambda: False) + + with pytest.raises(AuthenticationError) as exc_info: + config.validate_environment( + headers={}, + model="github_copilot/text-embedding-3-small", + messages=[], + optional_params={}, + litellm_params={}, + ) + + assert "GitHub Copilot shared device login is disabled on the LiteLLM proxy" in str(exc_info.value) + assert exc_info.value.status_code == 401 def test_per_user_session_token_wins_over_caller_authorization(): diff --git a/tests/unit/llms/github_copilot/messages/test_github_copilot_messages_transformation.py b/tests/unit/llms/github_copilot/messages/test_github_copilot_messages_transformation.py index 5cbb9a0a7b49..5ed360418029 100644 --- a/tests/unit/llms/github_copilot/messages/test_github_copilot_messages_transformation.py +++ b/tests/unit/llms/github_copilot/messages/test_github_copilot_messages_transformation.py @@ -5,6 +5,7 @@ from litellm.exceptions import AuthenticationError +from litellm.llms.github_copilot.authenticator import Authenticator from litellm.llms.github_copilot.common_utils import GetAPIKeyError from litellm.llms.github_copilot.messages.transformation import ( GithubCopilotAnthropicMessagesConfig, @@ -329,9 +330,7 @@ def test_validate_environment_uses_per_user_session_and_skips_authenticator(): from litellm.llms.github_copilot.per_user_auth import GithubCopilotUserSession config = GithubCopilotAnthropicMessagesConfig() - config.authenticator = MagicMock() - config.authenticator.get_api_key.side_effect = AssertionError("shared authenticator must not run") - config.authenticator.get_api_base.side_effect = AssertionError("shared authenticator must not run") + config.authenticator = Authenticator(shared_login_allowed=lambda: False) session = GithubCopilotUserSession(token="user-copilot-token", api_base="https://tenant.githubcopilot.com/") headers, api_base = config.validate_anthropic_messages_environment( @@ -343,7 +342,23 @@ def test_validate_environment_uses_per_user_session_and_skips_authenticator(): ) assert headers["Authorization"] == "Bearer user-copilot-token" assert api_base == "https://tenant.githubcopilot.com" - config.authenticator.get_api_key.assert_not_called() + + +def test_shared_authentication_error_is_raised_when_login_is_disabled(): + config = GithubCopilotAnthropicMessagesConfig() + config.authenticator = Authenticator(shared_login_allowed=lambda: False) + + with pytest.raises(AuthenticationError) as exc_info: + config.validate_anthropic_messages_environment( + headers={}, + model="github_copilot/claude-sonnet-5.5", + messages=[], + optional_params={}, + litellm_params={}, + ) + + assert "GitHub Copilot shared device login is disabled on the LiteLLM proxy" in str(exc_info.value) + assert exc_info.value.status_code == 401 def test_per_user_session_token_wins_over_caller_authorization(): diff --git a/tests/unit/llms/github_copilot/responses/test_github_copilot_responses_transformation.py b/tests/unit/llms/github_copilot/responses/test_github_copilot_responses_transformation.py index c3a62592039b..cf7b43bfc780 100644 --- a/tests/unit/llms/github_copilot/responses/test_github_copilot_responses_transformation.py +++ b/tests/unit/llms/github_copilot/responses/test_github_copilot_responses_transformation.py @@ -12,6 +12,8 @@ import httpx import pytest import litellm +from litellm.exceptions import AuthenticationError +from litellm.llms.github_copilot.authenticator import Authenticator from litellm.litellm_core_utils.get_model_cost_map import get_model_cost_map from litellm.types.utils import LlmProviders from litellm.utils import ProviderConfigManager @@ -759,8 +761,7 @@ def test_validate_environment_uses_per_user_session_and_skips_authenticator(): from litellm.llms.github_copilot.per_user_auth import GithubCopilotUserSession config = GithubCopilotResponsesAPIConfig() - config.authenticator = MagicMock() - config.authenticator.get_api_key.side_effect = AssertionError("shared authenticator must not run") + config.authenticator = Authenticator(shared_login_allowed=lambda: False) session = GithubCopilotUserSession(token="user-copilot-token", api_base="https://api.githubcopilot.com") headers = config.validate_environment( @@ -769,7 +770,21 @@ def test_validate_environment_uses_per_user_session_and_skips_authenticator(): litellm_params={"github_copilot_user_session": session}, ) assert headers["Authorization"] == "Bearer user-copilot-token" - config.authenticator.get_api_key.assert_not_called() + + +def test_shared_authentication_error_is_raised_when_login_is_disabled(): + config = GithubCopilotResponsesAPIConfig() + config.authenticator = Authenticator(shared_login_allowed=lambda: False) + + with pytest.raises(AuthenticationError) as exc_info: + config.validate_environment( + headers={}, + model="github_copilot/gpt-5.4", + litellm_params={}, + ) + + assert "GitHub Copilot shared device login is disabled on the LiteLLM proxy" in str(exc_info.value) + assert exc_info.value.status_code == 401 def test_per_user_session_token_wins_over_caller_authorization(): diff --git a/tests/unit/llms/github_copilot/test_github_copilot_authenticator.py b/tests/unit/llms/github_copilot/test_github_copilot_authenticator.py index 21d4901cf814..ae3b5de34f5e 100644 --- a/tests/unit/llms/github_copilot/test_github_copilot_authenticator.py +++ b/tests/unit/llms/github_copilot/test_github_copilot_authenticator.py @@ -49,6 +49,80 @@ def test_init(self): assert auth.api_key_file.endswith("/api-key.json") mock_makedirs.assert_called_once() + def test_disabled_shared_login_ignores_existing_tokens(self, tmp_path): + token_dir = tmp_path / "tokens" + token_dir.mkdir() + access_token_file = token_dir / "access-token" + access_token_file.write_text("valid-access-token") + api_key_info = { + "token": "valid-api-key", + "expires_at": 4102444800, + "endpoints": {"api": "https://api.githubcopilot.com"}, + } + api_key_file = token_dir / "api-key.json" + api_key_file.write_text(json.dumps(api_key_info)) + + with patch.dict(os.environ, {"GITHUB_COPILOT_TOKEN_DIR": str(token_dir)}): + auth = Authenticator(shared_login_allowed=lambda: False) + + with pytest.raises(GetAPIKeyError) as api_key_error: + auth.get_api_key() + with pytest.raises(GetAPIKeyError) as access_token_error: + auth.get_access_token() + + expected_message = ( + "GitHub Copilot shared device login is disabled on the LiteLLM proxy. Create an LLM credential with Auth " + "Type 'Per-user GitHub OAuth' and reference it from the deployment with litellm_credential_name" + ) + assert str(api_key_error.value) == expected_message + assert api_key_error.value.status_code == 401 + assert str(access_token_error.value) == expected_message + assert access_token_error.value.status_code == 401 + assert api_key_file.read_text() == json.dumps(api_key_info) + assert access_token_file.read_text() == "valid-access-token" + + def test_disabled_shared_login_does_not_create_token_dir(self, tmp_path): + token_dir = tmp_path / "not-created" + + with patch.dict(os.environ, {"GITHUB_COPILOT_TOKEN_DIR": str(token_dir)}): + Authenticator(shared_login_allowed=lambda: False) + + assert not token_dir.exists() + + def test_shared_login_predicate_is_checked_after_initialization(self, tmp_path): + token_dir = tmp_path / "tokens" + token_dir.mkdir() + api_key_info = { + "token": "valid-api-key", + "expires_at": 4102444800, + "endpoints": {"api": "https://api.githubcopilot.com"}, + } + (token_dir / "api-key.json").write_text(json.dumps(api_key_info)) + login_allowed = iter((True, False, False)) + + with patch.dict(os.environ, {"GITHUB_COPILOT_TOKEN_DIR": str(token_dir)}): + auth = Authenticator(shared_login_allowed=lambda: next(login_allowed)) + + with pytest.raises(GetAPIKeyError, match="GitHub Copilot shared device login is disabled"): + auth.get_api_key() + assert auth.get_api_base() is None + + def test_shared_login_enabled_keeps_reading_existing_tokens(self, tmp_path): + token_dir = tmp_path / "tokens" + token_dir.mkdir() + api_key_info = { + "token": "valid-api-key", + "expires_at": 4102444800, + "endpoints": {"api": "https://api.githubcopilot.com"}, + } + (token_dir / "api-key.json").write_text(json.dumps(api_key_info)) + + with patch.dict(os.environ, {"GITHUB_COPILOT_TOKEN_DIR": str(token_dir)}): + auth = Authenticator(shared_login_allowed=lambda: True) + + assert auth.get_api_key() == "valid-api-key" + assert auth.get_api_base() == "https://api.githubcopilot.com" + def test_ensure_token_dir(self): """Test that the token directory is created if it doesn't exist.""" with ( @@ -135,9 +209,7 @@ def test_get_access_token_failure(self, authenticator): def test_get_api_key_from_file(self, authenticator): """Test retrieving an API key from a file.""" future_time = (datetime.now() + timedelta(hours=1)).timestamp() - mock_api_key_data = json.dumps( - {"token": "mock-api-key", "expires_at": future_time} - ) + mock_api_key_data = json.dumps({"token": "mock-api-key", "expires_at": future_time}) with patch("builtins.open", mock_open(read_data=mock_api_key_data)): api_key = authenticator.get_api_key() @@ -146,9 +218,7 @@ def test_get_api_key_from_file(self, authenticator): def test_get_api_key_expired(self, authenticator): """Test refreshing an expired API key.""" past_time = (datetime.now() - timedelta(hours=1)).timestamp() - mock_expired_data = json.dumps( - {"token": "expired-api-key", "expires_at": past_time} - ) + mock_expired_data = json.dumps({"token": "expired-api-key", "expires_at": past_time}) mock_new_data = { "token": "new-api-key", "expires_at": (datetime.now() + timedelta(hours=1)).timestamp(), @@ -246,20 +316,14 @@ def test_login(self, authenticator): mock_token = "mock-access-token" with ( - patch.object( - authenticator, "_get_device_code", return_value=mock_device_code_data - ), - patch.object( - authenticator, "_poll_for_access_token", return_value=mock_token - ), + patch.object(authenticator, "_get_device_code", return_value=mock_device_code_data), + patch.object(authenticator, "_poll_for_access_token", return_value=mock_token), patch("builtins.print") as mock_print, ): result = authenticator._login() assert result == mock_token authenticator._get_device_code.assert_called_once() - authenticator._poll_for_access_token.assert_called_once_with( - "mock-device-code" - ) + authenticator._poll_for_access_token.assert_called_once_with("mock-device-code") mock_print.assert_called_once() def test_get_api_base_from_file(self, authenticator): diff --git a/tests/unit/llms/github_copilot/test_github_copilot_transformation.py b/tests/unit/llms/github_copilot/test_github_copilot_transformation.py index 2dbfae07526f..256671141c6e 100644 --- a/tests/unit/llms/github_copilot/test_github_copilot_transformation.py +++ b/tests/unit/llms/github_copilot/test_github_copilot_transformation.py @@ -1,7 +1,7 @@ import asyncio import json from datetime import datetime, timedelta -from typing import AsyncGenerator +from typing import AsyncGenerator, Final from unittest.mock import AsyncMock, MagicMock, mock_open, patch import pytest @@ -20,6 +20,7 @@ from litellm.llms.github_copilot.chat.transformation import GithubCopilotConfig from litellm.llms.github_copilot.common_utils import ( APIKeyExpiredError, + DEFAULT_GITHUB_COPILOT_API_BASE, GetAccessTokenError, GetAPIKeyError, GetDeviceCodeError, @@ -27,6 +28,33 @@ ) +def test_disabled_shared_login_allows_deployment_resolution_but_rejects_requests(): + config: Final = GithubCopilotConfig() + config.authenticator = Authenticator(shared_login_allowed=lambda: False) + + provider_info: Final = config.get_openai_compatible_provider_info( + model="github_copilot/gpt-4.1", + api_base=None, + api_key=None, + custom_llm_provider="github_copilot", + ) + assert provider_info == (DEFAULT_GITHUB_COPILOT_API_BASE, None, "github_copilot") + + with pytest.raises( + AuthenticationError, + match="GitHub Copilot shared device login is disabled on the LiteLLM proxy", + ): + config.validate_environment( + headers={}, + model="gpt-4.1", + messages=[{"role": "user", "content": "Hello"}], + optional_params={}, + litellm_params={}, + api_key=None, + api_base=None, + ) + + def test_github_copilot_config_get_openai_compatible_provider_info(): """Test the GitHub Copilot configuration provider info retrieval.""" @@ -37,9 +65,7 @@ def test_github_copilot_config_get_openai_compatible_provider_info(): config.authenticator = MagicMock() config.authenticator.get_api_key.return_value = mock_api_key # Test with dynamic endpoint - config.authenticator.get_api_base.return_value = ( - "https://api.enterprise.githubcopilot.com" - ) + config.authenticator.get_api_base.return_value = "https://api.enterprise.githubcopilot.com" # Test with default values model = "github_copilot/gpt-4" @@ -365,10 +391,6 @@ def test_x_initiator_header_system_only_messages(): assert headers["X-Initiator"] == "user" - - - - def test_copilot_vision_request_header_with_image(): """Test that Copilot-Vision-Request header is added when messages contain images""" config = GithubCopilotConfig() @@ -693,13 +715,8 @@ def test_transform_response_anthropic_native_tool_use(self): assert result.choices[0].message.tool_calls is not None assert len(result.choices[0].message.tool_calls) == 1 assert result.choices[0].message.tool_calls[0]["id"] == "toolu_01ABC" - assert ( - result.choices[0].message.tool_calls[0]["function"]["name"] == "get_weather" - ) - assert ( - '"Boston, MA"' - in result.choices[0].message.tool_calls[0]["function"]["arguments"] - ) + assert result.choices[0].message.tool_calls[0]["function"]["name"] == "get_weather" + assert '"Boston, MA"' in result.choices[0].message.tool_calls[0]["function"]["arguments"] def test_transform_response_anthropic_native_multiple_text_blocks(self): """All text blocks must be concatenated, not only the first.""" @@ -867,12 +884,8 @@ def test_passthrough_when_choices_present(self): @patch("litellm.llms.openai.openai.OpenAIChatCompletion._get_openai_client") -@patch( - "litellm.llms.openai.openai.OpenAIChatCompletion.make_sync_openai_chat_completion_request" -) -def test_openai_handler_repairs_github_copilot_empty_choices( - mock_request, mock_get_client -): +@patch("litellm.llms.openai.openai.OpenAIChatCompletion.make_sync_openai_chat_completion_request") +def test_openai_handler_repairs_github_copilot_empty_choices(mock_request, mock_get_client): """ The OpenAI SDK handler calls convert_to_model_response_object directly on the SDK's parsed output, bypassing transform_response. convert raises APIError on @@ -924,16 +937,51 @@ def test_validate_environment_uses_per_user_session_and_skips_authenticator(): from litellm.llms.github_copilot.per_user_auth import GithubCopilotUserSession config = GithubCopilotConfig() - config.authenticator = MagicMock() - config.authenticator.get_api_key.side_effect = AssertionError("shared authenticator must not run") + config.authenticator = Authenticator(shared_login_allowed=lambda: False) session = GithubCopilotUserSession(token="user-copilot-token", api_base="https://api.githubcopilot.com") headers = config.validate_environment( - headers={}, model="github_copilot/gpt-4o", messages=[], optional_params={}, + headers={}, + model="github_copilot/gpt-4o", + messages=[], + optional_params={}, litellm_params={"github_copilot_user_session": session}, ) assert headers["Authorization"] == "Bearer user-copilot-token" - config.authenticator.get_api_key.assert_not_called() + + +def test_shared_authentication_error_is_raised_when_login_is_disabled(): + config = GithubCopilotConfig() + config.authenticator = Authenticator(shared_login_allowed=lambda: False) + + with pytest.raises(AuthenticationError) as exc_info: + config.validate_environment( + headers={}, + model="github_copilot/gpt-4o", + messages=[], + optional_params={}, + litellm_params={}, + api_key=None, + api_base=None, + ) + + assert "GitHub Copilot shared device login is disabled on the LiteLLM proxy" in str(exc_info.value) + assert exc_info.value.status_code == 401 + + +def test_main_shared_authentication_error_is_mapped_to_authentication_error(): + from litellm.main import _get_github_copilot_shared_api_key + + authenticator = Authenticator(shared_login_allowed=lambda: False) + with pytest.raises(AuthenticationError) as exc_info: + _get_github_copilot_shared_api_key( + get_api_key=authenticator.get_api_key, + model="github_copilot/gpt-4.1", + custom_llm_provider="github_copilot", + ) + + assert "GitHub Copilot shared device login is disabled on the LiteLLM proxy" in str(exc_info.value) + assert exc_info.value.status_code == 401 def test_transform_response_carries_upstream_usage(): @@ -946,9 +994,7 @@ def test_transform_response_carries_upstream_usage(): "object": "chat.completion", "created": 1, "model": "github_copilot/gpt-4o", - "choices": [ - {"index": 0, "message": {"role": "assistant", "content": "hi"}, "finish_reason": "stop"} - ], + "choices": [{"index": 0, "message": {"role": "assistant", "content": "hi"}, "finish_reason": "stop"}], "usage": {"prompt_tokens": 11, "completion_tokens": 7, "total_tokens": 18}, }, ) diff --git a/tests/unit/proxy/conftest.py b/tests/unit/proxy/conftest.py index d86bdd84a625..2a60e731ca3a 100644 --- a/tests/unit/proxy/conftest.py +++ b/tests/unit/proxy/conftest.py @@ -170,6 +170,9 @@ def setup_and_teardown(): _restore_mutable_state(litellm, _LITELLM_STATE) _restore_mutable_state(litellm.proxy.proxy_server, _PROXY_SERVER_STATE) _flush_caches(_FLUSHABLE_CACHES) + from litellm.llms.github_copilot.authenticator import _SHARED_LOGIN_ALLOWED + + _SHARED_LOGIN_ALLOWED.set() loop = asyncio.get_event_loop_policy().new_event_loop() asyncio.set_event_loop(loop) @@ -178,13 +181,12 @@ def setup_and_teardown(): finally: loop.close() asyncio.set_event_loop(None) + _SHARED_LOGIN_ALLOWED.set() def pytest_collection_modifyitems(config, items): # Separate tests in 'test_amazing_proxy_custom_logger.py' and other tests - custom_logger_tests = [ - item for item in items if "custom_logger" in item.parent.name - ] + custom_logger_tests = [item for item in items if "custom_logger" in item.parent.name] other_tests = [item for item in items if "custom_logger" not in item.parent.name] # Sort tests based on their names @@ -209,9 +211,7 @@ def pytest_runtest_setup(item): from litellm.proxy import proxy_server item.stash[_proxy_module_globals_snapshot] = { - name: vars(proxy_server)[name] - for name in _PROXY_MODULE_GLOBALS_TO_ISOLATE - if name in vars(proxy_server) + name: vars(proxy_server)[name] for name in _PROXY_MODULE_GLOBALS_TO_ISOLATE if name in vars(proxy_server) } yield @@ -286,9 +286,7 @@ def build_cache_config(enable_cache: bool = True) -> Optional[Dict]: return {"cache": True, "cache_params": cache_params} -def build_minimal_proxy_config( - database_url: Optional[str] = None, **init_options -) -> Dict: +def build_minimal_proxy_config(database_url: Optional[str] = None, **init_options) -> Dict: """ Build a minimal proxy configuration YAML. @@ -317,9 +315,7 @@ def build_minimal_proxy_config( config["litellm_settings"].update(cache_config) if init_options.get("success_callback") is not None: - config["litellm_settings"]["success_callback"] = init_options[ - "success_callback" - ] + config["litellm_settings"]["success_callback"] = init_options["success_callback"] excluded_keys = { "master_key", @@ -335,9 +331,7 @@ def build_minimal_proxy_config( return config -def set_proxy_environment_variables( - monkeypatch, database_url: Optional[str] = None -) -> None: +def set_proxy_environment_variables(monkeypatch, database_url: Optional[str] = None) -> None: """ Set environment variables for database and Redis. @@ -358,9 +352,7 @@ def set_proxy_environment_variables( monkeypatch.setenv("REDIS_PASSWORD", redis_password) -def create_proxy_test_client( - monkeypatch, database_url: Optional[str] = None, **init_options -) -> TestClient: +def create_proxy_test_client(monkeypatch, database_url: Optional[str] = None, **init_options) -> TestClient: """ Create a proxy TestClient with optional database and Redis cache configuration. @@ -385,18 +377,14 @@ def create_proxy_test_client( cleanup_router_config_variables() filepath = os.path.dirname(os.path.abspath(__file__)) - default_config_fp = os.path.join( - filepath, "test_configs", "test_config_hosted_vllm_embedding.yaml" - ) + default_config_fp = os.path.join(filepath, "test_configs", "test_config_hosted_vllm_embedding.yaml") enable_cache = init_options.get("enable_cache", True) needs_redis = enable_cache and os.getenv("REDIS_HOST") is not None needs_db = (database_url or os.getenv("DATABASE_URL")) is not None if not os.path.exists(default_config_fp) or needs_redis or needs_db: - minimal_config = build_minimal_proxy_config( - database_url=database_url, **init_options - ) + minimal_config = build_minimal_proxy_config(database_url=database_url, **init_options) with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: yaml.dump(minimal_config, f) diff --git a/tests/unit/proxy/proxy_server/test_lifecycle.py b/tests/unit/proxy/proxy_server/test_lifecycle.py index 69f9c72c9c82..d74bd7c97a03 100644 --- a/tests/unit/proxy/proxy_server/test_lifecycle.py +++ b/tests/unit/proxy/proxy_server/test_lifecycle.py @@ -883,6 +883,9 @@ async def test_proxy_startup_event_honors_disable_model_info_refresh( clean_env = {k: v for k, v in os.environ.items() if k not in ("DATABASE_URL", "DIRECT_URL")} | { "LITELLM_DANGEROUSLY_PERMIT_WEAK_OR_UNSET_MASTER_KEY": "true" } + from litellm.llms.github_copilot.authenticator import _SHARED_LOGIN_ALLOWED + + _SHARED_LOGIN_ALLOWED.set() with ( patch.dict(os.environ, clean_env, clear=True), patch.object(ps, "scheduler", scheduler), @@ -890,6 +893,7 @@ async def test_proxy_startup_event_honors_disable_model_info_refresh( ): try: async with proxy_startup_event(app=None): + assert not _SHARED_LOGIN_ALLOWED.is_set() job = scheduler.get_job("refresh_model_info") finally: if scheduler.running: diff --git a/ui/litellm-dashboard/src/components/add_model/AddModelForm.integration.test.tsx b/ui/litellm-dashboard/src/components/add_model/AddModelForm.integration.test.tsx index 75297a508d2e..949bbd4fb341 100644 --- a/ui/litellm-dashboard/src/components/add_model/AddModelForm.integration.test.tsx +++ b/ui/litellm-dashboard/src/components/add_model/AddModelForm.integration.test.tsx @@ -701,7 +701,7 @@ describe("AddModelForm", () => { vi.mocked(modelCreateCall).mockClear(); const props = await renderAsAdmin(Providers.GITHUB_COPILOT); - await chooseSelectOption(user, screen.getByRole("combobox", { name: "Auth Type:" }), "Per-user GitHub OAuth"); + expect(await screen.findByRole("combobox", { name: "Auth Type:" })).toHaveTextContent("Per-user GitHub OAuth"); await user.click(screen.getByRole("button", { name: "Create credential" })); const dialog = await screen.findByRole("dialog"); const providerSelect = within(dialog).getByPlaceholderText("Select a provider"); @@ -746,42 +746,6 @@ describe("AddModelForm", () => { expect(modelParams).toMatchObject({ litellm_credential_name: "github-per-user" }); expect(modelParams).not.toHaveProperty("github_copilot_auth_type"); }); - - it("keeps shared GitHub device login inline when adding a model", async () => { - const user = userEvent.setup(); - vi.mocked(credentialCreateCall).mockClear(); - vi.mocked(modelCreateCall).mockClear(); - const props = await renderAsAdmin(Providers.GITHUB_COPILOT); - - expect(await screen.findByRole("combobox", { name: "Auth Type:" })).toHaveTextContent("Shared device login"); - expect(screen.queryByRole("button", { name: "Create credential" })).not.toBeInTheDocument(); - fireEvent.change(screen.getByLabelText("API Key"), { target: { value: "shared-device-token" } }); - props.handleOk.mockImplementation(async () => { - await handleAddModelSubmit( - { - api_key: "shared-device-token", - custom_llm_provider: Providers.GITHUB_COPILOT, - model_mappings: [ - { - public_name: "copilot-model", - litellm_model: "github_copilot/chat", - }, - ], - }, - "test-access-token", - { resetFields: vi.fn() }, - ); - return true; - }); - - await user.click(screen.getByRole("button", { name: "Add Model" })); - - await waitFor(() => expect(modelCreateCall).toHaveBeenCalledOnce()); - expect(credentialCreateCall).not.toHaveBeenCalled(); - expect(vi.mocked(modelCreateCall).mock.calls[0][1].litellm_params).toMatchObject({ - api_key: "shared-device-token", - }); - }); }); describe("cache control bindings reach the parent form store", () => { diff --git a/ui/litellm-dashboard/src/components/add_model/provider_auth_types.test.ts b/ui/litellm-dashboard/src/components/add_model/provider_auth_types.test.ts index 5467ff0085ad..3849fd19118b 100644 --- a/ui/litellm-dashboard/src/components/add_model/provider_auth_types.test.ts +++ b/ui/litellm-dashboard/src/components/add_model/provider_auth_types.test.ts @@ -79,12 +79,13 @@ describe("provider auth types", () => { expect(hiddenAuthFieldKeys(authTypes, "first")).toEqual(["second_key", "third_key"]); }); - it("defaults GitHub Copilot to shared device login and recognises a stored per-user credential", () => { + it("uses per-user OAuth as the only GitHub Copilot auth type and default", () => { const authTypes = authTypesFor("GITHUB_COPILOT"); - expect(inferAuthTypeId(authTypes, { api_key: "" })).toBe("shared_device_login"); + expect(authTypes).toHaveLength(1); + expect(inferAuthTypeId(authTypes, {})).toBe("per_user_oauth"); + expect(inferAuthTypeId(authTypes, { api_key: "" })).toBe("per_user_oauth"); expect(inferAuthTypeId(authTypes, { github_copilot_auth_type: "per_user_oauth" })).toBe("per_user_oauth"); expect(hiddenAuthFieldKeys(authTypes, "per_user_oauth")).toEqual(["api_base", "api_key"]); - expect(hiddenAuthFieldKeys(authTypes, "shared_device_login")).toEqual(["github_copilot_auth_type"]); }); }); diff --git a/ui/litellm-dashboard/src/components/add_model/provider_auth_types.ts b/ui/litellm-dashboard/src/components/add_model/provider_auth_types.ts index 6fe54955133e..2837b2bcd341 100644 --- a/ui/litellm-dashboard/src/components/add_model/provider_auth_types.ts +++ b/ui/litellm-dashboard/src/components/add_model/provider_auth_types.ts @@ -7,6 +7,7 @@ export interface ProviderAuthType { readonly fieldKeys: readonly string[]; readonly requiredFieldKeys: readonly string[]; readonly fixedValues?: Readonly>; + readonly hiddenFieldKeys?: readonly string[]; readonly credentialOnly?: boolean; } @@ -17,14 +18,6 @@ export const GITHUB_COPILOT_PER_USER_AUTH_TYPE = "per_user_oauth"; export const PROVIDER_AUTH_TYPES: Partial> = { GITHUB_COPILOT: [ - { - id: "shared_device_login", - label: "Shared device login", - description: - "One GitHub device login on the proxy host, stored in its token file, is used for every caller of models on this credential.", - fieldKeys: ["api_base", "api_key"], - requiredFieldKeys: [], - }, { id: GITHUB_COPILOT_PER_USER_AUTH_TYPE, label: "Per-user GitHub OAuth", @@ -33,6 +26,7 @@ export const PROVIDER_AUTH_TYPES: Partial { - const selectedFieldKeys = authTypes.find(({ id }) => id === selectedId)?.fieldKeys ?? []; + const selectedAuthType = authTypes.find(({ id }) => id === selectedId); + const selectedFieldKeys = selectedAuthType?.fieldKeys ?? []; + const fieldsToHide = [ + ...(selectedAuthType?.hiddenFieldKeys ?? []), + ...authTypes.filter(({ id }) => id !== selectedId).flatMap(({ fieldKeys }) => fieldKeys), + ]; - return authTypes - .filter(({ id }) => id !== selectedId) - .flatMap(({ fieldKeys }) => fieldKeys) - .filter( - (fieldKey, index, fieldKeys) => !selectedFieldKeys.includes(fieldKey) && fieldKeys.indexOf(fieldKey) === index, - ); + return fieldsToHide.filter( + (fieldKey, index, fieldKeys) => !selectedFieldKeys.includes(fieldKey) && fieldKeys.indexOf(fieldKey) === index, + ); }; diff --git a/ui/litellm-dashboard/src/components/add_model/provider_specific_fields.tsx b/ui/litellm-dashboard/src/components/add_model/provider_specific_fields.tsx index a38b077270f8..17c7378308f0 100644 --- a/ui/litellm-dashboard/src/components/add_model/provider_specific_fields.tsx +++ b/ui/litellm-dashboard/src/components/add_model/provider_specific_fields.tsx @@ -118,10 +118,16 @@ const ProviderSpecificFieldsContent: React.FC = ({ const credentialsFileRef = React.useRef(null); React.useEffect(() => { - if (selectedAuthType) { - onAuthTypeChange?.(selectedAuthType.id); + if (!selectedAuthType) { + return; + } + onAuthTypeChange?.(selectedAuthType.id); + if (context !== "model" || !selectedAuthType.credentialOnly) { + Object.entries(selectedAuthType.fixedValues ?? {}).forEach(([fieldKey, fixedValue]) => { + form.setValue(fieldKey, fixedValue); + }); } - }, [onAuthTypeChange, selectedAuthType]); + }, [context, form, onAuthTypeChange, selectedAuthType]); const pickCredentialsFile = (onLoaded: (contents: string) => void) => (event: React.ChangeEvent) => { const file = event.target.files?.[0]; diff --git a/ui/litellm-dashboard/src/components/model_add/CredentialModal.integration.test.tsx b/ui/litellm-dashboard/src/components/model_add/CredentialModal.integration.test.tsx index c28260ca0951..9e48157c208a 100644 --- a/ui/litellm-dashboard/src/components/model_add/CredentialModal.integration.test.tsx +++ b/ui/litellm-dashboard/src/components/model_add/CredentialModal.integration.test.tsx @@ -524,9 +524,7 @@ describe("CredentialModal with GitHub Copilot auth types", () => { const user = userEvent.setup(); const onSubmit = renderModal({ initialProvider: "GITHUB_COPILOT" }); - expect(await screen.findByRole("combobox", { name: "Auth Type:" })).toHaveTextContent("Shared device login"); - expect(await screen.findByLabelText("API Key")).toBeInTheDocument(); - await chooseOption(user, /^Auth Type:/, "Per-user GitHub OAuth"); + expect(await screen.findByRole("combobox", { name: "Auth Type:" })).toHaveTextContent("Per-user GitHub OAuth"); expect(screen.queryByLabelText("API Key")).not.toBeInTheDocument(); expect(screen.queryByLabelText("API Base")).not.toBeInTheDocument(); fill("Credential Name:", "copilot-per-user"); @@ -542,31 +540,16 @@ describe("CredentialModal with GitHub Copilot auth types", () => { ); }); - it("keeps the shared device login credential free of the per-user auth type", async () => { - const user = userEvent.setup(); - const onSubmit = renderModal({ initialProvider: "GITHUB_COPILOT" }); - - await screen.findByLabelText("API Key"); - fill("Credential Name:", "copilot-shared"); - await user.click(screen.getByRole("button", { name: "Add Credential" })); - - expect(onSubmit).toHaveBeenCalledWith( - { credential_name: "copilot-shared", custom_llm_provider: "GITHUB_COPILOT" }, - [], - ); - }); - - it("opens a stored per-user credential on its auth type and deletes it when switched to shared", async () => { + it("preserves the per-user auth type when editing a stored credential", async () => { const user = userEvent.setup(); const onSubmit = renderModal({ mode: "edit", existingCredential: perUserCopilotCredential }); expect(await screen.findByRole("combobox", { name: "Auth Type:" })).toHaveTextContent("Per-user GitHub OAuth"); - await chooseOption(user, /^Auth Type:/, "Shared device login"); await user.click(screen.getByRole("button", { name: "Update Credential" })); const [values, valuesToDelete] = onSubmit.mock.calls[0]; expect(values).toEqual({ credential_name: "copilot-per-user", custom_llm_provider: "GITHUB_COPILOT" }); - expect(valuesToDelete).toEqual(["github_copilot_auth_type"]); + expect(valuesToDelete).toEqual([]); }); });