Deduplicate Copilot bearer-prefix stripping in api-proxy#4951
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes a duplicated stripBearerPrefix() implementation in the api-proxy and standardizes upstream-response to use the canonical helper from providers/copilot-auth, aligning empty/whitespace-only credential handling to return undefined.
Changes:
- Removed local
stripBearerPrefix()fromcontainers/api-proxy/upstream-response.jsand imported the shared helper fromcontainers/api-proxy/providers/copilot-auth.js. - Updated
buildCopilotAuthErrorMessage()to rely on the canonical empty-value semantics (empty stripped values treated as unset). - Added a regression unit test ensuring an empty stripped BYOK key is treated as missing through the
upstream-responsepath.
Show a summary per file
| File | Description |
|---|---|
| containers/api-proxy/upstream-response.js | Switches bearer-prefix stripping to the shared canonical helper and exposes buildCopilotAuthErrorMessage via _testing for unit tests. |
| containers/api-proxy/server.auth.test.js | Adds a focused regression test covering the empty-stripped BYOK key behavior via upstream-response’s auth error message path. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
🔥 Smoke Test Results — Auth mode: PAT (COPILOT_GITHUB_TOKEN)
Overall: PASS cc
|
🔭 Smoke Test: API Proxy OTEL Tracing
All scenarios passed. ✅
|
|
Smoke Test: Copilot BYOK (Direct Mode) — PASS ✅
Running in direct BYOK mode (
|
🔥 Smoke Test Results — PASS
PR: Deduplicate Copilot bearer-prefix stripping in api-proxy Overall: PASS
|
|
Deduplicate Copilot bearer-prefix stripping in api-proxy ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Chroot Smoke Test Results
Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments.
|
Smoke Test Results — FAIL ❌
|
|
Smoke Test Results:
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) Overall: FAIL
|
Gemini Engine Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall Status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
stripBearerPrefix()existed in bothupstream-response.jsandproviders/copilot-auth.jswith the same regex but different empty-value semantics. This change removes the duplicate implementation and makesupstream-responseuse the shared canonical helper, so Bearer/token normalization now stays consistent in one place.What changed
stripBearerPrefix()fromcontainers/api-proxy/upstream-response.jscontainers/api-proxy/providers/copilot-auth.jsupstream-responseon the canonicalundefinedreturn for empty/whitespace-only credentials after prefix strippingBehavioral impact
buildCopilotAuthErrorMessage()now treats values like `****** the same way as the rest of the Copilot auth path: effectively unsetCoverage
upstream-responsecode path