Enable repository delegation in unified MCP gateway mode - #12753
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🟡 Changes recommended
Delegated calls can bypass revocation without gateway keys and fail under per-agent policies, while required integration coverage is absent.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Enables repository delegation in unified MCP gateway mode by sharing delegation state across control and data planes.
Changes:
- Initializes delegation during unified startup and serves the private control API.
- Authenticates and authorizes delegated GitHub tool calls.
- Adds shared delegation configuration and unit coverage.
File summaries
| File | Description |
|---|---|
internal/server/unified.go |
Attaches delegation runtime state. |
internal/server/middleware.go |
Adds delegated authentication. |
internal/server/middleware_auth.go |
Accepts live executor bearers. |
internal/server/delegation.go |
Implements control and authorization logic. |
internal/server/delegation_test.go |
Tests delegated calls and authentication. |
internal/server/backend_call.go |
Authorizes calls before execution. |
internal/proxy/delegation.go |
Reuses shared delegation configuration. |
internal/delegation/store.go |
Detects live executor bearers. |
internal/delegation/store_test.go |
Tests bearer revocation handling. |
internal/delegation/config.go |
Defines shared runtime configuration. |
internal/config/config_core.go |
Adds delegation to gateway configuration. |
internal/cmd/root.go |
Starts and stops unified delegation services. |
internal/cmd/proxy.go |
Returns shared runtime configuration. |
internal/cmd/delegation.go |
Manages the unified control listener. |
internal/cmd/delegation_test.go |
Tests control-listener status. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| func applyAuthIfConfiguredWithDelegation(apiKeys []string, delegatedAuthenticator func(string) bool, handler http.HandlerFunc) http.HandlerFunc { | ||
| if len(apiKeys) > 0 { | ||
| logAuth.Print("Auth key configured, applying middleware") | ||
| return authMiddleware(apiKeys, handler) | ||
| return authMiddlewareWithDelegation(apiKeys, delegatedAuthenticator, handler) |
| }() | ||
|
|
||
| sessionID := us.getSessionID(ctx) | ||
| ctx, err := us.authorizeDelegatedToolCall(ctx, serverID, toolName, args) |
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestStartUnifiedDelegationControlServesStatus(t *testing.T) { |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
lpcox
left a comment
There was a problem hiding this comment.
Requesting changes because the unified controller now starts, but the delegated data plane is still unusable under the exact gh-aw configuration and admits capabilities outside the closed delegation policy.
Before merge, add a real root-command integration test that launches the config-stdin unified mode with all five delegation environment inputs, the private listener, and AgentPolicies exactly as gh-aw emits them. Exercise status, reconcile, create-or-confirm, revoke, and revoke-by-labels over the control port; use the minted bearer through /mcp/github for an allowed call; prove denial for other repositories, tools, and non-tool MCP capabilities; then verify revocation and restart/recovery behavior.
The prior CI runs reported three testifylint failures. Head d7955c7 is specifically intended to fix those diagnostics, so the refreshed head still needs a green lint rerun.
|
@copilot address all review feedback |
Fixed in d7955c7. The referenced testifylint errors are resolved, and validation passed with |
Verified at d7955c7: the referenced testifylint diagnostics are cleared locally with |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Addressed in c58599e. The current branch carries the delegated authorization through the static AgentPolicies check only after store authorization, exposes delegated sessions through a filtered |
|
@copilot address the review feedback #12753 (comment) |
…nd listener-conflict coverage Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Addressed — see the reply on the linked discussion thread and the latest commit for the fix and added test coverage. |
🔒 mcpg Read-Only Stress — gvisorSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Overall: INCONCLUSIVE No write leaked. Part B could not distinguish gateway-level enforcement from backend
|
🔒 mcpg Read-Only Stress — defaultSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Overall: INCONCLUSIVE Notes:
|
lpcox
left a comment
There was a problem hiding this comment.
Verified on head bd2fe68. The dynamic-policy bypass is limited to calls already authorized by the delegation store, non-tool and malformed/batched JSON-RPC requests fail closed, and the root-command integration coverage now exercises the control/data-plane boundary, credential separation, listener failure, revocation, and restart persistence. Full local completion and integration suites pass.
github-repository-delegation-v1only initialized inproxymode, leaving gh-aw’s unified gateway without the private control listener even when all delegation activation inputs were present. AWF could receive a control endpoint, but reconciliation failed because no controller was serving it.Unified delegation activation
Private control plane
status,reconcile,create-or-confirm,revoke, andrevoke-by-labelsoperations through the private control endpoint.Delegated
/mcp/githubdata planeowner/repoand closedgithub-repository-read-v1tool set.