Refactor api-proxy request path into auditable header and upstream modules#5996
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the api-proxy’s HTTP request pipeline by extracting security-sensitive header construction and upstream HTTPS dispatch into dedicated modules, aiming to make auditing and future changes safer and more localized while preserving existing behavior.
Changes:
- Extracted request header validation/building into
request-headers.js(including strip/inject and content-length fixups). - Extracted upstream HTTPS request dispatch + model-not-supported backoff scheduling into
upstream-http.js. - Updated the api-proxy Docker image packaging to include the newly introduced modules, and added focused unit tests for both extracted modules.
Show a summary per file
| File | Description |
|---|---|
| containers/api-proxy/upstream-http.js | New module for upstream HTTPS request execution and retry scheduling. |
| containers/api-proxy/upstream-http.test.js | Adds unit tests covering upstream dispatch and model-not-supported backoff path. |
| containers/api-proxy/request-headers.js | New module centralizing request-id validation and upstream header construction/stripping. |
| containers/api-proxy/request-headers.test.js | Adds unit tests covering request-id validation and header strip/inject behaviors. |
| containers/api-proxy/proxy-request.js | Simplifies orchestrator by delegating header building and upstream sending to new modules. |
| containers/api-proxy/Dockerfile | Extends COPY list to ensure the new runtime modules are included in the built image. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Low
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
✅ Contribution Check completed successfully! PR #5996 follows the applicable CONTRIBUTING.md guidelines based on the provided metadata, diff, and contribution guide. No GitHub comment needed. |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
❌ Security Guard failed. Please review the logs for details. |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
✅ Smoke Claude passed |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed... |
|
✅ Build Test Suite completed successfully! |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 99.00% | 99.00% | ➡️ +0.00% |
| Statements | 98.95% | 98.95% | ➡️ +0.00% |
| Functions | 99.72% | 99.72% | ➡️ +0.00% |
| Branches | 95.44% | 95.41% | 📉 -0.03% |
Coverage comparison generated by scripts/ci/compare-coverage.ts
Smoke Test: Copilot BYOK (Direct) Mode ✅
Overall Status: PASS Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) via api-proxy sidecar. Inference authenticated with real credential held by sidecar; agent only sees placeholder. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"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:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🔬 Smoke Test Results
Overall: PASS — core connectivity verified.
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Services Connectivity
Overall: FAIL — Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Gemini Engine Smoke Test Results
Overall status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
Smoke Test: Copilot PAT Auth
Overall: PASS ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
Refactor api-proxy request path into auditable header and upstream modules GitHub reads: ✅ Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"
- "registry.npmjs.org"See Network Configuration for more information.
|
Smoke Test: API Proxy OpenTelemetry Tracing
All 5 scenarios pass. OTEL tracing integration is functional with 59 passing tests. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Chroot Version Comparison Results
Overall: ❌ Not all versions matched — Python and Node.js versions differ between host and chroot environment. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
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 Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
containers/api-proxy/proxy-request.jshad accumulated multiple security-sensitive concerns (header mutation, upstream dispatch, retry/backoff, orchestration) in one file, making review and targeted auditing harder. This PR continues the existing extraction pattern by isolating request-header and upstream transport logic into focused modules while preserving caller-facing behavior.What changed
containers/api-proxy/request-headers.jsisValidRequestId()andbuildRequestHeaders().x-request-id, Copilotx-initiator, content-length/transfer-encoding fixup, learned-header stripping).containers/api-proxy/upstream-http.jscreateSendUpstreamRequest(...).MODEL_NOT_SUPPORTED_RETRY_DELAYS_MSand recursive retry behavior via injected dependencies.containers/api-proxy/proxy-request.jsproxyRequest()now coordinates body collection/transform, guard enforcement, header builder, and upstream sender using the new modules.request-headers.jsandupstream-http.jstocontainers/api-proxy/DockerfileCOPYlist.Code shape after refactor (example)