Skip to content

Refactor shared OIDC runtime auth flow for OpenAI and Copilot adapters#4948

Merged
lpcox merged 4 commits into
mainfrom
copilot/duplicate-oidc-methods
Jun 14, 2026
Merged

Refactor shared OIDC runtime auth flow for OpenAI and Copilot adapters#4948
lpcox merged 4 commits into
mainfrom
copilot/duplicate-oidc-methods

Conversation

Copilot AI commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

The API proxy still duplicated OIDC runtime adapter logic between openai.js and copilot.js after provider initialization was centralized. This left security-critical token/header behavior split across multiple files.

  • Consolidate OIDC runtime adapter methods

    • Added createOidcRuntimeAdapterMethods(...) in containers/api-proxy/proxy-utils.js to centralize:
      • isEnabled() OIDC-aware readiness check
      • getOidcProvider()
      • getAwsOidcProvider()
    • Both OpenAI and Copilot adapters now spread these shared methods instead of re-declaring them.
  • Consolidate OIDC header resolution path

    • Added resolveOidcAuthHeaders(...) in containers/api-proxy/proxy-utils.js to centralize:
      • bearer-token provider branch (oidcProvider.getToken())
      • AWS OIDC branch (empty headers for SigV4 flow)
      • “not OIDC” signal (null) for static-auth fallback
    • openai.js and copilot.js now pass provider-specific header builders and keep only their static fallback behavior.
  • Add focused utility coverage

    • Added containers/api-proxy/proxy-utils.oidc.test.js for:
      • OIDC runtime enablement behavior
      • token-present / token-missing / AWS-OIDC / no-OIDC header resolution outcomes
const oidcHeaders = resolveOidcAuthHeaders({
  oidcProvider,
  awsOidcProvider,
  buildOidcHeaders: (token) => ({ Authorization: ['Bearer', token].join(' ') }),
});

if (oidcHeaders !== null) return oidcHeaders;
return buildStaticAuthHeaders(apiKey);

Copilot AI changed the title [WIP] Refactor duplicate OIDC runtime adapter methods Refactor shared OIDC runtime auth flow for OpenAI and Copilot adapters Jun 14, 2026
Copilot finished work on behalf of lpcox June 14, 2026 17:39
Copilot AI requested a review from lpcox June 14, 2026 17:39
@lpcox lpcox marked this pull request as ready for review June 14, 2026 17:39
Copilot AI review requested due to automatic review settings June 14, 2026 17:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the API proxy’s OIDC runtime authentication logic to eliminate duplicated, security-sensitive behavior across the OpenAI and Copilot provider adapters by centralizing shared enablement/provider-access methods and the OIDC header resolution path into proxy-utils.js, and adding unit tests for the new utilities.

Changes:

  • Added shared OIDC runtime adapter helpers (createOidcRuntimeAdapterMethods, resolveOidcAuthHeaders) to containers/api-proxy/proxy-utils.js.
  • Updated openai.js and copilot.js adapters to reuse the shared OIDC methods and centralized header resolution while keeping provider-specific static fallback/header shaping.
  • Added focused Jest coverage for the new OIDC utility helpers in proxy-utils.oidc.test.js.
Show a summary per file
File Description
containers/api-proxy/proxy-utils.oidc.test.js Adds unit tests covering the new shared OIDC runtime methods and header resolution outcomes.
containers/api-proxy/proxy-utils.js Introduces shared OIDC runtime adapter methods and centralized OIDC auth header resolution helper.
containers/api-proxy/providers/openai.js Replaces duplicated OIDC enablement/header logic with shared helpers while preserving OpenAI-specific header behavior.
containers/api-proxy/providers/copilot.js Replaces duplicated OIDC enablement/header logic with shared helpers while preserving Copilot-specific header behavior.

Copilot's findings

Tip

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

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

Comment on lines +51 to +69
it('returns an empty object for AWS OIDC request-signing flow', () => {
const headers = resolveOidcAuthHeaders({
oidcProvider: null,
awsOidcProvider: { isReady: () => true },
buildOidcHeaders: () => ({ Authorization: 'ignored-token' }),
});

expect(headers).toEqual({});
});

it('returns null when OIDC is not configured', () => {
const headers = resolveOidcAuthHeaders({
oidcProvider: null,
awsOidcProvider: null,
buildOidcHeaders: () => ({ Authorization: 'ignored-token' }),
});

expect(headers).toBeNull();
});
Comment on lines +74 to +78
// we surface a non-empty marker here to keep alwaysBind/isEnabled probes happy and
// resolve the real token lazily inside getAuthHeaders.
const authToken = staticAuthToken;
const oidcRuntimeMethods = createOidcRuntimeAdapterMethods({
staticAuthToken: authToken,
- Add test: bearer OIDC takes precedence over AWS OIDC when both configured
- Update copilot.js comment: authToken is typically undefined in OIDC mode,
  enablement is handled by oidcConfigured (not a 'non-empty marker')

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.84% 96.88% 📈 +0.04%
Statements 96.71% 96.75% 📈 +0.04%
Functions 98.80% 98.80% ➡️ +0.00%
Branches 91.25% 91.28% 📈 +0.03%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/workdir-setup.ts 92.6% → 94.4% (+1.85%) 92.6% → 94.4% (+1.85%)

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.

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

Comment thread containers/api-proxy/providers/openai.js Fixed
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Direct) Mode — Azure OpenAI (Foundry, api-key)

  • GitHub MCP Testing: ✅
  • GitHub.com connectivity: ✅
  • File I/O test: ✅
  • BYOK inference: ✅

Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)

Overall: PASS

@lpcox

🔑 BYOK (AOAI api-key) report filed by Smoke Copilot BYOK AOAI (api-key)

@github-actions

Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results — PASS

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

PR: Refactor shared OIDC runtime auth flow for OpenAI and Copilot adapters
Author: @CopilotAssignees: @lpcox, @Copilot

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

Smoke test — Auth mode: PAT (COPILOT_GITHUB_TOKEN) — @lpcox @Copilot

Test Result
GitHub MCP connectivity
GitHub.com HTTP ✅ 200
File write/read ❌ pre-step data unavailable (template vars not substituted)

Overall: FAIL

🔑 PAT report filed by Smoke Copilot PAT

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

@lpcox @Copilot
Smoke Test Results - Copilot BYOK AOAI Entra Mode

• 🔍 MCP API: ✅
• 🌐 GitHub.com HTTP: ✅
• 💾 File read/write: ✅
• ⚙️ BYOK inference: ✅

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

🪪 BYOK (AOAI Entra) report filed by Smoke Copilot BYOK AOAI (Entra)

@github-actions

Copy link
Copy Markdown
Contributor

🏗️ 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 #4948 ·

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Direct Mode) ✅ PASS

✅ GitHub MCP connectivity — listed 2 recent closed PRs successfully
✅ GitHub.com connectivity — HTTP 200
✅ File write/read — verified in /tmp/gh-aw/agent/
✅ BYOK inference — responding to prompt (direct mode active)

Mode: Direct BYOK (COPILOT_PROVIDER_API_KEY) via api-proxy sidecar → api.githubcopilot.com

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions

Copy link
Copy Markdown
Contributor

Reviewed merged PRs: refactor(api-proxy): split proxy-request.js into http-client.js and body-handler.js, Refactor OpenAI BYOK base URL parsing to reuse shared proxy URL normalization
✅ Browser title check
✅ File write/read check
❌ GitHub reads via gh CLI (auth unavailable) / fallback web reads used
❌ npm ci && npm run build (node missing)
Overall: FAIL

🔮 The oracle has spoken through Smoke Codex

@github-actions

Copy link
Copy Markdown
Contributor

Chroot Version Comparison — Smoke Test Results

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

Overall: ❌ Tests did not pass — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Summary
1. Module Loading otel.js loads cleanly; exports 14 symbols: startRequestSpan, setTokenAttributes, setBudgetAttributes, endSpan, endSpanError, shutdown, isEnabled, plus test-exported internals
2. Test Suite 39 tests, 0 failures across 12 describe blocks (module init, span creation, token attributes, budget attributes, exporter variants, shutdown)
3. Env Var Forwarding api-proxy-service-config.ts forwards GH_AW_OTLP_ENDPOINTS, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID, OTEL_SERVICE_NAME to the api-proxy container; observability-environment.ts auto-forwards all OTEL_* vars
4. Token Tracker Integration onUsage callback exists in token-tracker-http.js (line 251) and token-tracker-ws.js (line 208); wired in upstream-response.js and websocket-proxy.js to call otel.setTokenAttributes + otel.setBudgetAttributes
5. OTEL Diagnostics i️ No OTLP endpoint configured in this run — module initialized with FileSpanExporter fallback (writes to /var/log/api-proxy/otel.jsonl). Span export to a collector requires GH_AW_OTLP_ENDPOINTS or OTEL_EXPORTER_OTLP_ENDPOINT to be set

Overall: ✅ All functional scenarios pass. Scenario 5 is expected-pending (file fallback active; no collector endpoint wired for this PR run).

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: GitHub Actions Services Connectivity

Check Result
Redis PING ❌ No response (timeout)
PostgreSQL pg_isready no response
PostgreSQL SELECT 1 ❌ No response (timeout)

host.docker.internal resolves to 172.17.0.1 but ports 6379 and 5432 are unreachable.

Overall: ❌ FAIL

🔌 Service connectivity validated by Smoke Services

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Gemini

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 API: ✅ PASS
GitHub check: ✅ PASS
File verify: ✅ PASS

Total: PASS

💥 [THE END] — Illustrated by Smoke Claude

@lpcox lpcox merged commit e6436e9 into main Jun 14, 2026
87 of 94 checks passed
@lpcox lpcox deleted the copilot/duplicate-oidc-methods branch June 14, 2026 19:47
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.

4 participants