mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
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
This commit is contained in:
parent
40423e6ec0
commit
fed6b6e208
4 changed files with 104 additions and 72 deletions
|
|
@ -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]:
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue