mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix: harden Copilot API auth token handling (#557)
## Description Improve Copilot API authentication behavior by correctly handling incoming bearer tokens and ensuring required Copilot headers are present. ## 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 - Added token classification logic to distinguish short-lived Copilot API tokens (`tid_`) from GitHub OAuth tokens. - Updated auth flow to pass through valid existing Copilot API bearer tokens and replace unsuitable bearer tokens. - Added default `Copilot-Integration-Id` and `editor-version` headers when missing. - Improved Windows credential lookup to consider both GitHub CLI (`gh:`) and Copilot CLI credential target prefixes. - Added regression tests for pass-through, replacement, header injection, and token prefix classification. ## Testing Describe the tests you ran to verify your changes: - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ## Test Output ```text pytest -q tests/test_copilot_auth.py 29 passed in 0.40s ``` ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] 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 - [ ] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A ## Additional Notes This change is intentionally scoped to auth behavior and tests in: - `headroom/copilot_auth.py` - `tests/test_copilot_auth.py` --------- Co-authored-by: Abhinav Kaurav <abhinav.kaurav@e2open.com>
This commit is contained in:
parent
c0cadccff9
commit
6b0c09ffd5
3 changed files with 288 additions and 8 deletions
|
|
@ -32,3 +32,19 @@ secret:
|
|||
match: "sk-ant-oat01-oauth-fixture"
|
||||
- name: "Anthropic-shaped fixture token (PAYG via bearer)"
|
||||
match: "sk-ant-api03-payg-bearer-fixture"
|
||||
|
||||
# Minimal GitHub-shaped tokens used in tests/test_copilot_auth.py to
|
||||
# exercise _token_kind() prefix detection and _is_copilot_api_token().
|
||||
# Values are intentionally short/low-entropy — they carry no privilege.
|
||||
- name: "GitHub OAuth token fixture (test_copilot_auth)"
|
||||
match: "gho_x"
|
||||
- name: "GitHub Apps token fixture (test_copilot_auth)"
|
||||
match: "ghs_x"
|
||||
- name: "GitHub PAT fixture (test_copilot_auth)"
|
||||
match: "ghp_x"
|
||||
- name: "GitHub fine-grained PAT fixture (test_copilot_auth)"
|
||||
match: "github_pat_x"
|
||||
- name: "Copilot session token fixture (test_copilot_auth)"
|
||||
match: "tid_x"
|
||||
- name: "GitHub OAuth token fixture for exchange_token test"
|
||||
match: "gho_test"
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@ DEFAULT_TOKEN_EXCHANGE_URL = "https://api.github.com/copilot_internal/v2/token"
|
|||
DEFAULT_USER_INFO_URL = "https://api.github.com/copilot_internal/user"
|
||||
DEFAULT_GITHUB_HOST = "github.com"
|
||||
_TOKEN_EXPIRY_BUFFER_S = 60
|
||||
_DEFAULT_INTEGRATION_ID = "vscode-chat"
|
||||
_DEFAULT_EDITOR_VERSION = "vscode/1.104.1"
|
||||
_DEFAULT_USER_AGENT = "GitHubCopilotChat/0.1"
|
||||
|
||||
|
|
@ -203,26 +204,38 @@ def _read_windows_copilot_cli_oauth_token() -> str | None:
|
|||
return None
|
||||
|
||||
host = _github_host().lower()
|
||||
service_prefixes = [f"copilot-cli/{host}:"]
|
||||
bare_host = host.removeprefix("https://").removeprefix("http://")
|
||||
|
||||
gh_prefix = f"gh:{bare_host}:"
|
||||
copilot_prefixes = [f"copilot-cli/{host}:"]
|
||||
if "://" not in host:
|
||||
service_prefixes.append(f"copilot-cli/https://{host}:")
|
||||
copilot_prefixes.append(f"copilot-cli/https://{host}:")
|
||||
copilot_prefixes.append(f"copilot-cli/https://{host}/")
|
||||
|
||||
gh_tokens: list[str] = []
|
||||
copilot_tokens: list[str] = []
|
||||
|
||||
try:
|
||||
for idx in range(count.value):
|
||||
credential = credentials[idx].contents
|
||||
target = (credential.TargetName or "").strip().lower()
|
||||
if not any(target.startswith(prefix) for prefix in service_prefixes):
|
||||
continue
|
||||
if credential.CredentialBlobSize <= 0 or not credential.CredentialBlob:
|
||||
continue
|
||||
blob = ctypes.string_at(credential.CredentialBlob, credential.CredentialBlobSize)
|
||||
token = blob.decode("utf-8", errors="replace").strip()
|
||||
if token:
|
||||
return token
|
||||
if not token:
|
||||
continue
|
||||
if target.startswith(gh_prefix):
|
||||
gh_tokens.append(token)
|
||||
elif any(target.startswith(p) for p in copilot_prefixes):
|
||||
copilot_tokens.append(token)
|
||||
finally:
|
||||
if credentials:
|
||||
advapi32.CredFree(credentials)
|
||||
|
||||
for token in gh_tokens + copilot_tokens:
|
||||
return token
|
||||
|
||||
return None
|
||||
|
||||
|
||||
|
|
@ -617,13 +630,67 @@ def get_copilot_token_provider() -> CopilotTokenProvider:
|
|||
return _provider
|
||||
|
||||
|
||||
async def apply_copilot_api_auth(headers: dict[str, str], *, url: str) -> dict[str, str]:
|
||||
"""Replace Authorization with a fresh Copilot API token when targeting Copilot."""
|
||||
def _is_copilot_api_token(token: str) -> bool:
|
||||
"""Return True when the token looks like a short-lived Copilot API token.
|
||||
|
||||
Copilot API tokens currently use the "tid_" prefix.
|
||||
GitHub OAuth tokens (for example "gho_", "ghs_", "ghp_", "github_pat_")
|
||||
should be exchanged and must not be forwarded directly.
|
||||
"""
|
||||
normalized = token.strip()
|
||||
if not normalized:
|
||||
return False
|
||||
|
||||
if (
|
||||
normalized.startswith("gho_")
|
||||
or normalized.startswith("ghs_")
|
||||
or normalized.startswith("ghp_")
|
||||
or normalized.startswith("github_pat_")
|
||||
):
|
||||
return False
|
||||
|
||||
return normalized.startswith("tid_")
|
||||
|
||||
|
||||
def _token_kind(token: str) -> str:
|
||||
"""Return a non-sensitive label for the token type, safe to log."""
|
||||
t = token.strip()
|
||||
for prefix in ("tid_", "gho_", "ghs_", "ghp_", "github_pat_"):
|
||||
if t.startswith(prefix):
|
||||
return prefix + "***"
|
||||
return "unknown" if t else "empty"
|
||||
|
||||
|
||||
async def apply_copilot_api_auth(headers: dict[str, str], *, url: str) -> dict[str, str]:
|
||||
"""Apply Copilot auth headers for GitHub Copilot API requests."""
|
||||
resolved = dict(headers)
|
||||
if not is_copilot_api_url(url):
|
||||
return resolved
|
||||
|
||||
lower_keys = {k.lower() for k in resolved}
|
||||
if "copilot-integration-id" not in lower_keys:
|
||||
resolved["Copilot-Integration-Id"] = os.environ.get(
|
||||
"GITHUB_COPILOT_INTEGRATION_ID", _DEFAULT_INTEGRATION_ID
|
||||
)
|
||||
if "editor-version" not in lower_keys:
|
||||
resolved["editor-version"] = os.environ.get(
|
||||
"GITHUB_COPILOT_EDITOR_VERSION", _DEFAULT_EDITOR_VERSION
|
||||
)
|
||||
|
||||
incoming_auth = next((v for k, v in resolved.items() if k.lower() == "authorization"), None)
|
||||
if incoming_auth:
|
||||
scheme, _, raw_token = incoming_auth.partition(" ")
|
||||
if scheme.lower() == "bearer" and raw_token and _is_copilot_api_token(raw_token):
|
||||
logger.info(
|
||||
"apply_copilot_api_auth: passing through client token kind=%s",
|
||||
_token_kind(raw_token),
|
||||
)
|
||||
return resolved
|
||||
logger.info(
|
||||
"apply_copilot_api_auth: incoming token not suitable (kind=%s), will replace",
|
||||
_token_kind(raw_token) if raw_token else "none",
|
||||
)
|
||||
|
||||
token = await get_copilot_token_provider().get_api_token()
|
||||
for key in list(resolved):
|
||||
if key.lower() == "authorization":
|
||||
|
|
|
|||
|
|
@ -334,6 +334,155 @@ def test_apply_copilot_api_auth_replaces_authorization(monkeypatch: pytest.Monke
|
|||
assert "authorization" not in headers
|
||||
|
||||
|
||||
def test_apply_copilot_api_auth_passes_through_existing_api_token(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
async def fake_get_api_token() -> copilot_auth.CopilotAPIToken:
|
||||
raise AssertionError("provider should not be called for existing API token")
|
||||
|
||||
monkeypatch.setattr(
|
||||
copilot_auth.get_copilot_token_provider(),
|
||||
"get_api_token",
|
||||
fake_get_api_token,
|
||||
)
|
||||
|
||||
headers = asyncio.run(
|
||||
copilot_auth.apply_copilot_api_auth(
|
||||
{"authorization": "Bearer tid_existing_copilot_token"},
|
||||
url="https://api.githubcopilot.com/v1/chat/completions",
|
||||
)
|
||||
)
|
||||
|
||||
assert headers["authorization"] == "Bearer tid_existing_copilot_token"
|
||||
|
||||
|
||||
def test_apply_copilot_api_auth_replaces_github_oauth_bearer(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
async def fake_get_api_token() -> copilot_auth.CopilotAPIToken:
|
||||
return copilot_auth.CopilotAPIToken(
|
||||
token="copilot-session",
|
||||
expires_at=time.time() + 3600,
|
||||
api_url=copilot_auth.DEFAULT_API_URL,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
copilot_auth.get_copilot_token_provider(),
|
||||
"get_api_token",
|
||||
fake_get_api_token,
|
||||
)
|
||||
|
||||
headers = asyncio.run(
|
||||
copilot_auth.apply_copilot_api_auth(
|
||||
{"authorization": "Bearer gho_downstream_oauth"},
|
||||
url="https://api.githubcopilot.com/v1/chat/completions",
|
||||
)
|
||||
)
|
||||
|
||||
assert headers["Authorization"] == "Bearer copilot-session"
|
||||
assert "authorization" not in headers
|
||||
|
||||
|
||||
def test_apply_copilot_api_auth_replaces_non_bearer_auth(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
async def fake_get_api_token() -> copilot_auth.CopilotAPIToken:
|
||||
return copilot_auth.CopilotAPIToken(
|
||||
token="copilot-session",
|
||||
expires_at=time.time() + 3600,
|
||||
api_url=copilot_auth.DEFAULT_API_URL,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
copilot_auth.get_copilot_token_provider(),
|
||||
"get_api_token",
|
||||
fake_get_api_token,
|
||||
)
|
||||
|
||||
headers = asyncio.run(
|
||||
copilot_auth.apply_copilot_api_auth(
|
||||
{"authorization": "Basic abc123"},
|
||||
url="https://api.githubcopilot.com/v1/chat/completions",
|
||||
)
|
||||
)
|
||||
|
||||
assert headers["Authorization"] == "Bearer copilot-session"
|
||||
assert "authorization" not in headers
|
||||
|
||||
|
||||
def test_is_copilot_api_token_matches_expected_prefixes() -> None:
|
||||
assert copilot_auth._is_copilot_api_token("tid_session_token") is True
|
||||
assert copilot_auth._is_copilot_api_token("gho_oauth") is False
|
||||
assert copilot_auth._is_copilot_api_token("ghs_oauth") is False
|
||||
assert copilot_auth._is_copilot_api_token("ghp_oauth") is False
|
||||
assert copilot_auth._is_copilot_api_token("github_pat_example") is False
|
||||
assert copilot_auth._is_copilot_api_token("Bearer maybe") is False
|
||||
|
||||
|
||||
def test_apply_copilot_api_auth_injects_required_headers(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
async def fake_get_api_token() -> copilot_auth.CopilotAPIToken:
|
||||
return copilot_auth.CopilotAPIToken(
|
||||
token="copilot-session",
|
||||
expires_at=time.time() + 3600,
|
||||
api_url=copilot_auth.DEFAULT_API_URL,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
copilot_auth.get_copilot_token_provider(),
|
||||
"get_api_token",
|
||||
fake_get_api_token,
|
||||
)
|
||||
monkeypatch.delenv("GITHUB_COPILOT_INTEGRATION_ID", raising=False)
|
||||
monkeypatch.delenv("GITHUB_COPILOT_EDITOR_VERSION", raising=False)
|
||||
|
||||
headers = asyncio.run(
|
||||
copilot_auth.apply_copilot_api_auth(
|
||||
{},
|
||||
url="https://api.githubcopilot.com/v1/chat/completions",
|
||||
)
|
||||
)
|
||||
|
||||
assert headers["Authorization"] == "Bearer copilot-session"
|
||||
assert headers["Copilot-Integration-Id"] == "vscode-chat"
|
||||
assert headers["editor-version"] == "vscode/1.104.1"
|
||||
|
||||
|
||||
def test_apply_copilot_api_auth_preserves_existing_copilot_headers(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
async def fake_get_api_token() -> copilot_auth.CopilotAPIToken:
|
||||
return copilot_auth.CopilotAPIToken(
|
||||
token="copilot-session",
|
||||
expires_at=time.time() + 3600,
|
||||
api_url=copilot_auth.DEFAULT_API_URL,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
copilot_auth.get_copilot_token_provider(),
|
||||
"get_api_token",
|
||||
fake_get_api_token,
|
||||
)
|
||||
monkeypatch.setenv("GITHUB_COPILOT_INTEGRATION_ID", "should-not-override")
|
||||
monkeypatch.setenv("GITHUB_COPILOT_EDITOR_VERSION", "should-not-override")
|
||||
|
||||
headers = asyncio.run(
|
||||
copilot_auth.apply_copilot_api_auth(
|
||||
{
|
||||
"Authorization": "Bearer downstream-token",
|
||||
"Copilot-Integration-Id": "custom-integration",
|
||||
"Editor-Version": "custom-editor",
|
||||
},
|
||||
url="https://api.githubcopilot.com/v1/chat/completions",
|
||||
)
|
||||
)
|
||||
|
||||
assert headers["Copilot-Integration-Id"] == "custom-integration"
|
||||
assert headers["Editor-Version"] == "custom-editor"
|
||||
assert headers["Authorization"] == "Bearer copilot-session"
|
||||
|
||||
|
||||
def test_token_provider_reuses_oauth_token_without_exchange(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
|
@ -462,3 +611,51 @@ def test_read_windows_copilot_cli_oauth_token_returns_none_without_windll(
|
|||
monkeypatch.delattr(copilot_auth.ctypes, "WinDLL", raising=False)
|
||||
|
||||
assert copilot_auth._read_windows_copilot_cli_oauth_token() is None
|
||||
|
||||
|
||||
def test_is_copilot_api_token_returns_false_for_empty_string() -> None:
|
||||
assert copilot_auth._is_copilot_api_token("") is False
|
||||
assert copilot_auth._is_copilot_api_token(" ") is False
|
||||
|
||||
|
||||
def test_token_kind_returns_known_prefixes() -> None:
|
||||
assert copilot_auth._token_kind("tid_x") == "tid_***" # noqa: S105
|
||||
assert copilot_auth._token_kind("gho_x") == "gho_***" # noqa: S105
|
||||
assert copilot_auth._token_kind("ghs_x") == "ghs_***" # noqa: S105
|
||||
assert copilot_auth._token_kind("ghp_x") == "ghp_***" # noqa: S105
|
||||
assert copilot_auth._token_kind("github_pat_x") == "github_pat_***" # noqa: S105
|
||||
|
||||
|
||||
def test_token_kind_returns_unknown_for_unrecognised_token() -> None:
|
||||
assert copilot_auth._token_kind("some_random_token") == "unknown"
|
||||
|
||||
|
||||
def test_token_kind_returns_empty_for_blank_token() -> None:
|
||||
assert copilot_auth._token_kind("") == "empty"
|
||||
assert copilot_auth._token_kind(" ") == "empty"
|
||||
|
||||
|
||||
def test_exchange_token_sync_returns_payload_on_success(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
payload = {"token": "copilot-api", "expires_at": int(time.time()) + 3600}
|
||||
|
||||
class FakeResponse:
|
||||
def read(self) -> bytes:
|
||||
return json.dumps(payload).encode()
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *args):
|
||||
pass
|
||||
|
||||
monkeypatch.setattr(
|
||||
copilot_auth.urllib_request,
|
||||
"urlopen",
|
||||
lambda *args, **kwargs: FakeResponse(),
|
||||
)
|
||||
|
||||
result = copilot_auth.CopilotTokenProvider._exchange_token_sync(
|
||||
{"Authorization": "Bearer gho_test"} # noqa: S105
|
||||
)
|
||||
|
||||
assert result == payload
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue