mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## Description GHE Copilot credential discovery falls back straight to `github.com` when `GITHUB_COPILOT_HOST` is unset, even if the documented `GITHUB_COPILOT_API_URL` points at an enterprise host. This change keeps explicit-host precedence, then reuses the configured enterprise domain or a normalized custom API URL hostname for credential lookup, so Windows, macOS, Linux, GH CLI, and credential-file discovery search the same custom host instead of the public default. Closes #800. Attribution: https://github.com/headroomlabs-ai/headroom/issues/800#issuecomment-5044382263 narrowed the shared credential-host mismatch. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds new functionality) - [ ] Breaking change - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring ## Changes Made - Preserve explicit-host precedence, then fall back to the configured enterprise domain or a normalized custom API URL hostname only when the configured value is usable. - Normalize `api.` and `copilot-api.` prefixes before routing credential lookup, while keeping exact and segmented GitHub-hosted public domains plus public enterprise or malformed enterprise or API configuration fallback on `github.com`. - Add focused coverage for the base/head reproduction, explicit-host precedence, configured-enterprise precedence, public-enterprise, malformed-enterprise, and invalid-port fallback, prefixed-host normalization, adjacent-host exclusion, and GH CLI plus keychain forwarding. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_copilot_auth.py -q`) - [x] Linting passes - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_copilot_auth.py -q 105 passed in 0.58s uvx --from ruff==0.15.17 ruff check headroom/copilot_auth.py tests/test_copilot_auth.py All checks passed! uvx --from ruff==0.15.17 ruff format --check headroom/copilot_auth.py tests/test_copilot_auth.py 2 files already formatted git diff --check clean ``` ## Real Behavior Proof - Environment: Windows, isolated temporary credential file, local `origin/main` checkout plus this branch - Exact command / steps: With only `GITHUB_COPILOT_API_URL=https://api.ghe.example.com:8443/copilot` set and all other token sources disabled, run the same credential-file discovery reproduction against `origin/main` and this branch. - Observed result: `origin/main` selected `github.com` and resolved no token; the review branch selected `ghe.example.com` and resolved `gho-ghe`. - Not tested: live GitHub Enterprise Copilot tenant ## 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 own 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 did not edit `CHANGELOG.md`; Headroom generates release notes from the PR title ## Additional Notes The change does not alter API routing, token exchange, discovery order, or credential matching breadth, and it keeps the live tenant claim out of the PR body until an enterprise user reruns it.
This commit is contained in:
parent
e4076bbe99
commit
4a8157fa0a
2 changed files with 193 additions and 4 deletions
|
|
@ -110,7 +110,31 @@ def token_fingerprint(token: str) -> str:
|
|||
|
||||
|
||||
def _github_host() -> str:
|
||||
return (os.environ.get("GITHUB_COPILOT_HOST") or DEFAULT_GITHUB_HOST).strip().lower()
|
||||
explicit = os.environ.get("GITHUB_COPILOT_HOST", "").strip().lower()
|
||||
if explicit:
|
||||
return explicit
|
||||
|
||||
enterprise_domain = _configured_enterprise_domain()
|
||||
if enterprise_domain:
|
||||
return enterprise_domain
|
||||
|
||||
configured_url = os.environ.get("GITHUB_COPILOT_API_URL", "").strip()
|
||||
if configured_url:
|
||||
hostname = _configured_url_hostname(configured_url)
|
||||
if _is_public_copilot_api_host(hostname):
|
||||
return DEFAULT_GITHUB_HOST
|
||||
for prefix in ("copilot-api.", "api."):
|
||||
if hostname.startswith(prefix):
|
||||
hostname = hostname[len(prefix) :]
|
||||
break
|
||||
if hostname and hostname not in {
|
||||
DEFAULT_GITHUB_HOST,
|
||||
"api.github.com",
|
||||
"githubcopilot.com",
|
||||
}:
|
||||
return hostname
|
||||
|
||||
return DEFAULT_GITHUB_HOST
|
||||
|
||||
|
||||
def headroom_copilot_auth_path() -> Path:
|
||||
|
|
@ -132,8 +156,28 @@ def _enterprise_hostname(enterprise_url: str) -> str:
|
|||
normalized = normalize_copilot_enterprise_url(enterprise_url)
|
||||
if not normalized:
|
||||
return ""
|
||||
parsed = urlparse(f"https://{normalized}")
|
||||
return (parsed.hostname or normalized.split("/", 1)[0]).lower()
|
||||
try:
|
||||
parsed = urlparse(f"https://{normalized}")
|
||||
_ = parsed.port
|
||||
except ValueError:
|
||||
return ""
|
||||
hostname = (parsed.hostname or "").strip().lower()
|
||||
return hostname if hostname and " " not in hostname else ""
|
||||
|
||||
|
||||
def _configured_url_hostname(configured_url: str) -> str:
|
||||
raw = configured_url.strip()
|
||||
if not raw:
|
||||
return ""
|
||||
try:
|
||||
parsed = urlparse(raw)
|
||||
if not parsed.scheme or not parsed.netloc:
|
||||
return ""
|
||||
_ = parsed.port
|
||||
except ValueError:
|
||||
return ""
|
||||
hostname = (parsed.hostname or "").strip().lower()
|
||||
return hostname if hostname and " " not in hostname else ""
|
||||
|
||||
|
||||
def _copilot_subdomain_enterprise_host(enterprise_url: str) -> str | None:
|
||||
|
|
@ -148,7 +192,11 @@ def _copilot_subdomain_enterprise_host(enterprise_url: str) -> str | None:
|
|||
if host.startswith(prefix):
|
||||
host = host[len(prefix) :]
|
||||
break
|
||||
if not host or host in {"github.com", "www.github.com", "api.github.com"}:
|
||||
if (
|
||||
not host
|
||||
or host in {"github.com", "www.github.com", "api.github.com"}
|
||||
or _is_public_copilot_api_host(host)
|
||||
):
|
||||
return None
|
||||
return host
|
||||
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ def _isolated_copilot_auth(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> N
|
|||
"GH_TOKEN",
|
||||
"GITHUB_TOKEN",
|
||||
"GITHUB_COPILOT_API_URL",
|
||||
"GITHUB_COPILOT_HOST",
|
||||
"GITHUB_COPILOT_ENTERPRISE_URL",
|
||||
"GITHUB_COPILOT_ENTERPRISE_DOMAIN",
|
||||
"GITHUB_COPILOT_TOKEN_EXCHANGE_URL",
|
||||
|
|
@ -70,6 +71,86 @@ def test_default_oauth_domain_falls_back_to_github_com_when_env_blank(
|
|||
assert copilot_auth.default_oauth_domain() == "github.com"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("api_url", "expected"),
|
||||
[
|
||||
("https://api.GHE.Example.com:8443/copilot", "ghe.example.com"),
|
||||
("https://copilot-api.GHE.Example.com", "ghe.example.com"),
|
||||
("https://api.githubcopilot.com", "github.com"),
|
||||
("https://api.business.githubcopilot.com", "github.com"),
|
||||
("https://api.enterprise.githubcopilot.com", "github.com"),
|
||||
("https://api.individual.githubcopilot.com", "github.com"),
|
||||
("https://api.ghe.example.com:invalid/copilot", "github.com"),
|
||||
("https://[", "github.com"),
|
||||
("not a URL", "github.com"),
|
||||
("https://", "github.com"),
|
||||
],
|
||||
)
|
||||
def test_github_host_derives_from_api_url(
|
||||
monkeypatch: pytest.MonkeyPatch, api_url: str, expected: str
|
||||
) -> None:
|
||||
monkeypatch.setenv("GITHUB_COPILOT_API_URL", api_url)
|
||||
assert copilot_auth._github_host() == expected
|
||||
|
||||
|
||||
def test_github_host_explicit_value_wins_over_api_url(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("GITHUB_COPILOT_HOST", " Explicit.GHE.COM ")
|
||||
monkeypatch.setenv("GITHUB_COPILOT_API_URL", "https://other.example.com/api")
|
||||
assert copilot_auth._github_host() == "explicit.ghe.com"
|
||||
|
||||
|
||||
def test_github_host_uses_configured_enterprise_domain_before_api_url(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setenv("GITHUB_COPILOT_ENTERPRISE_URL", "https://enterprise.ghe.example.com")
|
||||
monkeypatch.setenv("GITHUB_COPILOT_API_URL", "https://api.other.example.com/api")
|
||||
assert copilot_auth._github_host() == "enterprise.ghe.example.com"
|
||||
|
||||
|
||||
def test_github_host_invalid_enterprise_url_falls_back_to_github_com(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setenv("GITHUB_COPILOT_ENTERPRISE_URL", "https://[")
|
||||
assert copilot_auth._github_host() == "github.com"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("env_name", "env_value"),
|
||||
[
|
||||
("GITHUB_COPILOT_ENTERPRISE_URL", "https://api.business.githubcopilot.com"),
|
||||
("GITHUB_COPILOT_ENTERPRISE_DOMAIN", "enterprise.githubcopilot.com"),
|
||||
],
|
||||
)
|
||||
def test_public_enterprise_config_keeps_public_defaults(
|
||||
monkeypatch: pytest.MonkeyPatch, env_name: str, env_value: str
|
||||
) -> None:
|
||||
monkeypatch.delenv("GITHUB_COPILOT_TOKEN_EXCHANGE_URL", raising=False)
|
||||
monkeypatch.delenv("GITHUB_COPILOT_USER_INFO_URL", raising=False)
|
||||
monkeypatch.setenv(env_name, env_value)
|
||||
|
||||
assert copilot_auth._github_host() == "github.com"
|
||||
assert copilot_auth.default_oauth_domain() == "github.com"
|
||||
assert copilot_auth._token_exchange_url() == copilot_auth.DEFAULT_TOKEN_EXCHANGE_URL
|
||||
assert copilot_auth._user_info_url() == copilot_auth.DEFAULT_USER_INFO_URL
|
||||
|
||||
|
||||
def test_enterprise_hostname_blank_returns_empty() -> None:
|
||||
assert copilot_auth._enterprise_hostname(" ") == ""
|
||||
|
||||
|
||||
def test_configured_url_hostname_blank_returns_empty() -> None:
|
||||
assert copilot_auth._configured_url_hostname(" ") == ""
|
||||
|
||||
|
||||
def test_copilot_subdomain_enterprise_host_rejects_blank_and_public_hosts() -> None:
|
||||
assert copilot_auth._copilot_subdomain_enterprise_host(" ") is None
|
||||
assert copilot_auth._copilot_subdomain_enterprise_host("https://github.com") is None
|
||||
assert (
|
||||
copilot_auth._copilot_subdomain_enterprise_host("https://api.business.githubcopilot.com")
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_read_cached_oauth_token_prefers_copilot_cli_before_generic_github_token(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
|
@ -601,6 +682,25 @@ def test_read_macos_keychain_oauth_token_uses_security(
|
|||
assert calls == ["github.com"]
|
||||
|
||||
|
||||
def test_keychain_and_secret_service_use_derived_host(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
calls: list[tuple[str, str]] = []
|
||||
|
||||
def fake_macos(*, host: str) -> None:
|
||||
calls.append(("macos", host))
|
||||
return None
|
||||
|
||||
def fake_linux(*, host: str) -> None:
|
||||
calls.append(("linux", host))
|
||||
return None
|
||||
|
||||
monkeypatch.setenv("GITHUB_COPILOT_API_URL", "https://ghe.example.com/api")
|
||||
monkeypatch.setattr(copilot_auth, "read_macos_keychain_token", fake_macos)
|
||||
monkeypatch.setattr(copilot_auth, "read_linux_secret_token", fake_linux)
|
||||
assert copilot_auth._read_macos_keychain_oauth_token() is None
|
||||
assert copilot_auth._read_linux_secret_oauth_token() is None
|
||||
assert calls == [("macos", "ghe.example.com"), ("linux", "ghe.example.com")]
|
||||
|
||||
|
||||
def test_read_cached_oauth_token_reads_hosts_file(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
) -> None:
|
||||
|
|
@ -625,6 +725,30 @@ def test_read_cached_oauth_token_reads_hosts_file(
|
|||
assert copilot_auth.read_cached_oauth_token() == "gho-file"
|
||||
|
||||
|
||||
def test_read_cached_oauth_token_reads_custom_api_host_file(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
) -> None:
|
||||
hosts = tmp_path / "hosts.json"
|
||||
hosts.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"ghe.example.com": {"oauth_token": "gho-ghe"},
|
||||
"adjacent.example.com": {"oauth_token": "gho-adjacent"},
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
monkeypatch.setenv("GITHUB_COPILOT_API_URL", "https://api.ghe.example.com:8443/copilot")
|
||||
monkeypatch.setenv("GITHUB_COPILOT_TOKEN_FILE", str(hosts))
|
||||
monkeypatch.setattr(copilot_auth, "_read_windows_copilot_cli_oauth_token", lambda: None)
|
||||
monkeypatch.setattr(copilot_auth, "_read_macos_keychain_oauth_token", lambda: None)
|
||||
monkeypatch.setattr(copilot_auth, "_read_gh_cli_oauth_token", lambda: None)
|
||||
|
||||
candidates = copilot_auth._read_file_oauth_token_candidates()
|
||||
assert [candidate.token for candidate in candidates] == ["gho-ghe"]
|
||||
assert copilot_auth.read_cached_oauth_token() == "gho-ghe"
|
||||
|
||||
|
||||
def test_read_cached_oauth_token_skips_expired_entries(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
) -> None:
|
||||
|
|
@ -662,6 +786,23 @@ def test_read_gh_cli_oauth_token_uses_hostname(monkeypatch: pytest.MonkeyPatch)
|
|||
assert calls == [["gh", "auth", "token", "--hostname", "example.ghe.com"]]
|
||||
|
||||
|
||||
def test_read_gh_cli_oauth_token_uses_api_url_hostname(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
calls: list[list[str]] = []
|
||||
|
||||
class CompletedProcess:
|
||||
returncode = 0
|
||||
stdout = "gho-gh-cli\n"
|
||||
|
||||
def fake_run(*args: object, **kwargs: object) -> CompletedProcess:
|
||||
calls.append(list(args[0]))
|
||||
return CompletedProcess()
|
||||
|
||||
monkeypatch.setenv("GITHUB_COPILOT_API_URL", "https://api.ghe.example.com/api")
|
||||
monkeypatch.setattr(copilot_auth, "run", fake_run)
|
||||
assert copilot_auth._read_gh_cli_oauth_token() == "gho-gh-cli"
|
||||
assert calls == [["gh", "auth", "token", "--hostname", "ghe.example.com"]]
|
||||
|
||||
|
||||
def test_read_gh_cli_oauth_token_returns_none_when_invocation_fails(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue