fix(github_copilot): address PR review feedback

Tighten the Anthropic Messages environment validation and web search interception behavior after review feedback. Avoid treating non-web-search requests as web-search-only paths.
This commit is contained in:
ririnto 2026-05-16 19:42:58 +09:00
parent 3063124e37
commit 323bf577a6
3 changed files with 80 additions and 29 deletions

View file

@ -130,19 +130,25 @@ class WebSearchInterceptionLogger(CustomLogger):
# includes a follow-up LLM call to synthesize the answer from search
# results. Short-circuiting those would skip that synthesis step and
# return raw search text — a regression for existing users.
#
# github_copilot is the exception: it has a BaseAnthropicMessagesConfig
# (added for thinking passthrough), but Copilot does not handle
# web_search tools natively, so we still need the short-circuit for
# web-search-only requests against Copilot.
try:
provider_enum = LlmProviders(provider_str)
anthropic_config = (
ProviderConfigManager.get_provider_anthropic_messages_config(
model=model, provider=provider_enum
if provider_enum != LlmProviders.GITHUB_COPILOT:
anthropic_config = (
ProviderConfigManager.get_provider_anthropic_messages_config(
model=model, provider=provider_enum
)
)
)
if anthropic_config is not None:
verbose_logger.debug(
f"WebSearchInterception: Skipping short-circuit for {provider_str} "
"(provider has native Anthropic Messages support, using agentic loop)"
)
return None
if anthropic_config is not None:
verbose_logger.debug(
f"WebSearchInterception: Skipping short-circuit for {provider_str} "
"(provider has native Anthropic Messages support, using agentic loop)"
)
return None
except (ValueError, Exception):
pass # unknown provider enum → safe to short-circuit

View file

@ -35,12 +35,15 @@ class GithubCopilotAnthropicMessagesConfig(AnthropicMessagesConfig):
) -> Tuple[dict, Optional[str]]:
"""
Validate environment for GitHub Copilot and add Copilot-specific headers.
The caller-supplied ``api_base`` is intentionally ignored. Routing this
request anywhere other than the authenticated Copilot endpoint would
leak the Copilot bearer token to a caller-controlled URL.
"""
# Get Copilot auth credentials
# Always use the Copilot endpoint resolved from the authenticated
# session, never the caller-supplied api_base.
dynamic_api_base = (
api_base
or self.authenticator.get_api_base()
or DEFAULT_GITHUB_COPILOT_API_BASE
self.authenticator.get_api_base() or DEFAULT_GITHUB_COPILOT_API_BASE
)
try:
dynamic_api_key = self.authenticator.get_api_key()
@ -61,6 +64,12 @@ class GithubCopilotAnthropicMessagesConfig(AnthropicMessagesConfig):
if "anthropic-version" not in headers:
headers["anthropic-version"] = "2023-06-01"
# Auto-inject anthropic-beta headers for advanced features
# (context_management, tool_search, output_format, speed)
headers = self._update_headers_with_anthropic_beta(
headers, optional_params, custom_llm_provider="github_copilot"
)
return headers, dynamic_api_base
def get_complete_url(
@ -74,8 +83,13 @@ class GithubCopilotAnthropicMessagesConfig(AnthropicMessagesConfig):
) -> str:
"""
Return the complete URL for GitHub Copilot /v1/messages endpoint.
The caller-supplied ``api_base`` is intentionally ignored to avoid
leaking the Copilot bearer token to a caller-controlled URL.
"""
api_base = api_base or DEFAULT_GITHUB_COPILOT_API_BASE
if not api_base.endswith("/v1/messages"):
api_base = f"{api_base}/v1/messages"
return api_base
resolved = (
self.authenticator.get_api_base() or DEFAULT_GITHUB_COPILOT_API_BASE
)
if not resolved.endswith("/v1/messages"):
resolved = f"{resolved}/v1/messages"
return resolved

View file

