Wire dynamic enclave delegation controller into the workflow runtime - #59046
Conversation
…gent isolation Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…on from run identity Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…e fallback Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…ify maxSchemaHashes mapping Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…gation gh-aw-mcpg v0.4.17 ships gh-aw-mcpg#12605, which implements the github-repository-delegation-v1 atomic bootstrap contract that dynamic agent enclaves consume. v0.4.16 advertised the controller but rejected the compiler's configuration, never started the real controller, and never handed AWF a private control endpoint. - Bump DefaultMCPGatewayVersion v0.4.16 -> v0.4.17 and pin the immutable digest sha256:29b2c79f..., recompiling all 299 lock files. - Keep MCPGDynamicRepositoryDelegationMinVersion at v0.4.17 so dynamic repository enclaves fail closed on any pinned mcpg older than v0.4.17, while static/legacy enclave profiles keep their v0.4.15 floor. - Update enclave docs and recommended example pins to v0.4.17. - Emit the delegation control endpoint via fmt.Fprintf so the manualpathconcat and staticcheck QF1012 linters pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Four critical and two moderate runtime compatibility and networking issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Wires dynamic GitHub enclave delegation into the workflow runtime using MCPG v0.4.17 while isolating primary-agent access.
Changes:
- Adds environment-based delegation bootstrap, persistent state, runtime expiry clamping, and a private control endpoint.
- Removes provisional stdin configuration and implicit primary-agent GitHub access.
- Updates version gates, tests, documentation, pins, and generated workflows.
File summaries
| File | Description |
|---|---|
pkg/workflow/tools_types.go |
Removes provisional delegation controller types. |
pkg/workflow/mcp_setup_gateway.go |
Adds controller bootstrap and topology. Critical (2 votes): generation is not a valid decimal uint64. Critical (2 votes): bridge-mode loopback binding is unreachable through Docker port publication. Critical (2 votes): the generated endpoint uses a nonexistent route instead of /internal/awf-enclave-mcp-control/. |
pkg/workflow/mcp_renderer.go |
Stops rendering the rejected controller configuration. |
pkg/workflow/mcp_gateway_config.go |
Prevents implicit primary-agent GitHub access. |
pkg/workflow/enclaves.go |
Builds delegation envelopes and runtime expiry. Critical (2 votes): the envelope does not match MCPG v0.4.17’s snake_case wire contract and is rejected at startup. Moderate (2 votes): port+10 can collide or exceed the valid port range. Moderate (2 votes): BSD date cannot parse all accepted RFC3339 values. |
pkg/workflow/enclaves_test.go |
Tests delegation generation and expiry behavior. |
pkg/workflow/enclave_github_proxy_test.go |
Tests primary-agent isolation. |
pkg/workflow/data/action_pins.json |
Adds the MCPG v0.4.17 pin. |
pkg/workflow/awf_env.go |
Excludes the private control endpoint from the primary agent. |
pkg/constants/version_constants.go |
Updates MCPG default and minimum versions. |
pkg/constants/version_constants_test.go |
Updates version expectations. |
pkg/actionpins/data/action_pins.json |
Adds the immutable MCPG image pin. |
docs/src/content/docs/experimental/enclaves.md |
Documents the runtime delegation contract. |
.github/workflows/schema-feature-coverage.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/github-remote-mcp-auth-test.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/feature-grower.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/daily-team-status.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/daily-spec-coverage-kiro.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/daily-schema-audit-cursor.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/daily-regression-audit-kiro.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/daily-max-ai-credits-test.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/daily-github-docs-seo-optimizer.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/daily-community-attribution.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/daily-byok-ollama-test.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/daily-action-setup-security-audit.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/copilot-pr-merged-report.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/workflows/codex-github-remote-mcp-test.lock.yml |
Regenerates with MCPG v0.4.17. |
.github/aw/enclaves.md |
Updates enclave authoring guidance. |
.github/aw/actions-lock.json |
Records the new MCPG pin. |
.changeset/upgrade-gh-aw-mcpg-v0-4-17.md |
Records the dependency upgrade. |
Review details
- Files reviewed: 68/315 changed files
- Comments generated: 6
- Review effort level: Balanced
| return map[string]any{ | ||
| "version": enclaveDynamicGitHubPolicy, | ||
| "runId": enclaveDelegationGeneration, | ||
| "backend": "github", | ||
| "allowedOwners": stringSliceOrEmpty(policy.AllowedOwners), |
There was a problem hiding this comment.
Fixed in becef83: buildMCPGatewayDelegationEnvelope now emits the exact v0.4.17 snake_case contract (run_id, enclave_backend, allowed_owners, allowed_repositories, tool_policy, allowed_schema_hashes, max_dynamic_schema_hashes, max_identity_ttl, expires_at) with no unknown fields, and TestDynamicEnclaveGatewayContract asserts the exact field set.
| yaml.WriteString(" mkdir -p \"" + enclaveDelegationStateDir + "\"\n") | ||
| yaml.WriteString(" chmod 700 \"" + enclaveDelegationStateDir + "\"\n") | ||
| yaml.WriteString(" export MCP_GATEWAY_DELEGATION_STATE_PATH=\"" + enclaveDelegationStateDir + "/state.json\"\n") | ||
| yaml.WriteString(" export MCP_GATEWAY_DELEGATION_GENERATION=\"" + enclaveDelegationGeneration + "\"\n") |
There was a problem hiding this comment.
Fixed in becef83: MCP_GATEWAY_DELEGATION_GENERATION now uses a plain decimal ${GITHUB_RUN_ATTEMPT} value; the envelope's run_id keeps the hyphenated run/attempt pair since that field is free-form.
| yaml.WriteString(" chmod 700 \"" + enclaveDelegationStateDir + "\"\n") | ||
| yaml.WriteString(" export MCP_GATEWAY_DELEGATION_STATE_PATH=\"" + enclaveDelegationStateDir + "/state.json\"\n") | ||
| yaml.WriteString(" export MCP_GATEWAY_DELEGATION_GENERATION=\"" + enclaveDelegationGeneration + "\"\n") | ||
| yaml.WriteString(" export MCP_GATEWAY_DELEGATION_CONTROL_LISTEN=\"127.0.0.1:" + strconv.Itoa(port+enclaveDelegationControlPortOffset) + "\"\n") |
There was a problem hiding this comment.
Fixed in becef83: the in-container listener now binds to 0.0.0.0 under bridge/network-isolation mode so Docker's -p publish can reach it, and stays 127.0.0.1 under --network host. The host-side -p publication remains 127.0.0.1 in both cases.
| // AWF-only private handoff: the control endpoint is never published to the | ||
| // primary-agent network, the enclave executor network, or the general MCP | ||
| // route, and is kept out of the primary agent's environment via --exclude-env. | ||
| fmt.Fprintf(yaml, " export AWF_ENCLAVE_GITHUB_DELEGATION_CONTROL_ENDPOINT=\"http://127.0.0.1:%d/control/%s\"\n", port+enclaveDelegationControlPortOffset, enclaveDynamicController) |
There was a problem hiding this comment.
Fixed in becef83: the control endpoint now targets the pinned mcpg build's actual /internal/awf-enclave-mcp-control/ API base path instead of /control/....
| // enclaveDelegationControlPortOffset is added to the job's MCP gateway data-plane | ||
| // port to derive the private, host-only listener port for mcpg's | ||
| // github-repository-delegation-v1 control plane. Deriving it from the (per-job | ||
| // configurable) data-plane port, rather than a single fixed literal, avoids a | ||
| // port collision on self-hosted runners that execute multiple concurrent jobs | ||
| // with distinct gateway ports on the same host. The control port is bound | ||
| // separately from MCP_GATEWAY_PORT and published only to loopback so neither | ||
| // the primary agent nor the enclave executor network can route to it. | ||
| enclaveDelegationControlPortOffset = 10 |
There was a problem hiding this comment.
Fixed in becef83/dba659c: validateSandboxConfig now rejects sandbox.mcp.port values that would push the derived control port out of the valid TCP range. Cross-job collisions between one job's control port and another job's data port remain an operator responsibility (documented in the constant's comment) since compile-time validation only sees a single job's configuration; a full collision-free multi-job port allocation is out of scope for this fix.
| // date -u -d is GNU coreutils syntax (Linux runners); fall back to BSD date's | ||
| // -j -f for portability, matching the pattern used elsewhere in this repo for | ||
| // parsing RFC3339 timestamps into epoch seconds on macOS runners. | ||
| fmt.Fprintf(&script, " GH_AW_ENCLAVE_DYNAMIC_CONFIGURED_EXPIRES_EPOCH=$(date -u -d %s +%%s 2>/dev/null || date -u -j -f \"%%Y-%%m-%%dT%%H:%%M:%%SZ\" %s +%%s)\n", escapedExpiresAt, escapedExpiresAt) |
There was a problem hiding this comment.
Fixed in becef83/dc0e2b8: buildDynamicEnclaveExpiryScript now canonicalizes the compiled expires-at to whole-second UTC RFC3339 before embedding it, so offsets and fractional seconds no longer break the BSD date fallback; a canonicalization failure now surfaces as an internal error rather than silently falling back to the raw value.
|
@copilot address review feedback |
… networking/path, port validation, and BSD date canonicalization Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…lently falling back Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…sponsibility, not detected at compile time Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Addressed all six items from the review: the envelope now emits mcpg's exact snake_case wire schema ( |
@copilot Please take the next forward-progress pass on PR #59046.
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
- "github.com"See Network Configuration for more information.
|
Re-checked HEAD (dba659c, 10 commits) and confirmed the six review threads are already addressed in code (becef83/dc0e2b8/dba659c) with replies posted on each thread citing the fixing commit. The remaining blocker is review follow-through, not fresh implementation: |
|
🎉 This pull request is included in a new release. Release: |
…amic enclave delegation) - Add Logs Storage Budget (--max-storage) documenting gh aw logs selective pruning/storage cap (#59049) - Add Dynamic Enclave Delegation Controller documenting enclaves[].agent.dynamic mode (#59046) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mcpg v0.4.16 rejects the provisional
gateway.delegationControllersstdin config the compiler emitted for dynamic GitHub enclaves, so AWF never received a control endpoint and dynamic policies were always rejected at runtime.Controller bootstrap
gateway.delegationControllersstdin field (MCPGatewayDelegationControllertype,MCPGatewayRuntimeConfig.DelegationControllers) that the real mcpg schema rejects.github-repository-delegation-v1controller:MCP_GATEWAY_DELEGATION_ENVELOPE,_CONTROL_KEY,_STATE_PATH,_GENERATION,_CONTROL_LISTEN.chmod 700mount) so controller state survives an in-run restart.Private control endpoint
AWF_ENCLAVE_GITHUB_DELEGATION_CONTROL_ENDPOINT, distinct from the executor-facingAWF_ENCLAVE_MCP_GATEWAY_ENDPOINT, bound to127.0.0.1only.port+10) rather than a fixed literal, avoiding collisions on self-hosted runners running concurrent jobs.Primary-agent isolation fix
tools.github. The backend stays registered for delegated identities, but the primary agent no longer gets implicit GitHub tool access:Runtime-relative expiry
expires-at <= now + timeoutcheck with a runtime clamp resolved at workflow setup:min(configured expires-at, job-start + enclave.timeout), computed via a GNU/BSDdate-portable shell script. Checked-in workflows no longer need a live-updating absolute timestamp.Version gating
MCPGDynamicRepositoryDelegationMinVersionaboveDefaultMCPGatewayVersionso dynamic enclave setup fails closed until a real post-mcpg-delegation-contract release is pinned.Out of scope
Exact mcpg wire-protocol validation against the unreleased compatible release, raising
AWFDynamicRepositoryEnclaveMinVersion/DefaultFirewallVersion, and DIFC isolation specifics — these depend on external releases not yet available.