Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion actions/setup/js/start_mcp_gateway.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1356,7 +1356,7 @@ async function main() {
// Write GitHub Actions step outputs
// -----------------------------------------------------------------------
if (githubOutput) {
const outputs = [`gateway-pid=${gatewayPid}`, `gateway-port=${gatewayPort}`, `gateway-agent-id=${agentId}`, `gateway-domain=${gatewayDomain}`].join("\n");
const outputs = [`gateway-pid=${gatewayPid}`, `gateway-port=${gatewayPort}`, `gateway-agent-id=${agentId}`, `gateway-api-key=${agentId}`, `gateway-domain=${gatewayDomain}`].join("\n");
try {
fs.appendFileSync(githubOutput, outputs + "\n");
} catch {
Expand Down
5 changes: 5 additions & 0 deletions actions/setup/js/start_mcp_gateway.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ describe("start_mcp_gateway logging", () => {
expect(source).toContain(`stdio: ["pipe", outputFd, stderrFd]`);
expect(source).toContain(`path.join(os.tmpdir(), "gh-aw-mcp-gateway-")`);
});

it("exports the gateway agent ID as the AWF API key handoff", () => {
const source = fs.readFileSync(new URL("./start_mcp_gateway.cjs", import.meta.url), "utf8");
expect(source).toContain("`gateway-api-key=${agentId}`");
});
});

describe("start_mcp_gateway health timeout", () => {
Expand Down
1 change: 1 addition & 0 deletions actions/setup/sh/start_mcp_gateway.sh
Original file line number Diff line number Diff line change
Expand Up @@ -524,5 +524,6 @@ echo ""
echo "gateway-pid=$GATEWAY_PID"
echo "gateway-port=${MCP_GATEWAY_PORT}"
echo "gateway-agent-id=${MCP_GATEWAY_AGENT_ID}"
echo "gateway-api-key=${MCP_GATEWAY_AGENT_ID}"
echo "gateway-domain=${MCP_GATEWAY_DOMAIN}"
} >> "$GITHUB_OUTPUT"
6 changes: 6 additions & 0 deletions actions/setup/sh/start_mcp_gateway_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,12 @@ test_validation_functions_exist() {
else
print_result "Configuration input handling missing" "FAIL"
fi

if grep -q 'gateway-api-key=${MCP_GATEWAY_AGENT_ID}' "$SCRIPT_PATH"; then
print_result "Gateway API key output handoff exists" "PASS"
else
print_result "Gateway API key output handoff missing" "FAIL"
fi

# Check for JSON validation
if grep -q "not valid JSON" "$SCRIPT_PATH"; then
Expand Down
3 changes: 3 additions & 0 deletions pkg/workflow/awf_env.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ func ComputeAWFExcludeEnvVarNames(workflowData *WorkflowData, coreSecretVarNames
if HasMCPServers(workflowData) {
addUnique("MCP_GATEWAY_AGENT_ID")
}
if enclavesEnabled(workflowData) {
addUnique("MCP_GATEWAY_API_KEY")
}

// GitHub MCP server token is always a secret when the GitHub tool is present.
if hasGitHubTool(workflowData.ParsedTools) {
Expand Down
3 changes: 3 additions & 0 deletions pkg/workflow/copilot_engine_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,9 @@ func (e *CopilotEngine) buildCopilotExecutionStep(workflowData *WorkflowData, co
// This is a security measure to prevent exposing unnecessary secrets to the AWF container
allowedSecrets := e.GetRequiredSecretNames(workflowData)
filteredEnv := FilterEnvForSecrets(env, allowedSecrets)
if enclavesEnabled(workflowData) {
filteredEnv["MCP_GATEWAY_API_KEY"] = "${{ steps.start-mcp-gateway.outputs.gateway-api-key }}"
}
// Inject GH_TOKEN for CLI proxy (added after filtering since it uses a special
// fallback expression that is always allowed when cli-proxy is enabled)
addCliProxyGHTokenToEnv(filteredEnv, workflowData)
Expand Down
39 changes: 38 additions & 1 deletion pkg/workflow/enclave_github_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,36 @@ func TestEnclaveGitHubMCPAgentPolicy(t *testing.T) {

func TestEnclaveGitHubMCPGatewayConfiguration(t *testing.T) {
data := enclaveGitHubIssuesWorkflowData()
data.Tools["github"] = map[string]any{}
data.SafeOutputs = &SafeOutputsConfig{AddComments: &AddCommentsConfig{}}
config := buildMCPGatewayConfig(data)

assert.Empty(t, config.AgentID)
assert.Equal(t, []string{"${MCP_GATEWAY_AGENT_ID}", "${AWF_ENCLAVE_GITHUB_MCP_AGENT_ID}"}, config.AgentIDs)
assert.Equal(t, []string{enclaveMCPServerName}, config.AgentPolicies["${MCP_GATEWAY_AGENT_ID}"].Servers)
assert.Equal(t, []string{enclaveMCPServerName, "github", constants.SafeOutputsMCPServerID.String()}, config.AgentPolicies["${MCP_GATEWAY_AGENT_ID}"].Servers)
assert.Equal(t, []string{"github"}, config.AgentPolicies["${AWF_ENCLAVE_GITHUB_MCP_AGENT_ID}"].Servers)

generatedServers := make(map[string]struct{})
for _, server := range collectMCPServersForManifest(data) {
generatedServers[server.Name] = struct{}{}
}
for agentID, policy := range config.AgentPolicies {
for _, server := range policy.Servers {
assert.Contains(t, generatedServers, server, "policy for %s references an unknown MCP server", agentID)
}
}
}

func TestToolsWithEnclaveGitHubIssuesUnionsTypedToolsets(t *testing.T) {
data := enclaveGitHubIssuesWorkflowData()
tools := map[string]any{
"github": map[string]any{"toolsets": []string{"context"}},
}

updated := toolsWithEnclaveGitHubIssues(tools, data)

assert.Equal(t, []string{"context", "issues"}, updated["github"].(map[string]any)["toolsets"])
assert.Equal(t, []string{"context"}, tools["github"].(map[string]any)["toolsets"], "original tools must remain unchanged")
}

func TestCompileEnclaveGitHubSharedGateway(t *testing.T) {
Expand All @@ -45,6 +69,11 @@ on: workflow_dispatch
strict: false
network: defaults
engine: copilot
tools:
github:
toolsets: [context]
safe-outputs:
add-comment:
sandbox:
agent:
id: awf
Expand Down Expand Up @@ -72,9 +101,17 @@ Read the assigned repository's issues through the enclave.

assert.Equal(t, 1, strings.Count(lock, "--name awmg-mcpg"))
assert.Contains(t, lock, `"agentIds": ["${MCP_GATEWAY_AGENT_ID}","${AWF_ENCLAVE_GITHUB_MCP_AGENT_ID}"]`)
assert.Contains(t, lock, `"safeoutputs": {`)
assert.Contains(t, lock, `"awf-enclave": {`)
assert.Contains(t, lock, `"required": false`)
assert.Contains(t, lock, `"GITHUB_TOOLSETS": "context,issues"`)
assert.Contains(t, lock, `"${MCP_GATEWAY_AGENT_ID}":{"servers":["awf-enclave","github","safeoutputs"],"tools":{"github":["get_me"]}}`)
assert.NotContains(t, lock, `"servers":["awf-enclave","github","safe-outputs"]`)
assert.Contains(t, lock, `"agentPolicies": {"${AWF_ENCLAVE_GITHUB_MCP_AGENT_ID}":{"servers":["github"],"tools":{"github":["list_issues","issue_read"]},"allow-only":{"min-integrity":"approved","repos":["octo-org/private-service"]}}`)
assert.Contains(t, lock, `AWF_ENCLAVE_GITHUB_MCP_AGENT_ID=$(openssl rand -base64 45 | tr -d '/+=')`)
assert.Contains(t, lock, `printf '%s=%s\n' AWF_ENCLAVE_GITHUB_MCP_AGENT_ID "$AWF_ENCLAVE_GITHUB_MCP_AGENT_ID"`)
assert.Contains(t, lock, `MCP_GATEWAY_API_KEY: ${{ steps.start-mcp-gateway.outputs.gateway-api-key }}`)
assert.Contains(t, lock, `--exclude-env MCP_GATEWAY_API_KEY`)
assert.Contains(t, lock, "--exclude-env AWF_ENCLAVE_GITHUB_MCP_AGENT_ID")
assert.NotContains(t, lock, "Enclave GitHub Proxy")
assert.NotContains(t, lock, "start_enclave_github_proxy")
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/enclaves.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ func writeEnclaveMCPJSON(yaml *strings.Builder, workflowData *WorkflowData, isLa
fmt.Fprintf(yaml, " %q: {\n", enclaveMCPServerName)
yaml.WriteString(" \"type\": \"http\",\n")
fmt.Fprintf(yaml, " \"url\": %q,\n", enclaveMCPUpstreamURL)
yaml.WriteString(" \"required\": false,\n")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making awf-enclave globally optional removes the fail-fast check for the deferred enclave path, so a broken enclave startup now escapes setup and only crashes later when the first enclave tool is invoked.

💡 Why this blocks

The deferred-server export already solves the eager-connect problem; adding "required": false on top changes behavior again by telling the gateway that missing enclave connectivity is acceptable. That hides real startup defects such as a dead enclave process, bad capability header, or missing readiness handoff, and turns them into harder-to-debug runtime failures during agent execution.

A safer fix is to preserve deferred startup while keeping an explicit readiness assertion before the agent step begins. If the gateway truly must tolerate a deferred backend here, the generated workflow needs a separate check that proves the enclave endpoint became reachable before any enclave tool can be used.

fmt.Fprintf(yaml, " \"headers\": {\"Authorization\": \"Bearer \\${%s}\"},\n", enclaveMCPCapabilityEnv)
fmt.Fprintf(yaml, " \"tools\": [")
for i, tool := range enabledEnclaveTools(workflowData) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/enclaves_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func TestGenerateEnclaveGatewayContract(t *testing.T) {
assert.Contains(t, generated, `AWF_ENCLAVE_MCP_GATEWAY_ENDPOINT="http://localhost:${MCP_GATEWAY_PORT}/mcp/awf-enclave"`)
assert.Contains(t, generated, `export GH_AW_MCP_DEFERRED_SERVERS="awf-enclave"`)
assert.NotContains(t, generated, `printf '%s=%s\n' GH_AW_MCP_DEFERRED_SERVERS`)
assert.NotContains(t, generated, `"required": false`)
assert.Contains(t, generated, `"required": false`)
assert.NotRegexp(t, `AWF_ENCLAVE_MCP_CAPABILITY=[0-9a-f]{64}`, generated)
gatewayCommand := strings.Index(generated, `export MCP_GATEWAY_DOCKER_COMMAND=`)
require.Greater(t, gatewayCommand, -1)
Expand Down Expand Up @@ -317,7 +317,7 @@ Use the enclave script executor.
assert.Less(t, gatewayKeyHandoff, awf)
assert.Less(t, deferred, awf)
assert.Contains(t, lock, `"awf-enclave"`)
assert.NotContains(t, lock, `"required": false`)
assert.Contains(t, lock, `"required": false`)
assert.Contains(t, lock, "--exclude-env MCP_GATEWAY_AGENT_ID")
if mountStart := strings.Index(lock, "- name: Mount MCP servers as CLIs"); mountStart >= 0 {
mountEnd := strings.Index(lock[mountStart:], "\n - name:")
Expand Down
6 changes: 5 additions & 1 deletion pkg/workflow/mcp_gateway_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,11 @@ func buildMCPGatewayConfig(workflowData *WorkflowData) *MCPGatewayRuntimeConfig
OTLPHeaders: workflowData.OTLPHeaders,
}
if enclaveGitHubIssuesEnabled(workflowData) {
primaryServers := collectMCPTools(workflowData)
manifestServers := collectMCPServersForManifest(workflowData)
primaryServers := make([]string, 0, len(manifestServers))
for _, server := range manifestServers {
primaryServers = append(primaryServers, server.Name)
}
primaryGitHubEnabled := false
if githubTool, hasGitHub := workflowData.Tools["github"]; hasGitHub && githubTool != false {
primaryGitHubEnabled = !isGitHubCLIModeEnabled(workflowData)
Expand Down
4 changes: 4 additions & 0 deletions pkg/workflow/mcp_setup_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ func toolsWithEnclaveGitHubIssues(tools map[string]any, workflowData *WorkflowDa
if !slices.Contains(toolsets, "issues") {
githubConfig["toolsets"] = append(toolsets, "issues")
}
} else if toolsets, ok := githubConfig["toolsets"].([]string); ok {
if !slices.Contains(toolsets, "issues") {
githubConfig["toolsets"] = append(toolsets, "issues")
}
} else if toolsets, ok := githubConfig["toolsets"].(string); ok {
if !slices.Contains(strings.Split(toolsets, ","), "issues") {
githubConfig["toolsets"] = toolsets + ",issues"
Expand Down
Loading