@ -6,10 +6,11 @@ import pytest
sys.path.insert(0, os.path.abspath("../.."))
from litellm.exceptions import AuthenticationError
from litellm.llms.github_copilot.common_utils import GetAPIKeyError
from litellm.llms.github_copilot.messages.transformation import (
GithubCopilotAnthropicMessagesConfig,
)
from litellm.llms.github_copilot.common_utils import GetAPIKeyError
def test_github_copilot_anthropic_messages_config_init():
@ -20,10 +21,12 @@ def test_github_copilot_anthropic_messages_config_init():
def test_github_copilot_anthropic_messages_get_complete_url():
"""Test URL construction for GitHub Copilot messages endpoint."""
"""URL is always resolved from the authenticator, never the caller."""
config = GithubCopilotAnthropicMessagesConfig()
config.authenticator = MagicMock()
config.authenticator.get_api_base.return_value = None
# Test with default api_base
# No api_base supplied -> default Copilot endpoint
url = config.get_complete_url(
api_base=None,
api_key=None,
@ -33,25 +36,28 @@ def test_github_copilot_anthropic_messages_get_complete_url():
)
assert url == "https://api.githubcopilot.com/v1/messages"
# Test with custom api_base
# Caller-supplied api_base must be ignored (token-exfiltration guard).
url = config.get_complete_url(
api_base="https://custom.api.com",
api_base="https://attacker.example.com",
api_key=None,
model="github_copilot/claude-haiku-4.5",
optional_params={},
litellm_params={},
)
assert url == "https://custom.api.com/v1/messages"
assert url == "https://api.githubcopilot.com/v1/messages"
# Test with api_base already ending with /v1/messages
# Authenticator-provided base is honored (e.g. business/enterprise tenants).
config.authenticator.get_api_base.return_value = (
"https://api.business.githubcopilot.com"
)
url = config.get_complete_url(
api_base="https://custom.api.com/v1/messages",
api_base="https://attacker.example.com",
api_key=None,
model="github_copilot/claude-haiku-4.5",
optional_params={},
litellm_params={},
)
assert url == "https://custom.api.com/v1/messages"
assert url == "https://api.business.githubcopilot.com/v1/messages"
def test_github_copilot_anthropic_messages_validate_environment():
@ -64,6 +70,7 @@ def test_github_copilot_anthropic_messages_validate_environment():
config.authenticator.get_api_base.return_value = None
headers = {}
# Pass a hostile api_base to confirm it is ignored.
validated_headers, api_base = config.validate_anthropic_messages_environment(
headers=headers,
model="github_copilot/claude-haiku-4.5",
@ -71,7 +78,7 @@ def test_github_copilot_anthropic_messages_validate_environment():
optional_params={},
litellm_params={},
api_key=None,
api_base=None,
api_base="https://attacker.example.com",
)
# Check that Copilot headers were added
@ -80,9 +87,33 @@ def test_github_copilot_anthropic_messages_validate_environment():
assert "Authorization" in validated_headers
assert "anthropic-version" in validated_headers
assert validated_headers["anthropic-version"] == "2023-06-01"
# api_base must come from the authenticator, never the caller.
assert api_base == "https://api.githubcopilot.com"
def test_github_copilot_anthropic_messages_validate_environment_injects_beta_headers():
"""Anthropic-beta headers must be auto-injected for advanced features
(context_management, output_format, etc.) — matches the parent
AnthropicMessagesConfig contract."""
config = GithubCopilotAnthropicMessagesConfig()
config.authenticator = MagicMock()
config.authenticator.get_api_key.return_value = "gh.test-key"
config.authenticator.get_api_base.return_value = None
validated_headers, _ = config.validate_anthropic_messages_environment(
headers={},
model="github_copilot/claude-haiku-4.5",
messages=[{"role": "user", "content": "Hello"}],
optional_params={"output_format": {"type": "json_object"}},
litellm_params={},
api_key=None,
api_base=None,
)
assert "anthropic-beta" in validated_headers
assert "structured-outputs-2025-11-13" in validated_headers["anthropic-beta"]
def test_github_copilot_anthropic_messages_validate_environment_auth_error():
"""Test error handling when authentication fails."""
config = GithubCopilotAnthropicMessagesConfig()
@ -93,7 +124,7 @@ def test_github_copilot_anthropic_messages_validate_environment_auth_error():
status_code=401, message="No valid API key found"
)
with pytest.raises(Exception): # AuthenticationError
with pytest.raises(AuthenticationError):
config.validate_anthropic_messages_environment(
headers={},
model="github_copilot/claude-haiku-4.5",