Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
4 changes: 4 additions & 0 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -10970,6 +10970,10 @@
"description": "GitHub App private key (e.g., '${{ secrets.APP_PRIVATE_KEY }}'). Required to mint a GitHub App token.",
"examples": ["${{ secrets.APP_PRIVATE_KEY }}"]
},
"ignore-if-missing": {
"type": "boolean",
"description": "If true, skip token minting when client-id/private-key resolve to empty strings at runtime. Defaults to false."
},
Comment on lines +12501 to +12504
"owner": {
"type": "string",
"description": "Optional owner of the GitHub App installation (defaults to current repository owner if not specified)"
Expand Down
24 changes: 24 additions & 0 deletions pkg/workflow/activation_github_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,30 @@ func TestActivationGitHubApp(t *testing.T) {
assert.Contains(t, stepsStr, "github-token: ${{ steps.activation-app-token.outputs.token }}", "Add-comment step should use app token")
})

t.Run("missing_key_ignore_adds_guard_and_fallback_token", func(t *testing.T) {
statusComment := true
workflowData := &WorkflowData{
Name: "Test Workflow",
AIReaction: "eyes",
StatusComment: &statusComment,
ActivationGitHubApp: &GitHubAppConfig{
AppID: "${{ secrets.GH_AW_APP_ID }}",
PrivateKey: "${{ secrets.GH_AW_APP_PRIVATE_KEY }}",
IgnoreIfMissing: true,
},
}

job, err := compiler.buildActivationJob(workflowData, false, "", "test.lock.yml")
require.NoError(t, err, "buildActivationJob should succeed")
require.NotNil(t, job)

stepsStr := strings.Join(job.Steps, "")
assert.Contains(t, stepsStr, "if: ${{ env.GH_AW_APP_CLIENT_ID != '' && env.GH_AW_APP_PRIVATE_KEY != '' }}")
assert.Contains(t, stepsStr, "GH_AW_APP_CLIENT_ID: ${{ secrets.GH_AW_APP_ID }}")
assert.Contains(t, stepsStr, "GH_AW_APP_PRIVATE_KEY: ${{ secrets.GH_AW_APP_PRIVATE_KEY }}")
assert.Contains(t, stepsStr, "github-token: ${{ steps.activation-app-token.outputs.token || secrets.GITHUB_TOKEN }}")
})

