From 197592495615d85e6086ed11f13fe5db6de1ec12 Mon Sep 17 00:00:00 2001 From: codgician <15964984+codgician@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:27:33 +0800 Subject: [PATCH] fix(github-copilot): constrain OAuth endpoints --- litellm/llms/github_copilot/authenticator.py | 68 +++++++++++++------ .../github_copilot/chat/transformation.py | 2 +- .../embedding/transformation.py | 2 +- .../responses/transformation.py | 2 +- ...github_copilot_embedding_transformation.py | 8 ++- ...github_copilot_responses_transformation.py | 12 ++-- .../test_github_copilot_authenticator.py | 52 +++++++++++++- .../test_github_copilot_transformation.py | 10 +-- 8 files changed, 119 insertions(+), 37 deletions(-) diff --git a/litellm/llms/github_copilot/authenticator.py b/litellm/llms/github_copilot/authenticator.py index 3ad35ebd620..e7d84835fee 100644 --- a/litellm/llms/github_copilot/authenticator.py +++ b/litellm/llms/github_copilot/authenticator.py @@ -23,15 +23,43 @@ 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 +def _https_hostname(url: str) -> str | None: + parsed_url = urlsplit(url) + if ( + parsed_url.scheme.lower() != "https" + or parsed_url.hostname is None + or parsed_url.username is not None + or parsed_url.password is not None + or parsed_url.query + or parsed_url.fragment + ): + return None + return parsed_url.hostname.lower() + + +def _configured_allowed_api_hosts() -> frozenset[str]: + configured_hosts = os.getenv("GITHUB_COPILOT_ALLOWED_API_HOSTS", "") + return frozenset(host.strip().lower() for host in configured_hosts.split(",") if host.strip()) + + +def _configured_oauth_hosts() -> tuple[str, ...]: + oauth_urls = ( + os.getenv("GITHUB_COPILOT_DEVICE_CODE_URL", DEFAULT_GITHUB_DEVICE_CODE_URL), + os.getenv("GITHUB_COPILOT_ACCESS_TOKEN_URL", DEFAULT_GITHUB_ACCESS_TOKEN_URL), + ) + return tuple(hostname for url in oauth_urls if (hostname := _https_hostname(url)) is not None) + + +def _is_trusted_api_base(api_base: str) -> bool: + hostname = _https_hostname(api_base) + if hostname is None: + return False + if hostname == "githubcopilot.com" or hostname.endswith(".githubcopilot.com"): + return True + if hostname in _configured_allowed_api_hosts(): + return True + return any( + hostname == oauth_host or hostname.endswith(f".{oauth_host}") for oauth_host in _configured_oauth_hosts() ) @@ -95,16 +123,18 @@ class Authenticator: status_code=401, ) - def get_api_base(self) -> str | None: - configured_api_base = os.getenv("GITHUB_COPILOT_API_BASE") - if configured_api_base is None: - return None - 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 configured_api_base + def get_api_base(self, api_base: str | None = None) -> str | None: + candidates = ( + ("deployment api_base", api_base), + ("GITHUB_COPILOT_API_BASE", os.getenv("GITHUB_COPILOT_API_BASE")), + ) + for source, candidate in candidates: + if candidate is None: + continue + if _is_trusted_api_base(candidate): + return candidate + verbose_logger.warning(f"Ignoring {source} because it is not a trusted HTTPS GitHub Copilot endpoint") + return None def _ensure_token_dir(self) -> None: """Ensure the token directory exists.""" diff --git a/litellm/llms/github_copilot/chat/transformation.py b/litellm/llms/github_copilot/chat/transformation.py index 2130dd10a9f..0609de186fa 100644 --- a/litellm/llms/github_copilot/chat/transformation.py +++ b/litellm/llms/github_copilot/chat/transformation.py @@ -34,7 +34,7 @@ class GithubCopilotConfig(OpenAIConfig): api_key: str | None, custom_llm_provider: str, ) -> tuple[str | None, str | None, str]: - dynamic_api_base: Final = self.authenticator.get_api_base() or DEFAULT_GITHUB_COPILOT_API_BASE + dynamic_api_base: Final = self.authenticator.get_api_base(api_base) or DEFAULT_GITHUB_COPILOT_API_BASE try: dynamic_api_key: Final = self.authenticator.get_api_key() except GetAPIKeyError as e: diff --git a/litellm/llms/github_copilot/embedding/transformation.py b/litellm/llms/github_copilot/embedding/transformation.py index e5e35059ce2..8740c633b76 100644 --- a/litellm/llms/github_copilot/embedding/transformation.py +++ b/litellm/llms/github_copilot/embedding/transformation.py @@ -97,7 +97,7 @@ class GithubCopilotEmbeddingConfig(BaseEmbeddingConfig): """ Get the complete URL for GitHub Copilot Embedding API endpoint. """ - effective_api_base = self.authenticator.get_api_base() or DEFAULT_GITHUB_COPILOT_API_BASE + effective_api_base = self.authenticator.get_api_base(api_base) or DEFAULT_GITHUB_COPILOT_API_BASE # Remove trailing slashes effective_api_base = effective_api_base.rstrip("/") diff --git a/litellm/llms/github_copilot/responses/transformation.py b/litellm/llms/github_copilot/responses/transformation.py index 526b240354f..759f1c66627 100644 --- a/litellm/llms/github_copilot/responses/transformation.py +++ b/litellm/llms/github_copilot/responses/transformation.py @@ -247,7 +247,7 @@ class GithubCopilotResponsesAPIConfig(OpenAIResponsesAPIConfig): """ Get the complete URL for GitHub Copilot Responses API endpoint. """ - effective_api_base = self.authenticator.get_api_base() or DEFAULT_GITHUB_COPILOT_API_BASE + effective_api_base = self.authenticator.get_api_base(api_base) or DEFAULT_GITHUB_COPILOT_API_BASE # Remove trailing slashes effective_api_base = effective_api_base.rstrip("/") diff --git a/tests/test_litellm/llms/github_copilot/embedding/test_github_copilot_embedding_transformation.py b/tests/test_litellm/llms/github_copilot/embedding/test_github_copilot_embedding_transformation.py index 87921a66297..2d4177634bf 100644 --- a/tests/test_litellm/llms/github_copilot/embedding/test_github_copilot_embedding_transformation.py +++ b/tests/test_litellm/llms/github_copilot/embedding/test_github_copilot_embedding_transformation.py @@ -76,7 +76,9 @@ def test_github_copilot_embedding_config_get_complete_url(): assert url == "https://api.githubcopilot.com/embeddings" # Test with custom API base from authenticator - config.authenticator.get_api_base.return_value = "https://api.enterprise.githubcopilot.com" + config.authenticator.get_api_base.side_effect = lambda api_base=None: ( + api_base or "https://api.enterprise.githubcopilot.com" + ) url = config.get_complete_url( api_base=None, api_key=None, @@ -87,13 +89,13 @@ def test_github_copilot_embedding_config_get_complete_url(): assert url == "https://api.enterprise.githubcopilot.com/embeddings" url = config.get_complete_url( - api_base="https://untrusted.example.com", + api_base="https://api.business.githubcopilot.com", api_key=None, model="github_copilot/text-embedding-3-small", optional_params={}, litellm_params={}, ) - assert url == "https://api.enterprise.githubcopilot.com/embeddings" + assert url == "https://api.business.githubcopilot.com/embeddings" def test_github_copilot_embedding_config_transform_request(): diff --git a/tests/test_litellm/llms/github_copilot/responses/test_github_copilot_responses_transformation.py b/tests/test_litellm/llms/github_copilot/responses/test_github_copilot_responses_transformation.py index de0fc41a6a9..925912bfb31 100644 --- a/tests/test_litellm/llms/github_copilot/responses/test_github_copilot_responses_transformation.py +++ b/tests/test_litellm/llms/github_copilot/responses/test_github_copilot_responses_transformation.py @@ -56,7 +56,9 @@ class TestGithubCopilotResponsesAPITransformation: """Test that get_complete_url returns correct GitHub Copilot endpoint""" # Mock authenticator to return default base mock_auth_instance = MagicMock() - mock_auth_instance.get_api_base.return_value = "https://api.individual.githubcopilot.com" + mock_auth_instance.get_api_base.side_effect = lambda api_base=None: ( + api_base or "https://api.individual.githubcopilot.com" + ) mock_authenticator_class.return_value = mock_auth_instance config = GithubCopilotResponsesAPIConfig() @@ -67,11 +69,11 @@ class TestGithubCopilotResponsesAPITransformation: f"Expected GitHub Copilot responses endpoint, got {url}" ) - custom_url = config.get_complete_url(api_base="https://untrusted.example.com", litellm_params={}) - assert custom_url == "https://api.individual.githubcopilot.com/responses" + custom_url = config.get_complete_url(api_base="https://api.business.githubcopilot.com", litellm_params={}) + assert custom_url == "https://api.business.githubcopilot.com/responses" - url_with_slash = config.get_complete_url(api_base="https://untrusted.example.com/", litellm_params={}) - assert url_with_slash == "https://api.individual.githubcopilot.com/responses" + url_with_slash = config.get_complete_url(api_base="https://api.githubcopilot.com/", litellm_params={}) + assert url_with_slash == "https://api.githubcopilot.com/responses" @patch("litellm.llms.github_copilot.responses.transformation.Authenticator") def test_validate_environment_default_headers(self, mock_authenticator_class, monkeypatch): 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 79fe94dc5c1..4e5023d11de 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 @@ -55,10 +55,10 @@ class TestGitHubCopilotAuthenticator: def test_get_api_base_prefers_environment(self, authenticator): with patch.dict( os.environ, - {"GITHUB_COPILOT_API_BASE": "https://configured.githubcopilot.example"}, + {"GITHUB_COPILOT_API_BASE": "https://api.enterprise.githubcopilot.com"}, clear=True, ): - assert authenticator.get_api_base() == "https://configured.githubcopilot.example" + assert authenticator.get_api_base() == "https://api.enterprise.githubcopilot.com" @pytest.mark.parametrize( "api_base", @@ -67,6 +67,7 @@ class TestGitHubCopilotAuthenticator: "https://user:password@api.githubcopilot.com", "https://api.githubcopilot.com?tenant=example", "https://api.githubcopilot.com#fragment", + "https://attacker.example.com", ), ) def test_get_api_base_rejects_insecure_configuration(self, authenticator, api_base): @@ -77,13 +78,58 @@ class TestGitHubCopilotAuthenticator: assert authenticator.get_api_base() is None mock_warning.assert_called_once_with( - "Ignoring GITHUB_COPILOT_API_BASE because it must be an HTTPS URL without credentials, query, or fragment" + "Ignoring GITHUB_COPILOT_API_BASE because it is not a trusted HTTPS GitHub Copilot endpoint" ) 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_api_base_trusts_enterprise_oauth_domain(self, authenticator): + environment = { + "GITHUB_COPILOT_API_BASE": "https://copilot-api.company.ghe.com", + "GITHUB_COPILOT_DEVICE_CODE_URL": "https://company.ghe.com/login/device/code", + "GITHUB_COPILOT_ACCESS_TOKEN_URL": "https://company.ghe.com/login/oauth/access_token", + } + with patch.dict(os.environ, environment, clear=True): + assert authenticator.get_api_base() == "https://copilot-api.company.ghe.com" + + def test_get_api_base_trusts_explicit_allowed_host(self, authenticator): + environment = { + "GITHUB_COPILOT_API_BASE": "https://copilot-proxy.example.com", + "GITHUB_COPILOT_ALLOWED_API_HOSTS": "copilot-proxy.example.com", + } + with patch.dict(os.environ, environment, clear=True): + assert authenticator.get_api_base() == "https://copilot-proxy.example.com" + + def test_get_api_base_prefers_trusted_deployment_endpoint(self, authenticator): + with patch.dict( + os.environ, + {"GITHUB_COPILOT_API_BASE": "https://api.individual.githubcopilot.com"}, + clear=True, + ): + assert ( + authenticator.get_api_base("https://api.enterprise.githubcopilot.com") + == "https://api.enterprise.githubcopilot.com" + ) + + def test_get_api_base_falls_back_from_untrusted_deployment_endpoint(self, authenticator): + with ( + patch.dict( + os.environ, + {"GITHUB_COPILOT_API_BASE": "https://api.individual.githubcopilot.com"}, + clear=True, + ), + patch("litellm.llms.github_copilot.authenticator.verbose_logger.warning") as mock_warning, + ): + assert ( + authenticator.get_api_base("https://attacker.example.com") == "https://api.individual.githubcopilot.com" + ) + + mock_warning.assert_called_once_with( + "Ignoring deployment api_base because it is not a trusted HTTPS GitHub Copilot endpoint" + ) + def test_get_github_headers(self, authenticator): headers = authenticator._get_github_headers() assert headers == { diff --git a/tests/test_litellm/llms/github_copilot/test_github_copilot_transformation.py b/tests/test_litellm/llms/github_copilot/test_github_copilot_transformation.py index 856a99d0859..e71224b1030 100644 --- a/tests/test_litellm/llms/github_copilot/test_github_copilot_transformation.py +++ b/tests/test_litellm/llms/github_copilot/test_github_copilot_transformation.py @@ -39,7 +39,9 @@ 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.side_effect = lambda api_base=None: ( + api_base or "https://api.enterprise.githubcopilot.com" + ) # Test with default values model = "github_copilot/gpt-4" @@ -59,14 +61,14 @@ def test_github_copilot_config_get_openai_compatible_provider_info(): assert custom_llm_provider == "github_copilot" api_base, _, _ = config._get_openai_compatible_provider_info( model=model, - api_base="https://untrusted.example.com", + api_base="https://api.business.githubcopilot.com", api_key=None, custom_llm_provider="github_copilot", ) - assert api_base == "https://api.enterprise.githubcopilot.com" + assert api_base == "https://api.business.githubcopilot.com" # Test fallback to default if no dynamic endpoint - config.authenticator.get_api_base.return_value = None + config.authenticator.get_api_base.side_effect = lambda api_base=None: None ( api_base, dynamic_api_key,