From 88e8a4b4036c8f0daff37f29b1fa53bac45606c1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 14:29:47 +0000 Subject: [PATCH 1/3] Initial plan From bdae9bcf4143921ad34b58afa8b9fd0dcddd787f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 15:00:59 +0000 Subject: [PATCH 2/3] fix: move checkout app token minting from activation job to agent job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub Actions runner (v2.308+) silently drops masked job outputs. actions/create-github-app-token calls ::add-mask:: on the token, so tokens passed from activation→agent via job outputs were being suppressed, causing 'Input required and not supplied: token'. Fix: mint checkout tokens directly in the agent job (same approach already used for github-mcp-app-token), so they are accessible as steps.checkout-app-token-{index}.outputs.token within the same job. Changes: - compiler_activation_job.go: remove checkout token minting + outputs - compiler_main_job.go: remove invariant blocking checkout minting in agent job - compiler_yaml_main_job.go: add checkout token minting before checkout steps - checkout_step_generator.go: update all token refs to steps.* form - checkout_manager.go: remove now-unused CheckoutAppTokenOutputs method - tests: update to verify new behavior (tokens in agent job) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7124be57-8902-4d7f-8689-588206019162 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/checkout_manager.go | 20 --------- pkg/workflow/checkout_manager_test.go | 8 +++- pkg/workflow/checkout_step_generator.go | 21 +++++---- pkg/workflow/compiler_activation_job.go | 19 -------- pkg/workflow/compiler_main_job.go | 14 +++--- pkg/workflow/compiler_yaml_main_job.go | 20 +++++++++ pkg/workflow/github_mcp_app_token_test.go | 55 ++++++++++++----------- 7 files changed, 72 insertions(+), 85 deletions(-) diff --git a/pkg/workflow/checkout_manager.go b/pkg/workflow/checkout_manager.go index 3d7fee38c6..1a72e83ad4 100644 --- a/pkg/workflow/checkout_manager.go +++ b/pkg/workflow/checkout_manager.go @@ -1,7 +1,6 @@ package workflow import ( - "fmt" "strings" "github.com/github/gh-aw/pkg/logger" @@ -284,25 +283,6 @@ func (cm *CheckoutManager) HasAppAuth() bool { return false } -// CheckoutAppTokenOutputs returns a map of activation job output names to their -// step token expressions, for all checkouts using app authentication. -// Output names follow the pattern "checkout_app_token_{index}". -// Step ID expressions follow the pattern "steps.checkout-app-token-{index}.outputs.token". -// These outputs are set by the activation job so the agent job can reference them as -// needs.activation.outputs.checkout_app_token_{index}. -func (cm *CheckoutManager) CheckoutAppTokenOutputs() map[string]string { - outputs := make(map[string]string) - for i, entry := range cm.ordered { - if entry.githubApp == nil { - continue - } - outputName := fmt.Sprintf("checkout_app_token_%d", i) - stepID := fmt.Sprintf("checkout-app-token-%d", i) - outputs[outputName] = fmt.Sprintf("${{ steps.%s.outputs.token }}", stepID) - } - return outputs -} - // HasExternalRootCheckout returns true if any checkout entry targets an external // repository (non-empty repository field) and writes to the workspace root (empty path). // When such a checkout exists, the workspace root is replaced with the external diff --git a/pkg/workflow/checkout_manager_test.go b/pkg/workflow/checkout_manager_test.go index dfd8ef7fa3..1b084c41c6 100644 --- a/pkg/workflow/checkout_manager_test.go +++ b/pkg/workflow/checkout_manager_test.go @@ -890,7 +890,9 @@ func TestDefaultCheckoutWithAppAuth(t *testing.T) { }) lines := cm.GenerateDefaultCheckoutStep(false, "", getPin) combined := strings.Join(lines, "") - assert.Contains(t, combined, "needs.activation.outputs.checkout_app_token_0", "checkout should reference app token step") + // Token is now minted in the agent job itself (same-job step reference) + assert.Contains(t, combined, "steps.checkout-app-token-0.outputs.token", "checkout should reference step output in same job") + assert.NotContains(t, combined, "needs.activation.outputs.checkout_app_token_0", "checkout must not reference activation job outputs (masked values are dropped by runner)") }) } @@ -908,7 +910,9 @@ func TestAdditionalCheckoutWithAppAuth(t *testing.T) { }) lines := cm.GenerateAdditionalCheckoutSteps(getPin) combined := strings.Join(lines, "") - assert.Contains(t, combined, "needs.activation.outputs.checkout_app_token_1", "additional checkout should reference app token at index 1") + // Token is now minted in the agent job itself (same-job step reference) + assert.Contains(t, combined, "steps.checkout-app-token-1.outputs.token", "additional checkout should reference step output at index 1") + assert.NotContains(t, combined, "needs.activation.outputs.checkout_app_token_1", "checkout must not reference activation job outputs (masked values are dropped by runner)") assert.Contains(t, combined, "other/repo", "should reference the additional repo") }) } diff --git a/pkg/workflow/checkout_step_generator.go b/pkg/workflow/checkout_step_generator.go index 993593f17c..fdeb55f259 100644 --- a/pkg/workflow/checkout_step_generator.go +++ b/pkg/workflow/checkout_step_generator.go @@ -37,8 +37,8 @@ func (cm *CheckoutManager) GenerateCheckoutAppTokenSteps(c *Compiler, permission // GenerateCheckoutAppTokenInvalidationSteps generates token invalidation steps // for all checkout entries that use app authentication. -// The tokens were minted in the activation job and are referenced via -// needs.activation.outputs.checkout_app_token_{index}. +// The tokens were minted in the agent job and are referenced via +// steps.checkout-app-token-{index}.outputs.token. func (cm *CheckoutManager) GenerateCheckoutAppTokenInvalidationSteps(c *Compiler) []string { var steps []string for i, entry := range cm.ordered { @@ -47,9 +47,11 @@ func (cm *CheckoutManager) GenerateCheckoutAppTokenInvalidationSteps(c *Compiler } checkoutManagerLog.Printf("Generating app token invalidation step for checkout index=%d", i) rawSteps := c.buildGitHubAppTokenInvalidationStep() - outputName := fmt.Sprintf("checkout_app_token_%d", i) + stepID := fmt.Sprintf("checkout-app-token-%d", i) for _, step := range rawSteps { - modified := strings.ReplaceAll(step, "steps.safe-outputs-app-token.outputs.token", "needs.activation.outputs."+outputName) + modified := strings.ReplaceAll(step, "steps.safe-outputs-app-token.outputs.token", "steps."+stepID+".outputs.token") + // Update the if condition to reference the step output (not activation outputs) + modified = strings.ReplaceAll(modified, "if: always() && steps.safe-outputs-app-token.outputs.token != ''", fmt.Sprintf("if: always() && steps.%s.outputs.token != ''", stepID)) // Update step name to indicate it's for checkout modified = strings.ReplaceAll(modified, "Invalidate GitHub App token", fmt.Sprintf("Invalidate checkout app token (%d)", i)) steps = append(steps, modified) @@ -163,9 +165,10 @@ func (cm *CheckoutManager) GenerateDefaultCheckoutStep( // Determine effective token: github-app-minted token takes precedence effectiveOverrideToken := override.token if override.githubApp != nil { - // The default checkout is always at index 0 in the ordered list + // The default checkout is always at index 0 in the ordered list. + // The token is minted in the agent job itself (same-job step reference). //nolint:gosec // G101: False positive - this is a GitHub Actions expression template placeholder, not a hardcoded credential - effectiveOverrideToken = "${{ needs.activation.outputs.checkout_app_token_0 }}" + effectiveOverrideToken = "${{ steps.checkout-app-token-0.outputs.token }}" } if effectiveOverrideToken != "" { fmt.Fprintf(&sb, " token: %s\n", effectiveOverrideToken) @@ -231,8 +234,9 @@ func generateCheckoutStepLines(entry *resolvedCheckout, index int, getActionPin // Determine effective token: github-app-minted token takes precedence effectiveToken := entry.token if entry.githubApp != nil { + // The token is minted in the agent job itself (same-job step reference). //nolint:gosec // G101: False positive - this is a GitHub Actions expression template placeholder, not a hardcoded credential - effectiveToken = fmt.Sprintf("${{ needs.activation.outputs.checkout_app_token_%d }}", index) + effectiveToken = fmt.Sprintf("${{ steps.checkout-app-token-%d.outputs.token }}", index) } if effectiveToken != "" { fmt.Fprintf(&sb, " token: %s\n", effectiveToken) @@ -319,8 +323,9 @@ func generateFetchStepLines(entry *resolvedCheckout, index int) string { // Determine authentication token token := entry.token if entry.githubApp != nil { + // The token is minted in the agent job itself (same-job step reference). //nolint:gosec // G101: False positive - this is a GitHub Actions expression template placeholder, not a hardcoded credential - token = fmt.Sprintf("${{ needs.activation.outputs.checkout_app_token_%d }}", index) + token = fmt.Sprintf("${{ steps.checkout-app-token-%d.outputs.token }}", index) } if token == "" { token = getEffectiveGitHubToken("") diff --git a/pkg/workflow/compiler_activation_job.go b/pkg/workflow/compiler_activation_job.go index 69d5e4639e..de39f59714 100644 --- a/pkg/workflow/compiler_activation_job.go +++ b/pkg/workflow/compiler_activation_job.go @@ -4,7 +4,6 @@ import ( "encoding/json" "errors" "fmt" - "maps" "slices" "strings" @@ -136,24 +135,6 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate outputs["activation_app_token_minting_failed"] = "${{ steps.activation-app-token.outcome == 'failure' }}" } - // 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}. - checkoutMgrForActivation := NewCheckoutManager(data.CheckoutConfigs) - if checkoutMgrForActivation.HasAppAuth() { - compilerActivationJobLog.Print("Generating checkout app token minting steps in activation job") - var checkoutPermissions *Permissions - if data.Permissions != "" { - parser := NewPermissionsParser(data.Permissions) - checkoutPermissions = parser.ToPermissions() - } else { - checkoutPermissions = NewPermissions() - } - checkoutAppTokenSteps := checkoutMgrForActivation.GenerateCheckoutAppTokenSteps(c, checkoutPermissions) - steps = append(steps, checkoutAppTokenSteps...) - maps.Copy(outputs, checkoutMgrForActivation.CheckoutAppTokenOutputs()) - } - // Add reaction step right after generate_aw_info so it is shown to the user as fast as // possible. generate_aw_info runs first so its data is captured even if the reaction fails. // This runs in the activation job so it can use any configured github-token or github-app. diff --git a/pkg/workflow/compiler_main_job.go b/pkg/workflow/compiler_main_job.go index 92d7386c92..cff1a6afd0 100644 --- a/pkg/workflow/compiler_main_job.go +++ b/pkg/workflow/compiler_main_job.go @@ -1,7 +1,6 @@ package workflow import ( - "errors" "fmt" "maps" "slices" @@ -70,15 +69,12 @@ 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 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. + // Checkout app tokens (checkout-app-token-*) are now minted directly in the agent job, + // for the same reason as the GitHub MCP App token: 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+). Minting within the agent job avoids + // the activation→agent output hop entirely. stepsContent := stepBuilder.String() - 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 if stepsContent != "" { diff --git a/pkg/workflow/compiler_yaml_main_job.go b/pkg/workflow/compiler_yaml_main_job.go index 0fbd24b744..b83a044078 100644 --- a/pkg/workflow/compiler_yaml_main_job.go +++ b/pkg/workflow/compiler_yaml_main_job.go @@ -35,6 +35,26 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat checkoutMgr.SetCrossRepoTargetRepo("${{ needs.activation.outputs.target_repo }}") } + // Mint checkout app tokens directly in the agent job before checkout steps are executed. + // Tokens 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 when used as job outputs (runner v2.308+). + // By minting here, the token is available as steps.checkout-app-token-{index}.outputs.token + // within the same job, just like the github-mcp-app-token pattern. + if checkoutMgr.HasAppAuth() { + compilerYamlLog.Print("Generating checkout app token minting steps in agent job") + var checkoutPermissions *Permissions + if data.Permissions != "" { + parser := NewPermissionsParser(data.Permissions) + checkoutPermissions = parser.ToPermissions() + } else { + checkoutPermissions = NewPermissions() + } + for _, step := range checkoutMgr.GenerateCheckoutAppTokenSteps(c, checkoutPermissions) { + yaml.WriteString(step) + } + } + // Add checkout step first if needed if needsCheckout { // Emit the default workspace checkout, applying any user-supplied overrides diff --git a/pkg/workflow/github_mcp_app_token_test.go b/pkg/workflow/github_mcp_app_token_test.go index d7af819182..1c9e9442d7 100644 --- a/pkg/workflow/github_mcp_app_token_test.go +++ b/pkg/workflow/github_mcp_app_token_test.go @@ -537,20 +537,20 @@ Test that write is rejected in tools.github.github-app.permissions. assert.Contains(t, err.Error(), "members", "Error should mention the offending scope") } -// 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. -// 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) { +// TestCheckoutAppTokensMintedInAgentJob verifies that checkout-related GitHub App token +// minting steps (create-github-app-token) appear directly in the agent job. +// Previously, checkout tokens were minted in the activation job and passed via job outputs, +// but the GitHub Actions runner silently drops masked values in job outputs (runner v2.308+), +// causing actions/checkout to fail with "Input required and not supplied: token". +// The fix mints checkout tokens in the agent job itself (same as github-mcp-app-token), +// so the token is accessible as steps.checkout-app-token-{index}.outputs.token. +func TestCheckoutAppTokensMintedInAgentJob(t *testing.T) { tests := []struct { name string markdown string }{ { - name: "checkout.github-app token not minted in agent job", + name: "checkout.github-app token minted in agent job", markdown: `--- on: issues permissions: @@ -564,11 +564,11 @@ checkout: private-key: ${{ secrets.APP_PRIVATE_KEY }} --- -Test workflow - checkout app token must not be minted in agent job. +Test workflow - checkout app token must be minted in agent job. `, }, { - name: "top-level github-app fallback for checkout not minted in agent job", + name: "top-level github-app fallback for checkout minted in agent job", markdown: `--- on: issues permissions: @@ -582,7 +582,7 @@ checkout: path: private --- -Test workflow - top-level github-app checkout token must not be minted in agent job. +Test workflow - top-level github-app checkout token must be minted in agent job. `, }, } @@ -603,21 +603,22 @@ Test workflow - top-level github-app checkout token must not be minted in agent require.NoError(t, err, "Failed to read lock file") lockContent := string(content) - // Locate the agent job section (after " agent:" and before the next top-level job) - agentJobStart := strings.Index(lockContent, "\n agent:\n") - require.NotEqual(t, -1, agentJobStart, "Agent job should be present") - - // Find the next top-level job after agent (or end of file) - nextJobStart := strings.Index(lockContent[agentJobStart+len("\n agent:\n"):], "\n ") - var agentJobContent string - if nextJobStart == -1 { - agentJobContent = lockContent[agentJobStart:] - } else { - agentJobContent = lockContent[agentJobStart : agentJobStart+len("\n agent:\n")+nextJobStart] - } - - assert.NotContains(t, agentJobContent, "create-github-app-token", - "Agent job must not mint checkout GitHub App tokens; checkout token minting must be in activation job") + // The token minting step must appear in the compiled workflow + assert.Contains(t, lockContent, "id: checkout-app-token-0", + "Checkout app token minting step must be present in the compiled workflow") + assert.Contains(t, lockContent, "create-github-app-token", + "Checkout app token minting step must use create-github-app-token action") + + // The token must be referenced via the step output (same-job), not via activation outputs. + // Activation job outputs of masked values are silently dropped by the runner (v2.308+). + assert.Contains(t, lockContent, "steps.checkout-app-token-0.outputs.token", + "Token must be referenced via step output within the same job") + assert.NotContains(t, lockContent, "needs.activation.outputs.checkout_app_token_0", + "Token must not be passed via activation job outputs (masked values are dropped by runner)") + + // The activation job must NOT expose checkout_app_token as a job output. + assert.NotContains(t, lockContent, "checkout_app_token_0: ${{ steps.checkout-app-token-0.outputs.token }}", + "Activation job must not expose checkout app token as a job output") }) } } From fe0f7378591943df372c3f79518222b3a5401f92 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 15:25:24 +0000 Subject: [PATCH 3/3] fix: address review comments - remove redundant ReplaceAll, add job-scoped assertions in test - checkout_step_generator.go: remove redundant second ReplaceAll in GenerateCheckoutAppTokenInvalidationSteps (the first pass already rewrites the if: condition when it replaces steps.safe-outputs-app-token) - github_mcp_app_token_test.go: use extractJobSection helper to assert the checkout-app-token-0 step is inside the agent job and NOT in the activation job, preventing a silent regression where the step could be emitted in the wrong job Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d385f64e-bf00-4ed1-87f1-20fff6a5a838 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/checkout_step_generator.go | 4 ++-- pkg/workflow/github_mcp_app_token_test.go | 23 +++++++++++++++++------ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/pkg/workflow/checkout_step_generator.go b/pkg/workflow/checkout_step_generator.go index fdeb55f259..6654d9710c 100644 --- a/pkg/workflow/checkout_step_generator.go +++ b/pkg/workflow/checkout_step_generator.go @@ -49,9 +49,9 @@ func (cm *CheckoutManager) GenerateCheckoutAppTokenInvalidationSteps(c *Compiler rawSteps := c.buildGitHubAppTokenInvalidationStep() stepID := fmt.Sprintf("checkout-app-token-%d", i) for _, step := range rawSteps { + // Replace all references to safe-outputs-app-token with the checkout-specific step ID. + // This covers both the `if:` condition and the `env:` token reference in one pass. modified := strings.ReplaceAll(step, "steps.safe-outputs-app-token.outputs.token", "steps."+stepID+".outputs.token") - // Update the if condition to reference the step output (not activation outputs) - modified = strings.ReplaceAll(modified, "if: always() && steps.safe-outputs-app-token.outputs.token != ''", fmt.Sprintf("if: always() && steps.%s.outputs.token != ''", stepID)) // Update step name to indicate it's for checkout modified = strings.ReplaceAll(modified, "Invalidate GitHub App token", fmt.Sprintf("Invalidate checkout app token (%d)", i)) steps = append(steps, modified) diff --git a/pkg/workflow/github_mcp_app_token_test.go b/pkg/workflow/github_mcp_app_token_test.go index 1c9e9442d7..347b0ead7d 100644 --- a/pkg/workflow/github_mcp_app_token_test.go +++ b/pkg/workflow/github_mcp_app_token_test.go @@ -603,22 +603,33 @@ Test workflow - top-level github-app checkout token must be minted in agent job. require.NoError(t, err, "Failed to read lock file") lockContent := string(content) - // The token minting step must appear in the compiled workflow - assert.Contains(t, lockContent, "id: checkout-app-token-0", - "Checkout app token minting step must be present in the compiled workflow") - assert.Contains(t, lockContent, "create-github-app-token", + // Extract the agent job section (from " agent:" to the next top-level job). + // The token minting step must be inside the agent job, not the activation job. + agentJobSection := extractJobSection(lockContent, "agent") + require.NotEmpty(t, agentJobSection, "Agent job should be present") + + // The token minting step must appear inside the agent job section + assert.Contains(t, agentJobSection, "id: checkout-app-token-0", + "Checkout app token minting step must be inside the agent job") + assert.Contains(t, agentJobSection, "create-github-app-token", "Checkout app token minting step must use create-github-app-token action") // The token must be referenced via the step output (same-job), not via activation outputs. // Activation job outputs of masked values are silently dropped by the runner (v2.308+). - assert.Contains(t, lockContent, "steps.checkout-app-token-0.outputs.token", + assert.Contains(t, agentJobSection, "steps.checkout-app-token-0.outputs.token", "Token must be referenced via step output within the same job") assert.NotContains(t, lockContent, "needs.activation.outputs.checkout_app_token_0", "Token must not be passed via activation job outputs (masked values are dropped by runner)") - // The activation job must NOT expose checkout_app_token as a job output. + // The activation job must NOT expose checkout_app_token as a job output or contain + // the minting step. assert.NotContains(t, lockContent, "checkout_app_token_0: ${{ steps.checkout-app-token-0.outputs.token }}", "Activation job must not expose checkout app token as a job output") + activationJobSection := extractJobSection(lockContent, "activation") + if activationJobSection != "" { + assert.NotContains(t, activationJobSection, "id: checkout-app-token-0", + "Checkout app token minting step must NOT be in the activation job") + } }) } }