From 7cdb14aa37ba4f4506b12cbe87eb5d8f6c789d51 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Apr 2026 21:24:34 +0000 Subject: [PATCH 1/2] Initial plan From 5657674a496a91057cebf72fd933554ae0c2235c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Apr 2026 22:01:49 +0000 Subject: [PATCH 2/2] fix: move GitHub MCP App token minting from activation job to agent job The GitHub Actions runner silently drops masked values when they are used as job outputs (runner v2.308+). Since actions/create-github-app-token unconditionally calls ::add-mask:: on the produced token, the github_mcp_app_token activation output was always empty, causing the GitHub MCP server to run unauthenticated. Fix by minting the GitHub MCP App token directly in the agent job (and any other consuming job), referencing it via steps.github-mcp-app-token.outputs.token instead of needs.activation.outputs.github_mcp_app_token. Checkout app tokens continue to be minted in the activation job since they are passed as job outputs differently and don't use create-github-app-token. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6bb5fdbe-f8ba-428f-945c-4a72f38c9a8b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_activation_job.go | 8 -- pkg/workflow/compiler_github_mcp_steps.go | 14 +-- pkg/workflow/compiler_main_job.go | 13 +-- pkg/workflow/compiler_yaml_main_job.go | 10 ++ pkg/workflow/copilot_engine_execution.go | 9 +- pkg/workflow/github_mcp_app_token_test.go | 125 ++++++++++++++-------- pkg/workflow/mcp_environment.go | 11 +- 7 files changed, 119 insertions(+), 71 deletions(-) diff --git a/pkg/workflow/compiler_activation_job.go b/pkg/workflow/compiler_activation_job.go index c64eb953aeb..4eea003dccb 100644 --- a/pkg/workflow/compiler_activation_job.go +++ b/pkg/workflow/compiler_activation_job.go @@ -121,14 +121,6 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate outputs["activation_app_token_minting_failed"] = "${{ steps.activation-app-token.outcome == 'failure' }}" } - // Mint the GitHub MCP app token in the activation job so that the agent job never - // receives the app-id / private-key secrets. The minted token is exposed as a job - // output and consumed by the agent job via needs.activation.outputs.github_mcp_app_token. - if data.ParsedTools != nil && data.ParsedTools.GitHub != nil && data.ParsedTools.GitHub.GitHubApp != nil { - steps = append(steps, c.generateGitHubMCPAppTokenMintingSteps(data)...) - outputs["github_mcp_app_token"] = "${{ steps.github-mcp-app-token.outputs.token }}" - } - // Mint checkout app tokens in the activation job so that the agent job never // receives the app-id / private-key secrets. Each token is exposed as a job output // and consumed by the agent job via needs.activation.outputs.checkout_app_token_{index}. diff --git a/pkg/workflow/compiler_github_mcp_steps.go b/pkg/workflow/compiler_github_mcp_steps.go index 501d0cf37ce..cdf6a9c3835 100644 --- a/pkg/workflow/compiler_github_mcp_steps.go +++ b/pkg/workflow/compiler_github_mcp_steps.go @@ -86,9 +86,11 @@ func (c *Compiler) generateGitHubMCPLockdownDetectionStep(yaml *strings.Builder, // permissions derived from the agent job's declared permissions plus any extra permissions // configured under tools.github.github-app.permissions. // -// The returned steps are intended to be added to the activation job so that the -// app-id / private-key secrets never reach the agent job. The minted token is then -// consumed in the agent job via needs.activation.outputs.github_mcp_app_token. +// The returned steps are added directly to the agent job so that the minted token is +// available as steps.github-mcp-app-token.outputs.token within that job. +// Minting happens inside the agent job (not the activation job) because +// actions/create-github-app-token calls ::add-mask:: on the produced token, and the +// GitHub Actions runner silently drops masked values when used as job outputs (runner v2.308+). func (c *Compiler) generateGitHubMCPAppTokenMintingSteps(data *WorkflowData) []string { // Check if GitHub tool has app configuration if data.ParsedTools == nil || data.ParsedTools.GitHub == nil || data.ParsedTools.GitHub.GitHubApp == nil { @@ -142,7 +144,7 @@ func (c *Compiler) generateGitHubMCPAppTokenMintingSteps(data *WorkflowData) []s // generateGitHubMCPAppTokenInvalidationStep generates a step to invalidate the GitHub App token for GitHub MCP server // This step always runs (even on failure) to ensure tokens are properly cleaned up. -// The token was minted in the activation job and is referenced via needs.activation.outputs.github_mcp_app_token. +// The token was minted in the agent job and is referenced via steps.github-mcp-app-token.outputs.token. func (c *Compiler) generateGitHubMCPAppTokenInvalidationStep(yaml *strings.Builder, data *WorkflowData) { // Check if GitHub tool has app configuration if data.ParsedTools == nil || data.ParsedTools.GitHub == nil || data.ParsedTools.GitHub.GitHubApp == nil { @@ -151,8 +153,8 @@ func (c *Compiler) generateGitHubMCPAppTokenInvalidationStep(yaml *strings.Build githubConfigLog.Print("Generating GitHub App token invalidation step for GitHub MCP server") - // The token was minted in the activation job; reference it via needs.activation.outputs. - const tokenExpr = "needs.activation.outputs.github_mcp_app_token" + // The token was minted in the agent job; reference it via steps output. + const tokenExpr = "steps.github-mcp-app-token.outputs.token" yaml.WriteString(" - name: Invalidate GitHub App token\n") fmt.Fprintf(yaml, " if: always() && %s != ''\n", tokenExpr) diff --git a/pkg/workflow/compiler_main_job.go b/pkg/workflow/compiler_main_job.go index f1348d73b67..58e065d2e8f 100644 --- a/pkg/workflow/compiler_main_job.go +++ b/pkg/workflow/compiler_main_job.go @@ -68,13 +68,14 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( return nil, fmt.Errorf("failed to generate main job steps: %w", err) } - // Compiler invariant: the agent job must not mint GitHub App tokens. - // All token minting (create-github-app-token) must happen in the activation job so that - // app-id / private-key secrets never reach the agent's environment. Fail fast during - // compilation if this invariant is violated to catch regressions early. + // Compiler invariant: the agent job must not mint checkout-related GitHub App tokens. + // Checkout token minting (checkout-app-token-*) must happen in the activation job. + // Note: the GitHub MCP App token (github-mcp-app-token) IS minted in the agent job — + // this is intentional because masked values are silently dropped by the runner when passed + // as job outputs (runner v2.308+), so the token must be minted within the job that uses it. stepsContent := stepBuilder.String() - if strings.Contains(stepsContent, "create-github-app-token") { - return nil, errors.New("compiler invariant violated: agent job contains a GitHub App token minting step (create-github-app-token); token minting must only occur in the activation job") + if strings.Contains(stepsContent, "id: checkout-app-token-") { + return nil, errors.New("compiler invariant violated: agent job contains a checkout GitHub App token minting step (checkout-app-token-*); checkout token minting must only occur in the activation job") } // Split the steps content into individual step entries diff --git a/pkg/workflow/compiler_yaml_main_job.go b/pkg/workflow/compiler_yaml_main_job.go index 62f3d025243..6485b6f1277 100644 --- a/pkg/workflow/compiler_yaml_main_job.go +++ b/pkg/workflow/compiler_yaml_main_job.go @@ -260,6 +260,16 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat // GH_AW_SAFE_OUTPUTS is now set at job level, no setup step needed + // Mint the GitHub MCP App token directly in the agent job. + // The token cannot be passed via job outputs from the activation job because + // actions/create-github-app-token calls ::add-mask:: on the token, and the + // GitHub Actions runner silently drops masked values in job outputs (runner v2.308+). + // By minting the token here, the app-id / private-key secrets are accessed only + // within this job and the minted token is available as steps.github-mcp-app-token.outputs.token. + for _, step := range c.generateGitHubMCPAppTokenMintingSteps(data) { + yaml.WriteString(step) + } + // Add GitHub MCP lockdown detection step if needed c.generateGitHubMCPLockdownDetectionStep(yaml, data) diff --git a/pkg/workflow/copilot_engine_execution.go b/pkg/workflow/copilot_engine_execution.go index 1718d929049..ca294d3add5 100644 --- a/pkg/workflow/copilot_engine_execution.go +++ b/pkg/workflow/copilot_engine_execution.go @@ -294,11 +294,12 @@ COPILOT_CLI_INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)" } if hasGitHubTool(workflowData.ParsedTools) { - // If GitHub App is configured, use the app token minted in the activation job. - // The token is passed via needs.activation.outputs to keep app-id/private-key - // secrets out of the agent job. + // 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 + // actions/create-github-app-token calls ::add-mask:: on the token, and the + // GitHub Actions runner silently drops masked values in job outputs (runner v2.308+). if workflowData.ParsedTools != nil && workflowData.ParsedTools.GitHub != nil && workflowData.ParsedTools.GitHub.GitHubApp != nil { - env["GITHUB_MCP_SERVER_TOKEN"] = "${{ needs.activation.outputs.github_mcp_app_token }}" + env["GITHUB_MCP_SERVER_TOKEN"] = "${{ steps.github-mcp-app-token.outputs.token }}" } else { customGitHubToken := getGitHubToken(workflowData.Tools["github"]) // Use effective token with precedence: custom > default diff --git a/pkg/workflow/github_mcp_app_token_test.go b/pkg/workflow/github_mcp_app_token_test.go index fdde4887785..d7af819182c 100644 --- a/pkg/workflow/github_mcp_app_token_test.go +++ b/pkg/workflow/github_mcp_app_token_test.go @@ -96,7 +96,7 @@ Test workflow with GitHub MCP app token minting. require.NoError(t, err, "Failed to read lock file") lockContent := string(content) - // Verify token minting step is present in the activation job + // Verify token minting step is present in the agent job assert.Contains(t, lockContent, "Generate GitHub App token", "Token minting step should be present") assert.Contains(t, lockContent, "actions/create-github-app-token", "Should use create-github-app-token action") assert.Contains(t, lockContent, "id: github-mcp-app-token", "Should use github-mcp-app-token as step ID") @@ -107,16 +107,17 @@ Test workflow with GitHub MCP app token minting. assert.Contains(t, lockContent, "permission-contents: read", "Should include contents read permission") assert.Contains(t, lockContent, "permission-issues: read", "Should include issues read permission") - // Verify token is exposed as an activation job output - assert.Contains(t, lockContent, "github_mcp_app_token: ${{ steps.github-mcp-app-token.outputs.token }}", "Activation job should expose github_mcp_app_token output") + // Verify the activation job does NOT expose github_mcp_app_token as a job output + // (masked values are silently dropped by the runner when used as job outputs) + assert.NotContains(t, lockContent, "github_mcp_app_token: ${{ steps.github-mcp-app-token.outputs.token }}", "Activation job must not expose github_mcp_app_token output") - // Verify token invalidation step is present in the agent job and references activation output + // Verify token invalidation step is present in the agent job and references the step output assert.Contains(t, lockContent, "Invalidate GitHub App token", "Token invalidation step should be present") assert.Contains(t, lockContent, "if: always()", "Invalidation step should always run") - assert.Contains(t, lockContent, "needs.activation.outputs.github_mcp_app_token", "Invalidation step should reference activation output") + assert.Contains(t, lockContent, "steps.github-mcp-app-token.outputs.token", "Invalidation step should reference agent job step output") - // Verify the app token is consumed from activation outputs in the agent job - assert.Contains(t, lockContent, "GITHUB_MCP_SERVER_TOKEN: ${{ needs.activation.outputs.github_mcp_app_token }}", "Should use activation output token for GitHub MCP Server") + // Verify the app token is consumed from the step output within the agent job + assert.Contains(t, lockContent, "GITHUB_MCP_SERVER_TOKEN: ${{ steps.github-mcp-app-token.outputs.token }}", "Should use agent job step token for GitHub MCP Server") } // TestGitHubMCPAppTokenAndGitHubTokenMutuallyExclusive tests that setting both app and github-token is rejected @@ -191,21 +192,21 @@ Test app token with remote GitHub MCP Server. require.NoError(t, err, "Failed to read lock file") lockContent := string(content) - // Verify token minting step is present in the activation job + // Verify token minting step is present in the agent job assert.Contains(t, lockContent, "Generate GitHub App token", "Token minting step should be present") assert.Contains(t, lockContent, "id: github-mcp-app-token", "Should use github-mcp-app-token as step ID") - // Verify the activation job exposes the token as an output - assert.Contains(t, lockContent, "github_mcp_app_token: ${{ steps.github-mcp-app-token.outputs.token }}", "Activation job should expose github_mcp_app_token output") + // Verify the activation job does NOT expose the token as a job output + assert.NotContains(t, lockContent, "github_mcp_app_token: ${{ steps.github-mcp-app-token.outputs.token }}", "Activation job must not expose github_mcp_app_token output") - // Verify the app token from activation outputs is used in the agent job - // The token should be referenced via needs.activation.outputs.github_mcp_app_token - if strings.Contains(lockContent, `"Authorization": "Bearer ${{ needs.activation.outputs.github_mcp_app_token }}"`) { - // Success - app token from activation is used in Authorization header - t.Log("App token from activation correctly used in remote mode Authorization header") + // Verify the app token from the agent job step is used + // The token should be referenced via steps.github-mcp-app-token.outputs.token + if strings.Contains(lockContent, `"Authorization": "Bearer ${{ steps.github-mcp-app-token.outputs.token }}"`) { + // Success - app token from step is used in Authorization header + t.Log("App token from agent job step correctly used in remote mode Authorization header") } else { // Also check for the env var reference pattern used by Claude engine - assert.Contains(t, lockContent, "GITHUB_MCP_SERVER_TOKEN: ${{ needs.activation.outputs.github_mcp_app_token }}", "Should use activation output token for GitHub MCP Server in remote mode") + assert.Contains(t, lockContent, "GITHUB_MCP_SERVER_TOKEN: ${{ steps.github-mcp-app-token.outputs.token }}", "Should use agent job step token for GitHub MCP Server in remote mode") } } @@ -312,9 +313,9 @@ Test that determine-automatic-lockdown is generated even when app is configured. assert.Contains(t, lockContent, "GITHUB_MCP_GUARD_MIN_INTEGRITY: ${{ steps.determine-automatic-lockdown.outputs.min_integrity }}", "Guard min-integrity env var should reference lockdown step output") assert.Contains(t, lockContent, "GITHUB_MCP_GUARD_REPOS: ${{ steps.determine-automatic-lockdown.outputs.repos }}", "Guard repos env var should reference lockdown step output") - // App token should still be minted (in activation job) and consumed via activation outputs + // App token should still be minted (now in agent job) and consumed via step output assert.Contains(t, lockContent, "id: github-mcp-app-token", "GitHub App token step should still be generated") - assert.Contains(t, lockContent, "GITHUB_MCP_SERVER_TOKEN: ${{ needs.activation.outputs.github_mcp_app_token }}", "App token from activation should be used for MCP server") + assert.Contains(t, lockContent, "GITHUB_MCP_SERVER_TOKEN: ${{ steps.github-mcp-app-token.outputs.token }}", "App token from agent job step should be used for MCP server") } // TestGitHubMCPAppTokenWithDependabotToolset tests that permission-vulnerability-alerts is included @@ -536,34 +537,18 @@ Test that write is rejected in tools.github.github-app.permissions. assert.Contains(t, err.Error(), "members", "Error should mention the offending scope") } -// TestAgentJobDoesNotMintGitHubAppTokens verifies the compiler invariant that no -// GitHub App token minting step (create-github-app-token) appears in the agent job. -// All minting must happen in the activation job so that app-id / private-key secrets +// TestCheckoutAppTokensNotMintedInAgentJob verifies that checkout-related GitHub App token +// minting steps (create-github-app-token) do NOT appear in the agent job. +// Checkout app tokens are minted in the activation job so that app-id / private-key secrets // never reach the agent's environment. -func TestAgentJobDoesNotMintGitHubAppTokens(t *testing.T) { +// Note: the GitHub MCP App token (tools.github.github-app) IS minted in the agent job — +// this is intentional because masked values are silently dropped by the runner when passed +// as job outputs (runner v2.308+). +func TestCheckoutAppTokensNotMintedInAgentJob(t *testing.T) { tests := []struct { name string markdown string }{ - { - name: "tools.github.github-app token not minted in agent job", - markdown: `--- -on: issues -permissions: - contents: read - issues: read -strict: false -tools: - github: - mode: local - github-app: - app-id: ${{ vars.APP_ID }} - private-key: ${{ secrets.APP_PRIVATE_KEY }} ---- - -Test workflow - MCP app token must not be minted in agent job. -`, - }, { name: "checkout.github-app token not minted in agent job", markdown: `--- @@ -632,7 +617,63 @@ Test workflow - top-level github-app checkout token must not be minted in agent } assert.NotContains(t, agentJobContent, "create-github-app-token", - "Agent job must not mint GitHub App tokens; minting must be in activation job") + "Agent job must not mint checkout GitHub App tokens; checkout token minting must be in activation job") }) } } + +// TestGitHubMCPAppTokenMintedInAgentJob verifies that the GitHub MCP App token +// (tools.github.github-app) IS minted directly in the agent job. +// This is required because actions/create-github-app-token calls ::add-mask:: on the +// produced token, and the GitHub Actions runner silently drops masked values when used +// as job outputs (runner v2.308+). By minting within the agent job the token is +// available as steps.github-mcp-app-token.outputs.token. +func TestGitHubMCPAppTokenMintedInAgentJob(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + markdown := `--- +on: issues +permissions: + contents: read + issues: read +strict: false +tools: + github: + mode: local + github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +--- + +Test workflow - MCP app token must be minted in agent job. +` + + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.md") + err := os.WriteFile(testFile, []byte(markdown), 0644) + require.NoError(t, err, "Failed to write test file") + + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "Workflow should compile successfully") + + lockFile := strings.TrimSuffix(testFile, ".md") + ".lock.yml" + content, err := os.ReadFile(lockFile) + require.NoError(t, err, "Failed to read lock file") + lockContent := string(content) + + // The minting step must be present somewhere in the compiled workflow + assert.Contains(t, lockContent, "create-github-app-token", + "GitHub MCP App token minting step must be present in the compiled workflow") + assert.Contains(t, lockContent, "id: github-mcp-app-token", + "GitHub MCP App token step must use github-mcp-app-token step ID") + + // The activation job must NOT expose github_mcp_app_token as a job output. + // Masked values are silently dropped by the runner when passed as job outputs (runner v2.308+). + assert.NotContains(t, lockContent, "github_mcp_app_token: ${{ steps.github-mcp-app-token.outputs.token }}", + "Activation job must not expose github_mcp_app_token as a job output") + + // The token must be referenced via the step output, not via activation job outputs. + assert.NotContains(t, lockContent, "needs.activation.outputs.github_mcp_app_token", + "Token must not be referenced via activation job outputs (masked values are dropped by runner)") + assert.Contains(t, lockContent, "steps.github-mcp-app-token.outputs.token", + "Token must be referenced via step output within the same job") +} diff --git a/pkg/workflow/mcp_environment.go b/pkg/workflow/mcp_environment.go index 28d963e43eb..550bb485027 100644 --- a/pkg/workflow/mcp_environment.go +++ b/pkg/workflow/mcp_environment.go @@ -69,12 +69,13 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor // Check if GitHub App is configured for token minting appConfigured := hasGitHubApp(githubTool) - // If GitHub App is configured, use the app token minted in the activation job. - // The token is passed via needs.activation.outputs to keep app-id/private-key - // secrets out of the agent job. + // 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 + // actions/create-github-app-token calls ::add-mask:: on the token, and the + // GitHub Actions runner silently drops masked values in job outputs (runner v2.308+). if appConfigured { - mcpEnvironmentLog.Print("Using GitHub App token from activation job for GitHub MCP server (overrides custom and default tokens)") - envVars["GITHUB_MCP_SERVER_TOKEN"] = "${{ needs.activation.outputs.github_mcp_app_token }}" + mcpEnvironmentLog.Print("Using GitHub App token from agent job step for GitHub MCP server (overrides custom and default tokens)") + envVars["GITHUB_MCP_SERVER_TOKEN"] = "${{ steps.github-mcp-app-token.outputs.token }}" } else { // Otherwise, use custom token or default fallback customGitHubToken := getGitHubToken(githubTool)