t.Run("app_token_minted_once_for_both_reaction_and_comment", func(t *testing.T) {
statusComment := true
workflowData := &WorkflowData{
Expand Down
9 changes: 9 additions & 0 deletions pkg/workflow/checkout_step_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ func (cm *CheckoutManager) GenerateDefaultCheckoutStep(
}
//nolint:gosec // G101: False positive - this is a GitHub Actions expression template placeholder, not a hardcoded credential
effectiveOverrideToken = fmt.Sprintf("${{ steps.checkout-app-token-%d.outputs.token }}", defaultIdx)
if override.githubApp.shouldIgnoreMissingKey() {
effectiveOverrideToken = combineTokenExpressions(effectiveOverrideToken, getEffectiveGitHubToken(override.token))
}
}
if effectiveOverrideToken != "" {
fmt.Fprintf(&sb, " token: %s\n", effectiveOverrideToken)
Expand Down Expand Up @@ -283,6 +286,9 @@ func generateCheckoutStepLines(entry *resolvedCheckout, index int, getActionPin
// 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("${{ steps.checkout-app-token-%d.outputs.token }}", index)
if entry.githubApp.shouldIgnoreMissingKey() {
effectiveToken = combineTokenExpressions(effectiveToken, getEffectiveGitHubToken(entry.token))
}
}
if effectiveToken != "" {
fmt.Fprintf(&sb, " token: %s\n", effectiveToken)
Expand Down Expand Up @@ -386,6 +392,9 @@ func generateFetchStepLines(entry *resolvedCheckout, index int) string {
// 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("${{ steps.checkout-app-token-%d.outputs.token }}", index)
if entry.githubApp.shouldIgnoreMissingKey() {
token = combineTokenExpressions(token, getEffectiveGitHubToken(entry.token))
}
}
if token == "" {
token = getEffectiveGitHubToken("")
Expand Down
12 changes: 12 additions & 0 deletions pkg/workflow/compiler_pre_activation_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,12 @@ func (c *Compiler) buildPreActivationAppTokenMintStep(app *GitHubAppConfig) []st

steps = append(steps, " - name: Generate GitHub App token for skip-if checks\n")
steps = append(steps, fmt.Sprintf(" id: %s\n", tokenStepID))
if app.shouldIgnoreMissingKey() {
steps = append(steps, " if: ${{ env.GH_AW_APP_CLIENT_ID != '' && env.GH_AW_APP_PRIVATE_KEY != '' }}\n")
steps = append(steps, " env:\n")
steps = append(steps, fmt.Sprintf(" GH_AW_APP_CLIENT_ID: %s\n", app.AppID))
steps = append(steps, fmt.Sprintf(" GH_AW_APP_PRIVATE_KEY: %s\n", app.PrivateKey))
}
steps = append(steps, fmt.Sprintf(" uses: %s\n", getActionPin("actions/create-github-app-token")))
steps = append(steps, " with:\n")
steps = append(steps, fmt.Sprintf(" client-id: %s\n", app.AppID))
Expand Down Expand Up @@ -689,6 +695,12 @@ func (c *Compiler) buildPreActivationAppTokenMintStep(app *GitHubAppConfig) []st
// When non-empty, callers should emit `with.github-token: <value>` in the step.
func (c *Compiler) resolvePreActivationSkipIfToken(data *WorkflowData) string {
if data.ActivationGitHubApp != nil {
if data.ActivationGitHubApp.shouldIgnoreMissingKey() {
return combineTokenExpressions(
fmt.Sprintf("${{ steps.%s.outputs.token }}", constants.PreActivationAppTokenStepID),
"${{ secrets.GITHUB_TOKEN }}",
)
}
return fmt.Sprintf("${{ steps.%s.outputs.token }}", constants.PreActivationAppTokenStepID)
}
if data.ActivationGitHubToken != "" {
Expand Down
7 changes: 6 additions & 1 deletion pkg/workflow/copilot_engine_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,12 @@ touch %s
// 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"] = "${{ steps.github-mcp-app-token.outputs.token }}"
tokenExpression := "${{ steps.github-mcp-app-token.outputs.token }}"
if workflowData.ParsedTools.GitHub.GitHubApp.shouldIgnoreMissingKey() {
customGitHubToken := getGitHubToken(workflowData.Tools["github"])
tokenExpression = combineTokenExpressions(tokenExpression, getEffectiveGitHubToken(customGitHubToken))
}
env["GITHUB_MCP_SERVER_TOKEN"] = tokenExpression
} else {
customGitHubToken := getGitHubToken(workflowData.Tools["github"])
// Use effective token with precedence: custom > default
Expand Down
25 changes: 24 additions & 1 deletion pkg/workflow/github_token.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,26 @@
package workflow

import "github.com/github/gh-aw/pkg/logger"
import (
"fmt"
"strings"

"github.com/github/gh-aw/pkg/logger"
)

var tokenLog = logger.New("workflow:github_token")

func unwrapGitHubExpression(expression string) string {
trimmed := strings.TrimSpace(expression)
if strings.HasPrefix(trimmed, "${{") && strings.HasSuffix(trimmed, "}}") {
return strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(trimmed, "${{"), "}}"))
}
return trimmed
}

func combineTokenExpressions(primaryExpression, fallbackExpression string) string {
return fmt.Sprintf("${{ %s || %s }}", unwrapGitHubExpression(primaryExpression), unwrapGitHubExpression(fallbackExpression))
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot review expression builder and reuse helpers

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in b793455: token expression composition now reuses shared helpers. combineTokenExpressions uses stripExpressionWrapper + RenderCondition, and buildIgnoreIfMissingCondition also renders through RenderCondition for consistent expression-builder usage.


// getEffectiveGitHubToken returns the GitHub token to use, with precedence:
// 1. Custom token passed as parameter (e.g., from tool-specific config)
// 2. Default fallback: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }}
Expand Down Expand Up @@ -120,6 +137,12 @@ func resolvePRCheckoutToken(safeOutputs *SafeOutputsConfig) (token string, isCus

// GitHub App token takes precedence over the safe-outputs level PAT
if safeOutputs.GitHubApp != nil {
if safeOutputs.GitHubApp.shouldIgnoreMissingKey() {
return combineTokenExpressions(
"${{ steps.safe-outputs-app-token.outputs.token }}",
getEffectiveSafeOutputGitHubToken(safeOutputs.GitHubToken),
), true
}
//nolint:gosec // G101: False positive - this is a GitHub Actions expression template placeholder, not a hardcoded credential
return "${{ steps.safe-outputs-app-token.outputs.token }}", true
}
Expand Down
11 changes: 10 additions & 1 deletion pkg/workflow/mcp_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,16 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor
// GitHub Actions runner silently drops masked values in job outputs (runner v2.308+).
if appConfigured {
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 }}"
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))
}
}
}
envVars["GITHUB_MCP_SERVER_TOKEN"] = tokenExpression
} else {
// Otherwise, use custom token or default fallback
customGitHubToken := getGitHubToken(githubTool)
Expand Down
42 changes: 37 additions & 5 deletions pkg/workflow/safe_outputs_app_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ var safeOutputsAppLog = logger.New("workflow:safe_outputs_app")

// GitHubAppConfig holds configuration for GitHub App-based token minting
type GitHubAppConfig struct {
AppID string `yaml:"client-id,omitempty"` // GitHub App client ID (or legacy app ID) (e.g., "${{ vars.APP_ID }}")
PrivateKey string `yaml:"private-key,omitempty"` // GitHub App private key (e.g., "${{ secrets.APP_PRIVATE_KEY }}")
Owner string `yaml:"owner,omitempty"` // Optional: owner of the GitHub App installation (defaults to current repository owner)
Repositories []string `yaml:"repositories,omitempty"` // Optional: comma or newline-separated list of repositories to grant access to
Permissions map[string]string `yaml:"permissions,omitempty"` // Optional: extra permission-* fields to merge into the minted token (nested wins over job-level)
AppID string `yaml:"client-id,omitempty"` // GitHub App client ID (or legacy app ID) (e.g., "${{ vars.APP_ID }}")
PrivateKey string `yaml:"private-key,omitempty"` // GitHub App private key (e.g., "${{ secrets.APP_PRIVATE_KEY }}")
IgnoreIfMissing bool `yaml:"ignore-if-missing,omitempty"` // If true, skip token minting when client-id/private-key resolve empty
Owner string `yaml:"owner,omitempty"` // Optional: owner of the GitHub App installation (defaults to current repository owner)
Repositories []string `yaml:"repositories,omitempty"` // Optional: comma or newline-separated list of repositories to grant access to
Permissions map[string]string `yaml:"permissions,omitempty"` // Optional: extra permission-* fields to merge into the minted token (nested wins over job-level)
Comment on lines +22 to +27
}

// ========================================
Expand Down Expand Up @@ -53,6 +54,15 @@ func parseAppConfig(appMap map[string]any) *GitHubAppConfig {
}
}

