Extract shared tokenAuthHeaders helper to deduplicate auth-prefix header assembly in copilot.js - #7875
Conversation
…fix logic Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The refactor preserves existing authentication behavior and has appropriate focused and existing regression coverage.
Pull request overview
Centralizes authorization-header construction and removes repeated Copilot auth-prefix assembly.
Changes:
- Adds reusable
tokenAuthHeaders. - Refactors Copilot header construction to use the helper.
- Adds focused helper tests.
File summaries
| File | Description |
|---|---|
containers/api-proxy/providers/auth-headers.js |
Adds and exports the shared helper. |
containers/api-proxy/providers/copilot.js |
Reuses centralized auth-prefix/header assembly. |
containers/api-proxy/providers/auth-headers.test.js |
Tests composition, merging, and immutability. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
✅ Copilot review passed with no inline comments. @copilot Add the |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
|
🔌 Smoke Services — All services reachable! ✅
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤
|
|
✅ Contribution Check completed successfully! Contribution check complete: PR #7875 follows the relevant CONTRIBUTING.md guidance. It includes unit tests for the new helper, keeps the change in the correct container directory, updates the code in a TypeScript/JS style consistent with the repo, and references the related issue (#7873) in the PR description. No additional reviewer comment needed.
|
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓
|
|
✅ Build Test Suite completed successfully!
|
|
✅ Smoke Claude passed Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.anthropic.com"See Network Configuration for more information.
|
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "msfeed25.pkgs.visualstudio.com"
- "registry.npmjs.org"See Network Configuration for more information.
|
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤
|
|
❌ Smoke Gemini reports failed. Facets need polishing...
|
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓
|
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅
|
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓
|
|
✅ Security Guard completed successfully! Security review complete - PR #7875 passes analysis. Refactoring extracts duplicate auth-header assembly into shared helper (tokenAuthHeaders). No security weakening: token handling unchanged, no capability additions, no validation regressions, maintains functional equivalence across three call sites. Consolidating security-critical code reduces inconsistency risk.
|
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded.
|
|
Smoke Test: Copilot Engine —
Overall: PASS
|
|
EGRESS_RESULT allow=pass deny=pass ✅ Allowed domain (github.com) reachable — HTTP 200 Overall status: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
github-actions
Bot
added
the
smoke-copilot-network-isolation
Smoke Test: Claude Engine Validation
Overall result: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.anthropic.com"See Network Configuration for more information.
|
✅ Smoke Test: Copilot BYOK (Direct) ModeTest Results:
Status: PASS — Direct BYOK mode fully operational
|
|
Smoke Test: Docker Sbx —
PR: #7875 Extract shared tokenAuthHeaders helper to deduplicate auth-prefix header assembly in copilot.js Overall: PASS
|
Smoke Test Results: GitHub Actions Services Connectivity
Overall: FAIL —
|
|
Extract shared tokenAuthHeaders helper to deduplicate auth-prefix header assembly in copilot.js
|
|
|
Chroot Version Comparison Results
Overall: FAILED — Node.js version mismatch between host and chroot environments.
|
📡 OTel Tracing Smoke Test Results
All scenarios pass or are expected-pending as designed. No regressions detected in the OTEL tracing integration.
|
|
PRs:
Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "msfeed25.pkgs.visualstudio.com"
- "registry.npmjs.org"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — PASS Notes:
|
Provider adapters repeated auth-header assembly logic across
openai.js,anthropic.js,copilot.js, andgoogle-adapter.js, withcopilot.jsin particular reimplementing the sameprefix + ' ' + tokenAuthorization-header concatenation in three separate places (models request, static headers, and/modelsauth resolution).Shared helper
tokenAuthHeaders(prefix, token, extraHeaders)tocontainers/api-proxy/providers/auth-headers.js— builds anAuthorization: <prefix> <token>header, optionally merged with extra headers.bearerAuthHeadersis now implemented in terms oftokenAuthHeaders('Bearer', ...), making it the single source of truth for Authorization-header assembly.Copilot adapter cleanup
containers/api-proxy/providers/copilot.jsnow callstokenAuthHeadersinstead of manually concatenating the auth prefix and token inbuildCopilotModelsRequest,buildStaticHeaders, andgetAuthHeaders.requiresGitHubTokenPrefix ? 'token' : 'Bearer') into a singlegithubTokenAuthPrefixconstant instead of recomputing it inline at each call site.Tests
tokenAuthHeaderscovering prefix/token composition, extra-header merging, non-mutation of inputs, and equivalence withbearerAuthHeaders.openai.js,anthropic.js, andgoogle-adapter.jsalready composed headers through the shared helpers and did not require changes.