Skip to content

refactor: extract validateAuthHeaderEnv and isValidHeaderName helpers#4783

Merged
lpcox merged 1 commit into
mainfrom
refactor/dedup-validate-custom-headers
Jun 11, 2026
Merged

refactor: extract validateAuthHeaderEnv and isValidHeaderName helpers#4783
lpcox merged 1 commit into
mainfrom
refactor/dedup-validate-custom-headers

Conversation

@lpcox

@lpcox lpcox commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

Centralizes the repeated HTTP header name validation pattern into two helpers in proxy-utils.js:

  1. validateAuthHeaderEnv(envVarName, rawValue, defaultHeader) — reads/trims/validates an env var as an HTTP header name, throwing with a descriptive error on failure. Replaces the throw-on-invalid IIFEs in anthropic.js and openai.js.

  2. isValidHeaderName(name) — boolean predicate wrapping http.validateHeaderName(). Replaces the try/catch in copilot-byok.js's header loop.

Fixes #4709

Centralizes HTTP header name validation into proxy-utils.js:
- validateAuthHeaderEnv(): throw-on-invalid pattern (anthropic, openai)
- isValidHeaderName(): boolean predicate (copilot-byok loop)

Fixes #4709

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 11, 2026 18:08
@github-actions

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.54% 96.58% 📈 +0.04%
Statements 96.46% 96.50% 📈 +0.04%
Functions 98.78% 98.78% ➡️ +0.00%
Branches 91.14% 91.18% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 90.2% → 91.4% (+1.23%) 90.2% → 91.4% (+1.23%)

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

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 repeated HTTP header-name validation logic used by api-proxy provider adapters into shared helpers in containers/api-proxy/proxy-utils.js, addressing #4709 and reducing duplicated validation/try-catch patterns across providers.

Changes:

  • Added validateAuthHeaderEnv(envVarName, rawValue, defaultHeader) to centralize trimming + header-name validation + consistent error messages for custom auth-header env vars.
  • Added isValidHeaderName(name) as a boolean wrapper around Node’s http.validateHeaderName() to simplify skip-on-invalid header handling.
  • Updated Anthropic/OpenAI adapters and the Copilot BYOK extra-header parser to use the new helpers instead of local duplicated logic.
Show a summary per file
File Description
containers/api-proxy/proxy-utils.js Introduces shared header-name validation helpers and exports them for adapter use.
containers/api-proxy/providers/openai.js Replaces inline env-var header validation IIFE with validateAuthHeaderEnv.
containers/api-proxy/providers/anthropic.js Replaces inline auth-header env-var validation with validateAuthHeaderEnv (defaulting to x-api-key).
containers/api-proxy/providers/copilot-byok.js Replaces per-header try/catch validation with isValidHeaderName for skip-on-invalid 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: 0

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot PAT Auth — PR #4783 by @lpcox

Test Result
GitHub MCP connectivity
GitHub.com HTTP ✅ HTTP 200
File write/read ⚠️ template vars unsubstituted

PR: refactor: extract validateAuthHeaderEnv and isValidHeaderName helpers
Auth mode: PAT (COPILOT_GITHUB_TOKEN)
Overall: PASS (2/2 live tests passed; file test skipped — pre-step data not injected)

🔑 PAT report filed by Smoke Copilot PAT

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Notes
S1: Module Loading otel.js loads successfully; exports: startRequestSpan, setTokenAttributes, setBudgetAttributes, endSpan, endSpanError, shutdown, isEnabled + test helpers
S2: Test Suite 39/39 tests passed in otel.test.js
S3: Env Var Forwarding api-proxy-service-config.ts forwards OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID, OTEL_SERVICE_NAME
S4: Token Tracker Integration onUsage callback present in token-tracker-http.js; invoked after normalized usage extracted
S5: OTEL Diagnostics Post-step pending (workflow still in progress)

All completed scenarios pass. S5 will populate once the workflow finishes.

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions

Copy link
Copy Markdown
Contributor

perf(doc-maintainer): reduce per-run token usage
perf(test-coverage-reporter): reduce token usage ~7-10% per run
✅ GitHub title check
✅ File write/read
✅ Build
✅ Discussion comment
Overall: 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

🔥 Smoke Test Results

PR: refactor: extract validateAuthHeaderEnv and isValidHeaderName helpers
Author: @lpcox

Test Result
GitHub MCP connectivity
GitHub.com HTTP (200)
File write/read

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Chroot Version Comparison Results

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

Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot

@github-actions

Copy link
Copy Markdown
Contributor

refactor: extract validateAuthHeaderEnv and isValidHeaderName helpers
refactor: extract createCounterGuard factory for guard modules

  1. GitHub MCP connectivity: ✅
  2. GitHub.com connectivity: ✅
  3. File Write/Read Test: ❌
  4. BYOK Inference Test: ✅

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

cc @lpcox

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

@github-actions

Copy link
Copy Markdown
Contributor

refactor: extract validateAuthHeaderEnv and isValidHeaderName helpers

  • GitHub MCP connectivity: ✅
  • GitHub.com connectivity: ✅
  • File write/read: ✅
  • 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

GitHub API: ✅ PASS
GitHub check: ✅ PASS
File verify: ✅ PASS

Total: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Direct) Mode ✅ PASS

Test Results:

  • ✅ MCP connectivity: 2 recent merged PRs fetched
  • ✅ GitHub.com connectivity: HTTP 200
  • ✅ File write/read: Working
  • ✅ BYOK inference: Active (responding via api-proxy → api.githubcopilot.com)

Mode: Direct BYOK (COPILOT_PROVIDER_API_KEY via api-proxy sidecar)
PR: #4783 — refactor: extract validateAuthHeaderEnv and isValidHeaderName helpers
Author: @lpcox | Assignees: @lpcox, @Copilot

🔑 BYOK report filed by Smoke Copilot BYOK

@lpcox lpcox merged commit 4cc9de2 into main Jun 11, 2026
83 of 86 checks passed
@lpcox lpcox deleted the refactor/dedup-validate-custom-headers branch June 11, 2026 18:38
@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: GitHub Actions Services Connectivity

Check Result
Redis PING ❌ timeout (no response from host.docker.internal:6379)
PostgreSQL pg_isready no response
PostgreSQL SELECT 1 ❌ timeout (no response from host.docker.internal:5432)

Overall: FAILhost.docker.internal is unreachable from this runner environment. Service containers are not accessible.

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

[Duplicate Code] Custom auth-header validation (validateHeaderName) duplicated across three provider adapters

2 participants