diff --git a/pkg/workflow/compiler_activation_job.go b/pkg/workflow/compiler_activation_job.go index c64eb953ae..4eea003dcc 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 501d0cf37c..cdf6a9c383 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 f1348d73b6..58e065d2e8 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 62f3d02524..6485b6f127 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 1718d92904..ca294d3add 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 fdde488778..d7af819182 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 28d963e43e..550bb48502 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)