diff --git a/docs/adr/37976-derive-github-app-owner-from-checkout-repository.md b/docs/adr/37976-derive-github-app-owner-from-checkout-repository.md new file mode 100644 index 00000000000..9d57c0af5e5 --- /dev/null +++ b/docs/adr/37976-derive-github-app-owner-from-checkout-repository.md @@ -0,0 +1,43 @@ +# ADR-37976: Derive Omitted GitHub App Owner from `checkout.repository` + +**Date**: 2026-06-09 +**Status**: Draft + +## Context + +When an agentic workflow authenticates with a GitHub App (`actions/create-github-app-token`), the minted token must be scoped to an installation owner. Until now, the `github-app.owner` field could only be a static literal; when omitted, the framework always defaulted to `${{ github.repository_owner }}` (the repository running the workflow). This broke multi-org workflows that target `owner/repo` pairs in *other* organizations at dispatch time — the token was minted against the wrong owner, so app-token minting failed. Authors worked around this by passing a redundant `trigger_org` input duplicating information already present in `checkout.repository`. + +## Decision + +We will derive the GitHub App installation owner from the single effective `checkout.repository` value when `github-app.owner` is omitted. For literal `owner/repo` values, the owner is extracted at compile time and emitted directly. For expression-based repositories (e.g. `${{ github.event.inputs.trigger_ref }}`), the compiler emits a generated pre-step that extracts the owner at runtime via `GITHUB_OUTPUT` and feeds it into the token-minting step. An explicit `github-app.owner` always takes precedence, and when no checkout repository is derivable the behavior falls back unchanged to `${{ github.repository_owner }}`. Derivation only applies when exactly one distinct repository is configured across checkouts; multiple distinct repositories yield no derivation. This applies to `checkout[*].github-app`, top-level `safe-outputs.github-app`, and `tools.github.github-app`. + +## Alternatives Considered + +### Alternative 1: Require an explicit `owner:` or `trigger_org` input +We could keep the status quo and require authors to always specify the owner explicitly (via `github-app.owner` or a `trigger_org` workflow input). This was rejected because it duplicates information already present in `checkout.repository`, is error-prone (the two values can drift), and adds boilerplate to every multi-org workflow. + +### Alternative 2: Always default to `github.repository_owner` +We could leave the default as the current repository owner and treat multi-org targeting as unsupported. This was rejected because it makes cross-org dispatch workflows impossible without manual overrides, which is the exact use case this change enables. + +### Alternative 3: Derive from any/all checkout repositories +We could attempt to derive an owner even when multiple distinct repositories are checked out (e.g. pick the first). This was rejected as ambiguous — minting a single token against one of several owners would silently choose the wrong installation. Restricting derivation to a single distinct repository keeps the inference unambiguous. + +## Consequences + +### Positive +- Multi-org workflows can mint correctly-scoped App tokens without a redundant `trigger_org` input. +- The derived owner stays in sync with `checkout.repository`, eliminating drift between two manually-maintained values. +- Token-minting step generation now uses stable step IDs directly (`checkout-app-token-N`, `github-mcp-app-token`, `safe-outputs-app-token`) instead of string-replacing generic safe-output step metadata, making the generator easier to reason about. + +### Negative +- Expression-based repositories now emit an additional generated runtime pre-step (`Derive GitHub App owner`), increasing the size and complexity of generated workflow YAML. +- Owner derivation is silently skipped when multiple distinct repositories are configured; authors targeting several orgs in one workflow must still set `owner:` explicitly and may not realize derivation did not apply. + +### Neutral +- A new internal helper file (`github_app_owner_derivation.go`) centralizes owner inference, literal extraction, and runtime-step generation. +- The `buildGitHubAppTokenMintStep*` helper family gains an `ownerSourceRepository` parameter threaded through checkout, GitHub MCP, and safe-outputs token generation; existing call sites pass `""` to preserve prior behavior. +- Activation / pre-activation token owner behavior is intentionally left unchanged. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/27177149721) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* diff --git a/pkg/workflow/checkout_step_generator.go b/pkg/workflow/checkout_step_generator.go index 99c0eb36542..45020086ac8 100644 --- a/pkg/workflow/checkout_step_generator.go +++ b/pkg/workflow/checkout_step_generator.go @@ -37,16 +37,14 @@ func (cm *CheckoutManager) GenerateCheckoutAppTokenSteps(c *Compiler, permission checkoutManagerLog.Printf("Generating app token minting step for checkout index=%d repo=%q", i, entry.key.repository) // Pass empty fallback so the app token defaults to github.event.repository.name. // Checkout-specific cross-repo scoping is handled via the explicit repository field. - appSteps := c.buildGitHubAppTokenMintStep(entry.githubApp, permissions, "") - stepID := fmt.Sprintf("checkout-app-token-%d", i) - for _, step := range appSteps { - modified := strings.ReplaceAll(step, "id: safe-outputs-app-token", "id: "+stepID) - // Rename the step to make it unique when multiple checkouts use app auth. - // This prevents duplicate step name errors when more than one checkout entry - // falls back to the top-level github-app (or has its own github-app configured). - modified = strings.ReplaceAll(modified, "name: Generate GitHub App token", fmt.Sprintf("name: Generate GitHub App token for checkout (%d)", i)) - steps = append(steps, modified) - } + steps = append(steps, c.buildGitHubAppTokenMintStepWithMeta( + entry.githubApp, + permissions, + "", + entry.key.repository, + fmt.Sprintf("Generate GitHub App token for checkout (%d)", i), + fmt.Sprintf("checkout-app-token-%d", i), + )...) } return steps } diff --git a/pkg/workflow/compiler_github_mcp_steps.go b/pkg/workflow/compiler_github_mcp_steps.go index e911088ec08..6b28209b233 100644 --- a/pkg/workflow/compiler_github_mcp_steps.go +++ b/pkg/workflow/compiler_github_mcp_steps.go @@ -136,15 +136,15 @@ func (c *Compiler) generateGitHubMCPAppTokenMintingSteps(data *WorkflowData) []s } // Generate the token minting step using the existing helper from safe_outputs_app.go - rawSteps := c.buildGitHubAppTokenMintStep(app, permissions, "") - - // Replace the default step ID with github-mcp-app-token to differentiate it from - // the safe-outputs app token. - var steps []string - for _, step := range rawSteps { - steps = append(steps, strings.ReplaceAll(step, "id: safe-outputs-app-token", "id: github-mcp-app-token")) - } - return steps + rawSteps := c.buildGitHubAppTokenMintStepWithMeta( + app, + permissions, + "", + inferSingleCheckoutRepositoryForGitHubAppOwner(data), + "Generate GitHub App token", + "github-mcp-app-token", + ) + return rawSteps } // generateParseGuardVarsStep generates a step that parses the blocked-users, trusted-users, and diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index 11707bd064f..423c4013244 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -436,7 +436,12 @@ func (c *Compiler) buildSafeOutputsJobFromParts( if hasWorkflowCallTrigger(data.On) { appTokenFallbackRepo = "${{ needs.activation.outputs.target_repo_name }}" } - appTokenSteps := c.buildGitHubAppTokenMintStep(data.SafeOutputs.GitHubApp, permissions, appTokenFallbackRepo) + appTokenSteps := c.buildGitHubAppTokenMintStepForRepository( + data.SafeOutputs.GitHubApp, + permissions, + appTokenFallbackRepo, + inferSingleCheckoutRepositoryForGitHubAppOwner(data), + ) // Calculate insertion index: after setup action (if present) and artifact downloads, but before checkout and safe output steps insertIndex := 0 diff --git a/pkg/workflow/compiler_yaml_step_generation.go b/pkg/workflow/compiler_yaml_step_generation.go index df3e63d18ea..3a0cbce1d54 100644 --- a/pkg/workflow/compiler_yaml_step_generation.go +++ b/pkg/workflow/compiler_yaml_step_generation.go @@ -135,7 +135,7 @@ func (c *Compiler) generateOTLPOIDCMintStep(data *WorkflowData) []string { if app := getOTLPGitHubAppTokenConfig(data.RawFrontmatter); app != nil { compilerYamlStepGenerationLog.Print("Generating OTLP GitHub App token mint step before setup") - return c.buildGitHubAppTokenMintStepWithMeta(app, nil, "", "Mint OTLP GitHub App token", "mint-otlp-oidc-token") + return c.buildGitHubAppTokenMintStepWithMeta(app, nil, "", "", "Mint OTLP GitHub App token", "mint-otlp-oidc-token") } githubApp := getOTLPGitHubApp(data.ParsedFrontmatter, data.RawFrontmatter) diff --git a/pkg/workflow/github_app_owner_derivation.go b/pkg/workflow/github_app_owner_derivation.go new file mode 100644 index 00000000000..74a011ea82c --- /dev/null +++ b/pkg/workflow/github_app_owner_derivation.go @@ -0,0 +1,111 @@ +package workflow + +import ( + "fmt" + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var githubAppOwnerDerivationLog = logger.New("workflow:github_app_owner_derivation") + +// inferSingleCheckoutRepositoryForGitHubAppOwner returns the single explicit checkout.repository +// value when the workflow targets exactly one distinct repository. It ignores the default checkout +// of the current repository and returns an empty string when multiple distinct repositories are +// configured. +func inferSingleCheckoutRepositoryForGitHubAppOwner(data *WorkflowData) string { + if data == nil { + return "" + } + + checkoutMgr := NewCheckoutManager(data.CheckoutConfigs) + var repository string + for _, entry := range checkoutMgr.ordered { + if entry.key.repository == "" { + continue + } + if repository == "" { + githubAppOwnerDerivationLog.Printf("Using checkout.repository %q as GitHub App owner source candidate", entry.key.repository) + repository = entry.key.repository + continue + } + if entry.key.repository != repository { + githubAppOwnerDerivationLog.Printf("Cannot infer a single GitHub App owner source: found multiple checkout repositories (%q, %q)", repository, entry.key.repository) + return "" + } + } + + if repository == "" && hasWorkflowCallTrigger(data.On) { + githubAppOwnerDerivationLog.Print("No explicit checkout.repository found for workflow_call; using needs.activation.outputs.target_repo as owner source") + return "${{ needs.activation.outputs.target_repo }}" + } + + if repository == "" { + githubAppOwnerDerivationLog.Print("No explicit checkout.repository found; GitHub App owner will fall back to github.repository_owner") + } + + return repository +} + +func buildGitHubAppOwnerResolutionSteps(repositoryExpr, stepName, stepID string) (string, []string) { + if owner, ok := deriveLiteralGitHubAppOwner(repositoryExpr); ok { + githubAppOwnerDerivationLog.Printf("Resolved GitHub App owner %q from literal repository %q", owner, repositoryExpr) + return owner, nil + } + if strings.TrimSpace(repositoryExpr) == "" { + githubAppOwnerDerivationLog.Print("No repository expression provided for GitHub App owner derivation; using github.repository_owner") + return "${{ github.repository_owner }}", nil + } + + ownerStepID := stepID + "-owner" + ownerStepName := strings.Replace(stepName, "Generate GitHub App token", "Derive GitHub App owner", 1) + if ownerStepName == stepName { + ownerStepName = "Derive GitHub App owner" + } + + var steps []string + steps = append(steps, fmt.Sprintf(" - name: %s\n", ownerStepName)) + steps = append(steps, fmt.Sprintf(" id: %s\n", ownerStepID)) + steps = append(steps, " env:\n") + steps = append(steps, fmt.Sprintf(" GH_AW_TARGET_REPOSITORY: %s\n", githubExpressionWhitespaceReplacer.Replace(repositoryExpr))) + steps = append(steps, " shell: bash\n") + steps = append(steps, " run: |\n") + steps = append(steps, " set -euo pipefail\n") + steps = append(steps, " echo \"[gh-aw] Deriving GitHub App owner from GH_AW_TARGET_REPOSITORY\"\n") + steps = append(steps, " repo=\"${GH_AW_TARGET_REPOSITORY%.wiki}\"\n") + steps = append(steps, " echo \"[gh-aw] Normalized repository candidate: $repo\"\n") + steps = append(steps, " owner=\"${repo%%/*}\"\n") + steps = append(steps, " if [[ -z \"$repo\" || -z \"$owner\" || \"$owner\" == \"$repo\" ]]; then\n") + steps = append(steps, " echo \"[gh-aw] Unable to derive GitHub App owner from repository: $GH_AW_TARGET_REPOSITORY\" >&2\n") + steps = append(steps, " exit 1\n") + steps = append(steps, " fi\n") + steps = append(steps, " echo \"[gh-aw] Derived GitHub App owner: $owner\"\n") + steps = append(steps, " echo \"owner=$owner\" >> \"$GITHUB_OUTPUT\"\n") + + return "${{ steps." + ownerStepID + ".outputs.owner }}", steps +} + +func resolveGitHubAppOwner(app *GitHubAppConfig, ownerSourceRepository, stepName, stepID string) (string, []string) { + if app != nil && strings.TrimSpace(app.Owner) != "" { + return app.Owner, nil + } + return buildGitHubAppOwnerResolutionSteps(ownerSourceRepository, stepName, stepID) +} + +func deriveLiteralGitHubAppOwner(repository string) (string, bool) { + repository = strings.TrimSpace(repository) + if repository == "" { + return "", false + } + + parts := strings.SplitN(repository, "/", 2) + if len(parts) != 2 { + return "", false + } + + owner := strings.TrimSpace(parts[0]) + if owner == "" || strings.Contains(owner, "${{") { + return "", false + } + return owner, true +} diff --git a/pkg/workflow/github_app_owner_derivation_test.go b/pkg/workflow/github_app_owner_derivation_test.go new file mode 100644 index 00000000000..69db5bda427 --- /dev/null +++ b/pkg/workflow/github_app_owner_derivation_test.go @@ -0,0 +1,277 @@ +//go:build !integration + +package workflow + +import ( + "strings" + "testing" + + "github.com/github/gh-aw/pkg/constants" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBuildGitHubAppTokenMintStepOwnerDerivation(t *testing.T) { + compiler := NewCompiler(WithVersion("1.0.0")) + + tests := []struct { + name string + app *GitHubAppConfig + ownerSourceRepo string + expectedOwner string + expectedContains string + unexpectedContains string + }{ + { + name: "no owner source falls back to github.repository_owner", + app: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + }, + expectedOwner: "owner: ${{ github.repository_owner }}", + unexpectedContains: "id: safe-outputs-app-token-owner", + }, + { + name: "explicit owner wins", + app: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + Owner: "explicit-org", + }, + ownerSourceRepo: "${{ github.event.inputs.trigger_ref }}", + expectedOwner: "owner: explicit-org", + unexpectedContains: "id: safe-outputs-app-token-owner", + }, + { + name: "literal repository derives owner without helper step", + app: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + }, + ownerSourceRepo: "acme/project", + expectedOwner: "owner: acme", + unexpectedContains: "id: safe-outputs-app-token-owner", + }, + { + name: "repository expression derives owner with helper step", + app: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + }, + ownerSourceRepo: "${{ github.event.inputs.trigger_ref }}", + expectedOwner: "owner: ${{ steps.safe-outputs-app-token-owner.outputs.owner }}", + expectedContains: "id: safe-outputs-app-token-owner", + }, + { + name: "bare repository name emits helper step", + app: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + }, + ownerSourceRepo: "just-a-repo", + expectedOwner: "owner: ${{ steps.safe-outputs-app-token-owner.outputs.owner }}", + expectedContains: "GH_AW_TARGET_REPOSITORY: just-a-repo", + }, + { + name: "repository expression includes wiki stripping", + app: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + }, + ownerSourceRepo: "${{ inputs.wiki_repo }}", + expectedOwner: "owner: ${{ steps.safe-outputs-app-token-owner.outputs.owner }}", + expectedContains: "repo=\"${GH_AW_TARGET_REPOSITORY%.wiki}\"", + }, + { + name: "repository expression is whitespace-normalized before yaml emission", + app: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + }, + ownerSourceRepo: "${{ inputs.target_\nrepo }}", + expectedOwner: "owner: ${{ steps.safe-outputs-app-token-owner.outputs.owner }}", + expectedContains: "GH_AW_TARGET_REPOSITORY: ${{ inputs.target_ repo }}", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + steps := compiler.buildGitHubAppTokenMintStepWithMeta( + tt.app, + nil, + "", + tt.ownerSourceRepo, + "Generate GitHub App token", + "safe-outputs-app-token", + ) + stepsStr := strings.Join(steps, "") + + assert.Contains(t, stepsStr, tt.expectedOwner, "case %q should include expected owner", tt.name) + if tt.expectedContains != "" { + assert.Contains(t, stepsStr, tt.expectedContains, "case %q should include expected content", tt.name) + } + if tt.unexpectedContains != "" { + assert.NotContains(t, stepsStr, tt.unexpectedContains, "case %q should not include unexpected content", tt.name) + } + + if strings.Contains(stepsStr, "id: safe-outputs-app-token-owner") { + ownerIdx := -1 + mintIdx := -1 + for i, step := range steps { + if strings.Contains(step, "id: safe-outputs-app-token-owner") { + ownerIdx = i + } + if strings.Contains(step, "id: safe-outputs-app-token\n") { + mintIdx = i + } + } + require.NotEqual(t, -1, ownerIdx, "case %q should include owner helper step id", tt.name) + require.NotEqual(t, -1, mintIdx, "case %q should include app token mint step id", tt.name) + require.Greater(t, mintIdx, ownerIdx, "case %q should emit owner derivation step before mint step", tt.name) + } + }) + } +} + +func TestWorkflowBuildersDeriveGitHubAppOwnerFromCheckoutRepository(t *testing.T) { + compiler := NewCompiler(WithVersion("1.0.0")) + compiler.jobManager = NewJobManager() + + data := &WorkflowData{ + Name: "Test Workflow", + On: `"on": "workflow_dispatch"`, + Permissions: `"permissions": + contents: read + issues: read + pull-requests: read`, + CheckoutConfigs: []*CheckoutConfig{ + { + Repository: "${{ github.event.inputs.trigger_ref }}", + Path: "target", + GitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.CHECKOUT_APP_ID }}", + PrivateKey: "${{ secrets.CHECKOUT_APP_PRIVATE_KEY }}", + Repositories: []string{"*"}, + }, + }, + }, + ParsedTools: &Tools{ + GitHub: &GitHubToolConfig{ + Mode: "local", + GitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.MCP_APP_ID }}", + PrivateKey: "${{ secrets.MCP_APP_PRIVATE_KEY }}", + Repositories: []string{"*"}, + }, + }, + }, + SafeOutputs: &SafeOutputsConfig{ + GitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.SAFE_OUTPUTS_APP_ID }}", + PrivateKey: "${{ secrets.SAFE_OUTPUTS_APP_PRIVATE_KEY }}", + Repositories: []string{"*"}, + }, + CreateIssues: &CreateIssuesConfig{TitlePrefix: "[automated] "}, + }, + } + + checkoutMgr := NewCheckoutManager(data.CheckoutConfigs) + checkoutSteps := strings.Join(checkoutMgr.GenerateCheckoutAppTokenSteps(compiler, nil), "") + assert.Contains(t, checkoutSteps, "id: checkout-app-token-0-owner", "checkout app token should include owner helper step") + assert.Contains(t, checkoutSteps, "owner: ${{ steps.checkout-app-token-0-owner.outputs.owner }}", "checkout app token should consume derived owner output") + assert.Contains(t, checkoutSteps, "GH_AW_TARGET_REPOSITORY: ${{ github.event.inputs.trigger_ref }}", "checkout owner helper should read target repository expression") + + mcpSteps := strings.Join(compiler.generateGitHubMCPAppTokenMintingSteps(data), "") + assert.Contains(t, mcpSteps, "id: github-mcp-app-token-owner", "github MCP token should include owner helper step") + assert.Contains(t, mcpSteps, "owner: ${{ steps.github-mcp-app-token-owner.outputs.owner }}", "github MCP token should consume derived owner output") + assert.Contains(t, mcpSteps, "GH_AW_TARGET_REPOSITORY: ${{ github.event.inputs.trigger_ref }}", "github MCP owner helper should read target repository expression") + + job, _, err := compiler.buildConsolidatedSafeOutputsJob(data, string(constants.AgentJobName), "test.md") + require.NoError(t, err, "safe outputs job compilation should succeed") + safeOutputsSteps := strings.Join(job.Steps, "") + assert.Contains(t, safeOutputsSteps, "id: safe-outputs-app-token-owner", "safe outputs app token should include owner helper step") + assert.Contains(t, safeOutputsSteps, "owner: ${{ steps.safe-outputs-app-token-owner.outputs.owner }}", "safe outputs app token should consume derived owner output") + assert.Contains(t, safeOutputsSteps, "GH_AW_TARGET_REPOSITORY: ${{ github.event.inputs.trigger_ref }}", "safe outputs owner helper should read target repository expression") +} + +func TestInferSingleCheckoutRepositoryForGitHubAppOwner(t *testing.T) { + t.Run("returns empty for multiple distinct repositories", func(t *testing.T) { + data := &WorkflowData{ + CheckoutConfigs: []*CheckoutConfig{ + {Repository: "org-a/repo"}, + {Repository: "org-b/other"}, + }, + } + assert.Empty(t, inferSingleCheckoutRepositoryForGitHubAppOwner(data), "multiple distinct repositories should not infer a single owner source") + }) + + t.Run("returns repository when repeated repositories are the same", func(t *testing.T) { + data := &WorkflowData{ + CheckoutConfigs: []*CheckoutConfig{ + {Repository: "org-a/repo"}, + {Repository: "org-a/repo"}, + }, + } + assert.Equal(t, "org-a/repo", inferSingleCheckoutRepositoryForGitHubAppOwner(data), "matching repositories should infer a single owner source") + }) + + t.Run("returns empty when no explicit repositories are configured", func(t *testing.T) { + data := &WorkflowData{ + CheckoutConfigs: []*CheckoutConfig{ + {Path: "default"}, + }, + } + assert.Empty(t, inferSingleCheckoutRepositoryForGitHubAppOwner(data), "no explicit repository should not infer owner source") + }) + + t.Run("workflow_call defaults to activation target repository", func(t *testing.T) { + data := &WorkflowData{ + On: `"on": + workflow_call: {}`, + CheckoutConfigs: []*CheckoutConfig{ + {Path: "default"}, + }, + } + assert.Equal(t, "${{ needs.activation.outputs.target_repo }}", inferSingleCheckoutRepositoryForGitHubAppOwner(data), "workflow_call should infer activation target repository when no explicit checkout repository is set") + }) +} + +func TestWorkflowCallSafeOutputsAppTokenUsesRepositoryOwnerSource(t *testing.T) { + compiler := NewCompiler(WithVersion("1.0.0")) + compiler.jobManager = NewJobManager() + + data := &WorkflowData{ + Name: "workflow-call-safe-outputs", + On: `"on": + workflow_call: {}`, + Permissions: `"permissions": + contents: read + issues: read`, + CheckoutConfigs: []*CheckoutConfig{ + { + Repository: "${{ inputs.target_repository }}", + GitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.CHECKOUT_APP_ID }}", + PrivateKey: "${{ secrets.CHECKOUT_APP_PRIVATE_KEY }}", + }, + }, + }, + SafeOutputs: &SafeOutputsConfig{ + GitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.SAFE_OUTPUTS_APP_ID }}", + PrivateKey: "${{ secrets.SAFE_OUTPUTS_APP_PRIVATE_KEY }}", + }, + CreateIssues: &CreateIssuesConfig{TitlePrefix: "[automated] "}, + }, + } + + job, _, err := compiler.buildConsolidatedSafeOutputsJob(data, string(constants.AgentJobName), "test.md") + require.NoError(t, err, "safe outputs job compilation should succeed for workflow_call") + require.NotNil(t, job, "safe outputs job should be generated for workflow_call") + + safeOutputsSteps := strings.Join(job.Steps, "") + assert.Contains(t, safeOutputsSteps, "id: safe-outputs-app-token-owner", "safe outputs app token should include owner helper for expression repository") + assert.Contains(t, safeOutputsSteps, "owner: ${{ steps.safe-outputs-app-token-owner.outputs.owner }}", "safe outputs app token should consume derived owner output") + assert.Contains(t, safeOutputsSteps, "GH_AW_TARGET_REPOSITORY: ${{ inputs.target_repository }}", "safe outputs owner helper should target explicit checkout repository expression") + assert.Contains(t, safeOutputsSteps, "repositories: ${{ needs.activation.outputs.target_repo_name }}", "workflow_call safe outputs app token should default repositories to target_repo_name") +} diff --git a/pkg/workflow/notify_comment.go b/pkg/workflow/notify_comment.go index 4bc37379f97..ef8cc541ce2 100644 --- a/pkg/workflow/notify_comment.go +++ b/pkg/workflow/notify_comment.go @@ -61,7 +61,12 @@ func (c *Compiler) buildConclusionJob(data *WorkflowData, mainJobName string, sa if hasWorkflowCallTrigger(data.On) { appTokenFallbackRepo = "${{ needs.activation.outputs.target_repo_name }}" } - steps = append(steps, c.buildGitHubAppTokenMintStep(data.SafeOutputs.GitHubApp, permissions, appTokenFallbackRepo)...) + steps = append(steps, c.buildGitHubAppTokenMintStepForRepository( + data.SafeOutputs.GitHubApp, + permissions, + appTokenFallbackRepo, + inferSingleCheckoutRepositoryForGitHubAppOwner(data), + )...) } // Add artifact download steps once (shared by noop and conclusion steps). diff --git a/pkg/workflow/safe_outputs_app_config.go b/pkg/workflow/safe_outputs_app_config.go index c0c6eda079e..513ab3df1be 100644 --- a/pkg/workflow/safe_outputs_app_config.go +++ b/pkg/workflow/safe_outputs_app_config.go @@ -22,7 +22,7 @@ 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 }}") 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) + Owner string `yaml:"owner,omitempty"` // Optional: owner of the GitHub App installation (defaults to checkout.repository owner when derivable, otherwise 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) } @@ -210,13 +210,19 @@ func (c *Compiler) mergeAppFromIncludedConfigs(topSafeOutputs *SafeOutputsConfig // workflow_call relay workflows so the token is scoped to the platform repo's NAME, not the full // owner/repo slug — actions/create-github-app-token expects repo names only when owner is also set). func (c *Compiler) buildGitHubAppTokenMintStep(app *GitHubAppConfig, permissions *Permissions, fallbackRepoExpr string) []string { - return c.buildGitHubAppTokenMintStepWithMeta(app, permissions, fallbackRepoExpr, "Generate GitHub App token", "safe-outputs-app-token") + return c.buildGitHubAppTokenMintStepWithMeta(app, permissions, fallbackRepoExpr, "", "Generate GitHub App token", "safe-outputs-app-token") } -func (c *Compiler) buildGitHubAppTokenMintStepWithMeta(app *GitHubAppConfig, permissions *Permissions, fallbackRepoExpr string, stepName string, stepID string) []string { +func (c *Compiler) buildGitHubAppTokenMintStepForRepository(app *GitHubAppConfig, permissions *Permissions, fallbackRepoExpr string, ownerSourceRepository string) []string { + return c.buildGitHubAppTokenMintStepWithMeta(app, permissions, fallbackRepoExpr, ownerSourceRepository, "Generate GitHub App token", "safe-outputs-app-token") +} + +func (c *Compiler) buildGitHubAppTokenMintStepWithMeta(app *GitHubAppConfig, permissions *Permissions, fallbackRepoExpr string, ownerSourceRepository string, stepName string, stepID string) []string { safeOutputsAppLog.Printf("Building GitHub App token mint step: owner=%s, repos=%d", app.Owner, len(app.Repositories)) var steps []string + owner, ownerSteps := resolveGitHubAppOwner(app, ownerSourceRepository, stepName, stepID) + steps = append(steps, ownerSteps...) steps = append(steps, fmt.Sprintf(" - name: %s\n", stepName)) steps = append(steps, fmt.Sprintf(" id: %s\n", stepID)) if app.shouldIgnoreMissingKey() { @@ -227,11 +233,7 @@ func (c *Compiler) buildGitHubAppTokenMintStepWithMeta(app *GitHubAppConfig, per steps = append(steps, fmt.Sprintf(" client-id: %s\n", app.AppID)) steps = append(steps, fmt.Sprintf(" private-key: %s\n", app.PrivateKey)) - // Add owner - default to current repository owner if not specified - owner := app.Owner - if owner == "" { - owner = "${{ github.repository_owner }}" - } + // Add owner - default to the derived checkout owner when available, otherwise current repository owner. steps = append(steps, fmt.Sprintf(" owner: %s\n", owner)) // Add repositories - behavior depends on configuration: diff --git a/pkg/workflow/safe_outputs_jobs.go b/pkg/workflow/safe_outputs_jobs.go index 243ce787ae7..7762cd7c123 100644 --- a/pkg/workflow/safe_outputs_jobs.go +++ b/pkg/workflow/safe_outputs_jobs.go @@ -70,7 +70,12 @@ func (c *Compiler) buildSafeOutputJob(data *WorkflowData, config SafeOutputJobCo if hasWorkflowCallTrigger(data.On) { appTokenFallbackRepo = "${{ needs.activation.outputs.target_repo_name }}" } - steps = append(steps, c.buildGitHubAppTokenMintStep(data.SafeOutputs.GitHubApp, config.Permissions, appTokenFallbackRepo)...) + steps = append(steps, c.buildGitHubAppTokenMintStepForRepository( + data.SafeOutputs.GitHubApp, + config.Permissions, + appTokenFallbackRepo, + inferSingleCheckoutRepositoryForGitHubAppOwner(data), + )...) } // Build the step based on action mode