Skip to content

Commit aa38e9c

Browse files
Tejas ChopraTejas Chopra
authored andcommitted
fix(copilot): match the completions override by host, not whole string
is_copilot_completions_host is asked about two different shapes: a bare base URL when routing, and the base URL plus the path when attaching auth. Comparing GITHUB_COPILOT_PROXY_URL by whole-string equality answered True for the first and False for the second, so an operator gateway was routed to correctly and then forwarded with no credentials -- a 401 on the one configuration that is the documented remedy for a custom deployment. Compare hosts instead, via a shared _url_host() that also tolerates a scheme-less value, and use it for the configured-API-URL check so a hand-written 'api.githubcopilot.com' is still recognised as public rather than read as a custom deployment.
1 parent 153a1d4 commit aa38e9c

2 files changed

Lines changed: 83 additions & 12 deletions

File tree

headroom/copilot_auth.py

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,18 @@ def reset_observed_completions_endpoint() -> None:
291291
_observed_completions_base_url = None
292292

293293

294+
def _url_host(value: str) -> str:
295+
"""Hostname for a URL, tolerating a scheme-less value.
296+
297+
Mirrors the normalization :func:`is_copilot_api_url` performs, so a host
298+
configured without "https://" is not silently treated as a different host.
299+
"""
300+
301+
parsed = urlparse(value)
302+
netloc_or_path = parsed.netloc.lower() or parsed.path.lower()
303+
return (parsed.hostname or netloc_or_path.split("/", 1)[0]).lower()
304+
305+
294306
def is_copilot_completions_host(url: str | None) -> bool:
295307
"""Return True when *url* already points at a Copilot inline-completions host.
296308
@@ -302,17 +314,17 @@ def is_copilot_completions_host(url: str | None) -> bool:
302314

303315
if not url:
304316
return False
305-
override = os.environ.get("GITHUB_COPILOT_PROXY_URL", "").strip().rstrip("/")
306-
if override and url.rstrip("/") == override:
307-
return True
308-
# Tolerate a scheme-less value the same way ``is_copilot_api_url`` does; a
309-
# configured host without "https://" must not silently stop being Copilot,
310-
# because the consequence is a request forwarded without credentials.
311-
parsed = urlparse(url)
312-
netloc_or_path = parsed.netloc.lower() or parsed.path.lower()
313-
host = (parsed.hostname or netloc_or_path.split("/", 1)[0]).lower()
317+
# Compare hosts, never whole strings: this is asked both about a bare base
318+
# URL (routing) and about a fully-built URL with the path appended (auth).
319+
# A string compare answers True for the first and False for the second, so
320+
# an operator override would route correctly and then be forwarded with no
321+
# credentials at all.
322+
host = _url_host(url)
314323
if not host:
315324
return False
325+
override = os.environ.get("GITHUB_COPILOT_PROXY_URL", "").strip()
326+
if override and host == _url_host(override):
327+
return True
316328
if host == "copilot-proxy.githubusercontent.com":
317329
return True
318330
# Per-SKU hosts GitHub hands out via `endpoints.proxy`, e.g.
@@ -353,9 +365,7 @@ def copilot_completions_base_url() -> str:
353365
if _observed_completions_base_url:
354366
return _observed_completions_base_url
355367
configured = _configured_api_url_override()
356-
if configured and not _is_public_copilot_api_host(
357-
(urlparse(configured).hostname or "").lower()
358-
):
368+
if configured and not _is_public_copilot_api_host(_url_host(configured)):
359369
return configured
360370
return DEFAULT_COMPLETIONS_PROXY_URL
361371

tests/test_copilot_vscode_completions_routing.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,3 +485,64 @@ def test_a_custom_deployment_still_keeps_its_own_host(
485485
monkeypatch.setenv("GITHUB_COPILOT_API_URL", configured_api_url)
486486

487487
assert copilot_completions_base_url() == configured_api_url
488+
489+
490+
# --------------------------------------------------------------------------- #
491+
# The two callers pass different shapes of URL
492+
# --------------------------------------------------------------------------- #
493+
def test_an_operator_override_is_matched_on_the_full_request_url(
494+
monkeypatch: pytest.MonkeyPatch,
495+
) -> None:
496+
"""Routing sees a base URL; auth sees the base URL *plus the path*.
497+
498+
Matching the override by whole-string equality answered True for the first
499+
and False for the second, so an operator gateway was routed to correctly and
500+
then forwarded with no credentials — a 401 on the one configuration that is
501+
the documented remedy for a custom deployment.
502+
"""
503+
monkeypatch.setenv("GITHUB_COPILOT_PROXY_URL", "https://gw.corp.internal")
504+
505+
base = "https://gw.corp.internal"
506+
full = f"https://gw.corp.internal{COMPLETIONS}"
507+
508+
assert copilot_auth.is_copilot_completions_host(base) is True
509+
assert copilot_auth.is_copilot_completions_host(full) is True
510+
assert copilot_auth.is_copilot_completions_host("https://gw.corp.internal/") is True
511+
assert copilot_auth.is_copilot_upstream_url(full) is True
512+
# A different host is still not the override.
513+
assert copilot_auth.is_copilot_completions_host(f"https://elsewhere.test{COMPLETIONS}") is False
514+
515+
516+
def test_an_operator_override_gateway_receives_credentials(
517+
monkeypatch: pytest.MonkeyPatch,
518+
) -> None:
519+
"""End of the same chain: the gateway must actually be authenticated."""
520+
521+
class _Token:
522+
token = "test-copilot-token"
523+
524+
class _Provider:
525+
async def get_api_token(self): # noqa: ANN202
526+
return _Token()
527+
528+
monkeypatch.setenv("GITHUB_COPILOT_PROXY_URL", "https://gw.corp.internal")
529+
monkeypatch.setattr(copilot_auth, "get_copilot_token_provider", lambda: _Provider())
530+
531+
resolved = asyncio.run(
532+
copilot_auth.apply_copilot_api_auth({}, url=f"https://gw.corp.internal{COMPLETIONS}")
533+
)
534+
535+
assert resolved.get("Authorization") == "Bearer test-copilot-token"
536+
537+
538+
@pytest.mark.parametrize(
539+
"configured",
540+
["api.githubcopilot.com", "api.business.githubcopilot.com"],
541+
)
542+
def test_a_scheme_less_public_capi_url_is_still_recognised(
543+
monkeypatch: pytest.MonkeyPatch, configured: str
544+
) -> None:
545+
"""A hand-written value without "https://" must not read as a custom host."""
546+
monkeypatch.setenv("GITHUB_COPILOT_API_URL", configured)
547+
548+
assert copilot_completions_base_url() == COMPLETIONS_PROXY

0 commit comments

Comments
 (0)