Commit 397803a
fix(copilot): bind the minted token to the integration ID we forward (#3164)
## Description
Reported from a Copilot CLI session:
```
[CopilotCLISession] Failed to fetch models: Error: 401 "unauthorized:
unable to validate HMAC for the given Copilot-Integration-ID"
[CopilotCLISession] Proxy URL configured (authType=hmac), skipping
client-side token validation
```
GitHub **binds a Copilot API token to the `Copilot-Integration-Id` it
was minted under** and verifies the pairing with an HMAC. Present a
token minted for integration A alongside a header naming integration B,
and you get exactly this error.
`apply_copilot_api_auth` applied the integration ID with *set-default*
semantics — `_set_header_default` returns early when the header is
already present — **before** deciding whose token to use:
```python
for name, value in _copilot_chat_header_defaults().items():
_set_header_default(resolved, name, value) # ← never overwrites
...
if incoming_auth and _is_forwardable_copilot_bearer_token(...):
return resolved # client's token kept
...
token = await get_copilot_token_provider().get_api_token() # ← REPLACED
```
The client always sends an ID, so when Headroom replaced the token — the
common case, logged as `incoming token not suitable (kind=unknown), will
replace` — the request left carrying **the client's integration ID next
to Headroom's token**, minted under `vscode-chat` via
`_copilot_token_exchange_headers`. A Copilot CLI session does not
identify as `vscode-chat`.
The second log line is why nothing caught it sooner: seeing a proxy URL,
the Copilot client reports `authType=hmac` and **skips its own token
validation**, deferring to the proxy. Nobody validates the pairing until
GitHub rejects it.
**Why this matters beyond one 401:** the failing call is *model
discovery*. When it fails the client falls back to its built-in model
list — which is why a user's selected model never appeared in telemetry
and all traffic surfaced as `gpt-4o-mini`.
Closes #
## 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
Restores one invariant: **the credential and the integration ID leave
together.**
- **Mint under the client's ID** rather than the proxy's default, so
GitHub's usage attribution keeps pointing at the surface that actually
made the call.
- **Overwrite the forwarded header to match what we minted** — but only
on the replace path. The pass-through branch returns earlier and keeps
the client's own ID beside the client's own token, which is equally a
matched pair.
- **Key the token cache by integration ID.** A single slot would hand a
`vscode-chat` token to a CLI session and reproduce the same 401 straight
from cache.
Two existing contracts deliberately preserved:
- Resolution order is **client header > `GITHUB_COPILOT_INTEGRATION_ID`
> built-in default**. The env var configures the *default* this proxy
sends; it does not override a client that stated its own identity.
Pinned by the existing
`test_apply_copilot_api_auth_preserves_existing_copilot_headers` (whose
fixture literally names the value `should-not-override`).
- The overwrite writes through the client's **existing key**, so a
lowercase `copilot-integration-id` does not gain a second capitalised
variant beside it — pinned by the existing
`..._preserves_existing_headers_case_insensitively`.
Existing test stubs for `get_api_token` gained the new keyword — the
same signature-drift hazard this repo just hit in
`RemoteKompressCompressor` (#3162).
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ pytest tests/ -q -k copilot
338 passed, 8 skipped
$ pytest tests/ -q # this branch
6 failed, 11386 passed, 587 skipped in 425.40s
All 6 also fail on clean origin/main, same machine — pre-existing, not regressions:
test_graceful_shutdown.py::test_run_server_installs_cancelled_error_filter
test_learn/test_integration.py::TestCodexIntegration::test_full_pipeline
test_release_workflows.py::test_no_native_tls_in_wheel_build_tree
test_providers/test_deepseek.py::... (3 litellm pricing tests)
$ ruff check headroom/
All checks passed!
$ mypy headroom/copilot_auth.py
0 errors
```
12 new tests: the mint/forward pairing, the pass-through branch keeping
the client's pair untouched, no duplicate case-variant header,
resolution order in both directions, blank/absent client values,
non-Copilot upstreams untouched, and per-integration cache isolation.
## Real Behavior Proof
- **Environment:** macOS, Python 3.12.13, branch on `origin/main` @
`a3821378`.
- **Exact command / steps:** drove `apply_copilot_api_auth` with the
reported shape — an unusable client bearer plus `Copilot-Integration-Id:
copilot-cli-chat` against `api.githubcopilot.com` — and compared the ID
the token would be **minted under** (via
`_copilot_token_exchange_headers`) against the ID actually
**forwarded**. Run against the same script before and after the change,
with `PYTHONPATH` pinned to the worktree.
- **Observed result:**
```
########## PRE-FIX ##########
token minted under : vscode-chat
header forwarded : copilot-cli-chat
-> GitHub would REJECT (401 HMAC)
########## POST-FIX ##########
token minted under : copilot-cli-chat
header forwarded : copilot-cli-chat
-> GitHub would ACCEPT
```
- **Not tested:** no live call to GitHub's CAPI — the HMAC is validated
server-side by GitHub and cannot be exercised offline. The claim
verified here is that the two halves now agree; that GitHub accepts a
correctly-paired credential is inferred from its error message, not
observed. **Worth one live Copilot CLI run before shipping to a
reporter.** The `GITHUB_COPILOT_API_TOKEN` path is also unchanged: an
externally-supplied token was minted under an integration this proxy
cannot know, so it is passed through as before.
## Runtime Rollout Safety
- **Rollout-managed feature(s):** none.
- **Minimum rollout channel:** n/a.
- **Stable/default behavior changed:** requests where Headroom replaces
the token now forward the integration ID the replacement was minted
under. For a client sending `vscode-chat` (VS Code, the previous
default) nothing changes at all — the resolved value is identical.
- **Kill switch / disable path:** setting
`GITHUB_COPILOT_INTEGRATION_ID` pins the value used for clients that
send none; clients that send one are unaffected either way.
- **Unsafe override required:** none.
- **Qualification impact:** model discovery should stop 401ing for
non-VS-Code Copilot surfaces, which restores the real model list.
- **Rollback path:** revert the commit; behavior returns to minting
under `vscode-chat` regardless of caller.
## 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
Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>1 parent 45cb1b9 commit 397803a
4 files changed
Lines changed: 372 additions & 32 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| 13 | + | |
13 | 14 | | |
14 | 15 | | |
15 | 16 | | |
| |||
902 | 903 | | |
903 | 904 | | |
904 | 905 | | |
905 | | - | |
| 906 | + | |
| 907 | + | |
| 908 | + | |
| 909 | + | |
| 910 | + | |
| 911 | + | |
| 912 | + | |
| 913 | + | |
| 914 | + | |
| 915 | + | |
| 916 | + | |
| 917 | + | |
| 918 | + | |
| 919 | + | |
| 920 | + | |
| 921 | + | |
| 922 | + | |
| 923 | + | |
| 924 | + | |
| 925 | + | |
| 926 | + | |
| 927 | + | |
| 928 | + | |
| 929 | + | |
| 930 | + | |
| 931 | + | |
| 932 | + | |
| 933 | + | |
| 934 | + | |
| 935 | + | |
| 936 | + | |
| 937 | + | |
| 938 | + | |
| 939 | + | |
| 940 | + | |
| 941 | + | |
| 942 | + | |
| 943 | + | |
| 944 | + | |
906 | 945 | | |
907 | 946 | | |
908 | 947 | | |
| |||
915 | 954 | | |
916 | 955 | | |
917 | 956 | | |
918 | | - | |
919 | | - | |
920 | | - | |
921 | | - | |
922 | | - | |
| 957 | + | |
923 | 958 | | |
924 | 959 | | |
925 | 960 | | |
| 961 | + | |
| 962 | + | |
| 963 | + | |
| 964 | + | |
| 965 | + | |
| 966 | + | |
| 967 | + | |
| 968 | + | |
| 969 | + | |
| 970 | + | |
| 971 | + | |
| 972 | + | |
| 973 | + | |
| 974 | + | |
| 975 | + | |
| 976 | + | |
926 | 977 | | |
927 | 978 | | |
928 | 979 | | |
| |||
932 | 983 | | |
933 | 984 | | |
934 | 985 | | |
935 | | - | |
| 986 | + | |
| 987 | + | |
| 988 | + | |
936 | 989 | | |
937 | 990 | | |
938 | 991 | | |
939 | | - | |
| 992 | + | |
940 | 993 | | |
941 | 994 | | |
942 | 995 | | |
| |||
1302 | 1355 | | |
1303 | 1356 | | |
1304 | 1357 | | |
1305 | | - | |
| 1358 | + | |
| 1359 | + | |
| 1360 | + | |
| 1361 | + | |
| 1362 | + | |
| 1363 | + | |
| 1364 | + | |
1306 | 1365 | | |
1307 | | - | |
| 1366 | + | |
| 1367 | + | |
| 1368 | + | |
| 1369 | + | |
| 1370 | + | |
| 1371 | + | |
| 1372 | + | |
| 1373 | + | |
| 1374 | + | |
| 1375 | + | |
| 1376 | + | |
| 1377 | + | |
| 1378 | + | |
| 1379 | + | |
| 1380 | + | |
1308 | 1381 | | |
1309 | 1382 | | |
1310 | 1383 | | |
| |||
1314 | 1387 | | |
1315 | 1388 | | |
1316 | 1389 | | |
1317 | | - | |
| 1390 | + | |
1318 | 1391 | | |
1319 | 1392 | | |
1320 | 1393 | | |
1321 | 1394 | | |
1322 | | - | |
| 1395 | + | |
1323 | 1396 | | |
1324 | 1397 | | |
1325 | 1398 | | |
| |||
1331 | 1404 | | |
1332 | 1405 | | |
1333 | 1406 | | |
1334 | | - | |
| 1407 | + | |
1335 | 1408 | | |
1336 | 1409 | | |
1337 | | - | |
1338 | | - | |
| 1410 | + | |
| 1411 | + | |
1339 | 1412 | | |
1340 | 1413 | | |
1341 | 1414 | | |
| |||
1348 | 1421 | | |
1349 | 1422 | | |
1350 | 1423 | | |
1351 | | - | |
| 1424 | + | |
1352 | 1425 | | |
1353 | 1426 | | |
1354 | | - | |
1355 | | - | |
| 1427 | + | |
| 1428 | + | |
1356 | 1429 | | |
1357 | 1430 | | |
1358 | | - | |
1359 | | - | |
| 1431 | + | |
| 1432 | + | |
| 1433 | + | |
| 1434 | + | |
1360 | 1435 | | |
1361 | 1436 | | |
1362 | 1437 | | |
| |||
1503 | 1578 | | |
1504 | 1579 | | |
1505 | 1580 | | |
1506 | | - | |
| 1581 | + | |
| 1582 | + | |
| 1583 | + | |
| 1584 | + | |
| 1585 | + | |
| 1586 | + | |
| 1587 | + | |
1507 | 1588 | | |
1508 | 1589 | | |
1509 | 1590 | | |
| |||
1533 | 1614 | | |
1534 | 1615 | | |
1535 | 1616 | | |
1536 | | - | |
| 1617 | + | |
1537 | 1618 | | |
1538 | 1619 | | |
1539 | 1620 | | |
1540 | 1621 | | |
| 1622 | + | |
| 1623 | + | |
| 1624 | + | |
| 1625 | + | |
| 1626 | + | |
| 1627 | + | |
| 1628 | + | |
| 1629 | + | |
| 1630 | + | |
| 1631 | + | |
| 1632 | + | |
| 1633 | + | |
| 1634 | + | |
| 1635 | + | |
1541 | 1636 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
986 | 986 | | |
987 | 987 | | |
988 | 988 | | |
989 | | - | |
| 989 | + | |
| 990 | + | |
| 991 | + | |
990 | 992 | | |
991 | 993 | | |
992 | 994 | | |
| |||
1018 | 1020 | | |
1019 | 1021 | | |
1020 | 1022 | | |
1021 | | - | |
| 1023 | + | |
| 1024 | + | |
| 1025 | + | |
1022 | 1026 | | |
1023 | 1027 | | |
1024 | 1028 | | |
| |||
1044 | 1048 | | |
1045 | 1049 | | |
1046 | 1050 | | |
1047 | | - | |
| 1051 | + | |
| 1052 | + | |
| 1053 | + | |
1048 | 1054 | | |
1049 | 1055 | | |
1050 | 1056 | | |
| |||
1117 | 1123 | | |
1118 | 1124 | | |
1119 | 1125 | | |
1120 | | - | |
| 1126 | + | |
| 1127 | + | |
| 1128 | + | |
1121 | 1129 | | |
1122 | 1130 | | |
1123 | 1131 | | |
| |||
1172 | 1180 | | |
1173 | 1181 | | |
1174 | 1182 | | |
1175 | | - | |
| 1183 | + | |
| 1184 | + | |
| 1185 | + | |
1176 | 1186 | | |
1177 | 1187 | | |
1178 | 1188 | | |
| |||
1203 | 1213 | | |
1204 | 1214 | | |
1205 | 1215 | | |
1206 | | - | |
| 1216 | + | |
| 1217 | + | |
| 1218 | + | |
1207 | 1219 | | |
1208 | 1220 | | |
1209 | 1221 | | |
| |||
1237 | 1249 | | |
1238 | 1250 | | |
1239 | 1251 | | |
1240 | | - | |
| 1252 | + | |
| 1253 | + | |
| 1254 | + | |
1241 | 1255 | | |
1242 | 1256 | | |
1243 | 1257 | | |
| |||
0 commit comments