diff --git a/docs/adr/53766-infer-github-app-permissions-and-events-for-bootstrap.md b/docs/adr/53766-infer-github-app-permissions-and-events-for-bootstrap.md new file mode 100644 index 00000000000..e83e41c5c80 --- /dev/null +++ b/docs/adr/53766-infer-github-app-permissions-and-events-for-bootstrap.md @@ -0,0 +1,44 @@ +# ADR-53766: Infer GitHub App Permissions and Events from Resolved Package Workflows in add-wizard Bootstrap + +**Date**: 2026-08-19 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +`gh aw add-wizard` bootstraps a GitHub App for packages that declare a `github-app` action in their `aw.yml` manifest. Previously, the manifest author had to manually list the App's `permissions` and `events` under `github-app`, duplicating information already expressed in each workflow's frontmatter (`permissions:`, `on:`, and `safe-outputs:` handlers). Manifests that omitted these fields silently produced a GitHub App scoped to only `metadata: read` with no subscribed webhook events, so the installed App could not actually perform the actions the package's workflows required (for example, writing issues or reacting to pull requests). + +### Decision + +We will infer the minimal GitHub App `permissions` and `events` requirements directly from the workflows reachable from a package's `aw.yml`, and merge them with any values still explicitly declared in the manifest. Permission inference reuses the same canonical helpers already used by the `.md` interactive workflow builder (`workflow.ComputeGitHubAppManifestPermissions`, which layers safe-outputs-derived permissions via `ComputePermissionsForSafeOutputs`/`SafeOutputsConfigFromKeys` on top of the raw top-level `permissions` block, keeping the highest scope seen per resource) and normalizes Actions permission keys to GitHub App manifest keys, dropping scopes with no App equivalent. Event inference uses `workflow.NormalizeGitHubAppWebhookEvents` to expand compiler-only triggers (command shorthands, `slash_command`, `label_command`, `reaction`, `status-comment`) to their underlying webhook events, map `pull_request_target` to `pull_request`, and filter out non-webhook triggers (`schedule`, `workflow_dispatch`, `repository_dispatch`). Inference is scoped to only the workflows resolved from the specific package associated with the selected bootstrap profile (`config.Profile.Source`), not the full set of sources passed to add-wizard, so an unrelated standalone workflow installed alongside a package cannot widen that package's App scope. A `--no-config` flag on `gh aw add-wizard` disables this inference entirely, restoring the previous behavior of only applying explicitly declared `permissions`/`events`. + +### Alternatives Considered + +#### Alternative 1: Require manifest authors to keep declaring permissions/events explicitly + +Leave the existing behavior unchanged and instead improve documentation or add a linter warning when `github-app` permissions/events are missing from `aw.yml`. Why not chosen: this still requires every package author to manually keep the manifest's App scopes in sync with the workflows' actual requirements, which is exactly the duplication and silent-drift problem the issue calls out; a warning does not prevent an under-scoped App from being created. + +#### Alternative 2: Hand-roll a separate permission/event derivation pass in the CLI package + +Implement bespoke frontmatter parsing inside `pkg/cli` to compute permissions and events for `aw.yml` inference, independent of the logic already used for `.md` workflow permission derivation in the interactive builder. Why not chosen: this would create two divergent code paths for deriving GitHub App requirements from workflow frontmatter — one for `.md` workflows and one for `aw.yml` packages — that would need to be kept in sync manually and would be prone to the same normalization bugs (Actions-style keys vs. App manifest keys, compiler-only trigger expansion) independently in each place. + +### Consequences + +#### Positive +- Package `aw.yml` manifests no longer need to duplicate `permissions`/`events` that are already derivable from their workflows; omitting them no longer silently produces an under-scoped App. +- Permission and event derivation for `aw.yml` packages and `.md` interactive workflow building now share one canonical implementation (`pkg/workflow/github_app_requirements.go`), reducing the risk of divergent or incorrect normalization logic. +- Inference is scoped per-package (via the bootstrap profile's own source), preserving least-privilege when multiple unrelated packages/workflows are installed together. + +#### Negative +- Inference adds a workflow-resolution pass (parsing every workflow reachable from the package) to `executeBootstrapProfile`, which is extra work compared to reading a few manifest fields directly, though bounded by the size of the package. +- Authors who want an App scoped more narrowly than what its workflows technically use (e.g., deliberately under-provisioning) must now use `--no-config` and revert to fully manual manifest declarations, since inferred values are additive/merged rather than a ceiling. + +#### Neutral +- `aw.yml` manifests may still declare `permissions`/`events` explicitly; declared values are merged with (not replaced by) inferred values, so manifests can supplement inference for cases the inference cannot see (e.g., App requirements unrelated to any workflow trigger). +- The `--no-config` flag preserves an escape hatch for the previous fully-manual behavior without removing any existing manifest fields or schema. + +--- + +*ADR created by adr-writer. Review and finalize before changing status from Draft to Accepted.* diff --git a/docs/src/content/docs/setup/cli.md b/docs/src/content/docs/setup/cli.md index 40f34479296..2c2ee507d52 100644 --- a/docs/src/content/docs/setup/cli.md +++ b/docs/src/content/docs/setup/cli.md @@ -190,7 +190,7 @@ gh aw add-wizard https://example.com/workflows/my-workflow.json # Arbitrary UR gh aw add-wizard githubnext/agentics/ci-doctor --no-secret # Skip secret prompt ``` -**Options:** `--no-secret`, `--dir/-d`, `--engine/-e`, `--no-gitattributes`, `--no-stop-after`, `--stop-after`, `--append`, `--no-security-scanner` +**Options:** `--no-secret`, `--dir/-d`, `--engine/-e`, `--no-gitattributes`, `--no-stop-after`, `--stop-after`, `--append`, `--no-security-scanner`, `--no-config` When the Copilot engine is selected, the wizard prompts the user to choose an authentication method: organization billing via [`permissions.copilot-requests: write`](/gh-aw/reference/auth/#copilot-requests-write-permission) (no PAT required), or a [`COPILOT_GITHUB_TOKEN`](/gh-aw/reference/auth/#copilot_github_token) personal access token (a separate token from the default `GITHUB_TOKEN`, because the agent needs elevated Copilot API access that the ephemeral workflow token does not carry). On the PAT path, the wizard auto-opens a preconfigured fine-grained PAT creation page (prefilled token name, expiration, and Copilot Requests permission). The GitHub page still must be completed manually in the browser. Users may paste either an existing suitable fine-grained PAT or a newly created one into the masked CLI prompt, but reuse should be based on the token's properties: personal-account resource owner, repository access set to Public repositories, and Copilot Requests permission available. If `COPILOT_GITHUB_TOKEN` already exists, the wizard still asks for the token again because GitHub does not expose stored secret values for validation. The flow does not rely on the PAT display name in GitHub's token list. The pasted token is then validated and stored as a repository secret. diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index c09871b9b7d..21efa90c44c 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -33,6 +33,11 @@ type AddInteractiveConfig struct { AppendText string // Extra content to append to the workflow on installation DisableSecurityScanner bool // Disable security scanning of workflow markdown content + // DisableGitHubAppPermissionInference disables inferring GitHub App + // permissions/events from the package's resolved workflows during bootstrap, + // so only permissions/events explicitly declared in aw.yml are applied. + DisableGitHubAppPermissionInference bool + // UseCopilotRequests indicates the user chose org-billing (copilot-requests) auth // instead of a PAT when setting up the Copilot engine during the wizard. // When true, COPILOT_GITHUB_TOKEN secret setup is skipped and @@ -166,7 +171,7 @@ func (c *AddInteractiveConfig) applyBootstrapConfigIfNeeded(ctx context.Context, return nil } if c.hasWriteAccess { - return executeBootstrapConfigForAdd(ctx, c.RepoOverride, c.WorkflowSpecs, profile, c.UseCopilotRequests, c.Verbose) + return executeBootstrapConfigForAdd(ctx, c.RepoOverride, c.WorkflowSpecs, profile, c.UseCopilotRequests, c.Verbose, c.DisableGitHubAppPermissionInference) } printBootstrapConfigTODO(os.Stderr, profile) return nil diff --git a/pkg/cli/add_wizard_command.go b/pkg/cli/add_wizard_command.go index 0f99e5ed012..9c1d8a1a810 100644 --- a/pkg/cli/add_wizard_command.go +++ b/pkg/cli/add_wizard_command.go @@ -54,6 +54,7 @@ Note: To create a new workflow from scratch, use the 'new' command instead.`, ` + string(constants.CLIExtensionPrefix) + ` add-wizard githubnext/agentics/ci-doctor --no-secret # Skip secret prompt ` + string(constants.CLIExtensionPrefix) + ` add-wizard githubnext/agentics/ci-doctor --append "custom footer" # Append custom content ` + string(constants.CLIExtensionPrefix) + ` add-wizard githubnext/agentics/ci-doctor --no-security-scanner # Skip security scan + ` + string(constants.CLIExtensionPrefix) + ` add-wizard githubnext/agentics/ci-doctor --no-config # Skip GitHub App permission/event inference from package workflows `, Args: func(cmd *cobra.Command, args []string) error { if len(args) < 1 { @@ -74,6 +75,7 @@ Note: To create a new workflow from scratch, use the 'new' command instead.`, skipSecret := noSecret || skipSecretLegacy appendText, _ := cmd.Flags().GetString("append") disableSecurityScanner := resolveDeprecatedBoolFlag(cmd, "no-security-scanner", "disable-security-scanner") + noGitHubAppInference, _ := cmd.Flags().GetBool("no-config") addWizardLog.Printf("Starting add-wizard: workflows=%v, engine=%s, verbose=%v", workflows, engineOverride, verbose) @@ -90,16 +92,17 @@ Note: To create a new workflow from scratch, use the 'new' command instead.`, } return RunAddInteractive(cmd.Context(), &AddInteractiveConfig{ - WorkflowSpecs: workflows, - Verbose: verbose, - EngineOverride: engineOverride, - NoGitattributes: noGitattributes, - WorkflowDir: workflowDir, - NoStopAfter: noStopAfter, - StopAfter: stopAfter, - SkipSecret: skipSecret, - AppendText: appendText, - DisableSecurityScanner: disableSecurityScanner, + WorkflowSpecs: workflows, + Verbose: verbose, + EngineOverride: engineOverride, + NoGitattributes: noGitattributes, + WorkflowDir: workflowDir, + NoStopAfter: noStopAfter, + StopAfter: stopAfter, + SkipSecret: skipSecret, + AppendText: appendText, + DisableSecurityScanner: disableSecurityScanner, + DisableGitHubAppPermissionInference: noGitHubAppInference, }) }, } @@ -131,6 +134,10 @@ Note: To create a new workflow from scratch, use the 'new' command instead.`, // for consistency with add and other install entry points) addSecurityScannerFlag(cmd) + // Add no-config flag to allow disabling automatic inference of GitHub App + // permissions/events from resolved package workflows. + cmd.Flags().Bool("no-config", false, "Disable inferring GitHub App permissions/events from the package's workflows; use only permissions/events declared in aw.yml") + // Register completions RegisterEngineFlagCompletion(cmd) RegisterDirFlagCompletion(cmd, "dir") diff --git a/pkg/cli/add_wizard_command_test.go b/pkg/cli/add_wizard_command_test.go index d4eeed34a94..a504d8131a7 100644 --- a/pkg/cli/add_wizard_command_test.go +++ b/pkg/cli/add_wizard_command_test.go @@ -49,4 +49,15 @@ func TestAddWizardCommand_ExamplesMentionNewFlags(t *testing.T) { assert.Contains(t, cmd.Example, "--append \"custom footer\"", "add-wizard examples should show append usage") assert.Contains(t, cmd.Example, "--no-security-scanner", "add-wizard examples should show no-security-scanner usage") + assert.Contains(t, cmd.Example, "--no-config", "add-wizard examples should show no-config usage") +} + +func TestAddWizardCommand_HasNoConfigFlag(t *testing.T) { + t.Parallel() + cmd := NewAddWizardCommand(func(string) error { return nil }) + require.NotNil(t, cmd) + + flag := cmd.Flags().Lookup("no-config") + require.NotNil(t, flag, "add-wizard should define no-config") + assert.Equal(t, "false", flag.DefValue, "no-config should default to false") } diff --git a/pkg/cli/bootstrap_config.go b/pkg/cli/bootstrap_config.go index 2ade8076f51..4b8b2aefdcd 100644 --- a/pkg/cli/bootstrap_config.go +++ b/pkg/cli/bootstrap_config.go @@ -70,7 +70,7 @@ func printBootstrapConfigTODO(w io.Writer, profile *resolvedBootstrapProfile) { // executeBootstrapConfigForAdd runs the bootstrap config actions interactively. // Used by add-wizard after the workflow PR has been created and merged. -func executeBootstrapConfigForAdd(ctx context.Context, repo string, sources []string, profile *resolvedBootstrapProfile, useCopilotRequests bool, verbose bool) error { +func executeBootstrapConfigForAdd(ctx context.Context, repo string, sources []string, profile *resolvedBootstrapProfile, useCopilotRequests bool, verbose bool, disableGitHubAppPermissionInference bool) error { if profile == nil || profile.Profile == nil || len(profile.Profile.Config) == 0 { return nil } @@ -88,11 +88,12 @@ func executeBootstrapConfigForAdd(ctx context.Context, repo string, sources []st } return executeBootstrapProfile(ctx, bootstrapProfileRunConfig{ - Repo: repo, - RepoDir: repoDir, - Sources: sources, - Profile: profile, - UseCopilotRequests: useCopilotRequests, - Verbose: verbose, + Repo: repo, + RepoDir: repoDir, + Sources: sources, + Profile: profile, + UseCopilotRequests: useCopilotRequests, + Verbose: verbose, + DisableGitHubAppPermissionInference: disableGitHubAppPermissionInference, }) } diff --git a/pkg/cli/bootstrap_profile_inference.go b/pkg/cli/bootstrap_profile_inference.go new file mode 100644 index 00000000000..ad20dc99c88 --- /dev/null +++ b/pkg/cli/bootstrap_profile_inference.go @@ -0,0 +1,154 @@ +package cli + +import ( + "context" + "maps" + "sort" + + "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/parser" + "github.com/github/gh-aw/pkg/workflow" +) + +var bootstrapInferenceLog = logger.New("cli:bootstrap_profile_inference") + +// inferBootstrapGitHubAppRequirements resolves every workflow reachable from sources +// (an aw.yml package) and merges their required GitHub App manifest permissions and +// webhook events into the minimal set required for a GitHub App to operate the +// package. This lets aw.yml manifests omit the github-app config[].permissions/events +// fields and still get a least-privilege App instead of the "metadata: read" +// fallback. +// +// Requirement derivation is delegated to pkg/workflow (ComputeGitHubAppManifestPermissions +// and NormalizeGitHubAppWebhookEvents), the same code path used to determine safe-outputs +// permissions and trigger normalization for standalone .md workflows, so aw.yml packages +// and directly-added .md workflows are held to identical permission/event rules. +func inferBootstrapGitHubAppRequirements(ctx context.Context, sources []string) (map[string]string, []string, error) { + if len(sources) == 0 { + return nil, nil, nil + } + resolved, err := ResolveWorkflows(ctx, sources, false) + if err != nil { + return nil, nil, err + } + + permissions := map[string]string{} + eventSet := map[string]struct{}{} + for _, candidate := range resolved.Workflows { + if candidate == nil || candidate.IsActionWorkflow || candidate.IsPackageSkillFile || candidate.IsPackageAgentFile { + continue + } + frontmatter, err := parser.ExtractFrontmatterFromContent(string(candidate.Content)) + if err != nil || frontmatter == nil { + continue + } + safeOutputs := bootstrapSafeOutputsConfigFromFrontmatter(frontmatter.Frontmatter) + for resource, level := range workflow.ComputeGitHubAppManifestPermissions(frontmatter.Frontmatter["permissions"], safeOutputs) { + permissions[resource] = mergeBootstrapPermissionLevel(permissions[resource], level) + } + for _, event := range bootstrapEventNamesFromOn(frontmatter.Frontmatter["on"]) { + eventSet[event] = struct{}{} + } + } + + events := make([]string, 0, len(eventSet)) + for event := range eventSet { + events = append(events, event) + } + sort.Strings(events) + + bootstrapInferenceLog.Printf("Inferred GitHub App requirements: permissions=%d, events=%d", len(permissions), len(events)) + if len(permissions) == 0 { + permissions = nil + } + return permissions, events, nil +} + +// bootstrapSafeOutputsConfigFromFrontmatter builds a minimal *workflow.SafeOutputsConfig +// from a workflow's raw "safe-outputs" frontmatter map, using the handler key names +// present (e.g. "create-issue", "add-comment"). This reuses workflow.SafeOutputsConfigFromKeys, +// the same helper the interactive workflow builder uses to compute safe-outputs-derived +// permissions for newly generated .md workflows, so package workflows are scoped with the +// same rules (e.g. an "issues: read" workflow with "create-issue" configured still yields +// "issues: write"). +func bootstrapSafeOutputsConfigFromFrontmatter(frontmatter map[string]any) *workflow.SafeOutputsConfig { + safeOutputsRaw, ok := frontmatter["safe-outputs"].(map[string]any) + if !ok || len(safeOutputsRaw) == 0 { + return nil + } + keys := make([]string, 0, len(safeOutputsRaw)) + for key := range safeOutputsRaw { + keys = append(keys, key) + } + return workflow.SafeOutputsConfigFromKeys(keys) +} + +// bootstrapPermissionLevelRank ranks permission levels so higher-privilege scopes are +// never downgraded when merging requirements across multiple workflows or between +// declared aw.yml values and inferred ones. +func bootstrapPermissionLevelRank(level string) int { + switch level { + case "write": + return 2 + case "read": + return 1 + default: + return 0 + } +} + +// mergeBootstrapPermissionLevel returns the higher of two permission levels +// (write > read > none), so a resource needed as "write" by one workflow is never +// downgraded by another workflow that only needs "read". +func mergeBootstrapPermissionLevel(existing, incoming string) string { + if existing == "" { + return incoming + } + if bootstrapPermissionLevelRank(incoming) > bootstrapPermissionLevelRank(existing) { + return incoming + } + return existing +} + +// bootstrapEventNamesFromOn extracts the GitHub App webhook events required by a +// workflow's "on" frontmatter value. It delegates to workflow.NormalizeGitHubAppWebhookEvents, +// the same trigger-normalization code path used elsewhere in the compiler, so +// compiler-only keys (slash_command, label_command, reaction, status-comment), +// command-trigger shorthands (e.g. "on: /my-bot"), and pull_request_target are all +// handled consistently rather than being reimplemented here. +func bootstrapEventNamesFromOn(raw any) []string { + return workflow.NormalizeGitHubAppWebhookEvents(raw) +} + +// mergeBootstrapGitHubAppRequirements combines explicitly declared manifest +// permissions/events (if any) with the inferred requirements from the package's +// resolved workflows, taking the union of events and the highest permission level +// per resource. +func mergeBootstrapGitHubAppRequirements(declaredPermissions map[string]string, declaredEvents []string, inferredPermissions map[string]string, inferredEvents []string) (map[string]string, []string) { + merged := make(map[string]string, len(declaredPermissions)+len(inferredPermissions)) + maps.Copy(merged, declaredPermissions) + for resource, level := range inferredPermissions { + merged[resource] = mergeBootstrapPermissionLevel(merged[resource], level) + } + if len(merged) == 0 { + merged = nil + } + + eventSet := make(map[string]struct{}, len(declaredEvents)+len(inferredEvents)) + for _, event := range declaredEvents { + eventSet[event] = struct{}{} + } + for _, event := range inferredEvents { + eventSet[event] = struct{}{} + } + events := make([]string, 0, len(eventSet)) + for event := range eventSet { + events = append(events, event) + } + sort.Strings(events) + if len(events) == 0 { + events = nil + } + + return merged, events +} diff --git a/pkg/cli/bootstrap_profile_inference_test.go b/pkg/cli/bootstrap_profile_inference_test.go new file mode 100644 index 00000000000..b02610027be --- /dev/null +++ b/pkg/cli/bootstrap_profile_inference_test.go @@ -0,0 +1,298 @@ +package cli + +import ( + "context" + "os" + "path/filepath" + "reflect" + "sort" + "testing" +) + +func TestMergeBootstrapPermissionLevel(t *testing.T) { + tests := []struct { + name string + existing string + incoming string + want string + }{ + {name: "empty existing takes incoming", existing: "", incoming: "read", want: "read"}, + {name: "write beats read", existing: "read", incoming: "write", want: "write"}, + {name: "read does not downgrade write", existing: "write", incoming: "read", want: "write"}, + {name: "equal levels unchanged", existing: "write", incoming: "write", want: "write"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := mergeBootstrapPermissionLevel(tt.existing, tt.incoming) + if got != tt.want { + t.Fatalf("mergeBootstrapPermissionLevel(%q, %q) = %q, want %q", tt.existing, tt.incoming, got, tt.want) + } + }) + } +} + +func TestBootstrapEventNamesFromOn(t *testing.T) { + tests := []struct { + name string + raw any + want []string + }{ + {name: "string trigger", raw: "issues", want: []string{"issues"}}, + {name: "list of triggers", raw: []any{"issues", "pull_request"}, want: []string{"issues", "pull_request"}}, + { + name: "map of triggers excludes non-webhook events", + raw: map[string]any{ + "issues": map[string]any{"types": []any{"opened"}}, + "schedule": []any{map[string]any{"cron": "0 0 * * *"}}, + "workflow_dispatch": nil, + "repository_dispatch": nil, + }, + want: []string{"issues"}, + }, + {name: "nil value", raw: nil, want: nil}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := bootstrapEventNamesFromOn(tt.raw) + sort.Strings(got) + want := append([]string(nil), tt.want...) + sort.Strings(want) + if len(got) != len(want) { + t.Fatalf("bootstrapEventNamesFromOn(%v) = %v, want %v", tt.raw, got, want) + } + for i := range got { + if got[i] != want[i] { + t.Fatalf("bootstrapEventNamesFromOn(%v) = %v, want %v", tt.raw, got, want) + } + } + }) + } +} + +func TestMergeBootstrapGitHubAppRequirements(t *testing.T) { + declaredPermissions := map[string]string{"contents": "read"} + declaredEvents := []string{"push"} + inferredPermissions := map[string]string{"contents": "write", "issues": "write"} + inferredEvents := []string{"issues", "push"} + + mergedPermissions, mergedEvents := mergeBootstrapGitHubAppRequirements(declaredPermissions, declaredEvents, inferredPermissions, inferredEvents) + + wantPermissions := map[string]string{"contents": "write", "issues": "write"} + if !reflect.DeepEqual(mergedPermissions, wantPermissions) { + t.Fatalf("merged permissions = %v, want %v", mergedPermissions, wantPermissions) + } + wantEvents := []string{"issues", "push"} + if !reflect.DeepEqual(mergedEvents, wantEvents) { + t.Fatalf("merged events = %v, want %v", mergedEvents, wantEvents) + } + + // Declared-only permissions/events with no inference produce the declared set unchanged. + mergedPermissions, mergedEvents = mergeBootstrapGitHubAppRequirements(declaredPermissions, declaredEvents, nil, nil) + if !reflect.DeepEqual(mergedPermissions, declaredPermissions) { + t.Fatalf("merged permissions with no inference = %v, want %v", mergedPermissions, declaredPermissions) + } + if !reflect.DeepEqual(mergedEvents, declaredEvents) { + t.Fatalf("merged events with no inference = %v, want %v", mergedEvents, declaredEvents) + } + + // No declared or inferred requirements yields nil (not empty maps/slices). + mergedPermissions, mergedEvents = mergeBootstrapGitHubAppRequirements(nil, nil, nil, nil) + if mergedPermissions != nil { + t.Fatalf("merged permissions = %v, want nil", mergedPermissions) + } + if mergedEvents != nil { + t.Fatalf("merged events = %v, want nil", mergedEvents) + } +} + +func TestInferBootstrapGitHubAppRequirements_MergesAcrossWorkflows(t *testing.T) { + dir := t.TempDir() + first := filepath.Join(dir, "first.md") + second := filepath.Join(dir, "second.md") + + firstContent := "---\non:\n issues:\n types: [opened]\npermissions:\n contents: read\n issues: write\n---\n\n# First\n" + secondContent := "---\non:\n pull_request:\n types: [opened]\n schedule:\n - cron: \"0 0 * * *\"\npermissions:\n contents: write\n---\n\n# Second\n" + + if err := os.WriteFile(first, []byte(firstContent), 0o644); err != nil { + t.Fatalf("failed to write first workflow: %v", err) + } + if err := os.WriteFile(second, []byte(secondContent), 0o644); err != nil { + t.Fatalf("failed to write second workflow: %v", err) + } + + permissions, events, err := inferBootstrapGitHubAppRequirements(context.Background(), []string{first, second}) + if err != nil { + t.Fatalf("inferBootstrapGitHubAppRequirements returned error: %v", err) + } + + wantPermissions := map[string]string{"contents": "write", "issues": "write"} + if !reflect.DeepEqual(permissions, wantPermissions) { + t.Fatalf("permissions = %v, want %v", permissions, wantPermissions) + } + wantEvents := []string{"issues", "pull_request"} + if !reflect.DeepEqual(events, wantEvents) { + t.Fatalf("events = %v, want %v (schedule must be excluded)", events, wantEvents) + } +} + +func TestInferBootstrapGitHubAppRequirements_NoSources(t *testing.T) { + permissions, events, err := inferBootstrapGitHubAppRequirements(context.Background(), nil) + if err != nil { + t.Fatalf("inferBootstrapGitHubAppRequirements returned error: %v", err) + } + if permissions != nil || events != nil { + t.Fatalf("expected nil permissions/events for no sources, got %v / %v", permissions, events) + } +} + +// TestInferBootstrapGitHubAppRequirements_SingleWorkflow covers the simplest case of a +// single workflow with a string "on" trigger and a small permissions block. +func TestInferBootstrapGitHubAppRequirements_SingleWorkflow(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "solo.md") + content := "---\non: issues\npermissions:\n issues: write\n---\n\n# Solo\n" + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write workflow: %v", err) + } + + permissions, events, err := inferBootstrapGitHubAppRequirements(context.Background(), []string{path}) + if err != nil { + t.Fatalf("inferBootstrapGitHubAppRequirements returned error: %v", err) + } + + wantPermissions := map[string]string{"issues": "write"} + if !reflect.DeepEqual(permissions, wantPermissions) { + t.Fatalf("permissions = %v, want %v", permissions, wantPermissions) + } + wantEvents := []string{"issues"} + if !reflect.DeepEqual(events, wantEvents) { + t.Fatalf("events = %v, want %v", events, wantEvents) + } +} + +// TestInferBootstrapGitHubAppRequirements_NoPermissionsOrEvents covers a workflow that +// declares neither a "permissions" block nor an "on" trigger recognized as a webhook +// event; both inferred maps/slices must come back nil rather than empty. +func TestInferBootstrapGitHubAppRequirements_NoPermissionsOrEvents(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "bare.md") + content := "---\non:\n schedule:\n - cron: \"0 0 * * *\"\n workflow_dispatch:\n---\n\n# Bare\n" + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write workflow: %v", err) + } + + permissions, events, err := inferBootstrapGitHubAppRequirements(context.Background(), []string{path}) + if err != nil { + t.Fatalf("inferBootstrapGitHubAppRequirements returned error: %v", err) + } + if permissions != nil { + t.Fatalf("permissions = %v, want nil", permissions) + } + if len(events) != 0 { + t.Fatalf("events = %v, want empty", events) + } +} + +// TestInferBootstrapGitHubAppRequirements_ThreeWorkflowsHighestScopeWins exercises +// merging across three workflows where the same resource is requested at different +// scopes (none/read/write) and events accumulate as a de-duplicated, sorted union. +func TestInferBootstrapGitHubAppRequirements_ThreeWorkflowsHighestScopeWins(t *testing.T) { + dir := t.TempDir() + paths := []string{ + filepath.Join(dir, "a.md"), + filepath.Join(dir, "b.md"), + filepath.Join(dir, "c.md"), + } + contents := []string{ + "---\non: issues\npermissions:\n contents: read\n issues: read\n---\n\n# A\n", + "---\non: pull_request\npermissions:\n contents: write\n issues: none\n---\n\n# B\n", + "---\non:\n - issues\n - pull_request\npermissions:\n contents: read\n issues: write\n---\n\n# C\n", + } + for i, path := range paths { + if err := os.WriteFile(path, []byte(contents[i]), 0o644); err != nil { + t.Fatalf("failed to write workflow %s: %v", path, err) + } + } + + permissions, events, err := inferBootstrapGitHubAppRequirements(context.Background(), paths) + if err != nil { + t.Fatalf("inferBootstrapGitHubAppRequirements returned error: %v", err) + } + + wantPermissions := map[string]string{"contents": "write", "issues": "write"} + if !reflect.DeepEqual(permissions, wantPermissions) { + t.Fatalf("permissions = %v, want %v", permissions, wantPermissions) + } + wantEvents := []string{"issues", "pull_request"} + if !reflect.DeepEqual(events, wantEvents) { + t.Fatalf("events = %v, want %v", events, wantEvents) + } +} + +// TestInferBootstrapGitHubAppRequirements_MapOnAllExcludedYieldsNoEvents verifies that +// when every trigger in a mapping-style "on" block is excluded from App inference +// (schedule/workflow_dispatch/repository_dispatch), no events are inferred at all. +func TestInferBootstrapGitHubAppRequirements_MapOnAllExcludedYieldsNoEvents(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "excluded.md") + content := "---\non:\n schedule:\n - cron: \"*/5 * * * *\"\n workflow_dispatch:\n repository_dispatch:\n types: [custom]\npermissions:\n contents: read\n---\n\n# Excluded\n" + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write workflow: %v", err) + } + + permissions, events, err := inferBootstrapGitHubAppRequirements(context.Background(), []string{path}) + if err != nil { + t.Fatalf("inferBootstrapGitHubAppRequirements returned error: %v", err) + } + wantPermissions := map[string]string{"contents": "read"} + if !reflect.DeepEqual(permissions, wantPermissions) { + t.Fatalf("permissions = %v, want %v", permissions, wantPermissions) + } + if len(events) != 0 { + t.Fatalf("events = %v, want empty", events) + } +} + +// TestInferBootstrapGitHubAppRequirements_InvalidFrontmatterSkipped verifies that a +// workflow file with unparsable frontmatter does not fail the whole inference pass; +// its contribution is simply skipped while valid sibling workflows still contribute. +func TestInferBootstrapGitHubAppRequirements_InvalidFrontmatterSkipped(t *testing.T) { + dir := t.TempDir() + badPath := filepath.Join(dir, "bad.md") + goodPath := filepath.Join(dir, "good.md") + + badContent := "---\non: [issues\npermissions:\n contents: read\n---\n\n# Bad\n" + goodContent := "---\non: pull_request\npermissions:\n contents: write\n---\n\n# Good\n" + if err := os.WriteFile(badPath, []byte(badContent), 0o644); err != nil { + t.Fatalf("failed to write bad workflow: %v", err) + } + if err := os.WriteFile(goodPath, []byte(goodContent), 0o644); err != nil { + t.Fatalf("failed to write good workflow: %v", err) + } + + permissions, events, err := inferBootstrapGitHubAppRequirements(context.Background(), []string{badPath, goodPath}) + if err != nil { + t.Fatalf("inferBootstrapGitHubAppRequirements returned error: %v", err) + } + wantPermissions := map[string]string{"contents": "write"} + if !reflect.DeepEqual(permissions, wantPermissions) { + t.Fatalf("permissions = %v, want %v", permissions, wantPermissions) + } + wantEvents := []string{"pull_request"} + if !reflect.DeepEqual(events, wantEvents) { + t.Fatalf("events = %v, want %v", events, wantEvents) + } +} + +// TestInferBootstrapGitHubAppRequirements_ResolutionErrorPropagates verifies that +// errors from resolving the underlying workflow sources (e.g. a nonexistent local +// file) are surfaced to the caller instead of being silently swallowed. +func TestInferBootstrapGitHubAppRequirements_ResolutionErrorPropagates(t *testing.T) { + dir := t.TempDir() + missing := filepath.Join(dir, "does-not-exist.md") + + _, _, err := inferBootstrapGitHubAppRequirements(context.Background(), []string{missing}) + if err == nil { + t.Fatal("expected an error for an unresolvable source, got nil") + } +} diff --git a/pkg/cli/bootstrap_profile_runner.go b/pkg/cli/bootstrap_profile_runner.go index f25d5121bac..db7fb65be73 100644 --- a/pkg/cli/bootstrap_profile_runner.go +++ b/pkg/cli/bootstrap_profile_runner.go @@ -33,9 +33,10 @@ var ( bootstrapSetSecret = func(_ context.Context, repo, name, value string) error { return setBootstrapRepoSecret(repo, name, value) } - bootstrapCreateGitHubApp = createBootstrapGitHubApp - bootstrapCheckOwnerType = checkSetupRepositoryOwnerType - bootstrapExchangeGitHubAppCode = bootstrapExchangeGitHubAppCodeImpl + bootstrapCreateGitHubApp = createBootstrapGitHubApp + bootstrapCheckOwnerType = checkSetupRepositoryOwnerType + bootstrapExchangeGitHubAppCode = bootstrapExchangeGitHubAppCodeImpl + bootstrapInferGitHubAppRequires = inferBootstrapGitHubAppRequirements ) type bootstrapProfileRunConfig struct { @@ -51,6 +52,10 @@ type bootstrapProfileRunConfig struct { // instead of a PAT. When true, copilot-auth config actions are skipped because // the workflow already has permissions.copilot-requests: write injected. UseCopilotRequests bool + // DisableGitHubAppPermissionInference disables inferring GitHub App + // permissions/events from the package's resolved workflows. When true, only + // permissions/events explicitly declared in aw.yml are applied to the App. + DisableGitHubAppPermissionInference bool } type bootstrapProfileExistingState struct { @@ -119,7 +124,23 @@ func executeBootstrapProfile(ctx context.Context, config bootstrapProfileRunConf return err } + var inferredPermissions map[string]string + var inferredEvents []string + if !config.DisableGitHubAppPermissionInference && hasBootstrapGitHubAppAction(config.Profile.Profile.Config) { + profileSources := config.Sources + if config.Profile.Source != "" { + profileSources = []string{config.Profile.Source} + } + inferredPermissions, inferredEvents, err = bootstrapInferGitHubAppRequires(ctx, profileSources) + if err != nil { + return err + } + } + for _, action := range config.Profile.Profile.Config { + if action.Type == "github-app" { + action.Permissions, action.Events = mergeBootstrapGitHubAppRequirements(action.Permissions, action.Events, inferredPermissions, inferredEvents) + } pending, err := bootstrapActionNeedsMutation(ctx, config.Repo, action, state, usesActionsToken) if err != nil { return err @@ -191,6 +212,15 @@ func applyBootstrapAction(ctx context.Context, config bootstrapProfileRunConfig, return nil } +func hasBootstrapGitHubAppAction(actions []repositoryPackageBootstrapAction) bool { + for _, action := range actions { + if action.Type == "github-app" { + return true + } + } + return false +} + func bootstrapProfileState(ctx context.Context, repo string) (*bootstrapProfileExistingState, error) { variableNames, err := listBootstrapRepoVariableNames(ctx, repo) if err != nil { diff --git a/pkg/cli/bootstrap_profile_runner_test.go b/pkg/cli/bootstrap_profile_runner_test.go index 209cbd3a07b..def24bbe013 100644 --- a/pkg/cli/bootstrap_profile_runner_test.go +++ b/pkg/cli/bootstrap_profile_runner_test.go @@ -5,6 +5,10 @@ package cli import ( "bytes" "context" + "errors" + "os" + "path/filepath" + "reflect" "strings" "testing" ) @@ -132,3 +136,324 @@ func TestPrintBootstrapConfigTODO_PreservesManifestOrder(t *testing.T) { } } } + +// TestExecuteBootstrapProfile_DisableGitHubAppPermissionInferenceSkipsResolution verifies +// that setting DisableGitHubAppPermissionInference on the run config skips inferring +// GitHub App requirements from the package's resolved workflows entirely. The inference +// function is stubbed to fail so any invocation surfaces as an error. +func TestExecuteBootstrapProfile_DisableGitHubAppPermissionInferenceSkipsResolution(t *testing.T) { + originalRunGH := runBootstrapGHContext + originalInfer := bootstrapInferGitHubAppRequires + t.Cleanup(func() { + runBootstrapGHContext = originalRunGH + bootstrapInferGitHubAppRequires = originalInfer + }) + runBootstrapGHContext = func(_ context.Context, _ string, _ ...string) ([]byte, error) { + return []byte("APP_ID\nAPP_PRIVATE_KEY\n"), nil + } + inferCalled := false + bootstrapInferGitHubAppRequires = func(_ context.Context, _ []string) (map[string]string, []string, error) { + inferCalled = true + return nil, nil, errors.New("inference must not run when disabled") + } + + profile := &resolvedBootstrapProfile{ + PackageID: "owner/repo", + Profile: &repositoryPackageBootstrap{ + Config: []repositoryPackageBootstrapAction{ + {Type: "github-app", AppIDVariable: "APP_ID", PrivateKeySecret: "APP_PRIVATE_KEY"}, + }, + }, + } + + err := executeBootstrapProfile(context.Background(), bootstrapProfileRunConfig{ + Repo: "octo/platform-ops", + Sources: nil, + Profile: profile, + DisableGitHubAppPermissionInference: true, + }) + if err != nil { + t.Fatalf("executeBootstrapProfile returned error with inference disabled: %v", err) + } + if inferCalled { + t.Fatal("expected inference function not to be called when DisableGitHubAppPermissionInference is true") + } +} + +// TestExecuteBootstrapProfile_InfersRequirementsFromRealWorkflows is an end-to-end +// integration test that exercises the real (unstubbed) inference pipeline: it resolves +// actual workflow files from disk and verifies the merged permissions/events reach +// GitHub App creation. The merge happens on a per-iteration local copy of the action +// (not written back into the profile), so this test drives the flow far enough to +// observe the merged values on the action passed to bootstrapCreateGitHubApp. +func TestExecuteBootstrapProfile_InfersRequirementsFromRealWorkflows(t *testing.T) { + restore := stubBootstrapGitHubAppCreationForInferenceTest(t) + + var capturedAction repositoryPackageBootstrapAction + bootstrapCreateGitHubApp = func(_ context.Context, _, _, _, _ string, action repositoryPackageBootstrapAction, _ bootstrapGitHubAppOverrides) (*bootstrapCreatedGitHubApp, error) { + capturedAction = action + return &bootstrapCreatedGitHubApp{ClientID: "client-id", PEM: "pem"}, nil + } + t.Cleanup(restore) + + dir := t.TempDir() + first := filepath.Join(dir, "first.md") + second := filepath.Join(dir, "second.md") + if err := os.WriteFile(first, []byte("---\non: issues\npermissions:\n issues: write\n---\n\n# First\n"), 0o644); err != nil { + t.Fatalf("failed to write first workflow: %v", err) + } + if err := os.WriteFile(second, []byte("---\non:\n pull_request:\n types: [opened]\n schedule:\n - cron: \"0 0 * * *\"\npermissions:\n contents: write\n---\n\n# Second\n"), 0o644); err != nil { + t.Fatalf("failed to write second workflow: %v", err) + } + + profile := &resolvedBootstrapProfile{ + PackageID: "owner/repo", + Profile: &repositoryPackageBootstrap{ + Config: []repositoryPackageBootstrapAction{ + {Type: "github-app", AppIDVariable: "APP_ID", PrivateKeySecret: "APP_PRIVATE_KEY"}, + }, + }, + } + + err := executeBootstrapProfile(context.Background(), bootstrapProfileRunConfig{ + Repo: "octo/platform-ops", + Sources: []string{first, second}, + Profile: profile, + }) + if err != nil { + t.Fatalf("executeBootstrapProfile returned error: %v", err) + } + + wantPermissions := map[string]string{"issues": "write", "contents": "write"} + if !reflect.DeepEqual(capturedAction.Permissions, wantPermissions) { + t.Fatalf("merged permissions = %v, want %v", capturedAction.Permissions, wantPermissions) + } + // mergeBootstrapGitHubAppRequirements always returns events sorted alphabetically, + // so this exact-order comparison is deterministic. + wantEvents := []string{"issues", "pull_request"} + if !reflect.DeepEqual(capturedAction.Events, wantEvents) { + t.Fatalf("merged events = %v, want %v (schedule must be excluded)", capturedAction.Events, wantEvents) + } +} + +// TestExecuteBootstrapProfile_InferredRequirementsMergeWithDeclaredManifestValues +// verifies that permissions/events declared explicitly in the aw.yml manifest's +// github-app action survive and are combined with (not replaced by) the values +// inferred from the package's resolved workflows. +func TestExecuteBootstrapProfile_InferredRequirementsMergeWithDeclaredManifestValues(t *testing.T) { + restore := stubBootstrapGitHubAppCreationForInferenceTest(t) + + var capturedAction repositoryPackageBootstrapAction + bootstrapCreateGitHubApp = func(_ context.Context, _, _, _, _ string, action repositoryPackageBootstrapAction, _ bootstrapGitHubAppOverrides) (*bootstrapCreatedGitHubApp, error) { + capturedAction = action + return &bootstrapCreatedGitHubApp{ClientID: "client-id", PEM: "pem"}, nil + } + t.Cleanup(restore) + + dir := t.TempDir() + wf := filepath.Join(dir, "wf.md") + if err := os.WriteFile(wf, []byte("---\non: issues\npermissions:\n issues: read\n---\n\n# Workflow\n"), 0o644); err != nil { + t.Fatalf("failed to write workflow: %v", err) + } + + profile := &resolvedBootstrapProfile{ + PackageID: "owner/repo", + Profile: &repositoryPackageBootstrap{ + Config: []repositoryPackageBootstrapAction{ + { + Type: "github-app", + AppIDVariable: "APP_ID", + PrivateKeySecret: "APP_PRIVATE_KEY", + Permissions: map[string]string{"contents": "read"}, + Events: []string{"push"}, + }, + }, + }, + } + + err := executeBootstrapProfile(context.Background(), bootstrapProfileRunConfig{ + Repo: "octo/platform-ops", + Sources: []string{wf}, + Profile: profile, + }) + if err != nil { + t.Fatalf("executeBootstrapProfile returned error: %v", err) + } + + // The manifest-declared contents:read is preserved, and the inferred issues:read + // (from the workflow's own "issues: read" permission) is added alongside it. + wantPermissions := map[string]string{"contents": "read", "issues": "read"} + if !reflect.DeepEqual(capturedAction.Permissions, wantPermissions) { + t.Fatalf("merged permissions = %v, want %v", capturedAction.Permissions, wantPermissions) + } + // mergeBootstrapGitHubAppRequirements always returns events sorted alphabetically, + // so this exact-order comparison is deterministic. + wantEvents := []string{"issues", "push"} + if !reflect.DeepEqual(capturedAction.Events, wantEvents) { + t.Fatalf("merged events = %v, want %v", capturedAction.Events, wantEvents) + } +} + +// TestExecuteBootstrapProfile_InferenceScopedToProfileSourceOnly verifies that when the +// resolved bootstrap profile carries a Source (the package that produced it), inference +// resolves only that source's workflows rather than every source config.Sources may +// contain. This prevents an unrelated standalone workflow/package installed alongside the +// bootstrap-profile package from leaking its permissions/events into this package's App. +func TestExecuteBootstrapProfile_InferenceScopedToProfileSourceOnly(t *testing.T) { + restore := stubBootstrapGitHubAppCreationForInferenceTest(t) + + var capturedAction repositoryPackageBootstrapAction + bootstrapCreateGitHubApp = func(_ context.Context, _, _, _, _ string, action repositoryPackageBootstrapAction, _ bootstrapGitHubAppOverrides) (*bootstrapCreatedGitHubApp, error) { + capturedAction = action + return &bootstrapCreatedGitHubApp{ClientID: "client-id", PEM: "pem"}, nil + } + t.Cleanup(restore) + + dir := t.TempDir() + packageWorkflow := filepath.Join(dir, "package.md") + unrelatedWorkflow := filepath.Join(dir, "unrelated.md") + if err := os.WriteFile(packageWorkflow, []byte("---\non: issues\npermissions:\n issues: write\n---\n\n# Package\n"), 0o644); err != nil { + t.Fatalf("failed to write package workflow: %v", err) + } + if err := os.WriteFile(unrelatedWorkflow, []byte("---\non: pull_request\npermissions:\n contents: write\n---\n\n# Unrelated\n"), 0o644); err != nil { + t.Fatalf("failed to write unrelated workflow: %v", err) + } + + profile := &resolvedBootstrapProfile{ + PackageID: "owner/repo", + Source: packageWorkflow, + Profile: &repositoryPackageBootstrap{ + Config: []repositoryPackageBootstrapAction{ + {Type: "github-app", AppIDVariable: "APP_ID", PrivateKeySecret: "APP_PRIVATE_KEY"}, + }, + }, + } + + // config.Sources includes the unrelated standalone workflow installed in the same + // run; only profile.Source's workflows should influence the inferred App scopes. + err := executeBootstrapProfile(context.Background(), bootstrapProfileRunConfig{ + Repo: "octo/platform-ops", + Sources: []string{packageWorkflow, unrelatedWorkflow}, + Profile: profile, + }) + if err != nil { + t.Fatalf("executeBootstrapProfile returned error: %v", err) + } + + wantPermissions := map[string]string{"issues": "write"} + if !reflect.DeepEqual(capturedAction.Permissions, wantPermissions) { + t.Fatalf("merged permissions = %v, want %v (unrelated workflow's contents:write must not leak in)", capturedAction.Permissions, wantPermissions) + } + wantEvents := []string{"issues"} + if !reflect.DeepEqual(capturedAction.Events, wantEvents) { + t.Fatalf("merged events = %v, want %v (unrelated workflow's pull_request must not leak in)", capturedAction.Events, wantEvents) + } +} + +// stubBootstrapGitHubAppCreationForInferenceTest stubs the collaborators needed to drive +// executeBootstrapProfile all the way to bootstrapCreateGitHubApp without any network +// access or interactive prompts, returning a restore func to reset all stubbed globals. +func stubBootstrapGitHubAppCreationForInferenceTest(t *testing.T) func() { + t.Helper() + originalRunGH := runBootstrapGHContext + originalCheckOwnerType := bootstrapCheckOwnerType + originalCreateApp := bootstrapCreateGitHubApp + originalUpsertVariable := bootstrapUpsertVariable + originalSetSecret := bootstrapSetSecret + t.Setenv(bootstrapGitHubAppModeEnv, "create") + + runBootstrapGHContext = func(_ context.Context, _ string, _ ...string) ([]byte, error) { + return []byte(""), nil + } + bootstrapCheckOwnerType = func(_ context.Context, _ string) (string, error) { + return "Organization", nil + } + bootstrapUpsertVariable = func(_ context.Context, _, _, _ string) error { return nil } + bootstrapSetSecret = func(_ context.Context, _, _, _ string) error { return nil } + // Safe default: callers are expected to override this immediately with a capturing + // stub, but default to a no-op (no network access) in case they don't. + bootstrapCreateGitHubApp = func(_ context.Context, _, _, _, _ string, _ repositoryPackageBootstrapAction, _ bootstrapGitHubAppOverrides) (*bootstrapCreatedGitHubApp, error) { + return &bootstrapCreatedGitHubApp{ClientID: "client-id", PEM: "pem"}, nil + } + + return func() { + runBootstrapGHContext = originalRunGH + bootstrapCheckOwnerType = originalCheckOwnerType + bootstrapCreateGitHubApp = originalCreateApp + bootstrapUpsertVariable = originalUpsertVariable + bootstrapSetSecret = originalSetSecret + } +} + +// TestExecuteBootstrapProfile_NoGitHubAppActionSkipsInferenceEntirely verifies that +// when the resolved bootstrap profile has no github-app action at all, inference is +// never invoked, even though valid, resolvable sources were provided. +func TestExecuteBootstrapProfile_NoGitHubAppActionSkipsInferenceEntirely(t *testing.T) { + originalRunGH := runBootstrapGHContext + originalInfer := bootstrapInferGitHubAppRequires + originalUpsertVariable := bootstrapUpsertVariable + t.Cleanup(func() { + runBootstrapGHContext = originalRunGH + bootstrapInferGitHubAppRequires = originalInfer + bootstrapUpsertVariable = originalUpsertVariable + }) + runBootstrapGHContext = func(_ context.Context, _ string, _ ...string) ([]byte, error) { + return []byte(""), nil + } + bootstrapUpsertVariable = func(_ context.Context, _, _, _ string) error { return nil } + inferCalled := false + bootstrapInferGitHubAppRequires = func(_ context.Context, _ []string) (map[string]string, []string, error) { + inferCalled = true + return nil, nil, errors.New("inference must not run without a github-app action") + } + + dir := t.TempDir() + wf := filepath.Join(dir, "wf.md") + if err := os.WriteFile(wf, []byte("---\non: issues\npermissions:\n issues: read\n---\n\n# Workflow\n"), 0o644); err != nil { + t.Fatalf("failed to write workflow: %v", err) + } + + profile := &resolvedBootstrapProfile{ + PackageID: "owner/repo", + Profile: &repositoryPackageBootstrap{ + Config: []repositoryPackageBootstrapAction{ + {Type: "repo-variable", Name: "MY_VAR", Default: "value"}, + }, + }, + } + + err := executeBootstrapProfile(context.Background(), bootstrapProfileRunConfig{ + Repo: "octo/platform-ops", + Sources: []string{wf}, + Profile: profile, + }) + if err != nil { + t.Fatalf("executeBootstrapProfile returned error even though no github-app action requires inference: %v", err) + } + if inferCalled { + t.Fatal("expected inference function not to be called when no github-app action is present") + } +} + +// TestExecuteBootstrapProfile_InferenceResolutionErrorPropagates verifies that when a +// github-app action is present and inference is enabled, a resolution failure for one +// of the package's sources aborts the whole bootstrap run with an error. +func TestExecuteBootstrapProfile_InferenceResolutionErrorPropagates(t *testing.T) { + profile := &resolvedBootstrapProfile{ + PackageID: "owner/repo", + Profile: &repositoryPackageBootstrap{ + Config: []repositoryPackageBootstrapAction{ + {Type: "github-app", AppIDVariable: "APP_ID", PrivateKeySecret: "APP_PRIVATE_KEY"}, + }, + }, + } + + err := executeBootstrapProfile(context.Background(), bootstrapProfileRunConfig{ + Repo: "octo/platform-ops", + Sources: []string{"/nonexistent/does-not-exist.md"}, + Profile: profile, + }) + if err == nil { + t.Fatal("expected an error when the package's workflow sources cannot be resolved") + } +} diff --git a/pkg/workflow/github_app_requirements.go b/pkg/workflow/github_app_requirements.go new file mode 100644 index 00000000000..4d0f80e5ff8 --- /dev/null +++ b/pkg/workflow/github_app_requirements.go @@ -0,0 +1,194 @@ +// This file provides the single, canonical code path used to derive the minimal +// GitHub App manifest requirements (permissions and webhook events) a workflow +// needs to operate. It is shared by every caller that must determine this "config" +// from a workflow's frontmatter — whether the workflow is reached directly as a +// standalone .md file or resolved from an aw.yml package manifest — so the same +// permission scopes, safe-outputs handling, and trigger normalization rules apply +// regardless of how the workflow was discovered. +package workflow + +import ( + "sort" + "strings" +) + +// permissionScopesWithoutAppManifestEquivalent lists Actions-only permission scopes +// that have no corresponding GitHub App manifest "default_permissions" entry and +// must never be copied into a bootstrap App manifest. id-token and attestations are +// ephemeral Actions/OIDC token claims; models and copilot-requests gate +// Actions-hosted AI features rather than API access granted to an installed App. +var permissionScopesWithoutAppManifestEquivalent = map[PermissionScope]bool{ + PermissionIdToken: true, + PermissionAttestations: true, + PermissionModels: true, + PermissionCopilotRequests: true, +} + +// GitHubAppManifestPermissionKey converts a workflow permission scope (as used in +// GitHub Actions frontmatter, e.g. "pull-requests") to the corresponding GitHub App +// manifest "default_permissions" key (e.g. "pull_requests"). It returns false when +// the scope has no GitHub App manifest equivalent (id-token, attestations, models, +// copilot-requests) and must be omitted from the manifest. +func GitHubAppManifestPermissionKey(scope PermissionScope) (string, bool) { + if permissionScopesWithoutAppManifestEquivalent[scope] { + return "", false + } + return strings.ReplaceAll(string(scope), "-", "_"), true +} + +// ComputeGitHubAppManifestPermissions derives the GitHub App manifest +// "default_permissions" map (keyed by manifest permission name, e.g. +// "pull_requests") required to operate a workflow, given its top-level frontmatter +// "permissions" value and its already parsed/merged safe-outputs configuration +// (which itself accounts for imports and per-handler apps). Standard Actions token +// scopes honor shorthand (read-all/write-all/"all: read") via Permissions.Get; +// GitHub App-only scopes (e.g. administration) are only included when explicitly +// declared, matching validateGitHubAppOnlyPermissions. Scopes with no GitHub App +// manifest equivalent, and scopes at "none", are omitted. +func ComputeGitHubAppManifestPermissions(frontmatterPermissions any, safeOutputs *SafeOutputsConfig) map[string]string { + permissions := NewPermissionsParserFromValue(frontmatterPermissions).ToPermissions() + // Merge() treats an explicit permissions map (even an empty one) as authoritative + // and clears any read-all/write-all shorthand on the receiver, so only merge when + // the safe-outputs config actually contributes permission scopes. + if safeOutputsPermissions := ComputePermissionsForSafeOutputs(safeOutputs); len(safeOutputsPermissions.permissions) > 0 { + permissions.Merge(safeOutputsPermissions) + } + + result := make(map[string]string) + addManifestPermission := func(scope PermissionScope, level PermissionLevel, ok bool) { + if !ok || level == "" || level == PermissionNone { + return + } + key, supported := GitHubAppManifestPermissionKey(scope) + if !supported { + return + } + result[key] = string(level) + } + + for _, scope := range GetAllPermissionScopes() { + level, ok := permissions.Get(scope) + addManifestPermission(scope, level, ok) + } + // GitHub App-only scopes must not be derived from read-all/write-all/"all: read" + // shorthand: only an explicit declaration in frontmatter grants them. + for _, scope := range GetAllGitHubAppOnlyScopes() { + level, ok := permissions.GetExplicit(scope) + addManifestPermission(scope, level, ok) + } + + if len(result) == 0 { + return nil + } + return result +} + +// nonWebhookOnTriggers lists workflow "on:" triggers that Actions supports but that +// are not deliverable to a GitHub App webhook subscription: they are driven by +// Actions scheduling/dispatch/composition, not by a webhook event delivered to an +// installed App. +var nonWebhookOnTriggers = map[string]bool{ + "schedule": true, + "workflow_dispatch": true, + "repository_dispatch": true, + "workflow_call": true, +} + +// commandTriggerDefaultWebhookEvents lists the underlying GitHub webhook events a +// "command:" trigger (or its "/name" shorthand, expanded to "slash_command" by the +// compiler) listens to by default, when it does not restrict itself with an +// explicit "events:" list. +var commandTriggerDefaultWebhookEvents = []string{"issues", "issue_comment", "pull_request", "pull_request_review_comment"} + +// labelCommandDefaultWebhookEvents lists the underlying GitHub webhook events a +// "label_command:" trigger listens to by default. +var labelCommandDefaultWebhookEvents = []string{"issues", "pull_request", "discussion"} + +// IsKnownGitHubWebhookEvent reports whether name is a recognized GitHub Actions/App +// webhook event type. +func IsKnownGitHubWebhookEvent(name string) bool { + return isKnownGitHubEvent(name) +} + +// rawOnSectionTriggerNames extracts the top-level trigger key(s) from a workflow's +// "on" frontmatter value, which may be a string, a list of strings, or a mapping of +// trigger name to trigger configuration. +func rawOnSectionTriggerNames(onValue any) []string { + var names []string + switch value := onValue.(type) { + case string: + names = append(names, value) + case []any: + for _, item := range value { + if name, ok := item.(string); ok { + names = append(names, name) + } + } + case map[string]any: + for name := range value { + names = append(names, name) + } + } + return names +} + +// NormalizeGitHubAppWebhookEvents extracts the set of GitHub App webhook events that +// a workflow's "on:" frontmatter section requires. It expands gh-aw's compiler-only +// command trigger shorthands ("/name" slash command strings, "command:", and +// "label_command:") to their underlying webhook events, maps pull_request_target to +// the pull_request webhook, and excludes both non-webhook Actions triggers +// (schedule, workflow_dispatch, repository_dispatch, workflow_call) and gh-aw's +// on:-section extension keys (reaction, status-comment, github-app, bots, roles, +// etc.) that are not themselves webhook events. +func NormalizeGitHubAppWebhookEvents(onValue any) []string { + eventSet := map[string]struct{}{} + addEvents := func(events []string) { + for _, event := range events { + eventSet[event] = struct{}{} + } + } + + for _, name := range rawOnSectionTriggerNames(onValue) { + name = strings.TrimSpace(name) + if name == "" { + continue + } + + if strings.HasPrefix(name, "/") { + // Slash command shorthand, e.g. "on: /my-bot". + addEvents(commandTriggerDefaultWebhookEvents) + continue + } + if strings.HasPrefix(name, "label-command ") { + addEvents(labelCommandDefaultWebhookEvents) + continue + } + + switch name { + case "slash_command", "command": + addEvents(commandTriggerDefaultWebhookEvents) + continue + case "label_command": + addEvents(labelCommandDefaultWebhookEvents) + continue + } + + if nonWebhookOnTriggers[name] || ghAwOnSectionKeys[name] { + continue + } + if !isKnownGitHubEvent(name) { + continue + } + if name == "pull_request_target" { + name = "pull_request" + } + eventSet[name] = struct{}{} + } + + events := make([]string, 0, len(eventSet)) + for event := range eventSet { + events = append(events, event) + } + sort.Strings(events) + return events +} diff --git a/pkg/workflow/github_app_requirements_test.go b/pkg/workflow/github_app_requirements_test.go new file mode 100644 index 00000000000..9af97bc6901 --- /dev/null +++ b/pkg/workflow/github_app_requirements_test.go @@ -0,0 +1,181 @@ +//go:build !integration + +package workflow + +import ( + "reflect" + "sort" + "testing" +) + +func TestGitHubAppManifestPermissionKey(t *testing.T) { + tests := []struct { + name string + scope PermissionScope + wantKey string + wantOK bool + }{ + {name: "hyphenated scope normalizes to underscore", scope: PermissionPullRequests, wantKey: "pull_requests", wantOK: true}, + {name: "security-events normalizes to underscore", scope: PermissionSecurityEvents, wantKey: "security_events", wantOK: true}, + {name: "single word scope unchanged", scope: PermissionContents, wantKey: "contents", wantOK: true}, + {name: "id-token has no manifest equivalent", scope: PermissionIdToken, wantOK: false}, + {name: "attestations has no manifest equivalent", scope: PermissionAttestations, wantOK: false}, + {name: "models has no manifest equivalent", scope: PermissionModels, wantOK: false}, + {name: "copilot-requests has no manifest equivalent", scope: PermissionCopilotRequests, wantOK: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + key, ok := GitHubAppManifestPermissionKey(tt.scope) + if ok != tt.wantOK { + t.Fatalf("GitHubAppManifestPermissionKey(%q) ok = %v, want %v", tt.scope, ok, tt.wantOK) + } + if ok && key != tt.wantKey { + t.Fatalf("GitHubAppManifestPermissionKey(%q) key = %q, want %q", tt.scope, key, tt.wantKey) + } + }) + } +} + +func TestComputeGitHubAppManifestPermissions(t *testing.T) { + tests := []struct { + name string + permissions any + safeOutputs *SafeOutputsConfig + want map[string]string + }{ + { + name: "nil permissions and no safe-outputs yields nil", + permissions: nil, + safeOutputs: nil, + want: nil, + }, + { + name: "top-level permissions normalize hyphenated keys to manifest keys", + permissions: map[string]any{ + "pull-requests": "write", + "security-events": "read", + }, + want: map[string]string{"pull_requests": "write", "security_events": "read"}, + }, + { + name: "none-level permissions are omitted", + permissions: map[string]any{"issues": "none"}, + want: nil, + }, + { + name: "scopes without a manifest equivalent are dropped", + permissions: map[string]any{"id-token": "write", "contents": "read"}, + want: map[string]string{"contents": "read"}, + }, + { + name: "safe-outputs derive write permissions even when top-level is read-only", + permissions: map[string]any{"issues": "read"}, + safeOutputs: SafeOutputsConfigFromKeys([]string{"create-issue"}), + want: map[string]string{"issues": "write"}, + }, + { + name: "app-only scopes require explicit declaration, not read-all/write-all shorthand", + permissions: map[string]any{ + "permissions": "write-all", + }, + }, + { + name: "app-only scope explicitly declared is included", + permissions: map[string]any{"administration": "write"}, + want: map[string]string{"administration": "write"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ComputeGitHubAppManifestPermissions(tt.permissions, tt.safeOutputs) + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("ComputeGitHubAppManifestPermissions() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestComputeGitHubAppManifestPermissionsWriteAllShorthandExcludesAppOnlyScopes(t *testing.T) { + got := ComputeGitHubAppManifestPermissions("write-all", nil) + if _, ok := got["administration"]; ok { + t.Fatalf("write-all shorthand must not implicitly grant GitHub App-only scopes, got %v", got) + } + if got["contents"] != "write" { + t.Fatalf("write-all shorthand should still grant standard scopes, got %v", got) + } +} + +func TestNormalizeGitHubAppWebhookEvents(t *testing.T) { + tests := []struct { + name string + on any + want []string + }{ + {name: "nil on value", on: nil, want: nil}, + {name: "string trigger", on: "issues", want: []string{"issues"}}, + {name: "list of triggers", on: []any{"issues", "pull_request"}, want: []string{"issues", "pull_request"}}, + { + name: "map excludes non-webhook triggers", + on: map[string]any{ + "issues": map[string]any{"types": []any{"opened"}}, + "schedule": []any{map[string]any{"cron": "0 0 * * *"}}, + "workflow_dispatch": nil, + "repository_dispatch": nil, + "workflow_call": nil, + }, + want: []string{"issues"}, + }, + { + name: "gh-aw compiler-only keys excluded", + on: map[string]any{ + "issues": nil, + "reaction": "eyes", + "status-comment": true, + }, + want: []string{"issues"}, + }, + { + name: "pull_request_target maps to pull_request", + on: map[string]any{"pull_request_target": nil}, + want: []string{"pull_request"}, + }, + { + name: "slash command shorthand expands to underlying webhook events", + on: "/my-bot", + want: []string{"issue_comment", "issues", "pull_request", "pull_request_review_comment"}, + }, + { + name: "slash_command key expands to underlying webhook events", + on: map[string]any{"slash_command": map[string]any{"name": "my-bot"}}, + want: []string{"issue_comment", "issues", "pull_request", "pull_request_review_comment"}, + }, + { + name: "label_command key expands to underlying webhook events", + on: map[string]any{"label_command": map[string]any{"name": "my-label"}}, + want: []string{"discussion", "issues", "pull_request"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := NormalizeGitHubAppWebhookEvents(tt.on) + sort.Strings(got) + want := append([]string(nil), tt.want...) + sort.Strings(want) + if len(got) == 0 && len(want) == 0 { + return + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("NormalizeGitHubAppWebhookEvents(%v) = %v, want %v", tt.on, got, want) + } + }) + } +} + +func TestIsKnownGitHubWebhookEvent(t *testing.T) { + if !IsKnownGitHubWebhookEvent("issues") { + t.Fatal("expected issues to be a known GitHub webhook event") + } + if IsKnownGitHubWebhookEvent("not-a-real-event") { + t.Fatal("expected not-a-real-event to be unknown") + } +}