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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 0 additions & 20 deletions pkg/workflow/checkout_manager.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package workflow

import (
"fmt"
"strings"

"github.com/github/gh-aw/pkg/logger"
Expand Down Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions pkg/workflow/checkout_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
})
}

Expand All @@ -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")
})
}
Expand Down
21 changes: 13 additions & 8 deletions pkg/workflow/checkout_step_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
// 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 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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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("")
Expand Down
19 changes: 0 additions & 19 deletions pkg/workflow/compiler_activation_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/json"
"errors"
"fmt"
"maps"
"slices"
"strings"

Expand Down Expand Up @@ -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.
Expand Down
14 changes: 5 additions & 9 deletions pkg/workflow/compiler_main_job.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package workflow

import (
"errors"
"fmt"
"maps"
"slices"
Expand Down Expand Up @@ -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 != "" {
Expand Down
20 changes: 20 additions & 0 deletions pkg/workflow/compiler_yaml_main_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
64 changes: 38 additions & 26 deletions pkg/workflow/github_mcp_app_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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.
`,
},
}
Expand All @@ -603,21 +603,33 @@ 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]
// 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, 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 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")
}

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")
})
}
}
Expand Down
Loading