Fail closed on invalid API proxy model allowlists - #9358
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Partially malformed policy arrays remain accepted instead of failing closed.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Enforces fail-closed API proxy model allowlists to prevent unauthorized fallback models.
Changes:
- Preserves explicitly empty allowlists through configuration.
- Rejects invalid policies and unauthorized requests.
- Adds tests and operator documentation.
| File | Description |
|---|---|
src/services/api-proxy-service-misc-forwarding.test.ts |
Tests empty-list forwarding. |
src/services/api-proxy-env-config.ts |
Preserves explicit allowlists. |
src/services/api-proxy-env-config.test.ts |
Tests environment serialization. |
docs/api-proxy-sidecar.md |
Documents BYOK restrictions. |
containers/api-proxy/server.model-policy.test.js |
Tests request enforcement. |
containers/api-proxy/guards/model-policy-guard.test.js |
Tests invalid-policy rejection. |
containers/api-proxy/guards/model-policy-guard.js |
Adds startup validation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (process.env.AWF_ALLOWED_MODELS !== undefined && !ALLOWED_MODELS) { | ||
| throw new Error('AWF_ALLOWED_MODELS must be a non-empty JSON array of model patterns'); | ||
| } | ||
| if (process.env.AWF_DISALLOWED_MODELS !== undefined && !DISALLOWED_MODELS) { | ||
| throw new Error('AWF_DISALLOWED_MODELS must be a non-empty JSON array of model patterns'); |
|
@copilot Fix the code for all comments in this review thread. When a review comment includes a suggested change, apply the suggestion exactly. Do not make changes beyond what is described in the linked review thread. |
Partially malformed model-policy arrays now fail closed for both allowlists and denylists. Focused tests pass. Commit: |
|
✅ Copilot review passed with no inline comments. @copilot Add the |
Documentation PreviewDocumentation has been built for this PR. To view locally:
Built from commit 12295c2 |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓
|
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤
|
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded.
|
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅
|
|
Smoke Cloud Hypervisor reports failed. Cloud Hypervisor + Copilot failed.
|
|
🔌 Smoke Services — All services reachable! ✅
|
|
✅ Security Guard completed successfully! Security review of PR #9358 complete: 1 security-relevant file changed (containers/api-proxy/guards/model-policy-guard.js). Changes implement fail-closed validation with early error throwing on misconfigured model allowlists/denylists. No security-weakening changes detected. No firewall rules loosened, no capabilities added, no ACL regressions. PR passes security review.
|
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed...
|
|
❌ Smoke Gemini reports failed. Facets need polishing...
|
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed...
|
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅
|
|
✅ Build Test Suite completed successfully!
|
|
✅ Smoke Claude passed
|
Smoke Test: Copilot BYOK (Direct) Mode ✅
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) via api-proxy → api.githubcopilot.com Overall Status: PASS
|
|
Smoke Copilot: PASS
|
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
Smoke Test: Claude Engine Validation
Overall result: PASS
|
|
Smoke services: ✅ Redis PONG · ✅ pg_isready accepting connections · ✅ psql SELECT 1 → 1. PASS
|
|
EGRESS_RESULT allow=pass deny=pass
|
|
OTEL smoke test
|
Smoke Test
|
Chroot Version Comparison
Result: Not all tests passed. Node.js differs between host and chroot, so the
|
🏗️ Build Test Suite Results
Overall: 6/8 ecosystems passed — FAIL Failures
|

A hardcoded harness fallback can request a model the BYOK operator did not approve. AWF already blocks models outside
apiProxy.allowedModels, but an empty or malformed policy could silently disable that protection.gpt-5.4request before upstream dispatch, while approved models continue to forward.{"apiProxy":{"allowedModels":["gpt-5.6-sol"]}}With this policy, a harness request for
gpt-5.4receives HTTP 403.