diff --git a/docs/adr/44444-checkout-safe-output-github-app-auth.md b/docs/adr/44444-checkout-safe-output-github-app-auth.md new file mode 100644 index 00000000000..99bc184c172 --- /dev/null +++ b/docs/adr/44444-checkout-safe-output-github-app-auth.md @@ -0,0 +1,48 @@ +# ADR-44444: Per-Checkout GitHub App Auth for safe_outputs Git Operations + +**Date**: 2026-07-09 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +The `safe_outputs` job performs git operations (checkout, push, PR creation) on behalf of agents. Until this change, these operations used either the global `safe-outputs.github-app` setting or the default `GITHUB_TOKEN`. In cross-repo workflows—where the target repository belongs to a different organization than the workflow's home repo—the default token lacks push access, and the global safe-outputs token is coarse-grained (applies uniformly to all safe_outputs operations regardless of target). Agents that check out an external repo (e.g., `OrgB/target-repo`) need a way to supply a GitHub App credential scoped specifically to that checkout's safe_outputs git operations without altering the agent's own checkout authentication. + +### Decision + +We will add a `checkout.safe-output-github-app` field (with `checkout.safe-outputs-github-app` as a backward-compatible alias) to `CheckoutConfig`. This field carries a `GitHubAppConfig` used exclusively by the `safe_outputs` job when minting tokens for checkout/push operations targeting that repository. Token resolution in `resolvePRCheckoutToken` now checks the checkout manager first—preferring an explicit repository match, then `current: true`, then the default checkout—before falling back to the existing global safe-outputs token chain. The agent job's own checkout authentication is unaffected. + +### Alternatives Considered + +#### Alternative 1: Use the existing global `safe-outputs.github-app` setting + +The global `safe-outputs.github-app` field already supports GitHub App credentials for all safe_outputs operations. Workflows could configure it once and rely on it for cross-repo pushes. + +This was not chosen because the global setting is a blunt instrument: it applies to every safe_outputs operation regardless of target repository, making it unsuitable when multiple checkouts target different organizations with different app registrations. It also conflates the safe_outputs credential with the overall workflow credential, which may have wider permissions than needed for a specific checkout target. + +#### Alternative 2: Add a `github-app` override field directly on each `safe_outputs` operation config + +Each operation (`create-pull-request`, `push-to-pull-request-branch`) could accept its own `github-app` override, keeping auth configuration co-located with the operation that needs it. + +This was not chosen because it would require duplicating the app config for every operation that targets the same repository, and it breaks the intuitive mapping between "a checked-out repository" and "the credentials used to push back to it." Anchoring the credential to the checkout config aligns with how `checkout.github-app` already works for agent auth, and lets the token-resolution logic leverage checkout ordering and the `current: true` flag. + +### Consequences + +#### Positive +- Cross-repo `safe_outputs` operations can use a fine-grained GitHub App token scoped to the exact target repository, without elevating the agent's own checkout credential. +- The resolution precedence (explicit repo match → `current: true` → default checkout) mirrors existing patterns in the checkout manager, keeping the mental model consistent. +- `ignore-if-missing` composes with the new field: when the app installation is absent, the resolver falls back to the global safe-outputs token chain transparently. + +#### Negative +- Each checkout entry can now carry two separate GitHub App configs (`github-app` for agent auth, `safe-output-github-app` for safe_outputs auth), increasing configuration surface area and the potential for user confusion about which credential applies when. +- The backward-compatible `safe-outputs-github-app` alias adds parser complexity and a permanent dual-key code path that must be maintained. + +#### Neutral +- The `resolvePRCheckoutToken` function signature gains a `*CheckoutManager` parameter; all existing callers that passed `nil` previously now pass an empty `NewCheckoutManager(nil)`, preserving existing behavior. +- Tests covering parser, checkout manager, step generator, and token resolution were added alongside the feature code, establishing coverage baselines for the new code paths. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/docs/src/content/docs/reference/checkout.md b/docs/src/content/docs/reference/checkout.md index 7e2e0bd07ec..90dd9070977 100644 --- a/docs/src/content/docs/reference/checkout.md +++ b/docs/src/content/docs/reference/checkout.md @@ -51,7 +51,7 @@ checkout: | `path` | string | Path within `GITHUB_WORKSPACE` to place the checkout. Defaults to workspace root. | | `github-token` | string | Token for authentication. Use `${{ secrets.MY_TOKEN }}` syntax. | | `github-app` | object | GitHub App credentials (`client-id` or `app-id` (deprecated), `private-key`, optional `owner`, `repositories`). Mutually exclusive with `github-token`. `app` is a deprecated alias for the field name. Run `gh aw fix` to auto-migrate `app-id` to `client-id`. | -| `safe-output-github-app` | object | Optional per-checkout GitHub App credentials used exclusively for safe_outputs git operations on this checkout target (`client-id` or `app-id` (deprecated), `private-key`, optional `owner`, `repositories`). Does not change agent/activation checkout auth. See [Cross-Organization safe_outputs Authentication](#cross-organization-safe_outputs-authentication-safe-output-github-app) for cross-org usage. | +| `safe-outputs-github-app` | object | Optional per-checkout GitHub App credentials used exclusively for safe_outputs git operations on this checkout target (`client-id` or `app-id` (deprecated), `private-key`, optional `owner`, `repositories`). Does not change agent/activation checkout auth. See [Cross-Organization safe_outputs Authentication](#cross-organization-safe_outputs-authentication-safe-outputs-github-app) for cross-org usage. | | `fetch-depth` | integer | Commits to fetch. `0` = full history, `1` = shallow clone (default). | | `fetch` | string \| string[] | Additional Git refs to fetch after checkout. See [Fetching Additional Refs](#fetching-additional-refs). | | `sparse-checkout` | string | Newline-separated patterns for sparse checkout (e.g., `.github/\nsrc/`). | @@ -154,11 +154,11 @@ checkout: force-clean-git-credentials: true ``` -## Cross-Organization safe_outputs Authentication (`safe-output-github-app`) +## Cross-Organization safe_outputs Authentication (`safe-outputs-github-app`) By default, the safe_outputs job uses `GITHUB_TOKEN` to check out repositories — `safe-outputs.github-app` and `safe-outputs.github-token` are **not** used for checkout and only govern PR/push API operations. This design prevents cross-org token confusion when the safe-outputs app is scoped to a target organization that differs from the workflow repository's organization. -For workflows that target a different organization, use `safe-output-github-app` on the relevant checkout entry to supply a checkout token for the safe_outputs job: +For workflows that target a different organization, use `safe-outputs-github-app` on the relevant checkout entry to supply a checkout token for the safe_outputs job: ```yaml wrap checkout: @@ -167,7 +167,7 @@ checkout: client-id: ${{ vars.APP_CLIENT_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} owner: OrgB - safe-output-github-app: + safe-outputs-github-app: client-id: ${{ vars.APP_CLIENT_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} owner: OrgB @@ -184,7 +184,7 @@ safe-outputs: In this configuration: - The agent job checks out `OrgB/target-repo` using the app token scoped to `OrgB`. -- The safe_outputs job checks out `OrgB/target-repo` using the `safe-output-github-app` token. +- The safe_outputs job checks out `OrgB/target-repo` using the `safe-outputs-github-app` token. - The safe_outputs job checks out the **workflow repository** using `GITHUB_TOKEN` (the default), not the `OrgB`-scoped app token — avoiding the cross-org authentication failure. - `safe-outputs.github-app` is used only for PR creation and push operations, not for checkout. diff --git a/docs/src/content/docs/specs/checkout-behavior-specification.md b/docs/src/content/docs/specs/checkout-behavior-specification.md index 3f8f7238c39..ea700f7b7a8 100644 --- a/docs/src/content/docs/specs/checkout-behavior-specification.md +++ b/docs/src/content/docs/specs/checkout-behavior-specification.md @@ -82,11 +82,11 @@ The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "S ### 3.1 Checkout Entry Parsing `checkout:` MUST accept either a single object or an array of objects. -Each entry MAY define: `repository`, `ref`, `path`, `github-token` (or legacy `token`), `github-app` (or deprecated alias `app`), `safe-output-github-app`, `fetch-depth`, `fetch`, `sparse-checkout`, `submodules`, `lfs`, `current`, `wiki`, and `force-clean-git-credentials`. +Each entry MAY define: `repository`, `ref`, `path`, `github-token` (or legacy `token`), `github-app` (or deprecated alias `app`), `safe-outputs-github-app`, `fetch-depth`, `fetch`, `sparse-checkout`, `submodules`, `lfs`, `current`, `wiki`, and `force-clean-git-credentials`. `github-token` and `github-app` MUST be mutually exclusive per entry. -`safe-output-github-app` applies only to safe_outputs git auth/token resolution. It MUST NOT change agent/activation checkout authentication behavior. +`safe-outputs-github-app` applies only to safe_outputs git auth/token resolution. It MUST NOT change agent/activation checkout authentication behavior. ### 3.2 Entry Merge Rules @@ -95,7 +95,7 @@ Entries with the same `(repository, path, wiki)` key MUST merge with these rules - `fetch-depth`: deepest wins (`0` wins over all) - `ref`: first non-empty wins - auth (`github-token` vs `github-app`): first auth wins -- safe_outputs auth (`safe-output-github-app`): first non-empty wins +- safe_outputs auth (`safe-outputs-github-app`): first non-empty wins - sparse patterns: union - fetch refs: union - `lfs`: OR @@ -144,7 +144,7 @@ Activation token precedence MUST be: `resolvePRCheckoutToken` precedence MUST be: -1. Checkout target `safe-output-github-app` minted token (with fallback chain when `ignore-if-missing: true`) +1. Checkout target `safe-outputs-github-app` minted token (with fallback chain when `ignore-if-missing: true`) 2. `${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }}` When safe_outputs checkout retention is enabled, checkouts without explicit entry tokens MUST persist the resolved PR checkout token so local git credentials match push/fetch token usage. @@ -236,8 +236,8 @@ Effective side-repo token precedence MUST be: - **T-CHK-008**: Trial mode repository/token override behavior - **T-CHK-009**: Side-repo target extraction and auth precedence - **T-CHK-010**: `force-clean-git-credentials` cleanup covers `.git/modules/**/config` -- **T-CHK-011**: `checkout.safe-output-github-app` is the sole supported safe_outputs auth override per checkout entry -- **T-CHK-012**: safe_outputs checkout token MUST NOT use `safe-outputs.github-app` or `safe-outputs.github-token`; only `safe-output-github-app` (per entry) or `GITHUB_TOKEN` are permitted +- **T-CHK-011**: `checkout.safe-outputs-github-app` is the sole supported safe_outputs auth override per checkout entry +- **T-CHK-012**: safe_outputs checkout token MUST NOT use `safe-outputs.github-app` or `safe-outputs.github-token`; only `safe-outputs-github-app` (per entry) or `GITHUB_TOKEN` are permitted - **T-CHK-013**: Checkout-manifest generation includes safe_outputs auth metadata without persisting resolved tokens ### 7.2 Compliance Checklist diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index e2b8f34b67f..8cdd286cfaa 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -13170,6 +13170,10 @@ "$ref": "#/$defs/github_app", "description": "GitHub App authentication. Mints a short-lived installation access token via actions/create-github-app-token. Mutually exclusive with github-token." }, + "safe-outputs-github-app": { + "$ref": "#/$defs/github_app", + "description": "GitHub App authentication used only by safe_outputs checkout/fetch/push operations for this checkout target. Does not change activation/agent checkout authentication." + }, "current": { "type": "boolean", "description": "Marks this checkout as the logical current repository for the workflow. When set to true, the AI agent will treat this repository as its primary working target. Only one checkout may have current set to true. Useful for central-repo workflows targeting a different repository." diff --git a/pkg/workflow/checkout_config_parser.go b/pkg/workflow/checkout_config_parser.go index 8a7855236b2..f03d40866b3 100644 --- a/pkg/workflow/checkout_config_parser.go +++ b/pkg/workflow/checkout_config_parser.go @@ -138,6 +138,29 @@ func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) { } } + // Parse app configuration for safe_outputs-only GitHub App authentication. + parseSafeOutputAppConfig := func(fieldName string, value any) (*GitHubAppConfig, error) { + appMap, ok := value.(map[string]any) + if !ok { + return nil, fmt.Errorf("checkout.%s must be an object", fieldName) + } + appConfig := parseAppConfig(appMap) + if appConfig.AppID == "" || appConfig.PrivateKey == "" { + return nil, fmt.Errorf("checkout.%s requires both client-id (or app-id) and private-key", fieldName) + } + return appConfig, nil + } + if v, ok := m["safe-outputs-github-app"]; ok { + appConfig, err := parseSafeOutputAppConfig("safe-outputs-github-app", v) + if err != nil { + return nil, err + } + cfg.SafeOutputGitHubApp = appConfig + } + if _, ok := m["safe-output-github-app"]; ok { + return nil, errors.New("checkout.safe-output-github-app is not supported; use checkout.safe-outputs-github-app") + } + // Validate mutual exclusivity of github-token and github-app if cfg.GitHubToken != "" && cfg.GitHubApp != nil { checkoutManagerLog.Print("Rejecting checkout config: github-token and github-app are mutually exclusive") diff --git a/pkg/workflow/checkout_manager.go b/pkg/workflow/checkout_manager.go index 10088468bc2..eccef6c590b 100644 --- a/pkg/workflow/checkout_manager.go +++ b/pkg/workflow/checkout_manager.go @@ -1,6 +1,7 @@ package workflow import ( + "fmt" "strings" "github.com/github/gh-aw/pkg/logger" @@ -63,6 +64,11 @@ type CheckoutConfig struct { // Mutually exclusive with GitHubToken. GitHubApp *GitHubAppConfig `json:"github-app,omitempty"` + // SafeOutputGitHubApp configures GitHub App-based authentication used only by + // safe_outputs git checkout/fetch/push operations for this checkout target. + // This does not change activation/agent checkout authentication. + SafeOutputGitHubApp *GitHubAppConfig `json:"safe-outputs-github-app,omitempty"` + // FetchDepth controls the number of commits to fetch. // 0 fetches all history (full clone). 1 is a shallow clone (default). FetchDepth *int `json:"fetch-depth,omitempty"` @@ -127,6 +133,7 @@ type resolvedCheckout struct { ref string // last non-empty ref wins token string // last non-empty github-token wins githubApp *GitHubAppConfig // GitHub App config (first non-nil wins) + safeOutputApp *GitHubAppConfig // safe_outputs-only GitHub App config (first non-nil wins) fetchDepth *int // nil means use default (1) sparsePatterns []string // merged sparse-checkout patterns submodules string @@ -291,6 +298,9 @@ func (cm *CheckoutManager) add(cfg *CheckoutConfig) { if cfg.GitHubApp != nil && entry.githubApp == nil && entry.token == "" { entry.githubApp = cfg.GitHubApp // first-seen auth wins (mutually exclusive with github-token) } + if cfg.SafeOutputGitHubApp != nil && entry.safeOutputApp == nil { + entry.safeOutputApp = cfg.SafeOutputGitHubApp // first-seen safe_outputs auth wins + } if cfg.SparseCheckout != "" { entry.sparsePatterns = mergeSparsePatterns(entry.sparsePatterns, cfg.SparseCheckout) } @@ -312,15 +322,16 @@ func (cm *CheckoutManager) add(cfg *CheckoutConfig) { checkoutManagerLog.Printf("Merged checkout for path=%q repository=%q", key.path, key.repository) } else { entry := &resolvedCheckout{ - key: key, - ref: cfg.Ref, - token: cfg.GitHubToken, - githubApp: cfg.GitHubApp, - fetchDepth: cfg.FetchDepth, - submodules: cfg.Submodules, - lfs: cfg.LFS, - current: cfg.Current, - cleanCreds: cfg.CleanGitCredentials, + key: key, + ref: cfg.Ref, + token: cfg.GitHubToken, + githubApp: cfg.GitHubApp, + safeOutputApp: cfg.SafeOutputGitHubApp, + fetchDepth: cfg.FetchDepth, + submodules: cfg.Submodules, + lfs: cfg.LFS, + current: cfg.Current, + cleanCreds: cfg.CleanGitCredentials, } if cfg.SparseCheckout != "" { entry.sparsePatterns = mergeSparsePatterns(nil, cfg.SparseCheckout) @@ -362,6 +373,68 @@ func (cm *CheckoutManager) HasAppAuth() bool { return false } +// HasSafeOutputAppAuth returns true if any checkout entry uses safe_outputs-only +// GitHub App authentication. +func (cm *CheckoutManager) HasSafeOutputAppAuth() bool { + for _, entry := range cm.ordered { + if entry.safeOutputApp != nil { + return true + } + } + return false +} + +// ResolveSafeOutputCheckoutTokenExpression returns a safe_outputs checkout token +// expression derived from checkout.safe-outputs-github-app for the target repo. +// The selected checkout precedence is: +// 1. explicit matching checkout.repository == targetRepo (when targetRepo is non-empty) +// 2. checkout marked current: true +// 3. default checkout override (workspace root) +func (cm *CheckoutManager) ResolveSafeOutputCheckoutTokenExpression(targetRepo string) (string, bool) { + findSafeOutputAppCheckoutIndex := func() int { + targetRepo = strings.TrimSpace(targetRepo) + if targetRepo != "" && targetRepo != "*" { + for idx, entry := range cm.ordered { + if entry.key.wiki || entry.safeOutputApp == nil { + continue + } + if entry.key.repository == targetRepo { + return idx + } + } + } + + for idx, entry := range cm.ordered { + if entry.key.wiki || entry.safeOutputApp == nil { + continue + } + if entry.current { + return idx + } + } + + if override := cm.GetDefaultCheckoutOverride(); override != nil && !override.key.wiki && override.safeOutputApp != nil { + if idx, ok := cm.index[override.key]; ok { + return idx + } + } + return -1 + } + + idx := findSafeOutputAppCheckoutIndex() + if idx < 0 { + return "", false + } + + //nolint:gosec // G101: False positive - this is a GitHub Actions expression template placeholder, not a hardcoded credential + token := fmt.Sprintf("${{ steps.checkout-safe-output-app-token-%d.outputs.token }}", idx) + app := cm.ordered[idx].safeOutputApp + if app != nil && app.shouldIgnoreMissingKey() { + token = combineTokenExpressions(token, getEffectiveSafeOutputGitHubToken("")) + } + return token, true +} + // resolveCheckoutPermissions determines the permissions used when minting checkout // GitHub App tokens. Both the agent job and the safe_outputs job resolve them the same // way: explicit cached permissions take precedence, then parsed frontmatter permissions, diff --git a/pkg/workflow/checkout_manager_test.go b/pkg/workflow/checkout_manager_test.go index 4ca1970d228..9ccdaaf5233 100644 --- a/pkg/workflow/checkout_manager_test.go +++ b/pkg/workflow/checkout_manager_test.go @@ -490,6 +490,35 @@ func TestParseCheckoutConfigs(t *testing.T) { assert.Equal(t, "${{ vars.CLIENT_ID }}", configs[0].GitHubApp.AppID, "client-id should populate AppID") }) + t.Run("safe-outputs-github-app config is parsed", func(t *testing.T) { + raw := map[string]any{ + "repository": "owner/target-repo", + "safe-outputs-github-app": map[string]any{ + "client-id": "${{ vars.SO_CLIENT_ID }}", + "private-key": "${{ secrets.SO_APP_PRIVATE_KEY }}", + }, + } + configs, err := ParseCheckoutConfigs(raw) + require.NoError(t, err, "safe-outputs-github-app config should parse without error") + require.Len(t, configs, 1) + require.NotNil(t, configs[0].SafeOutputGitHubApp, "safe-outputs-github-app config should be set") + assert.Equal(t, "${{ vars.SO_CLIENT_ID }}", configs[0].SafeOutputGitHubApp.AppID, "client-id should populate AppID") + assert.Equal(t, "${{ secrets.SO_APP_PRIVATE_KEY }}", configs[0].SafeOutputGitHubApp.PrivateKey, "private-key should be set") + }) + + t.Run("safe-output-github-app is rejected", func(t *testing.T) { + raw := map[string]any{ + "repository": "owner/target-repo", + "safe-output-github-app": map[string]any{ + "app-id": "${{ vars.SO_APP_ID }}", + "private-key": "${{ secrets.SO_APP_PRIVATE_KEY }}", + }, + } + _, err := ParseCheckoutConfigs(raw) + require.Error(t, err, "safe-output-github-app should be rejected") + assert.Contains(t, err.Error(), "checkout.safe-output-github-app is not supported; use checkout.safe-outputs-github-app") + }) + t.Run("github-token and github-app are mutually exclusive", func(t *testing.T) { raw := map[string]any{ "github-token": "${{ secrets.MY_TOKEN }}", @@ -1134,6 +1163,105 @@ func TestHasAppAuth(t *testing.T) { }) } +func TestHasSafeOutputAppAuth(t *testing.T) { + t.Run("returns false when no safe-output app configured", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {GitHubToken: "${{ secrets.MY_PAT }}"}, + }) + assert.False(t, cm.HasSafeOutputAppAuth(), "should be false when no safe-output app is configured") + }) + + t.Run("returns true when checkout has safe-output app", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + { + Repository: "owner/target-repo", + SafeOutputGitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.KEY }}", + }, + }, + }) + assert.True(t, cm.HasSafeOutputAppAuth(), "should be true when any checkout has safe-output app") + }) +} + +func TestResolveSafeOutputCheckoutTokenExpression(t *testing.T) { + t.Run("uses target-repo matching checkout safe-output app", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {Repository: "owner/a", Path: "./a"}, + { + Repository: "owner/b", + Path: "./b", + SafeOutputGitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.KEY }}", + }, + }, + }) + token, ok := cm.ResolveSafeOutputCheckoutTokenExpression("owner/b") + require.True(t, ok) + assert.Equal(t, "${{ steps.checkout-safe-output-app-token-1.outputs.token }}", token) + }) + + t.Run("falls back to current checkout when target repo not provided", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + { + Repository: "owner/current", + Current: true, + SafeOutputGitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.KEY }}", + }, + }, + }) + token, ok := cm.ResolveSafeOutputCheckoutTokenExpression("") + require.True(t, ok) + assert.Equal(t, "${{ steps.checkout-safe-output-app-token-0.outputs.token }}", token) + }) + + t.Run("ignore-if-missing uses default safe output token fallback", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + { + Repository: "owner/current", + Current: true, + SafeOutputGitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.KEY }}", + IgnoreIfMissing: true, + }, + }, + }) + token, ok := cm.ResolveSafeOutputCheckoutTokenExpression("") + require.True(t, ok) + assert.Equal( + t, + "${{ steps.checkout-safe-output-app-token-0.outputs.token || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }}", + token, + ) + }) +} + +func TestGenerateSafeOutputCheckoutAppTokenSteps(t *testing.T) { + compiler := NewCompiler() + permissions := NewPermissions() + + cm := NewCheckoutManager([]*CheckoutConfig{ + { + Repository: "owner/target", + SafeOutputGitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_KEY }}", + }, + }, + }) + + steps := cm.GenerateSafeOutputCheckoutAppTokenSteps(compiler, permissions) + require.NotEmpty(t, steps) + combined := strings.Join(steps, "") + assert.Contains(t, combined, "id: checkout-safe-output-app-token-0") + assert.Contains(t, combined, "Generate safe_outputs GitHub App token for checkout (0)") +} + func TestDefaultCheckoutWithAppAuth(t *testing.T) { getPin := func(ref string) string { return ref } diff --git a/pkg/workflow/checkout_step_generator.go b/pkg/workflow/checkout_step_generator.go index e3af1e11ca8..03e3a8aa660 100644 --- a/pkg/workflow/checkout_step_generator.go +++ b/pkg/workflow/checkout_step_generator.go @@ -51,6 +51,29 @@ func (cm *CheckoutManager) GenerateCheckoutAppTokenSteps(c *Compiler, permission return steps } +// GenerateSafeOutputCheckoutAppTokenSteps generates GitHub App token minting steps +// for checkout.safe-outputs-github-app entries. These steps are consumed only by the +// safe_outputs job when choosing the checkout/push token for PR operations. +func (cm *CheckoutManager) GenerateSafeOutputCheckoutAppTokenSteps(c *Compiler, permissions *Permissions) []string { + checkoutManagerLog.Printf("Building safe_outputs app token minting steps for %d checkout entries", len(cm.ordered)) + var steps []string + for checkoutIndex, entry := range cm.ordered { + if entry.safeOutputApp == nil { + continue + } + checkoutManagerLog.Printf("Generating safe_outputs app token minting step for checkout index=%d repo=%q", checkoutIndex, entry.key.repository) + steps = append(steps, collapseYAMLLinesIntoSteps(c.buildGitHubAppTokenMintStepWithMeta( + entry.safeOutputApp, + permissions, + "", + entry.key.repository, + fmt.Sprintf("Generate safe_outputs GitHub App token for checkout (%d)", checkoutIndex), + fmt.Sprintf("checkout-safe-output-app-token-%d", checkoutIndex), + ))...) + } + return steps +} + func collapseYAMLLinesIntoSteps(lines []string) []string { if len(lines) == 0 { return nil diff --git a/pkg/workflow/compiler_safe_outputs_steps.go b/pkg/workflow/compiler_safe_outputs_steps.go index cb6f5e49bb6..f714c56d3a4 100644 --- a/pkg/workflow/compiler_safe_outputs_steps.go +++ b/pkg/workflow/compiler_safe_outputs_steps.go @@ -44,8 +44,8 @@ func (c *Compiler) buildSharedPRCheckoutSteps(data *WorkflowData) []string { // removes the need for the handlers to inject a separate http.extraheader. The // same token is reused below for the "Configure Git credentials" step so both the // persisted checkout credential and the push remote agree on a single token. - prCheckoutToken, _ := resolvePRCheckoutToken(data.SafeOutputs) - checkoutMgr.SetPushToken(prCheckoutToken) + prCheckoutToken, _ := resolvePRCheckoutToken(data.SafeOutputs, checkoutMgr) + checkoutMgr.SetPushToken(resolveStaticCheckoutToken(data.SafeOutputs, checkoutMgr)) // Combined condition: run the checkout/git-config steps only when a create_pull_request // or push_to_pull_request_branch output will be processed. @@ -59,6 +59,9 @@ func (c *Compiler) buildSharedPRCheckoutSteps(data *WorkflowData) []string { if checkoutMgr.HasAppAuth() { steps = append(steps, injectStepCondition(checkoutMgr.GenerateCheckoutAppTokenSteps(c, resolveCheckoutPermissions(data)), condition)...) } + if checkoutMgr.HasSafeOutputAppAuth() { + steps = append(steps, checkoutMgr.GenerateSafeOutputCheckoutAppTokenSteps(c, resolveCheckoutPermissions(data))...) + } // Default workspace checkout (identical to the agent job). steps = append(steps, injectStepCondition( @@ -257,7 +260,7 @@ func (c *Compiler) buildHandlerManagerStep(data *WorkflowData) ([]string, error) // scenarios (allowed-repos). Without this, the handler falls back to the default // repo-scoped token which lacks access to other repos. if usesPatchesAndCheckouts(data.SafeOutputs) { - gitToken, isCustom := resolvePRCheckoutToken(data.SafeOutputs) + gitToken, isCustom := resolvePRCheckoutToken(data.SafeOutputs, NewCheckoutManager(data.CheckoutConfigs)) // Only override GITHUB_TOKEN when a custom token (app or PAT) is explicitly configured. // When no custom token is set, the default repo-scoped GITHUB_TOKEN from GitHub Actions // is already in the environment and overriding it with the same default is unnecessary. diff --git a/pkg/workflow/compiler_safe_outputs_steps_test.go b/pkg/workflow/compiler_safe_outputs_steps_test.go index 9ccdfa4f8e8..cfe2a8b0568 100644 --- a/pkg/workflow/compiler_safe_outputs_steps_test.go +++ b/pkg/workflow/compiler_safe_outputs_steps_test.go @@ -76,6 +76,32 @@ func TestBuildSharedPRCheckoutSteps(t *testing.T) { "token: ${{ steps.checkout-app-token-0.outputs.token }}", }, }, + { + name: "safe-output checkout app token is minted and used for git credentials", + safeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + TargetRepoSlug: "org/target-repo", + }, + }, + checkoutConfigs: []*CheckoutConfig{ + { + Repository: "org/target-repo", + Path: "./target-repo", + SafeOutputGitHubApp: &GitHubAppConfig{ + AppID: "12345", + PrivateKey: "test-key", + }, + }, + }, + checkContains: []string{ + "id: checkout-safe-output-app-token-0", + "token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }}", + "GIT_TOKEN: ${{ steps.checkout-safe-output-app-token-0.outputs.token }}", + }, + checkNotContains: []string{ + "id: checkout-safe-output-app-token-0\n if:", + }, + }, { name: "trial mode with target repo", trialMode: true, diff --git a/pkg/workflow/github_token.go b/pkg/workflow/github_token.go index eddbdc37c3c..e9f5ea15f64 100644 --- a/pkg/workflow/github_token.go +++ b/pkg/workflow/github_token.go @@ -117,9 +117,10 @@ func getEffectiveProjectGitHubToken(customToken string) string { // Applies the following precedence (highest to lowest): // 1. Per-config PAT: create-pull-request.github-token // 2. Per-config PAT: push-to-pull-request-branch.github-token -// 3. GitHub App minted token (if a github-app is configured) -// 4. safe-outputs level PAT: safe-outputs.github-token -// 5. Default fallback via getEffectiveSafeOutputGitHubToken() +// 3. Checkout-scoped safe-output GitHub App token (if configured for matching checkout) +// 4. safe-outputs GitHub App minted token (if a safe-outputs github-app is configured) +// 5. safe-outputs level PAT: safe-outputs.github-token +// 6. Default fallback via getEffectiveSafeOutputGitHubToken() // // Per-config tokens take precedence over the GitHub App so that individual operations // can override the app-wide authentication with a dedicated PAT when needed. @@ -127,7 +128,7 @@ func getEffectiveProjectGitHubToken(customToken string) string { // Returns: // - token: the effective GitHub Actions token expression to use for git operations // - isCustom: true when a custom non-default token was explicitly configured (per-config PAT, app, or safe-outputs PAT) -func resolvePRCheckoutToken(safeOutputs *SafeOutputsConfig) (token string, isCustom bool) { +func resolvePRCheckoutToken(safeOutputs *SafeOutputsConfig, checkoutMgr *CheckoutManager) (token string, isCustom bool) { if safeOutputs == nil { return getEffectiveSafeOutputGitHubToken(""), false } @@ -150,6 +151,13 @@ func resolvePRCheckoutToken(safeOutputs *SafeOutputsConfig) (token string, isCus return getEffectiveSafeOutputGitHubToken(perConfigToken), true } + if checkoutMgr != nil { + targetRepo := resolvePRCheckoutTargetRepo(safeOutputs) + if checkoutToken, ok := checkoutMgr.ResolveSafeOutputCheckoutTokenExpression(targetRepo); ok { + return checkoutToken, true + } + } + // GitHub App token takes precedence over the safe-outputs level PAT if safeOutputs.GitHubApp != nil { if safeOutputs.GitHubApp.shouldIgnoreMissingKey() { @@ -169,6 +177,23 @@ func resolvePRCheckoutToken(safeOutputs *SafeOutputsConfig) (token string, isCus return getEffectiveSafeOutputGitHubToken(""), false } +func resolvePRCheckoutTargetRepo(safeOutputs *SafeOutputsConfig) string { + if safeOutputs == nil { + return "" + } + if safeOutputs.CreatePullRequests != nil { + if repo := strings.TrimSpace(safeOutputs.CreatePullRequests.TargetRepoSlug); repo != "" && repo != "*" { + return repo + } + } + if safeOutputs.PushToPullRequestBranch != nil { + if repo := strings.TrimSpace(safeOutputs.PushToPullRequestBranch.TargetRepoSlug); repo != "" && repo != "*" { + return repo + } + } + return "" +} + // resolveStaticCheckoutToken returns the effective checkout token as a static GitHub Actions // expression (secret reference or default). Unlike resolvePRCheckoutToken, this function // never returns a step-output expression because step outputs are not accessible outside the job diff --git a/pkg/workflow/github_token_test.go b/pkg/workflow/github_token_test.go index 3b1738531a3..23a0684cc24 100644 --- a/pkg/workflow/github_token_test.go +++ b/pkg/workflow/github_token_test.go @@ -176,3 +176,77 @@ func TestCombineTokenExpressions(t *testing.T) { }) } } + +func TestResolvePRCheckoutToken(t *testing.T) { + t.Run("uses checkout safe-output app token when target-repo matches", func(t *testing.T) { + safeOutputs := &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + TargetRepoSlug: "owner/target", + }, + } + checkoutMgr := NewCheckoutManager([]*CheckoutConfig{ + { + Repository: "owner/target", + SafeOutputGitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_KEY }}", + }, + }, + }) + + token, isCustom := resolvePRCheckoutToken(safeOutputs, checkoutMgr) + if token != "${{ steps.checkout-safe-output-app-token-0.outputs.token }}" { + t.Fatalf("expected checkout safe-output app token, got %q", token) + } + if !isCustom { + t.Fatalf("expected isCustom=true") + } + }) + + t.Run("falls back to previous precedence when no checkout safe-output app exists", func(t *testing.T) { + safeOutputs := &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + GitHubToken: "${{ secrets.PR_PAT }}", + }, + }, + GitHubToken: "${{ secrets.SAFE_OUTPUTS_PAT }}", + } + + token, isCustom := resolvePRCheckoutToken(safeOutputs, NewCheckoutManager(nil)) + if token != "${{ secrets.PR_PAT }}" { + t.Fatalf("expected per-config PAT, got %q", token) + } + if !isCustom { + t.Fatalf("expected isCustom=true") + } + }) + + t.Run("per-config PAT takes precedence over checkout safe-output app token", func(t *testing.T) { + safeOutputs := &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + GitHubToken: "${{ secrets.PR_PAT }}", + }, + TargetRepoSlug: "owner/target", + }, + } + checkoutMgr := NewCheckoutManager([]*CheckoutConfig{ + { + Repository: "owner/target", + SafeOutputGitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_KEY }}", + }, + }, + }) + + token, isCustom := resolvePRCheckoutToken(safeOutputs, checkoutMgr) + if token != "${{ secrets.PR_PAT }}" { + t.Fatalf("expected per-config PAT token, got %q", token) + } + if !isCustom { + t.Fatalf("expected isCustom=true") + } + }) +}