Skip to content

Enable Copilot BYOK provider-env fallback, base-path routing, and hardened token isolation#2598

Merged
lpcox merged 4 commits intomainfrom
copilot/fix-byok-mode-issue
May 5, 2026
Merged

Enable Copilot BYOK provider-env fallback, base-path routing, and hardened token isolation#2598
lpcox merged 4 commits intomainfrom
copilot/fix-byok-mode-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 5, 2026

Copilot BYOK runs could fail when workflows set COPILOT_PROVIDER_API_KEY/COPILOT_PROVIDER_BASE_URL without COPILOT_API_KEY: AWF did not activate Copilot sidecar BYOK routing, while one-shot token isolation still replaced the real key with a placeholder. This change aligns key/target/base-path resolution with provider-style envs so sidecar activation and upstream routing remain coherent.

  • Copilot key resolution fallback

    • src/cli.ts now resolves copilotApiKey from:
      • COPILOT_API_KEY
      • fallback: COPILOT_PROVIDER_API_KEY
    • This ensures BYOK sidecar paths activate even when only provider key env is present.
  • Copilot target derivation from provider base URL

    • When --copilot-api-target and COPILOT_API_TARGET are unset, CLI derives copilotApiTarget from COPILOT_PROVIDER_BASE_URL hostname.
    • The resolved target is used consistently for runtime config and API-target allowlist resolution.
  • Copilot base-path derivation and propagation from provider base URL

    • CLI now derives copilotApiBasePath from the path component of COPILOT_PROVIDER_BASE_URL (or uses COPILOT_API_BASE_PATH when explicitly set).
    • src/services/api-proxy-service.ts now forwards COPILOT_API_BASE_PATH to the sidecar.
    • containers/api-proxy/providers/copilot.js now applies the base path via getBasePath() so BYOK providers using prefixed endpoints (for example /api/v1) are routed correctly instead of being sent to host root.
  • Agent token isolation hardening (feedback follow-up)

    • When --enable-api-proxy is enabled, agent env passthrough now explicitly excludes:
      • COPILOT_GITHUB_TOKEN
      • COPILOT_API_KEY
      • COPILOT_PROVIDER_API_KEY
    • This prevents real Copilot auth credentials from being injected into the agent container via inherited env passthrough (including --env-all), while preserving explicit placeholder behavior used for compatibility.
  • Focused unit coverage

    • Added cli tests for:
      • key precedence/fallback (COPILOT_API_KEY over COPILOT_PROVIDER_API_KEY)
      • hostname derivation from provider base URL (path/port/IPv4/IPv6/invalid input cases)
      • base-path derivation from provider base URL
      • provider-derived routing resolution (target + base path) used for runtime wiring
    • Added agent-service tests to verify:
      • provider auth key is not forwarded into agent env when api-proxy is enabled with --env-all
      • Copilot/provider placeholder values remain in place when BYOK config is active
    • Added api-proxy tests to verify:
      • COPILOT_API_BASE_PATH is forwarded by compose generation when configured
      • Copilot adapter normalizes and uses configured base path
copilotApiKey: process.env.COPILOT_API_KEY || process.env.COPILOT_PROVIDER_API_KEY;

const { copilotApiTarget, copilotApiBasePath } = resolveCopilotApiRouting(
  { copilotApiTarget: options.copilotApiTarget },
  process.env
);

@lpcox lpcox marked this pull request as ready for review May 5, 2026 22:43
Copilot AI changed the title [WIP] Fix BYOK mode failure when only COPILOT_PROVIDER_API_KEY is set Enable Copilot BYOK fallback when only provider env vars are set May 5, 2026
@lpcox lpcox requested a review from Mossaka as a code owner May 5, 2026 22:43
Copilot AI review requested due to automatic review settings May 5, 2026 22:43
Copilot finished work on behalf of lpcox May 5, 2026 22:43
Copilot AI requested a review from lpcox May 5, 2026 22:43
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 5, 2026

