From fed6b6e208ff894745e58b121866c97a991ad65c Mon Sep 17 00:00:00 2001 From: yyouretoast Date: Wed, 26 Aug 2026 14:32:30 +0400 Subject: [PATCH] fix(github_copilot): fix non-root Docker permission error and support token input - Remove eager _ensure_token_dir() from Authenticator.__init__() to prevent PermissionError during route resolution in non-root containers - Add PermissionError fallback to secure temp directory in _ensure_token_dir() - Accept access_token parameter in get_access_token/get_api_key/_refresh_api_key so tokens provided via UI API Key field or config are used directly - Pass api_key through in chat and embedding transformations Fixes #38329 --- litellm/llms/github_copilot/authenticator.py | 67 +++++++++--- .../github_copilot/chat/transformation.py | 4 +- .../embedding/transformation.py | 2 +- .../test_github_copilot_authenticator.py | 103 +++++++++--------- 4 files changed, 104 insertions(+), 72 deletions(-) diff --git a/litellm/llms/github_copilot/authenticator.py b/litellm/llms/github_copilot/authenticator.py index 80fd4f755e7..adca402601c 100644 --- a/litellm/llms/github_copilot/authenticator.py +++ b/litellm/llms/github_copilot/authenticator.py @@ -1,5 +1,6 @@ import json import os +import tempfile import time from datetime import datetime from typing import Any, Final @@ -37,18 +38,23 @@ class Authenticator: 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() - def get_access_token(self) -> str: + def get_access_token(self, access_token: str | None = None) -> str: """ Login to Copilot with retry 3 times. + Args: + access_token: Optional access token passed via config/request params. + Returns: str: The GitHub access token. Raises: GetAccessTokenError: If unable to obtain an access token after retries. """ + if access_token: + return access_token + try: with open(self.access_token_file, "r") as f: access_token = f.read().strip() @@ -62,6 +68,7 @@ class Authenticator: try: access_token = self._login() try: + self._ensure_token_dir() with open(self.access_token_file, "w") as f: f.write(access_token) except OSError: @@ -76,10 +83,13 @@ class Authenticator: status_code=401, ) - def get_api_key(self) -> str: + def get_api_key(self, access_token: str | None = None) -> str: """ Get the API key, refreshing if necessary. + Args: + access_token: Optional GitHub access token passed via config/request params. + Returns: str: The GitHub Copilot API key. @@ -105,9 +115,13 @@ class Authenticator: pass # Already logged in the try block try: - api_key_info = self._refresh_api_key() - with open(self.api_key_file, "w") as f: - json.dump(api_key_info, f) + api_key_info = self._refresh_api_key(access_token) + try: + self._ensure_token_dir() + with open(self.api_key_file, "w") as f: + json.dump(api_key_info, f) + except OSError as e: + verbose_logger.warning("Error saving API key to file (continuing with in-memory token): %s", e) token: Final = api_key_info.get("token") if token: return token @@ -116,12 +130,6 @@ class Authenticator: message="API key response missing token", status_code=401, ) - except OSError as e: - verbose_logger.error("Error saving API key to file: %s", e) - raise GetAPIKeyError( - message=f"Failed to save API key: {e}", - status_code=500, - ) except RefreshAPIKeyError as e: raise GetAPIKeyError( message=f"Failed to refresh API key: {e}", @@ -145,18 +153,21 @@ class Authenticator: verbose_logger.warning("Error reading API endpoint from file: %s", e) return None - def _refresh_api_key(self) -> dict[str, Any]: + def _refresh_api_key(self, access_token: str | None = None) -> dict[str, Any]: """ Refresh the API key using the access token. + Args: + access_token: Optional access token. If not provided, will call get_access_token(). + Returns: Dict[str, Any]: The API key information including token and expiration. Raises: RefreshAPIKeyError: If unable to refresh the API key. """ - access_token: Final = self.get_access_token() - headers: Final = self._get_github_headers(access_token) + resolved_token: Final = access_token or self.get_access_token() + headers: Final = self._get_github_headers(resolved_token) api_key_url: Final = os.getenv("GITHUB_COPILOT_API_KEY_URL", DEFAULT_GITHUB_API_KEY_URL) max_retries: Final = 3 @@ -183,9 +194,29 @@ class Authenticator: ) def _ensure_token_dir(self) -> None: - """Ensure the token directory exists.""" - if not os.path.exists(self.token_dir): - os.makedirs(self.token_dir, exist_ok=True) + """Ensure the token directory exists, falling back to temp directory on permission errors.""" + try: + if not os.path.exists(self.token_dir): + os.makedirs(self.token_dir, mode=0o700, exist_ok=True) + except (PermissionError, OSError) as e: + verbose_logger.warning( + "Cannot create token directory at %s (%s). Falling back to temp directory.", + self.token_dir, + e, + ) + self.token_dir = os.path.join(tempfile.gettempdir(), "litellm", "github_copilot") + try: + os.makedirs(self.token_dir, mode=0o700, exist_ok=True) + except OSError: + pass + self.access_token_file = os.path.join( + self.token_dir, + 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"), + ) def _get_github_headers(self, access_token: str | None = None) -> dict[str, str]: """ diff --git a/litellm/llms/github_copilot/chat/transformation.py b/litellm/llms/github_copilot/chat/transformation.py index 27a0028ce4a..dd3a4975e5d 100644 --- a/litellm/llms/github_copilot/chat/transformation.py +++ b/litellm/llms/github_copilot/chat/transformation.py @@ -42,7 +42,7 @@ class GithubCopilotConfig(OpenAIConfig): or DEFAULT_GITHUB_COPILOT_API_BASE ) try: - dynamic_api_key: Final = self.authenticator.get_api_key() + dynamic_api_key: Final = self.authenticator.get_api_key(access_token=api_key) except GetAPIKeyError as e: raise AuthenticationError( model=model, @@ -94,7 +94,7 @@ class GithubCopilotConfig(OpenAIConfig): # Add Copilot-specific headers (editor-version, user-agent, etc.) try: - copilot_api_key: Final = self.authenticator.get_api_key() + copilot_api_key: Final = self.authenticator.get_api_key(access_token=api_key) copilot_headers: Final = get_copilot_default_headers(copilot_api_key) validated_headers = {**copilot_headers, **validated_headers} except GetAPIKeyError: diff --git a/litellm/llms/github_copilot/embedding/transformation.py b/litellm/llms/github_copilot/embedding/transformation.py index 7ea7a89b4ca..15186a9d384 100644 --- a/litellm/llms/github_copilot/embedding/transformation.py +++ b/litellm/llms/github_copilot/embedding/transformation.py @@ -60,7 +60,7 @@ class GithubCopilotEmbeddingConfig(BaseEmbeddingConfig): """ try: # Get GitHub Copilot API key via OAuth - api_key = self.authenticator.get_api_key() + api_key = self.authenticator.get_api_key(access_token=api_key) if not api_key: raise AuthenticationError( 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 6c846a90c71..4055ec13bbf 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 @@ -1,6 +1,5 @@ import json import os -import time from datetime import datetime, timedelta from unittest.mock import MagicMock, mock_open, patch @@ -8,9 +7,7 @@ import pytest from litellm.llms.github_copilot.authenticator import Authenticator from litellm.llms.github_copilot.common_utils import ( - APIKeyExpiredError, GetAccessTokenError, - GetAPIKeyError, GetDeviceCodeError, RefreshAPIKeyError, ) @@ -19,13 +16,8 @@ from litellm.llms.github_copilot.common_utils import ( class TestGitHubCopilotAuthenticator: @pytest.fixture def authenticator(self): - with ( - patch("os.path.exists", return_value=False), - patch("os.makedirs") as mock_makedirs, - ): - auth = Authenticator() - mock_makedirs.assert_called_once() - return auth + auth = Authenticator() + return auth @pytest.fixture def mock_http_client(self): @@ -38,15 +30,13 @@ class TestGitHubCopilotAuthenticator: def test_init(self): """Test the initialization of the authenticator.""" - with ( - patch("os.path.exists", return_value=False), - patch("os.makedirs") as mock_makedirs, - ): + with patch("os.makedirs") as mock_makedirs: auth = Authenticator() - assert auth.token_dir.endswith("/github_copilot") - assert auth.access_token_file.endswith("/access-token") - assert auth.api_key_file.endswith("/api-key.json") - mock_makedirs.assert_called_once() + assert os.path.basename(auth.token_dir) == "github_copilot" + assert os.path.basename(auth.access_token_file) == "access-token" + assert os.path.basename(auth.api_key_file) == "api-key.json" + # Lazy init: directory is NOT created eagerly on __init__ + mock_makedirs.assert_not_called() def test_ensure_token_dir(self): """Test that the token directory is created if it doesn't exist.""" @@ -55,7 +45,20 @@ class TestGitHubCopilotAuthenticator: patch("os.makedirs") as mock_makedirs, ): auth = Authenticator() - mock_makedirs.assert_called_once_with(auth.token_dir, exist_ok=True) + auth._ensure_token_dir() + mock_makedirs.assert_called_once_with(auth.token_dir, mode=0o700, exist_ok=True) + + def test_ensure_token_dir_permission_error_fallback(self): + """Test that _ensure_token_dir falls back to temp directory on PermissionError.""" + auth = Authenticator() + original_dir = auth.token_dir + with ( + patch("os.path.exists", return_value=False), + patch("os.makedirs", side_effect=[PermissionError("Permission denied"), None]), + ): + auth._ensure_token_dir() + assert auth.token_dir != original_dir + assert "litellm" in auth.token_dir def test_get_github_headers(self, authenticator): """Test that GitHub headers are correctly generated.""" @@ -82,8 +85,7 @@ class TestGitHubCopilotAuthenticator: with ( patch.object(authenticator, "_login", return_value=mock_token), - patch("builtins.open", mock_open()), - patch("builtins.open", side_effect=IOError) as mock_read, + patch("builtins.open", side_effect=IOError), ): token = authenticator.get_access_token() assert token == mock_token @@ -106,9 +108,7 @@ class TestGitHubCopilotAuthenticator: 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() @@ -117,9 +117,7 @@ class TestGitHubCopilotAuthenticator: 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(), @@ -128,7 +126,7 @@ class TestGitHubCopilotAuthenticator: with ( patch("builtins.open", mock_open(read_data=mock_expired_data)), patch.object(authenticator, "_refresh_api_key", return_value=mock_new_data), - patch("json.dump") as mock_json_dump, + patch("json.dump"), ): api_key = authenticator.get_api_key() assert api_key == "new-api-key" @@ -217,20 +215,14 @@ class TestGitHubCopilotAuthenticator: 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): @@ -255,8 +247,10 @@ class TestGitHubCopilotAuthenticator: "user_code": "UC", "verification_uri": "https://example.com", } - with patch.dict(os.environ, {"GITHUB_COPILOT_DEVICE_CODE_URL": custom_url}), \ - patch("litellm.llms.github_copilot.authenticator._get_httpx_client", return_value=mock_client): + with ( + patch.dict(os.environ, {"GITHUB_COPILOT_DEVICE_CODE_URL": custom_url}), + patch("litellm.llms.github_copilot.authenticator._get_httpx_client", return_value=mock_client), + ): authenticator._get_device_code() assert mock_client.post.call_args[0][0] == custom_url @@ -269,8 +263,10 @@ class TestGitHubCopilotAuthenticator: "user_code": "UC", "verification_uri": "https://example.com", } - with patch.dict(os.environ, {"GITHUB_COPILOT_CLIENT_ID": custom_id}), \ - patch("litellm.llms.github_copilot.authenticator._get_httpx_client", return_value=mock_client): + with ( + patch.dict(os.environ, {"GITHUB_COPILOT_CLIENT_ID": custom_id}), + patch("litellm.llms.github_copilot.authenticator._get_httpx_client", return_value=mock_client), + ): authenticator._get_device_code() assert mock_client.post.call_args[1]["json"]["client_id"] == custom_id @@ -279,9 +275,11 @@ class TestGitHubCopilotAuthenticator: mock_client, mock_response = mock_http_client custom_url = "https://custom.example.com/token" mock_response.json.return_value = {"access_token": "tok"} - with patch.dict(os.environ, {"GITHUB_COPILOT_ACCESS_TOKEN_URL": custom_url}), \ - patch("litellm.llms.github_copilot.authenticator._get_httpx_client", return_value=mock_client), \ - patch("time.sleep"): + with ( + patch.dict(os.environ, {"GITHUB_COPILOT_ACCESS_TOKEN_URL": custom_url}), + patch("litellm.llms.github_copilot.authenticator._get_httpx_client", return_value=mock_client), + patch("time.sleep"), + ): authenticator._poll_for_access_token("dc") assert mock_client.post.call_args[0][0] == custom_url @@ -290,9 +288,11 @@ class TestGitHubCopilotAuthenticator: mock_client, mock_response = mock_http_client custom_id = "custom_client_id" mock_response.json.return_value = {"access_token": "tok"} - with patch.dict(os.environ, {"GITHUB_COPILOT_CLIENT_ID": custom_id}), \ - patch("litellm.llms.github_copilot.authenticator._get_httpx_client", return_value=mock_client), \ - patch("time.sleep"): + with ( + patch.dict(os.environ, {"GITHUB_COPILOT_CLIENT_ID": custom_id}), + patch("litellm.llms.github_copilot.authenticator._get_httpx_client", return_value=mock_client), + patch("time.sleep"), + ): authenticator._poll_for_access_token("dc") assert mock_client.post.call_args[1]["json"]["client_id"] == custom_id @@ -301,9 +301,10 @@ class TestGitHubCopilotAuthenticator: mock_client, mock_response = mock_http_client custom_url = "https://custom.example.com/api-key" mock_response.json.return_value = {"token": "api-tok", "expires_at": 9999999999} - with patch.dict(os.environ, {"GITHUB_COPILOT_API_KEY_URL": custom_url}), \ - patch("litellm.llms.github_copilot.authenticator._get_httpx_client", return_value=mock_client), \ - patch.object(authenticator, "get_access_token", return_value="access-tok"): + with ( + patch.dict(os.environ, {"GITHUB_COPILOT_API_KEY_URL": custom_url}), + patch("litellm.llms.github_copilot.authenticator._get_httpx_client", return_value=mock_client), + patch.object(authenticator, "get_access_token", return_value="access-tok"), + ): authenticator._refresh_api_key() assert mock_client.get.call_args[0][0] == custom_url -