diff --git a/pkg/workflow/activation_permissions_scope_test.go b/pkg/workflow/activation_permissions_scope_test.go index 4786f19f046..57212c6cd10 100644 --- a/pkg/workflow/activation_permissions_scope_test.go +++ b/pkg/workflow/activation_permissions_scope_test.go @@ -185,16 +185,17 @@ engine: copilot func TestAddActivationInteractionPermissionsMapFallsBackOnInvalidOnYAML(t *testing.T) { permsMap := map[PermissionScope]PermissionLevel{} - addActivationInteractionPermissionsMap(permsMap, "on: [", - /* hasReaction */ true, - /* reactionIncludesIssues */ true, - /* reactionIncludesPullRequests */ true, - /* reactionIncludesDiscussions */ true, - /* hasStatusComment */ true, - /* statusCommentIncludesIssues */ true, - /* statusCommentIncludesPullRequests */ true, - /* statusCommentIncludesDiscussions */ true, - ) + addActivationInteractionPermissionsMap(permsMap, activationInteractionPermissionsOptions{ + onSection: "on: [", + hasReaction: true, + reactionIncludesIssues: true, + reactionIncludesPullRequests: true, + reactionIncludesDiscussions: true, + hasStatusComment: true, + statusCommentIncludesIssues: true, + statusCommentIncludesPullRequests: true, + statusCommentIncludesDiscussions: true, + }) assert.Equal(t, PermissionWrite, permsMap[PermissionIssues], "fallback should include issues:write") assert.Equal(t, PermissionWrite, permsMap[PermissionPullRequests], "fallback should include pull-requests:write") @@ -204,16 +205,17 @@ func TestAddActivationInteractionPermissionsMapFallsBackOnInvalidOnYAML(t *testi func TestAddActivationInteractionPermissionsMapFallbackRespectsStatusCommentDiscussionsToggle(t *testing.T) { permsMap := map[PermissionScope]PermissionLevel{} - addActivationInteractionPermissionsMap(permsMap, "name: no-on-key", - /* hasReaction */ false, - /* reactionIncludesIssues */ true, - /* reactionIncludesPullRequests */ true, - /* reactionIncludesDiscussions */ true, - /* hasStatusComment */ true, - /* statusCommentIncludesIssues */ true, - /* statusCommentIncludesPullRequests */ true, - /* statusCommentIncludesDiscussions */ false, - ) + addActivationInteractionPermissionsMap(permsMap, activationInteractionPermissionsOptions{ + onSection: "name: no-on-key", + hasReaction: false, + reactionIncludesIssues: true, + reactionIncludesPullRequests: true, + reactionIncludesDiscussions: true, + hasStatusComment: true, + statusCommentIncludesIssues: true, + statusCommentIncludesPullRequests: true, + statusCommentIncludesDiscussions: false, + }) assert.Equal(t, PermissionWrite, permsMap[PermissionIssues], "fallback should include issues:write for status comments") _, hasPullRequests := permsMap[PermissionPullRequests] @@ -262,16 +264,17 @@ engine: copilot func TestAddActivationInteractionPermissionsMapFallbackRespectsStatusCommentIssuesToggle(t *testing.T) { permsMap := map[PermissionScope]PermissionLevel{} - addActivationInteractionPermissionsMap(permsMap, "name: no-on-key", - /* hasReaction */ false, - /* reactionIncludesIssues */ true, - /* reactionIncludesPullRequests */ true, - /* reactionIncludesDiscussions */ true, - /* hasStatusComment */ true, - /* statusCommentIncludesIssues */ false, - /* statusCommentIncludesPullRequests */ false, - /* statusCommentIncludesDiscussions */ true, - ) + addActivationInteractionPermissionsMap(permsMap, activationInteractionPermissionsOptions{ + onSection: "name: no-on-key", + hasReaction: false, + reactionIncludesIssues: true, + reactionIncludesPullRequests: true, + reactionIncludesDiscussions: true, + hasStatusComment: true, + statusCommentIncludesIssues: false, + statusCommentIncludesPullRequests: false, + statusCommentIncludesDiscussions: true, + }) _, hasIssues := permsMap[PermissionIssues] _, hasPullRequests := permsMap[PermissionPullRequests] @@ -345,16 +348,17 @@ engine: copilot func TestAddActivationInteractionPermissionsMapFallbackRespectsStatusCommentPullRequestsToggle(t *testing.T) { permsMap := map[PermissionScope]PermissionLevel{} - addActivationInteractionPermissionsMap(permsMap, "name: no-on-key", - /* hasReaction */ false, - /* reactionIncludesIssues */ true, - /* reactionIncludesPullRequests */ true, - /* reactionIncludesDiscussions */ true, - /* hasStatusComment */ true, - /* statusCommentIncludesIssues */ false, - /* statusCommentIncludesPullRequests */ false, - /* statusCommentIncludesDiscussions */ true, - ) + addActivationInteractionPermissionsMap(permsMap, activationInteractionPermissionsOptions{ + onSection: "name: no-on-key", + hasReaction: false, + reactionIncludesIssues: true, + reactionIncludesPullRequests: true, + reactionIncludesDiscussions: true, + hasStatusComment: true, + statusCommentIncludesIssues: false, + statusCommentIncludesPullRequests: false, + statusCommentIncludesDiscussions: true, + }) _, hasIssues := permsMap[PermissionIssues] _, hasPullRequests := permsMap[PermissionPullRequests] @@ -371,16 +375,17 @@ func TestActivationPermissionsIssueCommentReactionRequiresPullRequestsWrite(t *t permsMap := map[PermissionScope]PermissionLevel{} onSection := "on:\n issue_comment:\n types: [created]\n" - addActivationInteractionPermissionsMap(permsMap, onSection, - /* hasReaction */ true, - /* reactionIncludesIssues */ true, - /* reactionIncludesPullRequests */ true, - /* reactionIncludesDiscussions */ false, - /* hasStatusComment */ false, - /* statusCommentIncludesIssues */ false, - /* statusCommentIncludesPullRequests */ false, - /* statusCommentIncludesDiscussions */ false, - ) + addActivationInteractionPermissionsMap(permsMap, activationInteractionPermissionsOptions{ + onSection: onSection, + hasReaction: true, + reactionIncludesIssues: true, + reactionIncludesPullRequests: true, + reactionIncludesDiscussions: false, + hasStatusComment: false, + statusCommentIncludesIssues: false, + statusCommentIncludesPullRequests: false, + statusCommentIncludesDiscussions: false, + }) assert.Equal(t, PermissionWrite, permsMap[PermissionIssues], "issue_comment reaction should include issues:write") assert.Equal(t, PermissionWrite, permsMap[PermissionPullRequests], "issue_comment reaction should include pull-requests:write because PR comments use issue_comment event") diff --git a/pkg/workflow/compiler_activation_job.go b/pkg/workflow/compiler_activation_job.go index 17ecba03915..87b8db073ab 100644 --- a/pkg/workflow/compiler_activation_job.go +++ b/pkg/workflow/compiler_activation_job.go @@ -78,85 +78,50 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate func addActivationInteractionPermissions( perms *Permissions, - onSection string, - hasReaction bool, - reactionIncludesIssues bool, - reactionIncludesPullRequests bool, - reactionIncludesDiscussions bool, - hasStatusComment bool, - statusCommentIncludesIssues bool, - statusCommentIncludesPullRequests bool, - statusCommentIncludesDiscussions bool, + options activationInteractionPermissionsOptions, ) { if perms == nil { return } permsMap := make(map[PermissionScope]PermissionLevel) - addActivationInteractionPermissionsMap( - permsMap, - onSection, - hasReaction, - reactionIncludesIssues, - reactionIncludesPullRequests, - reactionIncludesDiscussions, - hasStatusComment, - statusCommentIncludesIssues, - statusCommentIncludesPullRequests, - statusCommentIncludesDiscussions, - ) + addActivationInteractionPermissionsMap(permsMap, options) for scope, level := range permsMap { perms.Set(scope, level) } } +type activationInteractionPermissionsOptions struct { + onSection string + hasReaction bool + reactionIncludesIssues bool + reactionIncludesPullRequests bool + reactionIncludesDiscussions bool + hasStatusComment bool + statusCommentIncludesIssues bool + statusCommentIncludesPullRequests bool + statusCommentIncludesDiscussions bool +} + func addActivationInteractionPermissionsMap( permsMap map[PermissionScope]PermissionLevel, - onSection string, - hasReaction bool, - reactionIncludesIssues bool, - reactionIncludesPullRequests bool, - reactionIncludesDiscussions bool, - hasStatusComment bool, - statusCommentIncludesIssues bool, - statusCommentIncludesPullRequests bool, - statusCommentIncludesDiscussions bool, + options activationInteractionPermissionsOptions, ) { - if !hasReaction && !hasStatusComment { + if !options.hasReaction && !options.hasStatusComment { return } // Fallback for unit tests or synthetic WorkflowData instances that do not populate the "on" section. // Real compiled workflows always have a populated trigger section. - if onSection == "" { + if options.onSection == "" { compilerActivationJobLog.Print("Empty on section while computing activation permissions; using broad fallback permissions") - addBroadActivationInteractionPermissions( - permsMap, - hasReaction, - reactionIncludesIssues, - reactionIncludesPullRequests, - reactionIncludesDiscussions, - hasStatusComment, - statusCommentIncludesIssues, - statusCommentIncludesPullRequests, - statusCommentIncludesDiscussions, - ) + addBroadActivationInteractionPermissions(permsMap, options) return } - eventSet, eventSetParsed := activationEventSet(onSection) + eventSet, eventSetParsed := activationEventSet(options.onSection) if !eventSetParsed { compilerActivationJobLog.Print("Unable to parse activation trigger events while computing permissions; using broad fallback permissions") - addBroadActivationInteractionPermissions( - permsMap, - hasReaction, - reactionIncludesIssues, - reactionIncludesPullRequests, - reactionIncludesDiscussions, - hasStatusComment, - statusCommentIncludesIssues, - statusCommentIncludesPullRequests, - statusCommentIncludesDiscussions, - ) + addBroadActivationInteractionPermissions(permsMap, options) return } @@ -167,10 +132,10 @@ func addActivationInteractionPermissionsMap( hasDiscussionEvent := eventSet["discussion"] hasDiscussionCommentEvent := eventSet["discussion_comment"] - if hasReaction { + if options.hasReaction { // Reactions on issues, issue comments, and pull requests use issues endpoints. - needsIssuesWriteForIssueEvents := reactionIncludesIssues && (hasIssuesEvent || hasIssueCommentEvent) - needsIssuesWriteForPullRequestEvents := reactionIncludesPullRequests && hasPullRequestEvent + needsIssuesWriteForIssueEvents := options.reactionIncludesIssues && (hasIssuesEvent || hasIssueCommentEvent) + needsIssuesWriteForPullRequestEvents := options.reactionIncludesPullRequests && hasPullRequestEvent needsIssuesWriteForReaction := needsIssuesWriteForIssueEvents || needsIssuesWriteForPullRequestEvents if needsIssuesWriteForReaction { permsMap[PermissionIssues] = PermissionWrite @@ -178,23 +143,23 @@ func addActivationInteractionPermissionsMap( // Reactions on pull requests and PR review comments require pull-requests: write. // issue_comment events also fire for PR comments (slash_command with events:[pull_request_comment] // compiles to issue_comment), so pull-requests: write is also needed when issue_comment is present. - if reactionIncludesPullRequests && (hasPullRequestEvent || hasPullRequestReviewCommentEvent || hasIssueCommentEvent) { + if options.reactionIncludesPullRequests && (hasPullRequestEvent || hasPullRequestReviewCommentEvent || hasIssueCommentEvent) { permsMap[PermissionPullRequests] = PermissionWrite } // Reactions on discussions use GraphQL discussion APIs. - if reactionIncludesDiscussions && (hasDiscussionEvent || hasDiscussionCommentEvent) { + if options.reactionIncludesDiscussions && (hasDiscussionEvent || hasDiscussionCommentEvent) { permsMap[PermissionDiscussions] = PermissionWrite } } - if hasStatusComment { + if options.hasStatusComment { // Status comments for issue and pull request related events use issue comment endpoints. - if (statusCommentIncludesIssues && (hasIssuesEvent || hasIssueCommentEvent)) || - (statusCommentIncludesPullRequests && (hasPullRequestEvent || hasPullRequestReviewCommentEvent)) { + if (options.statusCommentIncludesIssues && (hasIssuesEvent || hasIssueCommentEvent)) || + (options.statusCommentIncludesPullRequests && (hasPullRequestEvent || hasPullRequestReviewCommentEvent)) { permsMap[PermissionIssues] = PermissionWrite } // Status comments for discussions use discussion comment APIs and can be disabled via frontmatter. - if statusCommentIncludesDiscussions && (hasDiscussionEvent || hasDiscussionCommentEvent) { + if options.statusCommentIncludesDiscussions && (hasDiscussionEvent || hasDiscussionCommentEvent) { permsMap[PermissionDiscussions] = PermissionWrite } } @@ -202,28 +167,21 @@ func addActivationInteractionPermissionsMap( func addBroadActivationInteractionPermissions( permsMap map[PermissionScope]PermissionLevel, - hasReaction bool, - reactionIncludesIssues bool, - reactionIncludesPullRequests bool, - reactionIncludesDiscussions bool, - hasStatusComment bool, - statusCommentIncludesIssues bool, - statusCommentIncludesPullRequests bool, - statusCommentIncludesDiscussions bool, + options activationInteractionPermissionsOptions, ) { - if !hasReaction && !hasStatusComment { + if !options.hasReaction && !options.hasStatusComment { return } - needsIssuesWriteForReaction := hasReaction && (reactionIncludesIssues || reactionIncludesPullRequests) - needsIssuesWriteForStatusComment := statusCommentIncludesIssues || statusCommentIncludesPullRequests + needsIssuesWriteForReaction := options.hasReaction && (options.reactionIncludesIssues || options.reactionIncludesPullRequests) + needsIssuesWriteForStatusComment := options.statusCommentIncludesIssues || options.statusCommentIncludesPullRequests if needsIssuesWriteForReaction || needsIssuesWriteForStatusComment { permsMap[PermissionIssues] = PermissionWrite } - if hasReaction && reactionIncludesPullRequests { + if options.hasReaction && options.reactionIncludesPullRequests { permsMap[PermissionPullRequests] = PermissionWrite } - if (hasReaction && reactionIncludesDiscussions) || statusCommentIncludesDiscussions { + if (options.hasReaction && options.reactionIncludesDiscussions) || options.statusCommentIncludesDiscussions { permsMap[PermissionDiscussions] = PermissionWrite } } diff --git a/pkg/workflow/compiler_activation_job_builder.go b/pkg/workflow/compiler_activation_job_builder.go index 7cfb5dc413d..19102585539 100644 --- a/pkg/workflow/compiler_activation_job_builder.go +++ b/pkg/workflow/compiler_activation_job_builder.go @@ -135,15 +135,17 @@ func (c *Compiler) addActivationFeedbackAndValidationSteps(ctx *activationJobBui appPerms := NewPermissions() addActivationInteractionPermissions( appPerms, - data.On, - ctx.hasReaction, - ctx.reactionIssues, - ctx.reactionPullRequests, - ctx.reactionDiscussions, - ctx.hasStatusComment, - ctx.statusCommentIssues, - ctx.statusCommentPRs, - ctx.statusCommentDiscussions, + activationInteractionPermissionsOptions{ + onSection: data.On, + hasReaction: ctx.hasReaction, + reactionIncludesIssues: ctx.reactionIssues, + reactionIncludesPullRequests: ctx.reactionPullRequests, + reactionIncludesDiscussions: ctx.reactionDiscussions, + hasStatusComment: ctx.hasStatusComment, + statusCommentIncludesIssues: ctx.statusCommentIssues, + statusCommentIncludesPullRequests: ctx.statusCommentPRs, + statusCommentIncludesDiscussions: ctx.statusCommentDiscussions, + }, ) if ctx.shouldRemoveLabel { if slices.Contains(ctx.filteredLabelEvents, "issues") || slices.Contains(ctx.filteredLabelEvents, "pull_request") { @@ -490,18 +492,17 @@ func (c *Compiler) buildActivationPermissions(ctx *activationJobBuildContext) st if !ctx.data.StaleCheckDisabled { permsMap[PermissionActions] = PermissionRead } - addActivationInteractionPermissionsMap( - permsMap, - ctx.data.On, - ctx.hasReaction, - ctx.reactionIssues, - ctx.reactionPullRequests, - ctx.reactionDiscussions, - ctx.hasStatusComment, - ctx.statusCommentIssues, - ctx.statusCommentPRs, - ctx.statusCommentDiscussions, - ) + addActivationInteractionPermissionsMap(permsMap, activationInteractionPermissionsOptions{ + onSection: ctx.data.On, + hasReaction: ctx.hasReaction, + reactionIncludesIssues: ctx.reactionIssues, + reactionIncludesPullRequests: ctx.reactionPullRequests, + reactionIncludesDiscussions: ctx.reactionDiscussions, + hasStatusComment: ctx.hasStatusComment, + statusCommentIncludesIssues: ctx.statusCommentIssues, + statusCommentIncludesPullRequests: ctx.statusCommentPRs, + statusCommentIncludesDiscussions: ctx.statusCommentDiscussions, + }) // For centralized slash_command workflows, the compiled "on" section only contains // workflow_dispatch, so addActivationInteractionPermissionsMap above cannot detect the // original event types and skips write permissions. Supplement with a synthetic section @@ -509,18 +510,17 @@ func (c *Compiler) buildActivationPermissions(ctx *activationJobBuildContext) st if ctx.data.CommandCentralized && (ctx.hasReaction || ctx.hasStatusComment) { syntheticOn := buildCentralizedCommandOnSection(ctx.data.CommandEvents) if syntheticOn != "" { - addActivationInteractionPermissionsMap( - permsMap, - syntheticOn, - ctx.hasReaction, - ctx.reactionIssues, - ctx.reactionPullRequests, - ctx.reactionDiscussions, - ctx.hasStatusComment, - ctx.statusCommentIssues, - ctx.statusCommentPRs, - ctx.statusCommentDiscussions, - ) + addActivationInteractionPermissionsMap(permsMap, activationInteractionPermissionsOptions{ + onSection: syntheticOn, + hasReaction: ctx.hasReaction, + reactionIncludesIssues: ctx.reactionIssues, + reactionIncludesPullRequests: ctx.reactionPullRequests, + reactionIncludesDiscussions: ctx.reactionDiscussions, + hasStatusComment: ctx.hasStatusComment, + statusCommentIncludesIssues: ctx.statusCommentIssues, + statusCommentIncludesPullRequests: ctx.statusCommentPRs, + statusCommentIncludesDiscussions: ctx.statusCommentDiscussions, + }) } } if ctx.data.LockForAgent {