diff --git a/litellm/llms/github_copilot/authenticator.py b/litellm/llms/github_copilot/authenticator.py index 2b96ee6d79a..3ad35ebd620 100644 --- a/litellm/llms/github_copilot/authenticator.py +++ b/litellm/llms/github_copilot/authenticator.py @@ -2,10 +2,9 @@ import json import os import time from typing import Final +from urllib.parse import urlsplit import httpx -from pydantic import TypeAdapter, ValidationError -from typing_extensions import TypedDict from litellm._logging import verbose_logger from litellm.llms.custom_httpx.http_handler import _get_httpx_client @@ -18,22 +17,24 @@ from .common_utils import ( ) -class _LegacyCopilotEndpoints(TypedDict, total=False): - api: str - - -class _LegacyCopilotTokenCache(TypedDict, total=False): - endpoints: _LegacyCopilotEndpoints - - -_LEGACY_COPILOT_TOKEN_CACHE_ADAPTER = TypeAdapter(_LegacyCopilotTokenCache) - # Constants (default values — overridable via environment variables at call time) DEFAULT_GITHUB_CLIENT_ID: Final = "Iv1.b507a08c87ecfe98" 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" +def _is_secure_api_base(api_base: str) -> bool: + parsed_api_base = urlsplit(api_base) + return ( + parsed_api_base.scheme.lower() == "https" + and parsed_api_base.hostname is not None + and parsed_api_base.username is None + and parsed_api_base.password is None + and not parsed_api_base.query + and not parsed_api_base.fragment + ) + + class Authenticator: def __init__(self) -> None: """Initialize the GitHub Copilot authenticator with configurable token paths.""" @@ -46,14 +47,6 @@ class Authenticator: self.token_dir, os.getenv("GITHUB_COPILOT_ACCESS_TOKEN_FILE", "access-token"), ) - self.legacy_api_key_file = os.path.join( - self.token_dir, - os.getenv("GITHUB_COPILOT_API_KEY_FILE", "api-key.json"), - ) - if os.getenv("GITHUB_COPILOT_API_KEY_URL"): - verbose_logger.warning( - "GITHUB_COPILOT_API_KEY_URL is no longer used; LiteLLM sends the OAuth access token directly" - ) self._ensure_token_dir() def get_access_token(self) -> str: @@ -104,17 +97,14 @@ class Authenticator: def get_api_base(self) -> str | None: configured_api_base = os.getenv("GITHUB_COPILOT_API_BASE") - if configured_api_base: - return configured_api_base - try: - with open(self.legacy_api_key_file, "r") as legacy_api_key_file: - legacy_cache = _LEGACY_COPILOT_TOKEN_CACHE_ADAPTER.validate_json(legacy_api_key_file.read()) - except IOError: + if configured_api_base is None: return None - except ValidationError as e: - verbose_logger.warning(f"Error reading legacy GitHub Copilot API endpoint: {str(e)}") + if not _is_secure_api_base(configured_api_base): + verbose_logger.warning( + "Ignoring GITHUB_COPILOT_API_BASE because it must be an HTTPS URL without credentials, query, or fragment" + ) return None - return legacy_cache.get("endpoints", {}).get("api") + return configured_api_base def _ensure_token_dir(self) -> None: """Ensure the token directory exists.""" diff --git a/tests/test_litellm/llms/github_copilot/test_github_copilot_authenticator.py b/tests/test_litellm/llms/github_copilot/test_github_copilot_authenticator.py index a165a9f7d46..79fe94dc5c1 100644 --- a/tests/test_litellm/llms/github_copilot/test_github_copilot_authenticator.py +++ b/tests/test_litellm/llms/github_copilot/test_github_copilot_authenticator.py @@ -41,7 +41,6 @@ class TestGitHubCopilotAuthenticator: auth = Authenticator() assert auth.token_dir.endswith("/github_copilot") assert auth.access_token_file.endswith("/access-token") - assert auth.legacy_api_key_file.endswith("/api-key.json") mock_makedirs.assert_called_once() def test_ensure_token_dir(self): @@ -54,52 +53,37 @@ class TestGitHubCopilotAuthenticator: mock_makedirs.assert_called_once_with(auth.token_dir, exist_ok=True) def test_get_api_base_prefers_environment(self, authenticator): - with ( - patch.dict( - os.environ, - {"GITHUB_COPILOT_API_BASE": "https://configured.githubcopilot.example"}, - clear=True, - ), - patch("builtins.open", mock_open()) as mock_file, + with patch.dict( + os.environ, + {"GITHUB_COPILOT_API_BASE": "https://configured.githubcopilot.example"}, + clear=True, ): assert authenticator.get_api_base() == "https://configured.githubcopilot.example" - mock_file.assert_not_called() - def test_get_api_base_uses_legacy_endpoint(self, authenticator): - legacy_cache = '{"token":"ignored","endpoints":{"api":"https://api.enterprise.githubcopilot.com"}}' + @pytest.mark.parametrize( + "api_base", + ( + "http://api.githubcopilot.com", + "https://user:password@api.githubcopilot.com", + "https://api.githubcopilot.com?tenant=example", + "https://api.githubcopilot.com#fragment", + ), + ) + def test_get_api_base_rejects_insecure_configuration(self, authenticator, api_base): with ( - patch.dict(os.environ, {}, clear=True), - patch("builtins.open", mock_open(read_data=legacy_cache)), - ): - assert authenticator.get_api_base() == "https://api.enterprise.githubcopilot.com" - - def test_get_api_base_ignores_invalid_legacy_cache(self, authenticator): - with ( - patch.dict(os.environ, {}, clear=True), - patch("builtins.open", mock_open(read_data="not-json")), + patch.dict(os.environ, {"GITHUB_COPILOT_API_BASE": api_base}, clear=True), patch("litellm.llms.github_copilot.authenticator.verbose_logger.warning") as mock_warning, ): assert authenticator.get_api_base() is None - assert mock_warning.call_count == 1 - assert "Error reading legacy GitHub Copilot API endpoint" in mock_warning.call_args.args[0] - - def test_deprecated_api_key_url_warns(self): - with ( - patch.dict( - os.environ, - {"GITHUB_COPILOT_API_KEY_URL": "https://deprecated.example.com/token"}, - clear=True, - ), - patch("os.path.exists", return_value=True), - patch("litellm.llms.github_copilot.authenticator.verbose_logger.warning") as mock_warning, - ): - Authenticator() - mock_warning.assert_called_once_with( - "GITHUB_COPILOT_API_KEY_URL is no longer used; LiteLLM sends the OAuth access token directly" + "Ignoring GITHUB_COPILOT_API_BASE because it must be an HTTPS URL without credentials, query, or fragment" ) + def test_get_api_base_uses_default_when_unconfigured(self, authenticator): + with patch.dict(os.environ, {}, clear=True): + assert authenticator.get_api_base() is None + def test_get_github_headers(self, authenticator): headers = authenticator._get_github_headers() assert headers == {