Skip to content

fix(copilot): import a token any Copilot client left on disk - #3527

Open
olifarhaan wants to merge 1 commit into
charmbracelet:mainfrom
olifarhaan:fix-copilot-disk-token
Open

fix(copilot): import a token any Copilot client left on disk#3527
olifarhaan wants to merge 1 commit into
charmbracelet:mainfrom
olifarhaan:fix-copilot-disk-token

Conversation

@olifarhaan

Copy link
Copy Markdown

What

crush only picks up a Copilot token from ~/.config/github-copilot/apps.json when copilot.vim wrote it. A token any other Copilot client left there is ignored, so login starts a device flow with a perfectly good credential sitting on disk

Why it happened

RefreshTokenFromDisk (disk.go:23) looks up one hardcoded key, github.com:Iv1.b507a08c87ecfe98. Clients key their entry by host plus their own app id, so any other id, and the plain github.com form, miss. crush login copilot (login.go:149) and the copilot model switch in the TUI (ui.go:2209) both fall through to the device flow when that lookup comes back empty

codecompanion.nvim, avante.nvim and jcode all match on the host part of the key rather than one app id

Fix

Keep preferring our own app id, then fall back to any github.com entry that carries a token. Keys are sorted so a file with several entries always resolves to the same one instead of following map order. Enterprise hosts stay excluded, since a ghe.com token is not valid against api.github.com

Related to #3399

Proof

apps.json holding one entry from another client:

$ cat $HOME/.config/github-copilot/apps.json
{"github.com:Iv23liOtherClient":{"user":"octocat","oauth_token":"gho_token_written_by_another_client"}}

$ crush login copilot          # v0.88.1
Requesting device code from GitHub...

The following code should be on clipboard already:

B810-3318

$ crush login copilot          # this branch
Found existing GitHub Copilot token on disk. Using it to authenticate...
   ERROR  Unable to refresh token from disk: copilot token request failed: 401 Unauthorized - {
     "message": "Bad credentials",

The 401 is the placeholder token in the fixture. The point is which branch runs: before, the credential on disk is invisible and you get a device code

Tests

TestRefreshTokenFromDisk covers our app id, a foreign app id, the bare github.com key, our id winning when both are present, a ghe.com entry being skipped, a github.com.evil.example lookalike being rejected, an empty token being passed over, and a malformed file. TestRefreshTokenFromDiskIsStableAcrossCalls pins the ordering. Three of them fail on main

go test -race ./internal/oauth/... and golangci-lint v2.11 clean

One correction on the issue: its main claim is that the device-flow poller fires early. It cannot, ticker.Reset only runs after the response lands, so every gap is at least the mandated interval plus the round trip, and the 1s deltas in the capture look like whole-second timestamps. This fixes the other half it reports, that a token placed in apps.json is not picked up, so I have left it open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant