From 0dc86b8d293572890416a93713b1a67e3ea2e5ba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 12 Jun 2026 21:54:08 +0000 Subject: [PATCH 1/4] Initial plan From 89adc5f0b55e5cfee6cf621e16156244441dabc6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 12 Jun 2026 22:23:50 +0000 Subject: [PATCH 2/4] Add GitHubMCPMode enum and replace githubTool any with map[string]any Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/agent_validation.go | 2 +- pkg/workflow/claude_tools.go | 2 +- pkg/workflow/codex_mcp.go | 2 +- pkg/workflow/compiler_difc_proxy.go | 30 +-- pkg/workflow/compiler_difc_proxy_test.go | 6 +- pkg/workflow/compiler_github_mcp_steps.go | 6 +- pkg/workflow/compiler_yaml.go | 15 +- pkg/workflow/copilot_engine_execution.go | 6 +- pkg/workflow/copilot_engine_test.go | 2 +- pkg/workflow/copilot_github_mcp_test.go | 10 +- pkg/workflow/docker.go | 5 +- pkg/workflow/engine_helpers_shared_test.go | 8 +- pkg/workflow/github_lockdown_test.go | 2 +- pkg/workflow/github_readonly_test.go | 56 +---- pkg/workflow/github_toolset_expansion_test.go | 2 +- pkg/workflow/github_toolset_test.go | 14 +- pkg/workflow/mcp_config_test.go | 10 +- pkg/workflow/mcp_environment.go | 18 +- pkg/workflow/mcp_github_config.go | 228 ++++++++---------- pkg/workflow/mcp_renderer.go | 2 +- pkg/workflow/mcp_renderer_factory.go | 2 +- pkg/workflow/mcp_renderer_github.go | 24 +- pkg/workflow/mcp_renderer_types.go | 2 +- pkg/workflow/mcp_setup_generator.go | 19 +- pkg/workflow/safeoutputs_guard_policy_test.go | 8 +- pkg/workflow/tools_parser.go | 2 +- pkg/workflow/tools_types.go | 16 +- pkg/workflow/tools_validation_test.go | 2 +- 28 files changed, 225 insertions(+), 276 deletions(-) diff --git a/pkg/workflow/agent_validation.go b/pkg/workflow/agent_validation.go index 1998c546646..32f0f725052 100644 --- a/pkg/workflow/agent_validation.go +++ b/pkg/workflow/agent_validation.go @@ -199,7 +199,7 @@ func (c *Compiler) validatePiEngineRequirements(tools *ToolsConfig, engine Codin return nil } - if tools == nil || tools.GitHub == nil || tools.GitHub.Mode != "gh-proxy" { + if tools == nil || tools.GitHub == nil || tools.GitHub.Mode != GitHubMCPModeGHProxy { return errors.New("engine 'pi' requires tools.github.mode: gh-proxy") } diff --git a/pkg/workflow/claude_tools.go b/pkg/workflow/claude_tools.go index 92e40a97353..e40e7259608 100644 --- a/pkg/workflow/claude_tools.go +++ b/pkg/workflow/claude_tools.go @@ -322,7 +322,7 @@ func appendGitHubMCPTools(allowedTools []string, toolName string, toolValue any, } githubMode := getGitHubType(mcpConfig) defaultTools := constants.DefaultGitHubToolsLocal - if githubMode == "remote" { + if githubMode == GitHubMCPModeRemote { defaultTools = constants.DefaultGitHubToolsRemote } for _, defaultTool := range defaultTools { diff --git a/pkg/workflow/codex_mcp.go b/pkg/workflow/codex_mcp.go index 1c124eaa173..0a66ddaa112 100644 --- a/pkg/workflow/codex_mcp.go +++ b/pkg/workflow/codex_mcp.go @@ -56,7 +56,7 @@ func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]an renderer := createRenderer(false) // isLast is always false in TOML format switch toolName { case "github": - githubTool := expandedTools["github"] + githubTool, _ := expandedTools["github"].(map[string]any) renderer.RenderGitHubMCP(&mcpConfigContent, githubTool, workflowData) case "playwright": playwrightTool := expandedTools["playwright"] diff --git a/pkg/workflow/compiler_difc_proxy.go b/pkg/workflow/compiler_difc_proxy.go index 77ed48a0bca..87effc082fd 100644 --- a/pkg/workflow/compiler_difc_proxy.go +++ b/pkg/workflow/compiler_difc_proxy.go @@ -83,7 +83,8 @@ func hasDIFCGuardsConfigured(data *WorkflowData) bool { if !hasGitHub || githubTool == false { return false } - return len(getGitHubGuardPolicies(githubTool)) > 0 + toolConfig, _ := githubTool.(map[string]any) + return len(getGitHubGuardPolicies(toolConfig)) > 0 } // isIntegrityProxyEnabled returns true unless the user has explicitly disabled the DIFC proxy @@ -167,20 +168,15 @@ func hasPreAgentStepsWithGHToken(data *WorkflowData) bool { // endorser-min-integrity) are also included in the proxy policy. // // Returns an empty string if no guard policy fields are found. -func getDIFCProxyPolicyJSON(githubTool any, data *WorkflowData, gatewayConfig *MCPGatewayRuntimeConfig) string { - toolConfig, ok := githubTool.(map[string]any) - if !ok { - return "" - } - +func getDIFCProxyPolicyJSON(githubTool map[string]any, data *WorkflowData, gatewayConfig *MCPGatewayRuntimeConfig) string { policy := make(map[string]any) // Support both 'allowed-repos' (preferred) and deprecated 'repos' - repos, hasRepos := toolConfig["allowed-repos"] + repos, hasRepos := githubTool["allowed-repos"] if !hasRepos { - repos, hasRepos = toolConfig["repos"] + repos, hasRepos = githubTool["repos"] } - integrity, hasIntegrity := toolConfig["min-integrity"] + integrity, hasIntegrity := githubTool["min-integrity"] if !hasRepos && !hasIntegrity { return "" @@ -198,7 +194,7 @@ func getDIFCProxyPolicyJSON(githubTool any, data *WorkflowData, gatewayConfig *M } // Inject reaction fields when the feature flag is enabled and MCPG supports it. - injectIntegrityReactionFields(policy, toolConfig, data, gatewayConfig) + injectIntegrityReactionFields(policy, githubTool, data, gatewayConfig) guardPolicy := map[string]any{ "allow-only": policy, @@ -229,16 +225,16 @@ func resolveProxyContainerImage(gatewayConfig *MCPGatewayRuntimeConfig) string { func (c *Compiler) buildStartDIFCProxyStepYAML(data *WorkflowData) string { difcProxyLog.Print("Building Start DIFC proxy step YAML") - githubTool := data.Tools["github"] + githubToolConfig, _ := data.Tools["github"].(map[string]any) // Get MCP server token (same token the gateway uses for the GitHub MCP server) - customGitHubToken := getGitHubToken(githubTool) + customGitHubToken := getGitHubToken(githubToolConfig) effectiveToken := getEffectiveGitHubToken(customGitHubToken) // Build the simplified guard policy JSON (static fields only) // (plus reaction fields when integrity-reactions feature flag is enabled) ensureDefaultMCPGatewayConfig(data) - policyJSON := getDIFCProxyPolicyJSON(githubTool, data, data.SandboxConfig.MCP) + policyJSON := getDIFCProxyPolicyJSON(githubToolConfig, data, data.SandboxConfig.MCP) if policyJSON == "" { difcProxyLog.Print("Could not build DIFC proxy policy JSON, skipping proxy start") return "" @@ -498,10 +494,10 @@ const defaultCliProxyPolicyJSON = `{"allow-only":{"repos":"all","min-integrity": func (c *Compiler) buildStartCliProxyStepYAML(data *WorkflowData) string { difcProxyLog.Print("Building Start CLI proxy step YAML") - githubTool := data.Tools["github"] + githubToolConfig, _ := data.Tools["github"].(map[string]any) // Get token for the proxy - customGitHubToken := getGitHubToken(githubTool) + customGitHubToken := getGitHubToken(githubToolConfig) effectiveToken := getEffectiveGitHubToken(customGitHubToken) // Build the guard policy JSON (static fields only, plus reaction fields when enabled). @@ -509,7 +505,7 @@ func (c *Compiler) buildStartCliProxyStepYAML(data *WorkflowData) string { // calls return HTTP 503 ("proxy enforcement not configured"). Use the default // permissive policy when no guard policy is configured in the frontmatter. ensureDefaultMCPGatewayConfig(data) - policyJSON := getDIFCProxyPolicyJSON(githubTool, data, data.SandboxConfig.MCP) + policyJSON := getDIFCProxyPolicyJSON(githubToolConfig, data, data.SandboxConfig.MCP) if policyJSON == "" { policyJSON = defaultCliProxyPolicyJSON difcProxyLog.Print("No guard policy configured, using default CLI proxy policy") diff --git a/pkg/workflow/compiler_difc_proxy_test.go b/pkg/workflow/compiler_difc_proxy_test.go index de31248896c..541502bc7d8 100644 --- a/pkg/workflow/compiler_difc_proxy_test.go +++ b/pkg/workflow/compiler_difc_proxy_test.go @@ -210,7 +210,7 @@ func TestHasPreAgentStepsWithGHToken(t *testing.T) { func TestGetDIFCProxyPolicyJSON(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any expectedContains []string expectedAbsent []string expectEmpty bool @@ -221,8 +221,8 @@ func TestGetDIFCProxyPolicyJSON(t *testing.T) { expectEmpty: true, }, { - name: "non-map tool", - githubTool: false, + name: "empty map tool", + githubTool: map[string]any{}, expectEmpty: true, }, { diff --git a/pkg/workflow/compiler_github_mcp_steps.go b/pkg/workflow/compiler_github_mcp_steps.go index 6b28209b233..c0a20075748 100644 --- a/pkg/workflow/compiler_github_mcp_steps.go +++ b/pkg/workflow/compiler_github_mcp_steps.go @@ -26,10 +26,11 @@ func (c *Compiler) generateGitHubMCPLockdownDetectionStep(yaml *strings.Builder, githubConfigLog.Print("Skipping GitHub MCP lockdown detection step: GitHub tool not enabled") return } + githubToolMap, _ := githubTool.(map[string]any) // Skip when guard policy is already fully configured in the workflow. // The step is only needed to auto-configure guard policies for public repos. - if len(getGitHubGuardPolicies(githubTool)) > 0 { + if len(getGitHubGuardPolicies(githubToolMap)) > 0 { githubConfigLog.Print("Guard policy already configured in workflow, skipping automatic guard policy determination") return } @@ -168,9 +169,10 @@ func (c *Compiler) generateParseGuardVarsStep(yaml *strings.Builder, data *Workf githubConfigLog.Print("Skipping parse-guard-vars step: GitHub tool not enabled") return } + githubToolMap, _ := githubTool.(map[string]any) // Only generate the step when guard policies are configured. - if len(getGitHubGuardPolicies(githubTool)) == 0 { + if len(getGitHubGuardPolicies(githubToolMap)) == 0 { githubConfigLog.Print("Skipping parse-guard-vars step: no explicit guard policies configured") return } diff --git a/pkg/workflow/compiler_yaml.go b/pkg/workflow/compiler_yaml.go index 96de1957523..2975827fbc6 100644 --- a/pkg/workflow/compiler_yaml.go +++ b/pkg/workflow/compiler_yaml.go @@ -933,12 +933,15 @@ func (c *Compiler) generateCreateAwInfo(yaml *strings.Builder, data *WorkflowDat // Include lockdown validation env vars when lockdown is explicitly enabled. // validateLockdownRequirements is called from generate_aw_info.cjs and uses these vars. githubTool, hasGitHub := data.Tools["github"] - if hasGitHub && githubTool != false && hasGitHubLockdownExplicitlySet(githubTool) && getGitHubLockdown(githubTool) { - yaml.WriteString(" GITHUB_MCP_LOCKDOWN_EXPLICIT: \"true\"\n") - yaml.WriteString(" GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }}\n") - yaml.WriteString(" GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }}\n") - if customToken := getGitHubToken(githubTool); customToken != "" { - fmt.Fprintf(yaml, " CUSTOM_GITHUB_TOKEN: %s\n", customToken) + if hasGitHub && githubTool != false { + toolConfig, _ := githubTool.(map[string]any) + if hasGitHubLockdownExplicitlySet(toolConfig) && getGitHubLockdown(toolConfig) { + yaml.WriteString(" GITHUB_MCP_LOCKDOWN_EXPLICIT: \"true\"\n") + yaml.WriteString(" GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }}\n") + yaml.WriteString(" GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }}\n") + if customToken := getGitHubToken(toolConfig); customToken != "" { + fmt.Fprintf(yaml, " CUSTOM_GITHUB_TOKEN: %s\n", customToken) + } } } // Embed custom token weights only when custom model multipliers are configured. diff --git a/pkg/workflow/copilot_engine_execution.go b/pkg/workflow/copilot_engine_execution.go index 28ba307f191..b5c2ff8acd0 100644 --- a/pkg/workflow/copilot_engine_execution.go +++ b/pkg/workflow/copilot_engine_execution.go @@ -604,12 +604,14 @@ touch %s if workflowData.ParsedTools != nil && workflowData.ParsedTools.GitHub != nil && workflowData.ParsedTools.GitHub.GitHubApp != nil { tokenExpression := "${{ steps.github-mcp-app-token.outputs.token }}" if workflowData.ParsedTools.GitHub.GitHubApp.shouldIgnoreMissingKey() { - customGitHubToken := getGitHubToken(workflowData.Tools["github"]) + githubToolConfig, _ := workflowData.Tools["github"].(map[string]any) + customGitHubToken := getGitHubToken(githubToolConfig) tokenExpression = combineTokenExpressions(tokenExpression, getEffectiveGitHubToken(customGitHubToken)) } env["GITHUB_MCP_SERVER_TOKEN"] = tokenExpression } else { - customGitHubToken := getGitHubToken(workflowData.Tools["github"]) + githubToolConfig, _ := workflowData.Tools["github"].(map[string]any) + customGitHubToken := getGitHubToken(githubToolConfig) // Use effective token with precedence: custom > default effectiveToken := getEffectiveGitHubToken(customGitHubToken) env["GITHUB_MCP_SERVER_TOKEN"] = effectiveToken diff --git a/pkg/workflow/copilot_engine_test.go b/pkg/workflow/copilot_engine_test.go index c139962d8e1..7de9e58d08a 100644 --- a/pkg/workflow/copilot_engine_test.go +++ b/pkg/workflow/copilot_engine_test.go @@ -1444,7 +1444,7 @@ func TestCopilotEnginePromptFilePath(t *testing.T) { func TestCopilotEngineRenderGitHubMCPConfig(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any isLast bool expectedStrs []string }{ diff --git a/pkg/workflow/copilot_github_mcp_test.go b/pkg/workflow/copilot_github_mcp_test.go index 66ca7ba4936..a3cf313419b 100644 --- a/pkg/workflow/copilot_github_mcp_test.go +++ b/pkg/workflow/copilot_github_mcp_test.go @@ -13,7 +13,7 @@ import ( func TestRenderGitHubCopilotMCPConfig_AllowedTools(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any isLast bool expectedContent []string unexpectedContent []string @@ -122,7 +122,7 @@ func TestRenderGitHubCopilotMCPConfig_AllowedTools(t *testing.T) { func TestGetGitHubAllowedTools(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any expected []string }{ { @@ -169,8 +169,8 @@ func TestGetGitHubAllowedTools(t *testing.T) { expected: []string{"issue_read:1", "list_labels"}, }, { - name: "Not a map", - githubTool: "invalid", + name: "Nil tool", + githubTool: nil, expected: nil, }, } @@ -208,7 +208,7 @@ func TestGetGitHubAllowedTools(t *testing.T) { func TestGetGitHubGuardPoliciesToolCallLimits(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any expected map[string]any }{ { diff --git a/pkg/workflow/docker.go b/pkg/workflow/docker.go index 169acd7b64d..e5dfca26519 100644 --- a/pkg/workflow/docker.go +++ b/pkg/workflow/docker.go @@ -20,10 +20,11 @@ func collectDockerImages(tools map[string]any, workflowData *WorkflowData, actio imageSet := make(map[string]bool) // Use a set to avoid duplicates // Check for GitHub tool (uses Docker image) - if githubTool, hasGitHub := tools["github"]; hasGitHub { + if rawGithubTool, hasGitHub := tools["github"]; hasGitHub { + githubTool, _ := rawGithubTool.(map[string]any) githubType := getGitHubType(githubTool) // Only add if using local (Docker) mode - if githubType == "local" { + if githubType == GitHubMCPModeLocal { githubDockerImageVersion := getGitHubDockerImageVersion(githubTool) image := "ghcr.io/github/github-mcp-server:" + githubDockerImageVersion if !imageSet[image] { diff --git a/pkg/workflow/engine_helpers_shared_test.go b/pkg/workflow/engine_helpers_shared_test.go index 5bc8e5ae813..1aa7b59daff 100644 --- a/pkg/workflow/engine_helpers_shared_test.go +++ b/pkg/workflow/engine_helpers_shared_test.go @@ -367,7 +367,7 @@ func TestRenderJSONMCPConfig(t *testing.T) { options: JSONMCPConfigOptions{ ConfigPath: "/tmp/test-config.json", Renderers: MCPToolRenderers{ - RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) { + RenderGitHub: func(yaml *strings.Builder, githubTool map[string]any, isLast bool, workflowData *WorkflowData) { yaml.WriteString(" \"github\": { \"test\": true }") if !isLast { yaml.WriteString(",") @@ -406,7 +406,7 @@ func TestRenderJSONMCPConfig(t *testing.T) { options: JSONMCPConfigOptions{ ConfigPath: "/tmp/filtered-config.json", Renderers: MCPToolRenderers{ - RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) { + RenderGitHub: func(yaml *strings.Builder, githubTool map[string]any, isLast bool, workflowData *WorkflowData) { yaml.WriteString(" \"github\": { \"filtered\": true }") if !isLast { yaml.WriteString(",") @@ -442,7 +442,7 @@ func TestRenderJSONMCPConfig(t *testing.T) { options: JSONMCPConfigOptions{ ConfigPath: "/tmp/debug-config.json", Renderers: MCPToolRenderers{ - RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) { + RenderGitHub: func(yaml *strings.Builder, githubTool map[string]any, isLast bool, workflowData *WorkflowData) { yaml.WriteString(" \"github\": {}\n") }, RenderPlaywright: func(yaml *strings.Builder, playwrightTool any, isLast bool) {}, @@ -511,7 +511,7 @@ func TestRenderJSONMCPConfig_IsLastHandling(t *testing.T) { options := JSONMCPConfigOptions{ ConfigPath: "/tmp/test.json", Renderers: MCPToolRenderers{ - RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) { + RenderGitHub: func(yaml *strings.Builder, githubTool map[string]any, isLast bool, workflowData *WorkflowData) { callOrder = append(callOrder, "github") isLastValues = append(isLastValues, isLast) }, diff --git a/pkg/workflow/github_lockdown_test.go b/pkg/workflow/github_lockdown_test.go index 65b241c03a6..2c35fe8f246 100644 --- a/pkg/workflow/github_lockdown_test.go +++ b/pkg/workflow/github_lockdown_test.go @@ -54,7 +54,7 @@ func TestGitHubLockdownField(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - githubTool := tt.toolsMap["github"] + githubTool, _ := tt.toolsMap["github"].(map[string]any) result := getGitHubLockdown(githubTool) if result != tt.expected { t.Errorf("getGitHubLockdown() = %v, want %v", result, tt.expected) diff --git a/pkg/workflow/github_readonly_test.go b/pkg/workflow/github_readonly_test.go index 65670c2f4ed..e22ad2a90ae 100644 --- a/pkg/workflow/github_readonly_test.go +++ b/pkg/workflow/github_readonly_test.go @@ -5,57 +5,9 @@ package workflow import "testing" func TestGetGitHubReadOnly(t *testing.T) { - tests := []struct { - name string - githubTool any - expected bool - }{ - { - name: "read-only true", - githubTool: map[string]any{ - "read-only": true, - }, - expected: true, - }, - { - name: "read-only false is ignored (always read-only)", - githubTool: map[string]any{ - "read-only": false, - }, - expected: true, - }, - { - name: "no read-only field", - githubTool: map[string]any{}, - expected: true, // now defaults to true - }, - { - name: "read-only with other fields", - githubTool: map[string]any{ - "read-only": true, - "version": "latest", - "args": []string{"--verbose"}, - }, - expected: true, - }, - { - name: "nil tool", - githubTool: nil, - expected: true, // now defaults to true - }, - { - name: "string tool (not map)", - githubTool: "github", - expected: true, // now defaults to true - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := getGitHubReadOnly(tt.githubTool) - if result != tt.expected { - t.Errorf("getGitHubReadOnly() = %v, want %v", result, tt.expected) - } - }) + // getGitHubReadOnly always returns true; the GitHub MCP server is unconditionally read-only. + result := getGitHubReadOnly() + if !result { + t.Errorf("getGitHubReadOnly() = %v, want true", result) } } diff --git a/pkg/workflow/github_toolset_expansion_test.go b/pkg/workflow/github_toolset_expansion_test.go index 2e3318189e7..1bccca8da81 100644 --- a/pkg/workflow/github_toolset_expansion_test.go +++ b/pkg/workflow/github_toolset_expansion_test.go @@ -77,7 +77,7 @@ func TestExpandDefaultToolset(t *testing.T) { func TestGetGitHubToolsetsExpandsDefault(t *testing.T) { tests := []struct { name string - input any + input map[string]any expected string }{ { diff --git a/pkg/workflow/github_toolset_test.go b/pkg/workflow/github_toolset_test.go index 106834f13cd..e01d751c1f9 100644 --- a/pkg/workflow/github_toolset_test.go +++ b/pkg/workflow/github_toolset_test.go @@ -10,7 +10,7 @@ import ( func TestGetGitHubToolsets(t *testing.T) { tests := []struct { name string - input any + input map[string]any expected string }{ { @@ -54,8 +54,8 @@ func TestGetGitHubToolsets(t *testing.T) { expected: "context,repos,issues,pull_requests,discussions", }, { - name: "Non-map input returns action-friendly", - input: "not a map", + name: "Nil map input returns action-friendly", + input: nil, expected: "context,repos,issues,pull_requests", }, } @@ -73,7 +73,7 @@ func TestGetGitHubToolsets(t *testing.T) { func TestClaudeEngineGitHubToolsetsRendering(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any expectedInYAML []string notInYAML []string }{ @@ -162,7 +162,7 @@ func TestClaudeEngineGitHubToolsetsRendering(t *testing.T) { func TestCopilotEngineGitHubToolsetsRendering(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any expectedInYAML []string notInYAML []string }{ @@ -241,7 +241,7 @@ func TestCopilotEngineGitHubToolsetsRendering(t *testing.T) { func TestCodexEngineGitHubToolsetsRendering(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any expectedInYAML []string notInYAML []string }{ @@ -320,7 +320,7 @@ func TestCodexEngineGitHubToolsetsRendering(t *testing.T) { func TestGitHubToolsetsWithOtherConfiguration(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any expectedInYAML []string }{ { diff --git a/pkg/workflow/mcp_config_test.go b/pkg/workflow/mcp_config_test.go index 90765c2dadd..d0b55386a10 100644 --- a/pkg/workflow/mcp_config_test.go +++ b/pkg/workflow/mcp_config_test.go @@ -122,7 +122,7 @@ This is a test workflow for MCP configuration. func TestGenerateGitHubMCPConfig(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any expectedType string }{ { @@ -148,9 +148,9 @@ func TestGenerateGitHubMCPConfig(t *testing.T) { expectedType: "docker", }, { - name: "non-map github tool", - githubTool: "invalid", - // With Docker always enabled, invalid tool config defaults to docker (not services) + name: "nil github tool defaults to docker", + githubTool: nil, + // With Docker always enabled, nil tool config defaults to docker (not services) expectedType: "docker", }, } @@ -187,7 +187,7 @@ func TestGenerateGitHubMCPConfig(t *testing.T) { func TestMCPConfigurationEdgeCases(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any isLast bool expected string }{ diff --git a/pkg/workflow/mcp_environment.go b/pkg/workflow/mcp_environment.go index 54d4610f18e..a1686fead66 100644 --- a/pkg/workflow/mcp_environment.go +++ b/pkg/workflow/mcp_environment.go @@ -64,10 +64,10 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor // Check for GitHub MCP server token hasGitHub := slices.Contains(mcpTools, "github") if hasGitHub { - githubTool := tools["github"] + toolConfig, _ := tools["github"].(map[string]any) // Check if GitHub App is configured for token minting - appConfigured := hasGitHubApp(githubTool) + appConfigured := hasGitHubApp(toolConfig) // If GitHub App is configured, use the app token minted directly in the agent job. // The token cannot be passed via job outputs from the activation job because @@ -76,18 +76,16 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor if appConfigured { mcpEnvironmentLog.Print("Using GitHub App token from agent job step for GitHub MCP server (overrides custom and default tokens)") tokenExpression := "${{ steps.github-mcp-app-token.outputs.token }}" - if toolConfig, ok := githubTool.(map[string]any); ok { - if appMap, ok := toolConfig["github-app"].(map[string]any); ok { - if appConfig := parseAppConfig(appMap); appConfig.shouldIgnoreMissingKey() { - customGitHubToken := getGitHubToken(githubTool) - tokenExpression = combineTokenExpressions(tokenExpression, getEffectiveGitHubToken(customGitHubToken)) - } + if appMap, ok := toolConfig["github-app"].(map[string]any); ok { + if appConfig := parseAppConfig(appMap); appConfig.shouldIgnoreMissingKey() { + customGitHubToken := getGitHubToken(toolConfig) + tokenExpression = combineTokenExpressions(tokenExpression, getEffectiveGitHubToken(customGitHubToken)) } } envVars["GITHUB_MCP_SERVER_TOKEN"] = tokenExpression } else { // Otherwise, use custom token or default fallback - customGitHubToken := getGitHubToken(githubTool) + customGitHubToken := getGitHubToken(toolConfig) effectiveToken := getEffectiveGitHubToken(customGitHubToken) envVars["GITHUB_MCP_SERVER_TOKEN"] = effectiveToken } @@ -96,7 +94,7 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor // Skip only when guard policy is already explicitly set — in that case, the // determine-automatic-lockdown step is not generated. // Security: Pass step outputs through environment variables to prevent template injection. - guardPoliciesExplicit := len(getGitHubGuardPolicies(githubTool)) > 0 + guardPoliciesExplicit := len(getGitHubGuardPolicies(toolConfig)) > 0 if !guardPoliciesExplicit { envVars["GITHUB_MCP_GUARD_MIN_INTEGRITY"] = "${{ steps.determine-automatic-lockdown.outputs.min_integrity }}" envVars["GITHUB_MCP_GUARD_REPOS"] = "${{ steps.determine-automatic-lockdown.outputs.repos }}" diff --git a/pkg/workflow/mcp_github_config.go b/pkg/workflow/mcp_github_config.go index f253c507d32..f11f3649f77 100644 --- a/pkg/workflow/mcp_github_config.go +++ b/pkg/workflow/mcp_github_config.go @@ -82,12 +82,9 @@ func hasGitHubTool(parsedTools *Tools) bool { } // hasGitHubApp checks if a GitHub App is configured in the (merged) GitHub tool configuration -func hasGitHubApp(githubTool any) bool { - if toolConfig, ok := githubTool.(map[string]any); ok { - _, hasGitHubApp := toolConfig["github-app"] - return hasGitHubApp - } - return false +func hasGitHubApp(githubTool map[string]any) bool { + _, hasApp := githubTool["github-app"] + return hasApp } // isGitHubCLIModeEnabled returns true when GitHub prompt/runtime mode is explicitly set @@ -107,13 +104,13 @@ func isGitHubCLIModeEnabled(data *WorkflowData) bool { if toolConfig, ok := githubTool.(map[string]any); ok { if modeSetting, exists := toolConfig["mode"]; exists { if stringValue, ok := modeSetting.(string); ok { - switch modeValue := strings.ToLower(strings.TrimSpace(stringValue)); modeValue { - case "gh-proxy", "cli": + switch GitHubMCPMode(strings.ToLower(strings.TrimSpace(stringValue))) { + case GitHubMCPModeGHProxy, GitHubMCPModeCLI: return true - case "local", "remote": + case GitHubMCPModeLocal, GitHubMCPModeRemote: return false default: - githubConfigLog.Printf("Unrecognized tools.github.mode value: %s, falling back to legacy behavior", modeValue) + githubConfigLog.Printf("Unrecognized tools.github.mode value: %s, falling back to legacy behavior", stringValue) } } } @@ -124,10 +121,10 @@ func isGitHubCLIModeEnabled(data *WorkflowData) bool { // normalizeGitHubType normalizes and validates GitHub MCP transport values. // Supported values are `local` and `remote`. -func normalizeGitHubType(value string) (string, bool) { - normalizedValue := strings.ToLower(strings.TrimSpace(value)) +func normalizeGitHubType(value string) (GitHubMCPMode, bool) { + normalizedValue := GitHubMCPMode(strings.ToLower(strings.TrimSpace(value))) switch normalizedValue { - case "local", "remote": + case GitHubMCPModeLocal, GitHubMCPModeRemote: return normalizedValue, true default: return "", false @@ -136,37 +133,33 @@ func normalizeGitHubType(value string) (string, bool) { // getGitHubType extracts the MCP transport type from GitHub tool configuration // (local or remote). Supports both `type` (preferred) and legacy `mode` values. -func getGitHubType(githubTool any) string { - if toolConfig, ok := githubTool.(map[string]any); ok { - if typeSetting, exists := toolConfig["type"]; exists { - if stringValue, ok := typeSetting.(string); ok { - if normalizedValue, valid := normalizeGitHubType(stringValue); valid { - githubConfigLog.Printf("GitHub MCP type set explicitly: %s", normalizedValue) - return normalizedValue - } - githubConfigLog.Printf("Unrecognized tools.github.type value: %q, falling back to default", stringValue) +func getGitHubType(githubTool map[string]any) GitHubMCPMode { + if typeSetting, exists := githubTool["type"]; exists { + if stringValue, ok := typeSetting.(string); ok { + if normalizedValue, valid := normalizeGitHubType(stringValue); valid { + githubConfigLog.Printf("GitHub MCP type set explicitly: %s", normalizedValue) + return normalizedValue } + githubConfigLog.Printf("Unrecognized tools.github.type value: %q, falling back to default", stringValue) } - if modeSetting, exists := toolConfig["mode"]; exists { - if stringValue, ok := modeSetting.(string); ok { - if normalizedValue, valid := normalizeGitHubType(stringValue); valid { - githubConfigLog.Printf("GitHub MCP type read from legacy mode field: %s", normalizedValue) - return normalizedValue - } + } + if modeSetting, exists := githubTool["mode"]; exists { + if stringValue, ok := modeSetting.(string); ok { + if normalizedValue, valid := normalizeGitHubType(stringValue); valid { + githubConfigLog.Printf("GitHub MCP type read from legacy mode field: %s", normalizedValue) + return normalizedValue } } } githubConfigLog.Print("GitHub MCP mode: local (default)") - return "local" // default to local (Docker) + return GitHubMCPModeLocal // default to local (Docker) } // getGitHubToken extracts the custom github-token from GitHub tool configuration -func getGitHubToken(githubTool any) string { - if toolConfig, ok := githubTool.(map[string]any); ok { - if tokenSetting, exists := toolConfig["github-token"]; exists { - if stringValue, ok := tokenSetting.(string); ok { - return stringValue - } +func getGitHubToken(githubTool map[string]any) string { + if tokenSetting, exists := githubTool["github-token"]; exists { + if stringValue, ok := tokenSetting.(string); ok { + return stringValue } } return "" @@ -174,45 +167,38 @@ func getGitHubToken(githubTool any) string { // getGitHubReadOnly returns true always, since the GitHub MCP server is always read-only. // Setting read-only: false is not supported and will be flagged as a validation error. -func getGitHubReadOnly(_ any) bool { +func getGitHubReadOnly() bool { return true } // getGitHubLockdown checks if lockdown mode is enabled for GitHub tool // Defaults to constants.DefaultGitHubLockdown (false) -func getGitHubLockdown(githubTool any) bool { - if toolConfig, ok := githubTool.(map[string]any); ok { - if lockdownSetting, exists := toolConfig["lockdown"]; exists { - if boolValue, ok := lockdownSetting.(bool); ok { - return boolValue - } +func getGitHubLockdown(githubTool map[string]any) bool { + if lockdownSetting, exists := githubTool["lockdown"]; exists { + if boolValue, ok := lockdownSetting.(bool); ok { + return boolValue } } return constants.DefaultGitHubLockdown } // hasGitHubLockdownExplicitlySet checks if lockdown field is explicitly set in GitHub tool config -func hasGitHubLockdownExplicitlySet(githubTool any) bool { - if toolConfig, ok := githubTool.(map[string]any); ok { - _, exists := toolConfig["lockdown"] - return exists - } - return false +func hasGitHubLockdownExplicitlySet(githubTool map[string]any) bool { + _, exists := githubTool["lockdown"] + return exists } // getGitHubToolsets extracts the toolsets configuration from GitHub tool // Expands "default" to individual toolsets for action-friendly compatibility -func getGitHubToolsets(githubTool any) string { - if toolConfig, ok := githubTool.(map[string]any); ok { - if toolsetsSetting, exists := toolConfig["toolsets"]; exists { - // Handle array format only - if toolsets := parseStringSliceAny(toolsetsSetting, githubConfigLog); toolsets != nil { - toolsetsStr := strings.Join(toolsets, ",") - // Expand "default" to individual toolsets for action-friendly compatibility - resolved := expandDefaultToolset(toolsetsStr) - githubConfigLog.Printf("GitHub MCP toolsets resolved: %s", resolved) - return resolved - } +func getGitHubToolsets(githubTool map[string]any) string { + if toolsetsSetting, exists := githubTool["toolsets"]; exists { + // Handle array format only + if toolsets := parseStringSliceAny(toolsetsSetting, githubConfigLog); toolsets != nil { + toolsetsStr := strings.Join(toolsets, ",") + // Expand "default" to individual toolsets for action-friendly compatibility + resolved := expandDefaultToolset(toolsetsStr) + githubConfigLog.Printf("GitHub MCP toolsets resolved: %s", resolved) + return resolved } } // default to action-friendly toolsets (excludes "users" which GitHub Actions tokens don't support) @@ -262,12 +248,10 @@ func expandDefaultToolset(toolsetsStr string) string { // getGitHubAllowedTools extracts the allowed tools list from GitHub tool configuration // Returns the list of allowed tools, or nil if no allowed list is specified (which means all tools are allowed) -func getGitHubAllowedTools(githubTool any) []string { - if toolConfig, ok := githubTool.(map[string]any); ok { - if allowedSetting, exists := toolConfig["allowed"]; exists { - allowedTools, _ := parseGitHubAllowedToolsAndLimits(allowedSetting) - return allowedTools - } +func getGitHubAllowedTools(githubTool map[string]any) []string { + if allowedSetting, exists := githubTool["allowed"]; exists { + allowedTools, _ := parseGitHubAllowedToolsAndLimits(allowedSetting) + return allowedTools } return nil } @@ -324,45 +308,43 @@ func parseGitHubAllowedToolsAndLimits(allowedSetting any) ([]string, map[string] // the org/repo variable fallback expressions so that a centrally-configured variable extends the // per-workflow list rather than replacing it. // Returns nil if no guard policies are configured. -func getGitHubGuardPolicies(githubTool any) map[string]any { - if toolConfig, ok := githubTool.(map[string]any); ok { - var toolCallLimits map[string]int - if allowedSetting, exists := toolConfig["allowed"]; exists { - _, toolCallLimits = parseGitHubAllowedToolsAndLimits(allowedSetting) - } - hasToolCallLimits := len(toolCallLimits) > 0 +func getGitHubGuardPolicies(githubTool map[string]any) map[string]any { + var toolCallLimits map[string]int + if allowedSetting, exists := githubTool["allowed"]; exists { + _, toolCallLimits = parseGitHubAllowedToolsAndLimits(allowedSetting) + } + hasToolCallLimits := len(toolCallLimits) > 0 - // Support both 'allowed-repos' (preferred) and deprecated 'repos' - repos, hasRepos := toolConfig["allowed-repos"] - if !hasRepos { - repos, hasRepos = toolConfig["repos"] + // Support both 'allowed-repos' (preferred) and deprecated 'repos' + repos, hasRepos := githubTool["allowed-repos"] + if !hasRepos { + repos, hasRepos = githubTool["repos"] + } + integrity, hasIntegrity := githubTool["min-integrity"] + if hasRepos || hasIntegrity || hasToolCallLimits { + policy := map[string]any{} + if hasRepos { + policy["repos"] = normalizeGitHubRepositoryInReposScope(repos) + } else { + // Default repos to "all" when min-integrity is specified without repos. + // The MCP Gateway requires repos in the allow-only policy. + policy["repos"] = "all" } - integrity, hasIntegrity := toolConfig["min-integrity"] - if hasRepos || hasIntegrity || hasToolCallLimits { - policy := map[string]any{} - if hasRepos { - policy["repos"] = normalizeGitHubRepositoryInReposScope(repos) - } else { - // Default repos to "all" when min-integrity is specified without repos. - // The MCP Gateway requires repos in the allow-only policy. - policy["repos"] = "all" - } - if hasIntegrity { - policy["min-integrity"] = integrity - } - if hasToolCallLimits { - policy["tool-call-limits"] = toolCallLimits - } - // blocked-users, trusted-users, and approval-labels are parsed at runtime by the - // parse-guard-vars step. The step outputs proper JSON arrays (split on comma/newline, - // validated, jq-encoded) from both the compile-time static values and the - // GH_AW_GITHUB_* org/repo variables. - policy["blocked-users"] = guardExprSentinel + "${{ steps.parse-guard-vars.outputs.blocked_users }}" - policy["trusted-users"] = guardExprSentinel + "${{ steps.parse-guard-vars.outputs.trusted_users }}" - policy["approval-labels"] = guardExprSentinel + "${{ steps.parse-guard-vars.outputs.approval_labels }}" - return map[string]any{ - "allow-only": policy, - } + if hasIntegrity { + policy["min-integrity"] = integrity + } + if hasToolCallLimits { + policy["tool-call-limits"] = toolCallLimits + } + // blocked-users, trusted-users, and approval-labels are parsed at runtime by the + // parse-guard-vars step. The step outputs proper JSON arrays (split on comma/newline, + // validated, jq-encoded) from both the compile-time static values and the + // GH_AW_GITHUB_* org/repo variables. + policy["blocked-users"] = guardExprSentinel + "${{ steps.parse-guard-vars.outputs.blocked_users }}" + policy["trusted-users"] = guardExprSentinel + "${{ steps.parse-guard-vars.outputs.trusted_users }}" + policy["approval-labels"] = guardExprSentinel + "${{ steps.parse-guard-vars.outputs.approval_labels }}" + return map[string]any{ + "allow-only": policy, } } return nil @@ -463,7 +445,7 @@ func mcpgSupportsIntegrityReactions(gatewayConfig *MCPGatewayRuntimeConfig) bool // // This allows the gateway to read data from the GitHub MCP server and still write to safeoutputs. // Returns nil if no GitHub guard policies are configured. -func deriveSafeOutputsGuardPolicyFromGitHub(githubTool any) map[string]any { +func deriveSafeOutputsGuardPolicyFromGitHub(githubTool map[string]any) map[string]any { githubPolicies := getGitHubGuardPolicies(githubTool) if githubPolicies == nil { return nil @@ -556,13 +538,15 @@ func deriveWriteSinkGuardPolicyFromWorkflow(workflowData *WorkflowData) map[stri if workflowData == nil || workflowData.Tools == nil { return nil } - githubTool, hasGitHub := workflowData.Tools["github"] + rawGithubTool, hasGitHub := workflowData.Tools["github"] if !hasGitHub { return nil } + toolConfig, _ := rawGithubTool.(map[string]any) + // Try to derive from explicit guard policy first - policy := deriveSafeOutputsGuardPolicyFromGitHub(githubTool) + policy := deriveSafeOutputsGuardPolicyFromGitHub(toolConfig) if policy != nil { return policy } @@ -570,7 +554,7 @@ func deriveWriteSinkGuardPolicyFromWorkflow(workflowData *WorkflowData) map[stri // When no explicit guard policy is configured but automatic lockdown detection would run // (GitHub tool present and not disabled, no GitHub App configured), return accept=["*"] // because automatic lockdown always sets repos=all at runtime. - if githubTool != false && len(getGitHubGuardPolicies(githubTool)) == 0 && !hasGitHubApp(githubTool) { + if rawGithubTool != false && len(getGitHubGuardPolicies(toolConfig)) == 0 && !hasGitHubApp(toolConfig) { return map[string]any{ "write-sink": map[string]any{ "accept": []string{"*"}, @@ -581,25 +565,23 @@ func deriveWriteSinkGuardPolicyFromWorkflow(workflowData *WorkflowData) map[stri return nil } -func getGitHubDockerImageVersion(githubTool any) string { +func getGitHubDockerImageVersion(githubTool map[string]any) string { githubDockerImageVersion := string(constants.DefaultGitHubMCPServerVersion) // Default Docker image version // Extract version setting from tool properties - if toolConfig, ok := githubTool.(map[string]any); ok { - if versionSetting, exists := toolConfig["version"]; exists { - // Handle different version types - switch v := versionSetting.(type) { - case string: - githubDockerImageVersion = v - case int: - githubDockerImageVersion = strconv.Itoa(v) - case int64: - githubDockerImageVersion = strconv.FormatInt(v, 10) - case uint64: - githubDockerImageVersion = strconv.FormatUint(v, 10) - case float64: - // Use %g to avoid trailing zeros and scientific notation for simple numbers - githubDockerImageVersion = fmt.Sprintf("%g", v) - } + if versionSetting, exists := githubTool["version"]; exists { + // Handle different version types + switch v := versionSetting.(type) { + case string: + githubDockerImageVersion = v + case int: + githubDockerImageVersion = strconv.Itoa(v) + case int64: + githubDockerImageVersion = strconv.FormatInt(v, 10) + case uint64: + githubDockerImageVersion = strconv.FormatUint(v, 10) + case float64: + // Use %g to avoid trailing zeros and scientific notation for simple numbers + githubDockerImageVersion = fmt.Sprintf("%g", v) } } githubConfigLog.Printf("GitHub MCP Docker image version: %s", githubDockerImageVersion) diff --git a/pkg/workflow/mcp_renderer.go b/pkg/workflow/mcp_renderer.go index c0ee5d871ba..7fa438e1b66 100644 --- a/pkg/workflow/mcp_renderer.go +++ b/pkg/workflow/mcp_renderer.go @@ -148,7 +148,7 @@ func RenderJSONMCPConfig( switch toolName { case "github": - githubTool := tools["github"] + githubTool, _ := tools["github"].(map[string]any) options.Renderers.RenderGitHub(&configBuilder, githubTool, isLast, workflowData) case "playwright": playwrightTool := tools["playwright"] diff --git a/pkg/workflow/mcp_renderer_factory.go b/pkg/workflow/mcp_renderer_factory.go index 9e90051f712..1e9059aa9c9 100644 --- a/pkg/workflow/mcp_renderer_factory.go +++ b/pkg/workflow/mcp_renderer_factory.go @@ -175,7 +175,7 @@ func buildStandardJSONMCPRenderers( ) MCPToolRenderers { mcpRenderingLog.Printf("Building standard JSON MCP renderers") return MCPToolRenderers{ - RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) { + RenderGitHub: func(yaml *strings.Builder, githubTool map[string]any, isLast bool, workflowData *WorkflowData) { createRenderer(isLast).RenderGitHubMCP(yaml, githubTool, workflowData) }, RenderPlaywright: func(yaml *strings.Builder, playwrightTool any, isLast bool) { diff --git a/pkg/workflow/mcp_renderer_github.go b/pkg/workflow/mcp_renderer_github.go index d99a81734eb..e017e40ee65 100644 --- a/pkg/workflow/mcp_renderer_github.go +++ b/pkg/workflow/mcp_renderer_github.go @@ -14,9 +14,9 @@ import ( // RenderGitHubMCP generates the GitHub MCP server configuration // Supports both local (Docker) and remote (hosted) modes -func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, githubTool any, workflowData *WorkflowData) { +func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, githubTool map[string]any, workflowData *WorkflowData) { githubType := getGitHubType(githubTool) - readOnly := getGitHubReadOnly(githubTool) + readOnly := getGitHubReadOnly() // Get explicit lockdown value (only used when lockdown is explicitly configured) lockdown := getGitHubLockdown(githubTool) @@ -30,13 +30,11 @@ func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, github // the GitHub MCP server protocol does not expose that information. Warn if the // user configured reactions with the gateway path. if isFeatureEnabled(constants.IntegrityReactionsFeatureFlag, workflowData) { - if toolConfig, ok := githubTool.(map[string]any); ok { - if hasReactionFieldsInToolConfig(toolConfig) { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage( - "integrity-reactions: endorsement/disapproval reactions are ignored in MCP gateway mode because "+ - "reaction authors cannot be identified from the GitHub MCP server. Reactions are only enforced "+ - "in proxy mode (DIFC proxy / CLI proxy).")) - } + if hasReactionFieldsInToolConfig(githubTool) { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage( + "integrity-reactions: endorsement/disapproval reactions are ignored in MCP gateway mode because "+ + "reaction authors cannot be identified from the GitHub MCP server. Reactions are only enforced "+ + "in proxy mode (DIFC proxy / CLI proxy).")) } } shouldUseStepOutputForGuardPolicy := len(explicitGuardPolicies) == 0 && !hasGitHubApp(githubTool) @@ -55,7 +53,7 @@ func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, github yaml.WriteString(" \"github\": {\n") // Check if remote mode is enabled (type: remote) - if githubType == "remote" { + if githubType == GitHubMCPModeRemote { mcpRendererLog.Printf("GitHub MCP remote mode selected: copilot_fields=%t", r.options.IncludeCopilotFields) // Determine authorization value based on engine requirements // Copilot uses MCP passthrough syntax: "Bearer \${GITHUB_PERSONAL_ACCESS_TOKEN}" @@ -107,9 +105,9 @@ func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, github } // renderGitHubTOML generates GitHub MCP configuration in TOML format (for Codex engine) -func (r *MCPConfigRendererUnified) renderGitHubTOML(yaml *strings.Builder, githubTool any, workflowData *WorkflowData) { +func (r *MCPConfigRendererUnified) renderGitHubTOML(yaml *strings.Builder, githubTool map[string]any, workflowData *WorkflowData) { githubType := getGitHubType(githubTool) - readOnly := getGitHubReadOnly(githubTool) + readOnly := getGitHubReadOnly() lockdown := getGitHubLockdown(githubTool) toolsets := getGitHubToolsets(githubTool) @@ -152,7 +150,7 @@ func (r *MCPConfigRendererUnified) renderGitHubTOML(yaml *strings.Builder, githu fmt.Fprintf(yaml, " tool_timeout_sec = %d\n", toolTimeout) // Check if remote mode is enabled - if githubType == "remote" { + if githubType == GitHubMCPModeRemote { mcpRendererLog.Printf("GitHub MCP TOML remote mode: readonly_endpoint=%t", readOnly) // Remote mode - use hosted GitHub MCP server with streamable HTTP // Use readonly endpoint if read-only mode is enabled diff --git a/pkg/workflow/mcp_renderer_types.go b/pkg/workflow/mcp_renderer_types.go index 906a6046eb8..4476a9735bd 100644 --- a/pkg/workflow/mcp_renderer_types.go +++ b/pkg/workflow/mcp_renderer_types.go @@ -32,7 +32,7 @@ type RenderCustomMCPToolConfigHandler func(yaml *strings.Builder, toolName strin // MCPToolRenderers holds engine-specific rendering functions for each MCP tool type type MCPToolRenderers struct { - RenderGitHub func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) + RenderGitHub func(yaml *strings.Builder, githubTool map[string]any, isLast bool, workflowData *WorkflowData) RenderPlaywright func(yaml *strings.Builder, playwrightTool any, isLast bool) RenderCacheMemory func(yaml *strings.Builder, isLast bool, workflowData *WorkflowData) RenderAgenticWorkflows func(yaml *strings.Builder, isLast bool) diff --git a/pkg/workflow/mcp_setup_generator.go b/pkg/workflow/mcp_setup_generator.go index d1ea29f23ed..a8260073140 100644 --- a/pkg/workflow/mcp_setup_generator.go +++ b/pkg/workflow/mcp_setup_generator.go @@ -574,7 +574,8 @@ func generateMCPGatewaySetup(yaml *strings.Builder, tools map[string]any, mcpToo ensureDefaultMCPGatewayConfig(workflowData) gatewayConfig := workflowData.SandboxConfig.MCP port, domain, payloadDir, payloadPathPrefix, payloadSizeThreshold := resolveMCPGatewayValues(workflowData, gatewayConfig) - githubTool, hasGitHub := tools["github"] + githubToolRaw, hasGitHub := tools["github"] + githubTool, _ := githubToolRaw.(map[string]any) writeMCPGatewayExports(yaml, writeMCPGatewayExportsOptions{ engine: engine, workflowData: workflowData, @@ -657,7 +658,7 @@ type writeMCPGatewayExportsOptions struct { workflowData *WorkflowData gatewayConfig *MCPGatewayRuntimeConfig hasGitHub bool - githubTool any + githubTool map[string]any port int domain string payloadDir string @@ -712,7 +713,7 @@ func writeMCPGatewayExports(yaml *strings.Builder, opts writeMCPGatewayExportsOp yaml.WriteString(" export GH_AW_MCP_CLI_SERVERS=" + escapedCLIServersJSON + "\n") } } - if hasGitHub && getGitHubType(githubTool) == "remote" && engine.GetID() == "copilot" { + if hasGitHub && getGitHubType(githubTool) == GitHubMCPModeRemote && engine.GetID() == "copilot" { yaml.WriteString(" export GITHUB_PERSONAL_ACCESS_TOKEN=\"$GITHUB_MCP_SERVER_TOKEN\"\n") } if len(gatewayConfig.Env) > 0 { @@ -733,7 +734,7 @@ type buildMCPGatewayContainerCommandOptions struct { payloadDir string payloadPathPrefix string hasGitHub bool - githubTool any + githubTool map[string]any tools map[string]any } @@ -835,8 +836,8 @@ func appendMCPGatewayBaseEnvFlags(containerCmd *strings.Builder, payloadPathPref containerCmd.WriteString(" -e GITHUB_BASE_REF") } -func appendMCPGatewayConditionalEnvFlags(containerCmd *strings.Builder, workflowData *WorkflowData, engine CodingAgentEngine, hasGitHub bool, githubTool any, tools map[string]any) { - if hasGitHub && getGitHubType(githubTool) == "remote" && engine.GetID() == "copilot" { +func appendMCPGatewayConditionalEnvFlags(containerCmd *strings.Builder, workflowData *WorkflowData, engine CodingAgentEngine, hasGitHub bool, githubTool map[string]any, tools map[string]any) { + if hasGitHub && getGitHubType(githubTool) == GitHubMCPModeRemote && engine.GetID() == "copilot" { containerCmd.WriteString(" -e GITHUB_PERSONAL_ACCESS_TOKEN") } if IsMCPScriptsEnabled(workflowData.MCPScripts) { @@ -861,7 +862,7 @@ func appendMCPGatewayConditionalEnvFlags(containerCmd *strings.Builder, workflow } } -func appendMCPGatewayCustomAndHTTPEnvFlags(containerCmd *strings.Builder, workflowData *WorkflowData, gatewayConfig *MCPGatewayRuntimeConfig, mcpEnvVars map[string]string, hasGitHub bool, githubTool any, tools map[string]any, engine CodingAgentEngine) { +func appendMCPGatewayCustomAndHTTPEnvFlags(containerCmd *strings.Builder, workflowData *WorkflowData, gatewayConfig *MCPGatewayRuntimeConfig, mcpEnvVars map[string]string, hasGitHub bool, githubTool map[string]any, tools map[string]any, engine CodingAgentEngine) { if len(gatewayConfig.Env) > 0 { envVarNames := sliceutil.MapKeys(gatewayConfig.Env) sort.Strings(envVarNames) @@ -888,7 +889,7 @@ func appendMCPGatewayCustomAndHTTPEnvFlags(containerCmd *strings.Builder, workfl } } -func buildAddedGatewayEnvVarSet(workflowData *WorkflowData, gatewayConfig *MCPGatewayRuntimeConfig, hasGitHub bool, githubTool any, tools map[string]any, engine CodingAgentEngine) map[string]bool { +func buildAddedGatewayEnvVarSet(workflowData *WorkflowData, gatewayConfig *MCPGatewayRuntimeConfig, hasGitHub bool, githubTool map[string]any, tools map[string]any, engine CodingAgentEngine) map[string]bool { addedEnvVars := make(map[string]bool) standardEnvVars := []string{ "MCP_GATEWAY_PORT", "MCP_GATEWAY_DOMAIN", "MCP_GATEWAY_API_KEY", "MCP_GATEWAY_PAYLOAD_DIR", "DEBUG", @@ -906,7 +907,7 @@ func buildAddedGatewayEnvVarSet(workflowData *WorkflowData, gatewayConfig *MCPGa for _, envVar := range standardEnvVars { addedEnvVars[envVar] = true } - if hasGitHub && getGitHubType(githubTool) == "remote" && engine.GetID() == "copilot" { + if hasGitHub && getGitHubType(githubTool) == GitHubMCPModeRemote && engine.GetID() == "copilot" { addedEnvVars["GITHUB_PERSONAL_ACCESS_TOKEN"] = true } if IsMCPScriptsEnabled(workflowData.MCPScripts) { diff --git a/pkg/workflow/safeoutputs_guard_policy_test.go b/pkg/workflow/safeoutputs_guard_policy_test.go index 64f79f576c2..d2f518845c5 100644 --- a/pkg/workflow/safeoutputs_guard_policy_test.go +++ b/pkg/workflow/safeoutputs_guard_policy_test.go @@ -13,7 +13,7 @@ import ( func TestDeriveSafeOutputsGuardPolicyFromGitHub(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any expectedPolicies map[string]any expectNil bool description string @@ -234,10 +234,10 @@ func TestDeriveSafeOutputsGuardPolicyFromGitHub(t *testing.T) { description: "nil input should return nil", }, { - name: "non-map github tool", - githubTool: "invalid", + name: "empty github tool map", + githubTool: map[string]any{}, expectNil: true, - description: "non-map input should return nil", + description: "empty map input should return nil", }, } diff --git a/pkg/workflow/tools_parser.go b/pkg/workflow/tools_parser.go index 2c52cc91ebd..a786418a4e3 100644 --- a/pkg/workflow/tools_parser.go +++ b/pkg/workflow/tools_parser.go @@ -217,7 +217,7 @@ func parseGitHubTool(val any) *GitHubToolConfig { } if mode, ok := configMap["mode"].(string); ok { - config.Mode = mode + config.Mode = GitHubMCPMode(mode) } if mcpType, ok := configMap["type"].(string); ok { config.Type = mcpType diff --git a/pkg/workflow/tools_types.go b/pkg/workflow/tools_types.go index d639079d195..e4bf58fa86c 100644 --- a/pkg/workflow/tools_types.go +++ b/pkg/workflow/tools_types.go @@ -269,6 +269,20 @@ func (g GitHubToolsets) ToStringSlice() []string { return result } +// GitHubMCPMode represents the MCP transport/deployment mode for the GitHub tool. +type GitHubMCPMode string + +const ( + // GitHubMCPModeLocal runs the GitHub MCP server as a Docker container on the runner. + GitHubMCPModeLocal GitHubMCPMode = "local" + // GitHubMCPModeRemote connects to the hosted GitHub MCP service. + GitHubMCPModeRemote GitHubMCPMode = "remote" + // GitHubMCPModeGHProxy routes GitHub operations through the gh CLI proxy. + GitHubMCPModeGHProxy GitHubMCPMode = "gh-proxy" + // GitHubMCPModeCLI is a legacy alias for GitHubMCPModeGHProxy. + GitHubMCPModeCLI GitHubMCPMode = "cli" +) + // GitHubIntegrityLevel represents the minimum integrity level required for repository access type GitHubIntegrityLevel string @@ -291,7 +305,7 @@ type GitHubReposScope any // string or []any (YAML-parsed arrays are []any) // Can be nil (enabled with defaults), string, or an object with specific settings type GitHubToolConfig struct { Allowed GitHubAllowedTools `yaml:"allowed,omitempty"` - Mode string `yaml:"mode,omitempty"` + Mode GitHubMCPMode `yaml:"mode,omitempty"` Type string `yaml:"type,omitempty"` Version string `yaml:"version,omitempty"` Args []string `yaml:"args,omitempty"` diff --git a/pkg/workflow/tools_validation_test.go b/pkg/workflow/tools_validation_test.go index c384845f402..9b95f21823f 100644 --- a/pkg/workflow/tools_validation_test.go +++ b/pkg/workflow/tools_validation_test.go @@ -974,7 +974,7 @@ func TestGetDIFCProxyPolicyJSONWithReactions(t *testing.T) { tests := []struct { name string - githubTool any + githubTool map[string]any data *WorkflowData gatewayConfig *MCPGatewayRuntimeConfig expectedContains []string From 2607b3fc67a451fd5d77f166e4f8c3fdea72eaf2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 12 Jun 2026 23:02:09 +0000 Subject: [PATCH 3/4] docs(adr): add draft ADR-38945 for GitHubMCPMode enum and tool config narrowing --- ...ub-mcp-mode-enum-and-narrow-tool-config.md | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 docs/adr/38945-type-github-mcp-mode-enum-and-narrow-tool-config.md diff --git a/docs/adr/38945-type-github-mcp-mode-enum-and-narrow-tool-config.md b/docs/adr/38945-type-github-mcp-mode-enum-and-narrow-tool-config.md new file mode 100644 index 00000000000..423b4ab9f43 --- /dev/null +++ b/docs/adr/38945-type-github-mcp-mode-enum-and-narrow-tool-config.md @@ -0,0 +1,40 @@ +# ADR-38945: Type GitHub MCP Mode as an Enum and Narrow Tool Config to `map[string]any` + +**Date**: 2026-06-12 +**Status**: Draft + +## Context + +The GitHub MCP configuration code in `pkg/workflow/mcp_github_config.go` had grown around two weakly-typed conventions. First, roughly 19 helper functions accepted the GitHub tool config as `githubTool any` and each immediately re-asserted it to `map[string]any` internally, scattering the same `.(map[string]any)` type assertion (and its failure handling) across nearly every function and its callers. Second, the MCP "mode" of the GitHub server (`local`, `remote`, `gh-proxy`, `cli`) was modelled as a bare `string`, so the compiler relied on string literals like `== "remote"` and `!= "gh-proxy"` with no compile-time protection against typos or invalid values. Both patterns made the code harder to read and gave the type system no leverage to catch mistakes in a security-sensitive area (proxy policy, lockdown, token selection). + +## Decision + +We will introduce a named `GitHubMCPMode` string type in `tools_types.go` with constants `GitHubMCPModeLocal`, `GitHubMCPModeRemote`, `GitHubMCPModeGHProxy`, and `GitHubMCPModeCLI`, change `GitHubToolConfig.Mode` from `string` to `GitHubMCPMode`, and cast the parsed value once at the single ingestion point in `tools_parser.go`. We will also narrow every `githubTool any` parameter across `mcp_github_config.go` and its callers to `map[string]any`, performing the single `.(map[string]any)` assertion once at each call site (when reading `tools["github"]`) rather than inside every helper. `normalizeGitHubType`/`getGitHubType` return `GitHubMCPMode`, and all bare string comparisons are replaced with the typed constants. We chose this because pushing the type assertion to the boundary and naming the mode values makes the security-relevant configuration paths self-documenting and compiler-checked, with no behavioral change (nil maps remain safe since Go returns zero values on reads). + +## Alternatives Considered + +### Alternative 1: Keep `githubTool any` and the bare-string mode +Leave the existing signatures unchanged and continue asserting to `map[string]any` inside each helper. Rejected because it perpetuates ~19 duplicated assertions and offers no compile-time defense for mode values; the duplication and stringly-typed comparisons are exactly the maintenance and correctness hazard this PR exists to remove. + +### Alternative 2: Model the tool config as a concrete struct end-to-end +Replace the `map[string]any` representation entirely with a fully-typed `GitHubToolConfig` struct threaded through all helpers. Rejected (for now) as too large a change: the tool config is parsed from open-ended frontmatter and shared with generic MCP rendering paths that operate on `map[string]any`, so a full struct migration would ripple far beyond this PR. Narrowing to `map[string]any` plus a typed mode enum captures most of the type-safety benefit at a fraction of the blast radius. + +## Consequences + +### Positive +- The single `.(map[string]any)` assertion moves to each call site, eliminating ~19 duplicated assertions and their inconsistent failure handling inside helpers. +- `GitHubMCPMode` constants give the compiler authority over mode values, preventing typo'd or invalid mode strings in security-sensitive logic (proxy, lockdown, token selection). +- Helper signatures (`map[string]any`, `GitHubMCPMode`) now document intent directly, improving readability for future contributors. + +### Negative +- Wide, mechanical change surface: ~30 non-test files plus 11 test files touched, increasing review effort and rebase/merge-conflict risk against concurrent work in `pkg/workflow`. +- Test inputs that previously exercised non-map values (e.g. `"invalid"`, `false`, `"not a map"`) are replaced with `nil`/empty maps, so the "wrong type passed in" branch is no longer covered at the helper level — that contract now lives at the call-site assertion. +- The type assertion at each call site silently yields a `nil` map on mismatch (`, _`), so a malformed `tools["github"]` value degrades to zero-value behavior rather than an explicit error. + +### Neutral +- `getGitHubReadOnly()` loses its unused parameter and now takes no arguments, since it unconditionally returns `true`. +- The change is intended to be behavior-preserving; the GitHub MCP server remains unconditionally read-only and nil-map reads continue to return zero values. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/27446718826) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* From 2ecbc860b32e671640270b6b7a367b35c6230be2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 13 Jun 2026 03:59:43 +0000 Subject: [PATCH 4/4] fix: guard docker image pull against disabled GitHub tool and accept cli as pi engine mode Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/agent_validation.go | 3 ++- pkg/workflow/docker.go | 21 ++++++++++++--------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/pkg/workflow/agent_validation.go b/pkg/workflow/agent_validation.go index 32f0f725052..2991d0ca9cf 100644 --- a/pkg/workflow/agent_validation.go +++ b/pkg/workflow/agent_validation.go @@ -199,7 +199,8 @@ func (c *Compiler) validatePiEngineRequirements(tools *ToolsConfig, engine Codin return nil } - if tools == nil || tools.GitHub == nil || tools.GitHub.Mode != GitHubMCPModeGHProxy { + if tools == nil || tools.GitHub == nil || + (tools.GitHub.Mode != GitHubMCPModeGHProxy && tools.GitHub.Mode != GitHubMCPModeCLI) { return errors.New("engine 'pi' requires tools.github.mode: gh-proxy") } diff --git a/pkg/workflow/docker.go b/pkg/workflow/docker.go index e5dfca26519..b262745a119 100644 --- a/pkg/workflow/docker.go +++ b/pkg/workflow/docker.go @@ -21,15 +21,18 @@ func collectDockerImages(tools map[string]any, workflowData *WorkflowData, actio // Check for GitHub tool (uses Docker image) if rawGithubTool, hasGitHub := tools["github"]; hasGitHub { - githubTool, _ := rawGithubTool.(map[string]any) - githubType := getGitHubType(githubTool) - // Only add if using local (Docker) mode - if githubType == GitHubMCPModeLocal { - githubDockerImageVersion := getGitHubDockerImageVersion(githubTool) - image := "ghcr.io/github/github-mcp-server:" + githubDockerImageVersion - if !imageSet[image] { - images = append(images, image) - imageSet[image] = true + // Only proceed when the value is an actual config map; a boolean false + // means the tool is explicitly disabled. + if githubTool, ok := rawGithubTool.(map[string]any); ok { + githubType := getGitHubType(githubTool) + // Only add if using local (Docker) mode + if githubType == GitHubMCPModeLocal { + githubDockerImageVersion := getGitHubDockerImageVersion(githubTool) + image := "ghcr.io/github/github-mcp-server:" + githubDockerImageVersion + if !imageSet[image] { + images = append(images, image) + imageSet[image] = true + } } } }