Deduplicate JSON body object parsing across api-proxy transforms and model rewrite paths#4337
Deduplicate JSON body object parsing across api-proxy transforms and model rewrite paths#4337Copilot wants to merge 3 commits into
Conversation
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
This PR reduces drift risk in the API proxy by centralizing the repeated “parse JSON body and ensure it’s a non-array object” guard into a shared utility, then refactoring the existing body-transform and model-rewrite call sites to use it.
Changes:
- Added
parseBodyAsObject(body)incontainers/api-proxy/body-utils.jsto standardize JSON object parsing and validation. - Refactored five call sites across
body-transform.js,providers/copilot.js, andmodel-resolver.jsto use the shared parser. - Added focused Jest coverage for the shared parsing behavior in
containers/api-proxy/body-utils.test.js.
Show a summary per file
| File | Description |
|---|---|
| containers/api-proxy/providers/copilot.js | Switches BYOK extra-body-field injection to use the shared JSON object parser. |
| containers/api-proxy/model-resolver.js | Switches model rewrite path to use the shared JSON object parser. |
| containers/api-proxy/body-transform.js | Switches transform utilities to use the shared JSON object parser. |
| containers/api-proxy/body-utils.js | Introduces the centralized parseBodyAsObject helper. |
| containers/api-proxy/body-utils.test.js | Adds unit tests to lock in shared parsing behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 0
|
@copilot fix this failing ci check https://github.com/github/gh-aw-firewall/actions/runs/26985429834/job/79637011619?pr=4337 |
The root cause was that |
Smoke Test: Claude Engine
Result: PASS
|
|
Smoke: Copilot BYOK (Direct) — PASS ✅
Running in direct BYOK mode (
|
|
PR titles:
✅ GitHub PR query Overall status: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
🧪 Smoke Test Results
PR: Deduplicate JSON body object parsing across api-proxy transforms and model rewrite paths Overall: PASS
|
🔬 Smoke Test: API Proxy OpenTelemetry Tracing
All scenarios passed. OTEL tracing integration is complete and working correctly.
|
🧪 Chroot Version Comparison Results
Overall: ❌ Not all versions match. Go versions are in sync. Python and Node.js differ between host and chroot environments.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL —
|
|
Smoke Test: 1/4 PASS (Connectivity, File, Bash OK; MCP FAIL). Overall: 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.
|
The API proxy repeated the same JSON body parse-and-validate guard across five request-processing functions, creating drift risk for future hardening or behavior changes. This change centralizes that guard and updates all identified call sites to use it.
Shared body parsing utility
containers/api-proxy/body-utils.jswithparseBodyAsObject(body).Call site refactors (5 sites)
containers/api-proxy/body-transform.jssanitizeNullToolCallTypesinjectSteeringMessageinjectStreamOptionscontainers/api-proxy/providers/copilot.jsinjectByokExtraBodyFieldscontainers/api-proxy/model-resolver.jsrewriteModelInBodyFocused coverage for shared behavior
containers/api-proxy/body-utils.test.jsto assert object parse success and null-return behavior for invalid/non-object inputs.