// Parse ignore-if-missing behavior (optional): true to skip minting when key inputs are empty
if ignoreIfMissing, exists := appMap["ignore-if-missing"]; exists {
if ignore, ok := ignoreIfMissing.(bool); ok {
appConfig.IgnoreIfMissing = ignore
} else {
safeOutputsAppLog.Printf("Ignoring github-app.ignore-if-missing: expected boolean, got %T", ignoreIfMissing)
}
Comment on lines +58 to +64
}

// Parse owner (optional)
if owner, exists := appMap["owner"]; exists {
if ownerStr, ok := owner.(string); ok {
Expand Down Expand Up @@ -92,6 +102,13 @@ func parseAppConfig(appMap map[string]any) *GitHubAppConfig {
return appConfig
}

func (app *GitHubAppConfig) shouldIgnoreMissingKey() bool {
if app == nil {
return false
}
return app.IgnoreIfMissing
}

// ========================================
// App Configuration Merging
// ========================================
Expand Down Expand Up @@ -153,6 +170,12 @@ func (c *Compiler) buildGitHubAppTokenMintStep(app *GitHubAppConfig, permissions

steps = append(steps, " - name: Generate GitHub App token\n")
steps = append(steps, " id: safe-outputs-app-token\n")
if app.shouldIgnoreMissingKey() {
steps = append(steps, " if: ${{ env.GH_AW_APP_CLIENT_ID != '' && env.GH_AW_APP_PRIVATE_KEY != '' }}\n")
steps = append(steps, " env:\n")
steps = append(steps, fmt.Sprintf(" GH_AW_APP_CLIENT_ID: %s\n", app.AppID))
steps = append(steps, fmt.Sprintf(" GH_AW_APP_PRIVATE_KEY: %s\n", app.PrivateKey))
}
steps = append(steps, fmt.Sprintf(" uses: %s\n", getActionPin("actions/create-github-app-token")))
steps = append(steps, " with:\n")
steps = append(steps, fmt.Sprintf(" client-id: %s\n", app.AppID))
Expand Down Expand Up @@ -441,6 +464,12 @@ func (c *Compiler) buildActivationAppTokenMintStep(app *GitHubAppConfig, permiss

steps = append(steps, " - name: Generate GitHub App token for activation\n")
steps = append(steps, " id: activation-app-token\n")
if app.shouldIgnoreMissingKey() {
steps = append(steps, " if: ${{ env.GH_AW_APP_CLIENT_ID != '' && env.GH_AW_APP_PRIVATE_KEY != '' }}\n")
steps = append(steps, " env:\n")
steps = append(steps, fmt.Sprintf(" GH_AW_APP_CLIENT_ID: %s\n", app.AppID))
steps = append(steps, fmt.Sprintf(" GH_AW_APP_PRIVATE_KEY: %s\n", app.PrivateKey))
}
steps = append(steps, fmt.Sprintf(" uses: %s\n", getActionPin("actions/create-github-app-token")))
steps = append(steps, " with:\n")
steps = append(steps, fmt.Sprintf(" client-id: %s\n", app.AppID))
Expand Down Expand Up @@ -485,6 +514,9 @@ func (c *Compiler) buildActivationAppTokenMintStep(app *GitHubAppConfig, permiss
// a reference to that step's output (${{ steps.activation-app-token.outputs.token }}).
func (c *Compiler) resolveActivationToken(data *WorkflowData) string {
if data.ActivationGitHubApp != nil {
if data.ActivationGitHubApp.shouldIgnoreMissingKey() {
return combineTokenExpressions("${{ steps.activation-app-token.outputs.token }}", "${{ secrets.GITHUB_TOKEN }}")
}
return "${{ steps.activation-app-token.outputs.token }}"
}
if data.ActivationGitHubToken != "" {
Expand Down
50 changes: 50 additions & 0 deletions pkg/workflow/safe_outputs_app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,56 @@ Test workflow with minimal app configuration.
assert.Empty(t, workflowData.SafeOutputs.GitHubApp.Repositories)
}

func TestSafeOutputsAppIgnoreIfMissing(t *testing.T) {
compiler := NewCompiler(WithVersion("1.0.0"))

markdown := `---
on: issues
safe-outputs:
add-comment:
github-app:
app-id: ${{ secrets.GH_AW_APP_ID }}
private-key: ${{ secrets.GH_AW_APP_PRIVATE_KEY }}
ignore-if-missing: true
---

# Test Workflow
`

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")

workflowData, err := compiler.ParseWorkflowFile(testFile)
require.NoError(t, err, "Failed to parse markdown content")
require.NotNil(t, workflowData.SafeOutputs, "SafeOutputs should not be nil")
require.NotNil(t, workflowData.SafeOutputs.GitHubApp, "GitHub app configuration should be parsed")
assert.True(t, workflowData.SafeOutputs.GitHubApp.IgnoreIfMissing)

job, _, err := compiler.buildConsolidatedSafeOutputsJob(workflowData, "main", testFile)
require.NoError(t, err, "Failed to build safe_outputs job")
require.NotNil(t, job, "Job should not be nil")

stepsStr := strings.Join(job.Steps, "")
assert.Contains(t, stepsStr, "if: ${{ env.GH_AW_APP_CLIENT_ID != '' && env.GH_AW_APP_PRIVATE_KEY != '' }}")
assert.Contains(t, stepsStr, "GH_AW_APP_CLIENT_ID: ${{ secrets.GH_AW_APP_ID }}")
assert.Contains(t, stepsStr, "GH_AW_APP_PRIVATE_KEY: ${{ secrets.GH_AW_APP_PRIVATE_KEY }}")
assert.Contains(t, stepsStr, "github-token: ${{ steps.safe-outputs-app-token.outputs.token || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }}")
}

func TestSafeOutputsAppIgnoreIfMissingInvalidType(t *testing.T) {
app := parseAppConfig(map[string]any{
"client-id": "${{ vars.APP_ID }}",
"private-key": "${{ secrets.APP_PRIVATE_KEY }}",
"ignore-if-missing": "not-a-bool",
})

require.NotNil(t, app)
assert.False(t, app.IgnoreIfMissing)
assert.False(t, app.shouldIgnoreMissingKey())
}

// TestSafeOutputsAppWithoutSafeOutputs tests that app without safe outputs doesn't break
func TestSafeOutputsAppWithoutSafeOutputs(t *testing.T) {
compiler := NewCompiler(WithVersion("1.0.0"))
Expand Down
18 changes: 18 additions & 0 deletions pkg/workflow/safe_outputs_env.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,15 @@ func (c *Compiler) addSafeOutputGitHubTokenForConfig(steps *[]string, data *Work

// If app is configured, use app token
if data.SafeOutputs != nil && data.SafeOutputs.GitHubApp != nil {
if data.SafeOutputs.GitHubApp.shouldIgnoreMissingKey() {
effectiveCustomToken := configToken
if effectiveCustomToken == "" {
effectiveCustomToken = safeOutputsToken
}
fallbackToken := getEffectiveSafeOutputGitHubToken(effectiveCustomToken)
*steps = append(*steps, fmt.Sprintf(" github-token: %s\n", combineTokenExpressions("${{ steps.safe-outputs-app-token.outputs.token }}", fallbackToken)))
return
}
*steps = append(*steps, " github-token: ${{ steps.safe-outputs-app-token.outputs.token }}\n")
return
}
Expand All @@ -212,6 +221,15 @@ func (c *Compiler) addSafeOutputCopilotGitHubTokenForConfig(steps *[]string, dat

// If app is configured, use app token
if data.SafeOutputs != nil && data.SafeOutputs.GitHubApp != nil {
if data.SafeOutputs.GitHubApp.shouldIgnoreMissingKey() {
effectiveCustomToken := configToken
if effectiveCustomToken == "" {
effectiveCustomToken = safeOutputsToken
}
fallbackToken := getEffectiveCopilotRequestsToken(effectiveCustomToken)
*steps = append(*steps, fmt.Sprintf(" github-token: %s\n", combineTokenExpressions("${{ steps.safe-outputs-app-token.outputs.token }}", fallbackToken)))
return
}
*steps = append(*steps, " github-token: ${{ steps.safe-outputs-app-token.outputs.token }}\n")
return
}
Expand Down
Loading