@copilot make sure that auth tokens are never injected into the agent container

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 86.76% 86.88% 📈 +0.12%
Statements 86.70% 86.82% 📈 +0.12%
Functions 81.22% 81.39% 📈 +0.17%
Branches 79.45% 79.63% 📈 +0.18%
📁 Per-file Coverage Changes (3 files)
File Lines (Before → After) Statements (Before → After)
src/services/agent-service.ts 95.5% → 95.6% (+0.05%) 95.3% → 95.3% (+0.05%)
src/container-lifecycle.ts 87.1% → 88.2% (+1.14%) 87.5% → 88.6% (+1.11%)
src/cli.ts 28.1% → 31.6% (+3.49%) 28.1% → 32.0% (+3.93%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the CLI’s Copilot BYOK env resolution so workflows that only set provider-style Copilot env vars can still activate the API proxy sidecar and derive a Copilot target for allowlisting/routing.

Changes:

  • Added resolveCopilotApiKey() to fall back from COPILOT_API_KEY to COPILOT_PROVIDER_API_KEY.
  • Added deriveCopilotApiTargetFromProviderBaseUrl() and threaded its result into CLI config + allowlist resolution.
  • Added unit tests for env precedence and hostname derivation edge cases.
Show a summary per file
File Description
src/cli.ts Adds Copilot BYOK env fallback helpers and uses the resolved target/key in CLI runtime config and allowlist setup.
src/cli.test.ts Adds focused helper tests for Copilot env-key precedence and provider-base-URL hostname derivation.

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: 2

Comment thread src/cli.ts
Comment on lines +151 to +156
const candidate = trimmed.includes('://')
? trimmed
: `https://${trimmed}`;

try {
return new URL(candidate).hostname || undefined;
Comment thread src/cli.ts
options.copilotApiTarget ||
process.env.COPILOT_API_TARGET ||
copilotApiTargetFromProviderBaseUrl;

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 5, 2026

@copilot address review feedback

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 5, 2026

@copilot make sure that auth tokens are never injected into the agent container

Implemented in 48e065b. I hardened agent env filtering so COPILOT_GITHUB_TOKEN, COPILOT_API_KEY, and COPILOT_PROVIDER_API_KEY are excluded from passthrough when --enable-api-proxy is on, and added tests that verify real Copilot/provider tokens are not injected into the agent (including --env-all cases).

Copilot AI changed the title Enable Copilot BYOK fallback when only provider env vars are set Enable Copilot BYOK fallback and harden agent token isolation for provider env vars May 5, 2026
Copilot finished work on behalf of lpcox May 5, 2026 22:52
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 5, 2026

@copilot address review feedback

Addressed in 9d0cb5d. I implemented Copilot provider base-path routing so COPILOT_PROVIDER_BASE_URL paths (for example /api/v1) are preserved via COPILOT_API_BASE_PATH, wired this through CLI/config/api-proxy env, and added regression coverage for routing resolution plus target/base-path propagation.

Copilot AI changed the title Enable Copilot BYOK fallback and harden agent token isolation for provider env vars Enable Copilot BYOK provider-env fallback, base-path routing, and hardened token isolation May 5, 2026
Copilot finished work on behalf of lpcox May 5, 2026 22:59
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Smoke Test Results
✅ GitHub MCP: Last 2 merged PRs retrieved
✅ Playwright: github.com title verified
✅ File Writing: Test file created
✅ Bash: File verified

Status: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Smoke Test: Copilot BYOK — PR by @Copilot, assignees: @lpcox @Copilot

Test Result
GitHub MCP connectivity
GitHub.com HTTP ⚠️ pre-step data unavailable (template vars not expanded)
File write/read ⚠️ pre-step data unavailable (template vars not expanded)
BYOK inference (api-proxy → api.githubcopilot.com)

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com.

Overall: PASS (core BYOK path confirmed working)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

🔥 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP connectivity
File write/read

Overall: PASS

PR: Enable Copilot BYOK provider-env fallback, base-path routing, and hardened token isolation
Author: @Copilot | Assignees: @lpcox @Copilot

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

🔮 Codex smoke oracle: FAIL
Merged PRs: api-proxy: inject X-Initiator: agent default on all Copilot-bound requests to prevent billing inflation | refactor(entrypoint): deduplicate signal-handler + token-cleanup into shared function
Queried PRs: feat(api-proxy): OIDC authentication for Azure OpenAI (Entra-only) | Enable Copilot BYOK provider-env fallback, base-path routing, and hardened token isolation
✅ GitHub PR review/query, Playwright, file/bash, discussion comment, npm build
❌ safeinputs-gh unavailable; github-discussion-query unavailable; Tavily exposed no callable search tool
Overall status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color 1/1 passed ✅ PASS
Go env 1/1 passed ✅ PASS
Go uuid 1/1 passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx All passed ✅ PASS
Node.js execa All passed ✅ PASS
Node.js p-limit All passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #2598 · ● 545K ·

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Smoke Test Results

  • Redis PING: ❌ (no response / timeout)
  • PostgreSQL pg_isready: ❌ (no response)
  • PostgreSQL SELECT 1: ❌ (no response / timeout)

Overall: FAILhost.docker.internal service containers are not reachable from this environment.

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Chroot Version Comparison — Smoke Test Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ NO
Node.js v24.14.1 v20.20.2 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot. Go versions match.

Tested by Smoke Chroot

@lpcox lpcox merged commit 94754b0 into main May 5, 2026
62 of 69 checks passed
@lpcox lpcox deleted the copilot/fix-byok-mode-issue branch May 5, 2026 23:23
@github-actions github-actions Bot mentioned this pull request May 6, 2026
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.

BYOK mode fails when only COPILOT_PROVIDER_API_KEY is set (without COPILOT_API_KEY)

3 participants