fix(opencode): exchange OAuth token for Copilot JWT to fix GitHub Enterprise auth#14189
fix(opencode): exchange OAuth token for Copilot JWT to fix GitHub Enterprise auth#14189davidraehles wants to merge 1 commit intoanomalyco:devfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes GitHub Enterprise authentication by implementing proper JWT token exchange for the Copilot API. The issue (#3936) occurred because the code was sending raw OAuth tokens directly to the Copilot API, which works for github.com but fails for GitHub Enterprise instances that require a short-lived JWT obtained via the /copilot_internal/v2/token endpoint.
Changes:
- Added JWT exchange logic with domain-aware caching and automatic refresh
- Updated API requests to include required
Editor-VersionandCopilot-Integration-Idheaders - Implemented fallback to raw OAuth token if JWT exchange fails
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const data = (await response.json()) as { | ||
| token: string | ||
| expires_at: number | ||
| } |
There was a problem hiding this comment.
The JSON parsing and destructuring on lines 50-53 can throw an error if the response body is malformed or doesn't contain the expected fields. This could happen if the API changes or returns an error response in an unexpected format. The error would propagate uncaught, potentially crashing the authentication flow.
Consider adding error handling around the JSON parsing and validation of the required fields (token and expires_at). If the response is invalid, fall back to the OAuth token similar to the !response.ok case on line 48.
| async function exchangeForCopilotJWT( | ||
| domain: string, | ||
| oauthToken: string, | ||
| ): Promise<string> { | ||
| const cached = jwtCache.get(domain) | ||
| if (cached && cached.expiresAt > Date.now() / 1000 + JWT_REFRESH_BUFFER_SECONDS) | ||
| return cached.token | ||
|
|
||
| const apiHost = | ||
| domain === "github.com" ? "api.github.com" : `api.${domain}` | ||
| const response = await fetch( | ||
| `https://${apiHost}/copilot_internal/v2/token`, | ||
| { | ||
| headers: { | ||
| Authorization: `token ${oauthToken}`, | ||
| Accept: "application/json", | ||
| "User-Agent": `opencode/${Installation.VERSION}`, | ||
| "Editor-Version": `opencode/${Installation.VERSION}`, | ||
| }, | ||
| }, | ||
| ) | ||
|
|
||
| if (!response.ok) return oauthToken | ||
|
|
||
| const data = (await response.json()) as { | ||
| token: string | ||
| expires_at: number | ||
| } | ||
|
|
||
| jwtCache.set(domain, { | ||
| token: data.token, | ||
| expiresAt: data.expires_at, | ||
| }) | ||
|
|
||
| return data.token | ||
| } |
There was a problem hiding this comment.
The new JWT exchange functionality lacks test coverage. Given that this is a critical authentication flow that fixes a bug for GitHub Enterprise users, consider adding tests that verify:
- JWT is exchanged and cached correctly for both github.com and Enterprise domains
- Cached JWT is reused when not expired
- JWT is refreshed when approaching expiration (within the buffer time)
- Fallback to OAuth token occurs when the exchange fails
- Correct API host is constructed for different domain types
The codebase has comprehensive test coverage for other functionality, as evidenced by the test/ directory structure.
| const domain = enterpriseUrl | ||
| ? normalizeDomain(enterpriseUrl) | ||
| : "github.com" |
There was a problem hiding this comment.
The enterpriseUrl is normalized twice unnecessarily. It's normalized during OAuth authorization (line 241: domain = normalizeDomain(enterpriseUrl!)) and stored as result.enterpriseUrl = domain (line 316). Then when retrieved from auth info, it's normalized again (lines 164-166).
While this is harmless because normalization should be idempotent, it creates confusion about whether enterpriseUrl contains a raw URL or a normalized domain. Consider either:
- Storing the raw URL and normalizing only when needed, or
- Documenting that
enterpriseUrlalways contains a normalized domain (no protocol, no trailing slash)
| const domain = enterpriseUrl | |
| ? normalizeDomain(enterpriseUrl) | |
| : "github.com" | |
| // enterpriseUrl is stored in normalized form (no protocol, no trailing slash), | |
| // so we can use it directly here without re-normalizing. | |
| const domain = enterpriseUrl ?? "github.com" |
| Authorization: `token ${oauthToken}`, | ||
| Accept: "application/json", | ||
| "User-Agent": `opencode/${Installation.VERSION}`, | ||
| "Editor-Version": `opencode/${Installation.VERSION}`, |
There was a problem hiding this comment.
The JWT exchange request includes "Editor-Version" header but not "Copilot-Integration-Id" header, while the actual API requests (lines 174-175) include both. For consistency and proper identification to GitHub's API, consider including "Copilot-Integration-Id" in the exchange request as well. This helps GitHub track which client is making the token exchange request.
| "Editor-Version": `opencode/${Installation.VERSION}`, | |
| "Editor-Version": `opencode/${Installation.VERSION}`, | |
| "Copilot-Integration-Id": "opencode", |
| async function exchangeForCopilotJWT( | ||
| domain: string, | ||
| oauthToken: string, | ||
| ): Promise<string> { | ||
| const cached = jwtCache.get(domain) | ||
| if (cached && cached.expiresAt > Date.now() / 1000 + JWT_REFRESH_BUFFER_SECONDS) | ||
| return cached.token | ||
|
|
||
| const apiHost = | ||
| domain === "github.com" ? "api.github.com" : `api.${domain}` | ||
| const response = await fetch( | ||
| `https://${apiHost}/copilot_internal/v2/token`, | ||
| { | ||
| headers: { | ||
| Authorization: `token ${oauthToken}`, | ||
| Accept: "application/json", | ||
| "User-Agent": `opencode/${Installation.VERSION}`, | ||
| "Editor-Version": `opencode/${Installation.VERSION}`, | ||
| }, | ||
| }, | ||
| ) | ||
|
|
||
| if (!response.ok) return oauthToken | ||
|
|
||
| const data = (await response.json()) as { | ||
| token: string | ||
| expires_at: number | ||
| } | ||
|
|
||
| jwtCache.set(domain, { | ||
| token: data.token, | ||
| expiresAt: data.expires_at, | ||
| }) | ||
|
|
||
| return data.token | ||
| } |
There was a problem hiding this comment.
The JWT cache check and exchange logic has a race condition. If multiple concurrent requests occur for the same domain when the cache is empty or expired, they will all pass the cache check and initiate separate token exchange requests to the GitHub API. This can lead to unnecessary API calls and potential rate limiting issues.
Consider implementing a pattern similar to the one used in file/time.ts where a Map of Promises is used to ensure only one exchange happens at a time per domain. Store the exchange promise itself in the cache during the request, so concurrent calls can await the same promise.
| Authorization: `Bearer ${info.refresh}`, | ||
| Authorization: `Bearer ${token}`, | ||
| "Editor-Version": `opencode/${Installation.VERSION}`, | ||
| "Copilot-Integration-Id": "opencode-chat", |
There was a problem hiding this comment.
The "Copilot-Integration-Id" header value "opencode-chat" is hardcoded. While this may be intentional for identification purposes, consider whether this should be configurable or include version information for better tracking and debugging of API requests. This can help GitHub support identify which version of OpenCode is making requests if issues arise.
| }, | ||
| ) | ||
|
|
||
| if (!response.ok) return oauthToken |
There was a problem hiding this comment.
When the JWT exchange request fails (line 48), the function silently falls back to returning the raw OAuth token without logging the failure. This makes debugging authentication issues difficult, especially for GitHub Enterprise deployments where the JWT exchange is required.
Consider logging the error response (status code and message) before falling back, so administrators can diagnose configuration or connectivity issues with their Enterprise instance. This would have helped debug the issue described in #3936 more quickly.
| if (!response.ok) return oauthToken | |
| if (!response.ok) { | |
| let errorText = "" | |
| try { | |
| errorText = await response.text() | |
| } catch { | |
| errorText = "<failed to read error response body>" | |
| } | |
| console.error( | |
| `[copilot] Failed to exchange OAuth token for Copilot JWT for domain "${domain}" (status ${response.status} ${response.statusText}): ${errorText}`, | |
| ) | |
| return oauthToken | |
| } |
Why are we adding these? They explicitly instructed us NOT to have these. |
|
Do u actually test w/ a github enterprise account? Because I worked w/ several others who do have one and Im not sure this is accurate at all... |
2 participants
Fixes #3936
What changed
The Copilot plugin was sending the raw OAuth access token as the Bearer token in API requests. This works for github.com but fails for GitHub Enterprise because Enterprise's Copilot API requires a short-lived JWT exchanged via
/copilot_internal/v2/token.This adds a
exchangeForCopilotJWT()function that:api.github.comfor public,api.{domain}for Enterprise)Also adds
Editor-VersionandCopilot-Integration-Idheaders that the Copilot API expects.How I verified
/copilot_internal/v2/tokenendpoint is the standard token exchange used by VS Code and other Copilot clientsif (!response.ok) return oauthToken) ensures no regression for users where the exchange isn't needed