Skip to content

feat(api-proxy): harden Copilot BYOK resolution and add Azure provider routing#4162

Open
Copilot wants to merge 6 commits into
mainfrom
copilot/fix-copilot-api-key-handling
Open

feat(api-proxy): harden Copilot BYOK resolution and add Azure provider routing#4162
Copilot wants to merge 6 commits into
mainfrom
copilot/fix-copilot-api-key-handling

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 1, 2026

resolveCopilotApiKey() no longer treats COPILOT_API_KEY as an upstream BYOK credential, aligning BYOK key lookup with the documented provider path (COPILOT_PROVIDER_API_KEY / COPILOT_PROVIDER_BASE_URL).
This PR also includes follow-up fixes needed to preserve offline behavior and adds Azure OpenAI BYOK routing support in the API proxy.

What changed

  • BYOK resolver behavior

    • resolveCopilotApiKey() now reads only COPILOT_PROVIDER_API_KEY.
    • If COPILOT_PROVIDER_API_KEY is unset and COPILOT_API_KEY is set, the resolver returns undefined (legacy key is not forwarded).
    • Added one-time deprecation warning behavior for legacy-only usage, with matching unit test updates.
  • Offline + BYOK compatibility fix

    • Restored expected container env setup for the copilotGithubToken path when host-side provider key is absent.
    • Ensures offline mode remains enabled in AWF sandbox flows and avoids regressions caused by accidental online-mode behavior.
  • Azure OpenAI/Cognitive Services BYOK routing

    • Added Azure target detection in the Copilot provider adapter.
    • Uses api-key header auth for Azure targets (instead of Bearer).
    • Injects api-version query param (default 2024-10-21, configurable).
    • Extended config/schema/types and tests for copilotProviderTarget options (including basePath and azureApiVersion), plus related docs updates.
  • Security maintenance

    • Included docs-site dependency updates to resolve high-severity audit findings.

Test updates

  • Updated resolver tests for provider-only key semantics and deprecation behavior.
  • Updated CLI/env-resolution tests for provider-only BYOK handling.
  • Added/updated API proxy Azure routing tests and config/schema validation coverage.

Copilot AI changed the title [WIP] Stop treating COPILOT_API_KEY as a BYOK upstream credential Stop treating COPILOT_API_KEY as a BYOK upstream credential Jun 1, 2026
Copilot finished work on behalf of lpcox June 1, 2026 23:19
Copilot AI requested a review from lpcox June 1, 2026 23:19
@lpcox lpcox marked this pull request as ready for review June 1, 2026 23:22
Copilot AI review requested due to automatic review settings June 1, 2026 23:22
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.35% 96.41% 📈 +0.06%
Statements 96.21% 96.25% 📈 +0.04%
Functions 98.27% 98.27% ➡️ +0.00%
Branches 90.44% 90.49% 📈 +0.05%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/services/api-proxy-service.ts 98.9% → 99.0% (+0.01%) 98.0% → 97.1% (-0.93%)
src/config-writer.ts 89.3% → 90.9% (+1.65%) 89.3% → 90.9% (+1.65%)

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

@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 AWF’s Copilot BYOK credential resolution so that resolveCopilotApiKey() no longer treats COPILOT_API_KEY (Copilot CLI session auth / sentinel-prone) as an upstream BYOK credential, aligning BYOK key lookup to COPILOT_PROVIDER_API_KEY and adding deprecation signaling when legacy-only env is detected.

Changes:

  • Updated resolveCopilotApiKey() to read only COPILOT_PROVIDER_API_KEY and ignore COPILOT_API_KEY (with a one-time warning).
  • Added a process-local “warn once” latch plus a reset helper for tests.
  • Updated unit tests and CLI env-resolution tests to reflect provider-only BYOK semantics and warning behavior.
Show a summary per file
File Description
src/copilot-api-resolver.ts Switches BYOK key resolution to provider-only and adds one-time deprecation warning behavior.
src/copilot-api-resolver.test.ts Updates resolver tests for new precedence/ignore rules and warning latch behavior.
src/cli.test.ts Updates CLI env-resolution expectations to match provider-only BYOK key semantics.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 2

Comment on lines +7 to +11
let copilotApiKeyDeprecationWarned = false;

export function __resetCopilotApiKeyDeprecationLatchForTesting(): void {
copilotApiKeyDeprecationWarned = false;
}
Comment thread src/copilot-api-resolver.ts Outdated
Comment on lines +24 to +29
if (env.COPILOT_API_KEY !== undefined && !copilotApiKeyDeprecationWarned) {
logger.warn(
'COPILOT_API_KEY is deprecated for BYOK and will be ignored; use COPILOT_PROVIDER_API_KEY and COPILOT_PROVIDER_BASE_URL instead.'
);
copilotApiKeyDeprecationWarned = true;
}
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@lpcox lpcox changed the title Stop treating COPILOT_API_KEY as a BYOK upstream credential fix: stop treating COPILOT_API_KEY as a BYOK upstream credential Jun 1, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

Documentation Preview

Documentation build failed for this PR. View logs.

Built from commit 06d1a98

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

When resolveCopilotApiKey() returns undefined (no COPILOT_PROVIDER_API_KEY
on host), the copilotGithubToken path still needs COPILOT_OFFLINE=true,
COPILOT_PROVIDER_BASE_URL, and COPILOT_PROVIDER_API_KEY placeholder to be
set in the agent container. Without these, the Copilot CLI runs in online
mode, enforcing MCP server policies and using stream_options — both of
which cause failures in the AWF sandbox.

