Harden and document the mcp gateway Authorization header contract against workspace credential exposure - #7794
Conversation
There was a problem hiding this comment.
Pull request overview
Extracts and documents the enclave MCP upstream authorization template, but does not address the client-facing gateway credential exposure in #7787.
Changes:
- Adds a named authorization-template constant.
- Adds a regression test for the literal template.
- Documents credential-handling requirements.
Show a summary per file
| File | Description |
|---|---|
src/enclave/gateway.ts |
Extracts the upstream header template. |
src/enclave/gateway.test.ts |
Tests that the template remains unresolved. |
docs/enclaves-architecture.md |
Documents workspace credential exposure. |
docs/awf-config-spec.md |
Adds authorization handling requirements. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Balanced
| type: 'http', | ||
| url: ENCLAVE_MCP_UPSTREAM_URL, | ||
| headers: { Authorization: 'Bearer ' + '$' + `{${ENCLAVE_MCP_CAPABILITY_ENV}}` }, | ||
| headers: { Authorization: ENCLAVE_MCP_AUTHORIZATION_HEADER_TEMPLATE }, |
| cannot bake a real secret into the contract it returns. Any config-adapter | ||
| that renders this contract into an engine or tool config file (for example | ||
| under `GITHUB_WORKSPACE`) MUST copy this template string verbatim and let the | ||
| MCP client resolve it from its own process environment at request time. |
| argument so it cannot bake a real secret into the contract. Any config adapter | ||
| that renders this contract into an engine/tool config file MUST copy the | ||
| template verbatim and resolve it from the MCP client's own process environment | ||
| at request time; persisting the resolved capability to any agent-readable file |
|
@copilot address review feedback |
Addressed in 48b1745: removed the misleading AWF upstream-only exposure claim and documented the separate mcpg client-to-gateway credential hop, including the runtime-only/no-persistence requirement for adapters consuming mcpg's rewritten output. Targeted gateway tests, type-check, docs lint, secret scanning, and CodeQL pass. |
|
✅ Copilot review passed with no inline comments. @copilot Add the |
|
✅ Contribution Check completed successfully! Contribution check complete for PR #7794: the PR includes a clear description, references a related issue, adds documentation updates, and includes a regression test for the new contract constant. No contribution-guideline issues found.
|
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓
|
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅
|
|
🔌 Smoke Services — All services reachable! ✅
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤
|
|
❌ Smoke Gemini reports failed. Facets need polishing...
|
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓
|
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓
|
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓
|
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓
|
|
🔌 Smoke Services — All services reachable! ✅
|
|
🚀 Security Guard has started processing this pull request |
|
✅ Contribution Check completed successfully! PR #7794 follows CONTRIBUTING.md: it includes tests for the new contract invariant, updates the relevant docs, references a related issue in the description, and keeps the change in existing src/docs files. No contribution-guideline issues found.
|
|
✅ Build Test Suite completed successfully!
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤
|
|
Copilot Network Isolation Smoke Test
Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
🧪 Smoke Test: Docker Sbx — PASS ✅
Overall: PASS cc
|
Smoke Test: Claude Engine Validation
Overall result: PASS
|
Smoke Test: Copilot BYOK (Direct) Mode ✅
Mode: Direct BYOK (COPILOT_PROVIDER_API_KEY) → api-proxy sidecar → api.githubcopilot.com Status: PASS
|
Smoke Test: Services Connectivity
Overall: FAIL —
|
|
Smoke Test: Copilot Engine —
Overall: PASS
|
|
Harden and document the mcp gateway Authorization header contract against workspace credential exposure
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) Overall: PASS Cc:
|
Chroot vs Host Version Comparison
|
|
Smoke TestPR titles:
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — PASS Notes:
|
|
Smoke Test: API Proxy OTEL Tracing — Results
All 5 scenarios pass. No unexpected failures.
|
Engine config adapters that consume AWF's MCP gateway handoff can write the
Authorizationheader verbatim into workspace-visible config files, making the gateway credential readable by any file-read tool, not just code execution. The concretely reported adapters (pydantic.md,crush.md,convert_gateway_config_codex.cjs) live ingithub/gh-aw, not this repo — this PR hardens and documents the AWF-side contract those adapters build on so the fix has an unambiguous, tested API to rely on.Explicit, named contract constant
Authorizationheader value inbuildEnclaveMcpgUpstreamContract()intoENCLAVE_MCP_AUTHORIZATION_HEADER_TEMPLATE, a literal environment-variable-reference template (never the resolved capability).buildEnclaveMcpgUpstreamContract()takes no environment argument, so it structurally cannot bake a real secret into the contract it returns.Regression test
Documentation
docs/enclaves-architecture.mdanddocs/awf-config-spec.md§14.3 now spell out that any config adapter rendering this contract into a file must copy the template verbatim and resolve it from the MCP client's own process environment at request time — never persist the resolved value to any agent-readable file.