Fix Copilot proxy auth and GHEC model discovery - #8038
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Three moderate issues leave integration-ID precedence, diagnostics, and GHEC header preservation incomplete.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes Copilot authentication, GHEC model discovery, placeholder handling, and safe diagnostics.
Changes:
- Selects authorization schemes by credential type.
- Adds canonical GHEC request handling and integration identity support.
- Rejects legacy credential-isolation placeholders.
- Adds targeted regression tests.
Required fixes:
- Forward
GITHUB_COPILOT_INTEGRATION_IDinto the sidecar environment. - Expose the new diagnostic fields through the management
/reflectresponse. - Preserve caller-provided, case-variant integration IDs on GHEC requests.
File summaries
| File | Description |
|---|---|
containers/api-proxy/request-headers.test.js |
Tests GHEC integration headers. |
containers/api-proxy/request-headers.js |
Recognizes GHEC hosts; integration-ID deduplication needs correction. |
containers/api-proxy/providers/copilot.js |
Updates authentication, discovery, identity, and diagnostics; environment forwarding and reflection output remain incomplete. |
containers/api-proxy/providers/copilot-auth.js |
Classifies credentials and rejects legacy placeholders. |
containers/api-proxy/copilot-auth.test.js |
Tests credential resolution and authorization schemes. |
containers/api-proxy/copilot-adapter-enterprise.test.js |
Adds GHEC and fine-grained PAT regression coverage. |
Review details
Suppressed comments (1)
containers/api-proxy/providers/copilot.js:194
- When Copilot uses an OIDC provider,
resolveHeaders()sends a Bearer OIDC token, but these static values reportcredential_kind: none,inference_credential_source: none, and can even reporttokenas the scheme on a GHES-classified target. Make the reflected values depend onoidcConfigured/authProvider; otherwise the new safe diagnostics misdescribe the credential actually used for inference.
credential_kind: apiKey ? 'byok' : githubToken && githubToken.startsWith('github_pat_') ? 'fine_grained_pat' : githubToken ? 'github_token' : 'none',
selected_scheme: authPrefix,
inference_credential_source: inferenceCredentialSource,
inference_selected_scheme: authPrefix,
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const integrationId = (env.COPILOT_INTEGRATION_ID || '').trim() | ||
| || (env.GITHUB_COPILOT_INTEGRATION_ID || '').trim() | ||
| || 'agentic-workflows'; |
| modelsPath, | ||
| reflectionConfigured: !!authToken || oidcConfigured, | ||
| reflectionModelsPath: modelsPath, | ||
| reflectionExtra: { |
| const copilotHost = isCopilotIntegrationHost(targetHost); | ||
| for (const [name, value] of Object.entries(injectHeaders)) { | ||
| if (!copilotHost && name.toLowerCase() === 'copilot-integration-id') continue; | ||
| headers[name] = value; |
|
@copilot address review feedback |
Addressed all three review items in commit |
|
✅ Copilot review passed with no inline comments. @copilot Add the |
|
🛡️ 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.
|
|
✅ Build Test Suite completed successfully!
|
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded.
|
|
🔌 Smoke Services — All services reachable! ✅
|
|
❌ Smoke Gemini reports failed. Facets need polishing...
|
|
❌ Contribution Check failed. Please review the logs for details.
|
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅
|
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤
|
|
✅ Security Guard completed successfully! Security review complete: PR #8038 contains no security vulnerabilities. The changes improve Copilot proxy auth handling (fine-grained PAT detection, GHEC model discovery, legacy placeholder filtering) while maintaining existing security boundaries. All 4 security-relevant files show defensive improvements with no weakened DROP/REJECT, expanded ACCEPT, auth regressions, or capability additions.
|
|
✅ 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.
|
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 Mode) ✅ PASS
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) with api-proxy sidecar injection.
|
|
EGRESS_RESULT allow=pass deny=pass ✅ Allowed domain (github.com) reachable — Overall: 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.
|
Smoke Test: Services Connectivity
Overall: FAIL —
|
|
Smoke Test: Copilot Engine —
Overall: PASS
|
Chroot Version Comparison Results
Overall: FAILED — Node.js version mismatch between host and chroot environments (host: v24.19.0, chroot: v2.98.0). The
|
|
Docker Sbx Smoke Test
Overall: PASS cc
|
|
PR titles:
GitHub merged PR review: ✅ Warning Firewall blocked 7 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "msfeed25.pkgs.visualstudio.com"
- "www.google.com"
- "www.gstatic.com"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — PASS Notes:
|
|
GitHub MCP Testing (#8026, #8021): ❌ (got #8038, #8025) Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) FAIL cc
|
|
Smoke Test: API Proxy OTel Tracing — Results
Overall: PASS — all implemented scenarios succeeded; Scenario 5's absence of api-proxy-specific spans is expected given no live proxied LLM request occurred in this run.
|
|
GitHub MCP connectivity: ✅ Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra Overall status: PASS /cc
|
…d GITHUB_COPILOT_INTEGRATION_ID fallback Corrects docs/auth-matrix.md and docs/environment.md to reflect PR #8038 (Fix Copilot proxy auth and GHEC model discovery, merged 2026-09-02): - COPILOT_GITHUB_TOKEN values starting with github_pat_ (fine-grained PATs) now always use the Bearer auth prefix on every Copilot target (github.com, GHEC, GHES, Business), overriding the previous target-based token/Bearer selection that still applies to classic PATs and OAuth tokens. - Copilot-Integration-Id now also falls back to GITHUB_COPILOT_INTEGRATION_ID when COPILOT_INTEGRATION_ID is unset. - Cross-referenced the stronger fine-grained-PAT guarantee from the existing Classic PAT + COPILOT_MODEL remediation guidance in docs/environment.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…d GITHUB_COPILOT_INTEGRATION_ID fallback (#8055) Corrects docs/auth-matrix.md and docs/environment.md to reflect PR #8038 (Fix Copilot proxy auth and GHEC model discovery, merged 2026-09-02): - COPILOT_GITHUB_TOKEN values starting with github_pat_ (fine-grained PATs) now always use the Bearer auth prefix on every Copilot target (github.com, GHEC, GHES, Business), overriding the previous target-based token/Bearer selection that still applies to classic PATs and OAuth tokens. - Copilot-Integration-Id now also falls back to GITHUB_COPILOT_INTEGRATION_ID when COPILOT_INTEGRATION_ID is unset. - Cross-referenced the stronger fine-grained-PAT guarantee from the existing Classic PAT + COPILOT_MODEL remediation guidance in docs/environment.md. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fine-grained GitHub PATs were sent with the target-selected
tokenscheme on Business, Enterprise, and GHEC endpoints. Canonical GHEC targets also missed GitHub-hosted model-discovery handling, while a legacy isolation placeholder could override the real GitHub credential for inference.Credential selection
Bearerforgithub_pat_fine-grained PATs on every Copilot target.tokenbehavior for OAuth and classic PATs on applicable targets.placeholder-token-for-credential-isolationalongside current and offline BYOK placeholders.GHEC request shape
copilot-api.<tenant>.ghe.comtargets as GitHub-hosted catalog endpoints.X-GitHub-Api-Version: 2026-07-01to startup and proxied/models.COPILOT_INTEGRATION_ID, thenGITHUB_COPILOT_INTEGRATION_ID, then the default.Copilot-Integration-Idfor GHEC without synthesizing interaction headers.Safe diagnostics