The offline+BYOK block now fires when either copilotApiKey OR
copilotGithubToken is present, matching the pre-existing behavior where
the old resolver returned the dummy sentinel as a truthy value.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@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.

@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.

@github-actions github-actions Bot mentioned this pull request Jun 2, 2026
@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.

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Jun 2, 2026

@copilot update the pr summary to reflect all commits and their motivations

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 2, 2026

@copilot update the pr summary to reflect all commits and their motivations

Updated summary to reflect all commits and motivations:

  • 633bcfd — switched Copilot BYOK key resolution to provider-only (COPILOT_PROVIDER_API_KEY), so COPILOT_API_KEY is no longer treated as an upstream BYOK credential.
  • 1ec58a2 — fixed the copilotGithubToken path to still force offline+BYOK container env setup when no provider key is present on host, preventing online-mode regressions in AWF sandbox.
  • ac9a296 — added Azure OpenAI/Cognitive Services BYOK routing support in the API proxy (host detection, api-key auth header, api-version handling, and config/schema/docs updates).
  • 2938168 — resolved high-severity docs-site audit vulnerabilities to keep docs dependencies compliant and secure.
  • 5c3fd0f — initial planning commit used to track execution checklist for this branch.

If you want, I can also condense this into a shorter 2–3 bullet PR description format.

Copilot AI changed the title fix: stop treating COPILOT_API_KEY as a BYOK upstream credential feat(api-proxy): harden Copilot BYOK resolution and add Azure provider routing Jun 2, 2026
Copilot finished work on behalf of lpcox June 2, 2026 00:51
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Jun 2, 2026

@copilot address review feedback unless it undoes subsequent commits #4162 (review)

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 2, 2026

@copilot address review feedback unless it undoes subsequent commits #4162 (review)

Addressed in 76c3539.

  • Removed the exported test-only latch reset helper from production API and updated tests to avoid relying on exported internals.
  • Updated deprecation warning behavior to skip known sentinel/placeholder and blank/whitespace COPILOT_API_KEY values, reducing expected-flow noise.
  • Added/updated resolver tests for warn-once behavior and sentinel/blank suppression.

Copilot finished work on behalf of lpcox June 2, 2026 01:34
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Smoke Test: Claude Engine

  • ✅ GitHub API: 2 recent PR entries found
  • ✅ GitHub check: playwright_check=PASS
  • ✅ File verify: smoke-test-claude-26792817182.txt exists

Result: PASS (3/3)

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

🔭 Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Notes
1. Module Loading otel.js loads; exports: startRequestSpan, setTokenAttributes, endSpan, endSpanError, shutdown, isEnabled + internals
2. Test Suite 33/33 tests pass (spans, token attrs, parent ctx propagation, OTLP export, file fallback, graceful degradation)
3. Env Var Forwarding api-proxy-service.ts forwards OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID, OTEL_SERVICE_NAME
4. Token Tracker Integration onUsage callback exists in token-tracker-http.js; invoked after usage extraction as OTEL hook point
5. OTEL Diagnostics FileSpanExporter writes NDJSON to /var/log/api-proxy/otel.jsonl when OTLP endpoint is unset; ProxyAwareOtlpExporter routes via Squid when configured

All scenarios pass. OTEL tracing integration is complete and working.

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

🔍 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP connectivity ⚠️ (pre-step data unavailable)
File write/read ⚠️ (pre-step data unavailable)

Overall: PARTIAL — MCP confirmed working; pre-step outputs not injected (template vars unexpanded).

PR: feat(api-proxy): harden Copilot BYOK resolution and add Azure provider routing
Author: @Copilot | Assignees: @lpcox, @Copilot

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

🔥 Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP connectivity
GitHub.com HTTP connectivity ⚠️ (template vars unresolved — pre-step data unavailable)
File write/read ⚠️ (template vars unresolved — pre-step data unavailable)
BYOK inference (agent responding)

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

PR: feat(api-proxy): harden Copilot BYOK resolution and add Azure provider routing — author @Copilot, assignees @lpcox @Copilot

Overall: PARTIAL (2/2 verifiable tests pass; 2 tests had unresolved CI template variables)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Smoke test results:\n- fix(workflow): prevent multiline GITHUB_OUTPUT in red-team benchmark ✅\n- fix(test): correct smoke-claude version assertion to match v0.76.1 ✅\n- GitHub query ✅\n- GitHub title ✅\n- File write/read ✅\n- Discussion lookup ✅\n- Build ✅\nOverall: PASS

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 Jun 2, 2026

Smoke test FAIL: GitHub MCP missing. Connectivity PASS (via proxy). File/Bash PASS.

Warning

Firewall blocked 1 domain

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

  • localhost

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

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Chroot Version Comparison

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ NO
Node.js v24.16.0 v22.22.3 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

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

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 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 #4162 · sonnet46 887.6K ·

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Smoke Test Results

  • Redis PING: ❌ (connection timed out)
  • PostgreSQL pg_isready: ❌ (no response)
  • PostgreSQL SELECT 1: ❌ (not attempted — pg_isready failed)

Overall: FAILhost.docker.internal is not reachable from this environment. Service containers appear unreachable.

🔌 Service connectivity validated by Smoke Services

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.

Proposal: Stop treating COPILOT_API_KEY as a BYOK upstream credential

3 participants