Add github-app.missing-key ignore mode and guard App token minting across workflow paths#33033
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
github-app.missing-key ignore mode and guard App token minting across workflow paths
|
@copilot refactor field to "ignore-if-missing: true" instead of enum. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Updated in |
There was a problem hiding this comment.
Pull request overview
Adds an opt-in mode intended to skip GitHub App token minting when App credentials are unavailable and fall back to non-App tokens.
Changes:
- Adds a GitHub App missing-secret configuration path and token-expression fallback helpers.
- Guards App token mint steps across safe-outputs, activation, pre-activation, checkout, and MCP paths.
- Adds tests for safe-outputs and activation ignore-mode generation.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/safe_outputs_env.go |
Adds fallback token wiring for safe-output token consumers. |
pkg/workflow/safe_outputs_app_test.go |
Adds parsing/build tests for safe-output App ignore behavior. |
pkg/workflow/safe_outputs_app_config.go |
Adds App config flag parsing and guarded token minting/fallback resolution. |
pkg/workflow/mcp_environment.go |
Adds MCP GitHub token fallback composition when App minting is skipped. |
pkg/workflow/github_token.go |
Adds token expression composition helpers and checkout fallback wiring. |
pkg/workflow/copilot_engine_execution.go |
Applies MCP GitHub token fallback in Copilot execution env. |
pkg/workflow/compiler_pre_activation_job.go |
Guards pre-activation App token minting and fallback token resolution. |
pkg/workflow/checkout_step_generator.go |
Adds fallback token expressions for App-authenticated checkout/fetch steps. |
pkg/workflow/activation_github_token_test.go |
Adds activation App ignore-mode generation test coverage. |
pkg/parser/schemas/main_workflow_schema.json |
Extends GitHub App schema with the new missing-secret option. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (3)
pkg/workflow/safe_outputs_app_config.go:471
- This uses step-local
envvalues in the same step'sifcondition. Those values are not available when GitHub Actions evaluates the condition, so activation App token minting is skipped even when the credentials are set, and activation falls back to the default token instead of using the configured App.
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))
pkg/workflow/safe_outputs_app_config.go:519
- When App minting is skipped, this fallback ignores
data.ActivationGitHubToken. Workflows that configure both an activationgithub-tokenfallback and an App in ignore mode will still use onlysecrets.GITHUB_TOKEN, regressing cross-repo or higher-permission activation paths that already rely onon.github-token.
if data.ActivationGitHubApp.shouldIgnoreMissingKey() {
return combineTokenExpressions("${{ steps.activation-app-token.outputs.token }}", "${{ secrets.GITHUB_TOKEN }}")
}
pkg/workflow/compiler_pre_activation_job.go:702
- The ignore-mode fallback for pre-activation skip-if checks also drops
data.ActivationGitHubToken. If users have anon.github-tokenfor authenticated search queries, missing App credentials will fall back to onlyGITHUB_TOKEN, which can make existing cross-repo or elevated-permission skip checks fail.
if data.ActivationGitHubApp.shouldIgnoreMissingKey() {
return combineTokenExpressions(
fmt.Sprintf("${{ steps.%s.outputs.token }}", constants.PreActivationAppTokenStepID),
"${{ secrets.GITHUB_TOKEN }}",
)
- Files reviewed: 10/10 changed files
- Comments generated: 6
| "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." | ||
| }, |
| 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)) |
| 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)) |
| 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) |
| 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 |
| // 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) | ||
| } |
|
@copilot review all comments, update pr title and description |
…ation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot use expression builder to create expressions |
…d wrappers Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in Updated PR title: Updated PR description:
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| 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)) | ||
| } |
There was a problem hiding this comment.
@copilot review expression builder and reuse helpers
There was a problem hiding this comment.
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.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Updated in |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
conclusion(and other downstream) jobs could hard-fail on fork PRs or other contexts where App secrets are unavailable, becauseactions/create-github-app-tokenwas invoked unconditionally. This change adds an explicitmissing-keybehavior ongithub-app, preserving default fail-fast behavior while allowing opt-in graceful degradation.Config model + schema
github-app.missing-keywith supported values:error(default): current behavior, fail when key inputs are empty.ignore: skip minting when key inputs are empty.main_workflow_schema.jsonto document and validate the new field.errorwith warning logging.Token minting guards (all github-app entry points)
on.github-app/ skip-if checks)if+ env indirection so empty secrets do not crash the job.Fallback token resolution when minting is skipped
${{ ... || ... }}chains.missing-key: ignore, token consumers now fall back to existing non-App token chains instead of requiring the minted App token.Illustrative behavior
Generated mint step is guarded, and downstream token usage resolves like: