mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## Description PR https://github.com/headroomlabs-ai/headroom/pull/2445 added the missing OpenCode subscription path, but the shared Copilot subscription resolver still lets Business and Enterprise payload hosts route through segmented `*.githubcopilot.com` domains and still drops an explicit `GITHUB_COPILOT_API_URL` pin on two resolution paths. This follow-up moves the final hosted-route decision back into the shared resolver, normalizes `api.business.githubcopilot.com` and `api.enterprise.githubcopilot.com` to the generic host by default, and makes the explicit pin win on token exchange, explicit API token, and Copilot-token candidate resolution. Both `headroom wrap copilot --subscription` and `headroom wrap opencode --copilot-subscription` inherit the same fix because they already consume the same `CopilotSubscriptionTokenResolution.api_url`. Refs #2441. Attribution: https://github.com/headroomlabs-ai/headroom/pull/2445#issuecomment-5026212395 reported and narrowed the Business or Enterprise regression, and https://github.com/headroomlabs-ai/headroom/pull/2445#issuecomment-5026863498 scoped the shared-resolver follow-up that this change implements. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Centralize subscription hosted-route selection so explicit `GITHUB_COPILOT_API_URL` pins win on token exchange, explicit API token, and Copilot-token candidate resolution. - Normalize `api.business.githubcopilot.com` and `api.enterprise.githubcopilot.com` to `https://api.githubcopilot.com` by default, extending the existing individual-seat normalization. - Extend focused auth and wrapper tests so both subscription wrappers prove the corrected shared resolver output and the private-proxy isolation contract stays intact. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_copilot_auth.py -q`) - [x] Unit tests pass (`uv run pytest tests/test_cli/test_wrap_copilot.py -q`) - [x] Unit tests pass (`uv run pytest tests/test_cli/test_wrap_opencode.py -q`) - [x] Unit tests pass (`uv run pytest tests/test_cli/test_wrap_persistent.py -q`) - [x] Linting passes (`uv run ruff check .`) - [x] Formatting check passes (`uv run ruff format . --check`) - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_copilot_auth.py -q -> 84 passed uv run pytest tests/test_cli/test_wrap_copilot.py -q -> 31 passed uv run pytest tests/test_cli/test_wrap_opencode.py -q -> 44 passed in 143.46s uv run pytest tests/test_cli/test_wrap_persistent.py -q -> 31 passed uv run ruff check . -> All checks passed! uv run ruff format . --check -> 1331 files already formatted ``` ## Real Behavior Proof - Environment: Windows - Exact command / steps: Run the focused auth, Copilot wrapper, OpenCode wrapper, and persistent-proxy pytest files after implementing the shared resolver change, then ask lucasp1337 to rerun the Business or Enterprise `--copilot-subscription` scenario from PR https://github.com/headroomlabs-ai/headroom/pull/2445#issuecomment-5026212395 on a real seat. - Observed result: Focused auth and wrapper pytest runs passed locally, including the enterprise-host exchange reproduction row, explicit-pin precedence on all three producer paths, both subscription wrapper routes, and the private-proxy isolation regression. Live Business or Enterprise success stays behind reporter retest. - Not tested: live Business or Enterprise tenant run ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes No `CHANGELOG.md` edit is needed because Headroom generates release notes from conventional commits. Risk for maintainers: PR https://github.com/headroomlabs-ai/headroom/pull/641 manually validated a Business seat against the GitHub-returned hosted domain in June on `gpt-5.4`, so generic-by-default could affect tenants that genuinely require a dedicated host. This follow-up keeps the documented escape hatch intact by making `GITHUB_COPILOT_API_URL` win on every path. Live-seat proof boundary: lucasp1337 offered to retest on a Business or Enterprise seat in PR https://github.com/headroomlabs-ai/headroom/pull/2445#issuecomment-5026212395. Keep any live success claim behind that rerun.
This commit is contained in:
parent
8c8fae0d0b
commit
2eca5ee114
4 changed files with 207 additions and 21 deletions
|
|
@ -178,7 +178,7 @@ def default_oauth_domain() -> str:
|
|||
return domain if domain else DEFAULT_GITHUB_HOST
|
||||
|
||||
|
||||
def _configured_api_url() -> str:
|
||||
def _configured_api_url_override() -> str | None:
|
||||
api_url = os.environ.get("GITHUB_COPILOT_API_URL", "").strip()
|
||||
if api_url:
|
||||
return api_url.rstrip("/")
|
||||
|
|
@ -187,6 +187,13 @@ def _configured_api_url() -> str:
|
|||
if enterprise_domain:
|
||||
return copilot_api_url_from_enterprise_url(enterprise_domain).rstrip("/")
|
||||
|
||||
return None
|
||||
|
||||
|
||||
def _configured_api_url() -> str:
|
||||
configured = _configured_api_url_override()
|
||||
if configured:
|
||||
return configured
|
||||
return DEFAULT_API_URL
|
||||
|
||||
|
||||
|
|
@ -774,16 +781,25 @@ def _api_url_from_payload(payload: dict[str, Any] | None) -> str | None:
|
|||
|
||||
|
||||
def _subscription_api_url_from_user_info_payload(payload: dict[str, Any] | None) -> str:
|
||||
configured = _configured_api_url_override()
|
||||
if configured:
|
||||
return configured
|
||||
|
||||
api_url = _api_url_from_payload(payload)
|
||||
if not api_url:
|
||||
return _configured_api_url()
|
||||
return DEFAULT_API_URL
|
||||
|
||||
host = urlparse(api_url).netloc.lower()
|
||||
if host in {"api.githubcopilot.com", "api.individual.githubcopilot.com"}:
|
||||
return _configured_api_url()
|
||||
if host in {
|
||||
"api.githubcopilot.com",
|
||||
"api.individual.githubcopilot.com",
|
||||
"api.business.githubcopilot.com",
|
||||
"api.enterprise.githubcopilot.com",
|
||||
}:
|
||||
return DEFAULT_API_URL
|
||||
if host.endswith(".githubcopilot.com"):
|
||||
return api_url
|
||||
return _configured_api_url()
|
||||
return DEFAULT_API_URL
|
||||
|
||||
|
||||
def _subscription_api_url_from_user_info(oauth_token: str) -> str:
|
||||
|
|
@ -791,8 +807,8 @@ def _subscription_api_url_from_user_info(oauth_token: str) -> str:
|
|||
|
||||
|
||||
def _api_url_from_exchange_payload(payload: dict[str, Any], *, oauth_token: str) -> str:
|
||||
configured = _configured_api_url()
|
||||
if configured != DEFAULT_API_URL:
|
||||
configured = _configured_api_url_override()
|
||||
if configured:
|
||||
return configured
|
||||
|
||||
api_url = _api_url_from_payload(payload)
|
||||
|
|
|
|||
|
|
@ -987,7 +987,7 @@ def test_wrap_copilot_subscription_uses_resolved_subscription_endpoint(
|
|||
assert env["COPILOT_PROVIDER_BEARER_TOKEN"] == "copilot-api"
|
||||
|
||||
|
||||
def test_wrap_copilot_subscription_normalizes_individual_public_endpoint(
|
||||
def test_wrap_copilot_subscription_normalizes_enterprise_host(
|
||||
runner: CliRunner,
|
||||
wrap_modules: tuple[types.ModuleType, click.Group],
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
|
|
@ -1019,7 +1019,7 @@ def test_wrap_copilot_subscription_normalizes_individual_public_endpoint(
|
|||
lambda _headers: {
|
||||
"token": "copilot-api",
|
||||
"expires_at": 9999999999,
|
||||
"endpoints": {"api": "https://api.individual.githubcopilot.com"},
|
||||
"endpoints": {"api": "https://api.enterprise.githubcopilot.com"},
|
||||
}
|
||||
),
|
||||
),
|
||||
|
|
|
|||
|
|
@ -33,6 +33,12 @@ def _set_test_home(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
|
|||
monkeypatch.delenv("OPENCODE_CONFIG", raising=False)
|
||||
|
||||
|
||||
def _clear_copilot_route_config(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.delenv("GITHUB_COPILOT_API_URL", raising=False)
|
||||
monkeypatch.delenv("GITHUB_COPILOT_ENTERPRISE_URL", raising=False)
|
||||
monkeypatch.delenv("GITHUB_COPILOT_ENTERPRISE_DOMAIN", raising=False)
|
||||
|
||||
|
||||
def _subscription_resolution() -> CopilotSubscriptionTokenResolution:
|
||||
return CopilotSubscriptionTokenResolution(
|
||||
token="copilot-api-secret",
|
||||
|
|
@ -50,13 +56,14 @@ def _subscription_resolution() -> CopilotSubscriptionTokenResolution:
|
|||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_wrap_opencode_copilot_subscription_handoffs_seed_after_actual_port(
|
||||
def test_wrap_opencode_copilot_subscription_normalizes_enterprise_host_and_handoffs_seed_after_actual_port(
|
||||
runner: CliRunner,
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.chdir(tmp_path)
|
||||
_set_test_home(monkeypatch, tmp_path)
|
||||
_clear_copilot_route_config(monkeypatch)
|
||||
monkeypatch.setenv("GITHUB_COPILOT_API_TOKEN", "inherited-api-secret")
|
||||
monkeypatch.setenv("GITHUB_COPILOT_REFRESH_OAUTH_TOKEN", "inherited-refresh-secret")
|
||||
monkeypatch.setenv("GITHUB_COPILOT_API_TOKEN_EXPIRES_AT", "999.0")
|
||||
|
|
@ -77,11 +84,33 @@ def test_wrap_opencode_copilot_subscription_handoffs_seed_after_actual_port(
|
|||
|
||||
with (
|
||||
patch.object(wrap_mod.shutil, "which", return_value="opencode"),
|
||||
patch.object(
|
||||
wrap_mod,
|
||||
"_require_copilot_subscription_resolution",
|
||||
return_value=_subscription_resolution(),
|
||||
patch(
|
||||
"headroom.copilot_auth.iter_oauth_token_candidates",
|
||||
return_value=[
|
||||
type(
|
||||
"_Candidate",
|
||||
(),
|
||||
{
|
||||
"token": "gho-oauth",
|
||||
"source": "headroom-copilot-auth:/tmp/copilot_auth.json",
|
||||
"confidence": "copilot-oauth",
|
||||
"validate_for_subscription": True,
|
||||
},
|
||||
)()
|
||||
],
|
||||
),
|
||||
patch(
|
||||
"headroom.copilot_auth.CopilotTokenProvider._exchange_token_sync",
|
||||
staticmethod(
|
||||
lambda _headers: {
|
||||
"token": "copilot-api-secret",
|
||||
"expires_at": 123.5,
|
||||
"refresh_token": "copilot-refresh-secret",
|
||||
"endpoints": {"api": "https://api.enterprise.githubcopilot.com"},
|
||||
}
|
||||
),
|
||||
),
|
||||
patch("headroom.copilot_auth._fetch_copilot_user_info", return_value=None),
|
||||
patch.object(wrap_mod, "_ensure_proxy", side_effect=fake_ensure_proxy),
|
||||
patch.object(wrap_mod, "_launch_tool", side_effect=fake_launch_tool),
|
||||
):
|
||||
|
|
@ -101,7 +130,7 @@ def test_wrap_opencode_copilot_subscription_handoffs_seed_after_actual_port(
|
|||
ensure = captured["ensure"]
|
||||
assert ensure["openai_api_url"] == "https://api.githubcopilot.com"
|
||||
assert ensure["copilot_api_token"] == "copilot-api-secret"
|
||||
assert ensure["copilot_refresh_oauth_token"] == "copilot-refresh-secret"
|
||||
assert ensure["copilot_refresh_oauth_token"] == "gho-oauth"
|
||||
assert ensure["copilot_api_token_expires_at"] == 123.5
|
||||
launch = captured["launch"]
|
||||
assert launch["port"] == 9010
|
||||
|
|
@ -137,6 +166,7 @@ def test_wrap_opencode_copilot_subscription_rejects_incompatible_modes(
|
|||
) -> None:
|
||||
monkeypatch.chdir(tmp_path)
|
||||
_set_test_home(monkeypatch, tmp_path)
|
||||
_clear_copilot_route_config(monkeypatch)
|
||||
config_file = tmp_path / ".config" / "opencode" / "opencode.json"
|
||||
config_file.parent.mkdir(parents=True)
|
||||
config_file.write_text("{}", encoding="utf-8")
|
||||
|
|
@ -157,6 +187,7 @@ def test_wrap_opencode_copilot_subscription_rejects_headroom_backend_env(
|
|||
) -> None:
|
||||
monkeypatch.chdir(tmp_path)
|
||||
_set_test_home(monkeypatch, tmp_path)
|
||||
_clear_copilot_route_config(monkeypatch)
|
||||
monkeypatch.setenv("HEADROOM_BACKEND", "anyllm")
|
||||
with patch.object(wrap_mod, "_ensure_proxy", side_effect=AssertionError("proxy launched")):
|
||||
result = runner.invoke(
|
||||
|
|
@ -174,6 +205,7 @@ def test_wrap_opencode_copilot_subscription_requires_login_before_launch(
|
|||
) -> None:
|
||||
monkeypatch.chdir(tmp_path)
|
||||
_set_test_home(monkeypatch, tmp_path)
|
||||
_clear_copilot_route_config(monkeypatch)
|
||||
with (
|
||||
patch.object(
|
||||
wrap_mod,
|
||||
|
|
@ -197,6 +229,7 @@ def test_wrap_opencode_copilot_subscription_cleans_up_proxy_on_config_failure(
|
|||
) -> None:
|
||||
monkeypatch.chdir(tmp_path)
|
||||
_set_test_home(monkeypatch, tmp_path)
|
||||
_clear_copilot_route_config(monkeypatch)
|
||||
|
||||
class _FakeProxy:
|
||||
def __init__(self) -> None:
|
||||
|
|
|
|||
|
|
@ -173,7 +173,7 @@ def test_resolve_subscription_bearer_token_does_not_fallback_to_unexchanged_oaut
|
|||
assert copilot_auth.resolve_subscription_bearer_token() is None
|
||||
|
||||
|
||||
def test_resolve_subscription_bearer_token_details_exchanges_oauth_candidate(
|
||||
def test_subscription_enterprise_host_repro(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.delenv("GITHUB_COPILOT_API_TOKEN", raising=False)
|
||||
|
|
@ -202,7 +202,7 @@ def test_resolve_subscription_bearer_token_details_exchanges_oauth_candidate(
|
|||
return {
|
||||
"token": "copilot-api",
|
||||
"expires_at": int(time.time()) + 3600,
|
||||
"endpoints": {"api": "https://api.business.githubcopilot.com"},
|
||||
"endpoints": {"api": "https://api.enterprise.githubcopilot.com"},
|
||||
}
|
||||
|
||||
monkeypatch.setattr(
|
||||
|
|
@ -217,7 +217,7 @@ def test_resolve_subscription_bearer_token_details_exchanges_oauth_candidate(
|
|||
assert resolution.token == "copilot-api"
|
||||
assert resolution.source == "headroom-copilot-auth:/tmp/copilot_auth.json:token-exchange"
|
||||
assert resolution.confidence == "copilot-token-exchange"
|
||||
assert resolution.api_url == "https://api.business.githubcopilot.com"
|
||||
assert resolution.api_url == copilot_auth.DEFAULT_API_URL
|
||||
assert resolution.token_fingerprint == copilot_auth.token_fingerprint("copilot-api")
|
||||
assert resolution.refresh_oauth_token == "gho-oauth"
|
||||
assert isinstance(resolution.api_token_expires_at, float)
|
||||
|
|
@ -259,13 +259,13 @@ def test_resolve_subscription_exchange_uses_cloud_enterprise_advertised_api(
|
|||
monkeypatch.setattr(
|
||||
copilot_auth,
|
||||
"_fetch_copilot_user_info",
|
||||
lambda _token: {"endpoints": {"api": "https://api.business.githubcopilot.com"}},
|
||||
lambda _token: {"endpoints": {"api": "https://api.enterprise.githubcopilot.com"}},
|
||||
)
|
||||
|
||||
resolution = copilot_auth.resolve_subscription_bearer_token_details()
|
||||
|
||||
assert resolution is not None
|
||||
assert resolution.api_url == "https://api.business.githubcopilot.com"
|
||||
assert resolution.api_url == copilot_auth.DEFAULT_API_URL
|
||||
assert copilot_auth._token_exchange_url() == "https://api.github.com/copilot_internal/v2/token"
|
||||
|
||||
|
||||
|
|
@ -286,7 +286,144 @@ def test_api_url_from_exchange_payload_rejects_non_copilot_host(
|
|||
oauth_token="gho-oauth",
|
||||
)
|
||||
|
||||
assert resolved == "https://api.business.githubcopilot.com"
|
||||
assert resolved == copilot_auth.DEFAULT_API_URL
|
||||
|
||||
|
||||
def _resolve_subscription_producer_path(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
producer: str,
|
||||
payload_host: str,
|
||||
configured_api_url: str | None = None,
|
||||
enterprise_domain: str | None = None,
|
||||
) -> str:
|
||||
with monkeypatch.context() as patch:
|
||||
patch.delenv("GITHUB_COPILOT_API_TOKEN", raising=False)
|
||||
patch.delenv("GITHUB_COPILOT_API_URL", raising=False)
|
||||
patch.delenv("GITHUB_COPILOT_ENTERPRISE_DOMAIN", raising=False)
|
||||
if configured_api_url is not None:
|
||||
patch.setenv("GITHUB_COPILOT_API_URL", configured_api_url)
|
||||
if enterprise_domain is not None:
|
||||
patch.setenv("GITHUB_COPILOT_ENTERPRISE_DOMAIN", enterprise_domain)
|
||||
|
||||
payload = {"endpoints": {"api": payload_host}}
|
||||
if producer == "exchange":
|
||||
patch.setattr(
|
||||
copilot_auth,
|
||||
"iter_oauth_token_candidates",
|
||||
lambda: [
|
||||
copilot_auth.CopilotTokenCandidate(
|
||||
token="gho-oauth", source="test", confidence="test"
|
||||
)
|
||||
],
|
||||
)
|
||||
patch.setattr(
|
||||
copilot_auth.CopilotTokenProvider,
|
||||
"_exchange_token_sync",
|
||||
staticmethod(lambda _headers: {"token": "tid-api", **payload}),
|
||||
)
|
||||
elif producer == "explicit":
|
||||
patch.setenv("GITHUB_COPILOT_API_TOKEN", "tid-api")
|
||||
patch.setattr(copilot_auth, "_fetch_copilot_user_info", lambda _token: payload)
|
||||
else:
|
||||
patch.setattr(
|
||||
copilot_auth,
|
||||
"iter_oauth_token_candidates",
|
||||
lambda: [
|
||||
copilot_auth.CopilotTokenCandidate(
|
||||
token="tid_api", source="test", confidence="test"
|
||||
)
|
||||
],
|
||||
)
|
||||
patch.setattr(copilot_auth, "_fetch_copilot_user_info", lambda _token: payload)
|
||||
|
||||
resolution = copilot_auth.resolve_subscription_bearer_token_details()
|
||||
assert resolution is not None
|
||||
return resolution.api_url
|
||||
|
||||
|
||||
@pytest.mark.parametrize("producer", ["exchange", "explicit", "candidate"])
|
||||
def test_subscription_api_url_pin_precedence(
|
||||
monkeypatch: pytest.MonkeyPatch, producer: str
|
||||
) -> None:
|
||||
assert (
|
||||
_resolve_subscription_producer_path(
|
||||
monkeypatch,
|
||||
producer,
|
||||
"https://api.enterprise.githubcopilot.com",
|
||||
configured_api_url="https://api.pinned.example.com",
|
||||
)
|
||||
== "https://api.pinned.example.com"
|
||||
)
|
||||
assert (
|
||||
_resolve_subscription_producer_path(
|
||||
monkeypatch,
|
||||
producer,
|
||||
"https://api.other.githubcopilot.com",
|
||||
configured_api_url=copilot_auth.DEFAULT_API_URL,
|
||||
)
|
||||
== copilot_auth.DEFAULT_API_URL
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("producer", ["exchange", "explicit", "candidate"])
|
||||
def test_subscription_enterprise_domain_precedence(
|
||||
monkeypatch: pytest.MonkeyPatch, producer: str
|
||||
) -> None:
|
||||
assert (
|
||||
_resolve_subscription_producer_path(
|
||||
monkeypatch,
|
||||
producer,
|
||||
"https://api.business.githubcopilot.com",
|
||||
enterprise_domain="ghe.example.com",
|
||||
)
|
||||
== "https://copilot-api.ghe.example.com"
|
||||
)
|
||||
|
||||
|
||||
def test_subscription_unknown_host_passthrough(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
assert (
|
||||
copilot_auth._subscription_api_url_from_user_info_payload(
|
||||
{"endpoints": {"api": "https://api.other.githubcopilot.com"}}
|
||||
)
|
||||
== "https://api.other.githubcopilot.com"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"payload_host",
|
||||
[
|
||||
"https://api.githubcopilot.com",
|
||||
"https://api.individual.githubcopilot.com",
|
||||
"https://api.business.githubcopilot.com",
|
||||
"https://api.enterprise.githubcopilot.com",
|
||||
],
|
||||
)
|
||||
def test_subscription_known_hosts_normalize_to_default(payload_host: str) -> None:
|
||||
assert (
|
||||
copilot_auth._subscription_api_url_from_user_info_payload(
|
||||
{"endpoints": {"api": payload_host}}
|
||||
)
|
||||
== copilot_auth.DEFAULT_API_URL
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"payload",
|
||||
[
|
||||
None,
|
||||
{},
|
||||
{"endpoints": {}},
|
||||
{"endpoints": {"api": " "}},
|
||||
{"endpoints": {"api": 4}},
|
||||
{"endpoints": {"api": "https://api.openai.com/v1"}},
|
||||
],
|
||||
)
|
||||
def test_subscription_payload_host_fallback(
|
||||
monkeypatch: pytest.MonkeyPatch, payload: dict[str, object] | None
|
||||
) -> None:
|
||||
assert copilot_auth._subscription_api_url_from_user_info_payload(payload) == (
|
||||
copilot_auth.DEFAULT_API_URL
|
||||
)
|
||||
|
||||
|
||||
def test_api_url_from_exchange_payload_normalizes_individual_public_host(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue