diff --git a/pkg/workflow/activation_permissions_scope_test.go b/pkg/workflow/activation_permissions_scope_test.go index 57212c6cd10..0f783dab9a7 100644 --- a/pkg/workflow/activation_permissions_scope_test.go +++ b/pkg/workflow/activation_permissions_scope_test.go @@ -224,6 +224,28 @@ func TestAddActivationInteractionPermissionsMapFallbackRespectsStatusCommentDisc assert.False(t, hasDiscussions, "fallback should omit discussions:write when status-comment.discussions is false and reactions are disabled") } +func TestAddActivationInteractionPermissionsMapFallbackIgnoresStatusCommentDefaultsWhenDisabled(t *testing.T) { + permsMap := map[PermissionScope]PermissionLevel{} + + addActivationInteractionPermissionsMap(permsMap, activationInteractionPermissionsOptions{ + onSection: "on: [", + hasReaction: true, + reactionIncludesIssues: false, + reactionIncludesPullRequests: false, + reactionIncludesDiscussions: true, + hasStatusComment: false, + statusCommentIncludesIssues: true, + statusCommentIncludesPullRequests: true, + statusCommentIncludesDiscussions: true, + }) + + _, hasIssues := permsMap[PermissionIssues] + assert.False(t, hasIssues, "fallback should not include issues:write when status-comment is disabled") + _, hasPullRequests := permsMap[PermissionPullRequests] + assert.False(t, hasPullRequests, "fallback should not include pull-requests:write for discussions-only reactions") + assert.Equal(t, PermissionWrite, permsMap[PermissionDiscussions], "fallback should include discussions:write for discussions reactions") +} + func TestActivationPermissionsStatusCommentIssuesDisabled(t *testing.T) { tmpDir := testutil.TempDir(t, "activation-perms-status-comment-issues-disabled") testFile := filepath.Join(tmpDir, "status-comment-issues-disabled.md") @@ -543,3 +565,203 @@ engine: copilot assert.Contains(t, activationJobSection, "pull-requests: write", "activation job should include pull-requests:write for slash_command PR comment reactions") assert.NotContains(t, activationJobSection, "discussions: write", "activation job should not include discussions:write for slash_command PR comment reactions") } + +// Tests for workflow_call trigger + reaction/status-comment permissions (issue #39372). +// When a workflow declares workflow_call as its trigger it acts as a reusable workflow and can +// be called from ANY caller event. The compiler cannot know the caller's event type at compile +// time, so it must grant the full set of permissions that the configured reactions / status-comments +// could require. + +func TestActivationPermissionsWorkflowCallReaction(t *testing.T) { + tmpDir := testutil.TempDir(t, "activation-perms-workflow-call-reaction") + testFile := filepath.Join(tmpDir, "workflow-call-reaction.md") + testContent := `--- +on: + workflow_call: + reaction: eyes +engine: copilot +--- + +# Reusable workflow with reaction +` + + err := os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err, "failed to write test workflow") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "failed to compile workflow") + + lockContent, err := os.ReadFile(stringutil.MarkdownToLockFile(testFile)) + require.NoError(t, err, "failed to read generated lock file") + + activationJobSection := extractJobSection(string(lockContent), string(constants.ActivationJobName)) + assert.Contains(t, activationJobSection, "issues: write", "activation job should include issues: write when workflow_call + reaction are configured") + assert.Contains(t, activationJobSection, "pull-requests: write", "activation job should include pull-requests: write when workflow_call + reaction are configured") + assert.Contains(t, activationJobSection, "discussions: write", "activation job should include discussions: write when workflow_call + reaction are configured") +} + +func TestActivationPermissionsWorkflowCallReactionDiscussionsOnly(t *testing.T) { + tmpDir := testutil.TempDir(t, "activation-perms-workflow-call-reaction-discussions-only") + testFile := filepath.Join(tmpDir, "workflow-call-reaction-discussions-only.md") + testContent := `--- +on: + workflow_call: + reaction: + type: eyes + issues: false + pull-requests: false + discussions: true +engine: copilot +--- + +# Reusable workflow with discussions-only reaction +` + + err := os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err, "failed to write test workflow") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "failed to compile workflow") + + lockContent, err := os.ReadFile(stringutil.MarkdownToLockFile(testFile)) + require.NoError(t, err, "failed to read generated lock file") + + activationJobSection := extractJobSection(string(lockContent), string(constants.ActivationJobName)) + assert.NotContains(t, activationJobSection, "issues: write", "activation job should not include issues: write when workflow_call + discussions-only reaction are configured") + assert.NotContains(t, activationJobSection, "pull-requests: write", "activation job should not include pull-requests: write when workflow_call + discussions-only reaction are configured") + assert.Contains(t, activationJobSection, "discussions: write", "activation job should include discussions: write when workflow_call + discussions-only reaction are configured") +} + +func TestActivationPermissionsWorkflowCallStatusComment(t *testing.T) { + tmpDir := testutil.TempDir(t, "activation-perms-workflow-call-status-comment") + testFile := filepath.Join(tmpDir, "workflow-call-status-comment.md") + testContent := `--- +on: + workflow_call: + reaction: none + status-comment: true +engine: copilot +--- + +# Reusable workflow with status-comment only +` + + err := os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err, "failed to write test workflow") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "failed to compile workflow") + + lockContent, err := os.ReadFile(stringutil.MarkdownToLockFile(testFile)) + require.NoError(t, err, "failed to read generated lock file") + + activationJobSection := extractJobSection(string(lockContent), string(constants.ActivationJobName)) + assert.Contains(t, activationJobSection, "issues: write", "activation job should include issues: write when workflow_call + status-comment are configured") + // pull-requests:write is only needed for PR reactions (addBroadActivationInteractionPermissions only sets it for + // hasReaction && reactionIncludesPullRequests); PR status-comments post via the issues API so issues:write suffices. + assert.NotContains(t, activationJobSection, "pull-requests: write", "activation job should not include pull-requests: write for status-comment-only (PR status-comments use the issues API scope, not pull-requests)") + // discussions:write is included because status-comment defaults include discussions (statusCommentIncludesDiscussions=true by default) + assert.Contains(t, activationJobSection, "discussions: write", "activation job should include discussions: write when workflow_call + status-comment are configured (discussions enabled by default)") +} + +func TestActivationPermissionsWorkflowCallAndIssuesTriggerReaction(t *testing.T) { + tmpDir := testutil.TempDir(t, "activation-perms-workflow-call-and-issues-reaction") + testFile := filepath.Join(tmpDir, "workflow-call-and-issues-reaction.md") + testContent := `--- +on: + workflow_call: + issues: + types: [labeled] + reaction: eyes +engine: copilot +--- + +# Reusable workflow with workflow_call and issues trigger +` + + err := os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err, "failed to write test workflow") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "failed to compile workflow") + + lockContent, err := os.ReadFile(stringutil.MarkdownToLockFile(testFile)) + require.NoError(t, err, "failed to read generated lock file") + + activationJobSection := extractJobSection(string(lockContent), string(constants.ActivationJobName)) + assert.Contains(t, activationJobSection, "issues: write", "activation job should include issues: write when workflow_call+issues triggers + reaction are configured") + assert.Contains(t, activationJobSection, "pull-requests: write", "activation job should include pull-requests: write when workflow_call is present (broad permissions)") + assert.Contains(t, activationJobSection, "discussions: write", "activation job should include discussions: write when workflow_call is present (broad permissions)") +} + +func TestActivationPermissionsWorkflowCallReactionWithGitHubApp(t *testing.T) { + tmpDir := testutil.TempDir(t, "activation-perms-workflow-call-reaction-app") + testFile := filepath.Join(tmpDir, "workflow-call-reaction-app.md") + testContent := `--- +on: + workflow_call: + github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_KEY }} + reaction: eyes +engine: copilot +--- + +# Reusable workflow with reaction and activation GitHub App +` + + err := os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err, "failed to write test workflow") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "failed to compile workflow") + + lockContent, err := os.ReadFile(stringutil.MarkdownToLockFile(testFile)) + require.NoError(t, err, "failed to read generated lock file") + + activationJobSection := extractJobSection(string(lockContent), string(constants.ActivationJobName)) + assert.Contains(t, activationJobSection, "permission-issues: write", "activation app token should include permission-issues: write when workflow_call + reaction are configured") + assert.Contains(t, activationJobSection, "permission-pull-requests: write", "activation app token should include permission-pull-requests: write when workflow_call + reaction are configured") + assert.Contains(t, activationJobSection, "permission-discussions: write", "activation app token should include permission-discussions: write when workflow_call + reaction are configured") +} + +func TestActivationPermissionsWorkflowCallReactionDiscussionsOnlyWithGitHubApp(t *testing.T) { + tmpDir := testutil.TempDir(t, "activation-perms-workflow-call-reaction-discussions-only-app") + testFile := filepath.Join(tmpDir, "workflow-call-reaction-discussions-only-app.md") + testContent := `--- +on: + workflow_call: + github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_KEY }} + reaction: + type: eyes + issues: false + pull-requests: false + discussions: true +engine: copilot +--- + +# Reusable workflow with discussions-only reaction and activation GitHub App +` + + err := os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err, "failed to write test workflow") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "failed to compile workflow") + + lockContent, err := os.ReadFile(stringutil.MarkdownToLockFile(testFile)) + require.NoError(t, err, "failed to read generated lock file") + + activationJobSection := extractJobSection(string(lockContent), string(constants.ActivationJobName)) + assert.NotContains(t, activationJobSection, "permission-issues: write", "activation app token should not include permission-issues: write for workflow_call + discussions-only reaction") + assert.NotContains(t, activationJobSection, "permission-pull-requests: write", "activation app token should not include permission-pull-requests: write for workflow_call + discussions-only reaction") + assert.Contains(t, activationJobSection, "permission-discussions: write", "activation app token should include permission-discussions: write for workflow_call + discussions-only reaction") +} diff --git a/pkg/workflow/compiler_activation_job.go b/pkg/workflow/compiler_activation_job.go index f68e6b98c3d..761f0e3ccb3 100644 --- a/pkg/workflow/compiler_activation_job.go +++ b/pkg/workflow/compiler_activation_job.go @@ -185,14 +185,16 @@ func addBroadActivationInteractionPermissions( } needsIssuesWriteForReaction := options.hasReaction && (options.reactionIncludesIssues || options.reactionIncludesPullRequests) - needsIssuesWriteForStatusComment := options.statusCommentIncludesIssues || options.statusCommentIncludesPullRequests + needsIssuesWriteForStatusComment := options.hasStatusComment && + (options.statusCommentIncludesIssues || options.statusCommentIncludesPullRequests) if needsIssuesWriteForReaction || needsIssuesWriteForStatusComment { permsMap[PermissionIssues] = PermissionWrite } if options.hasReaction && options.reactionIncludesPullRequests { permsMap[PermissionPullRequests] = PermissionWrite } - if (options.hasReaction && options.reactionIncludesDiscussions) || options.statusCommentIncludesDiscussions { + if (options.hasReaction && options.reactionIncludesDiscussions) || + (options.hasStatusComment && options.statusCommentIncludesDiscussions) { permsMap[PermissionDiscussions] = PermissionWrite } } diff --git a/pkg/workflow/compiler_activation_job_builder.go b/pkg/workflow/compiler_activation_job_builder.go index 51b38178b79..151eca9f93b 100644 --- a/pkg/workflow/compiler_activation_job_builder.go +++ b/pkg/workflow/compiler_activation_job_builder.go @@ -249,6 +249,21 @@ func buildActivationAppTokenPermissions(ctx *activationJobBuildContext) *Permiss statusCommentIncludesDiscussions: ctx.statusCommentDiscussions, }, ) + if hasWorkflowCallTrigger(ctx.data.On) && (ctx.hasReaction || ctx.hasStatusComment) { + addActivationInteractionPermissions( + appPerms, + activationInteractionPermissionsOptions{ + hasReaction: ctx.hasReaction, + reactionIncludesIssues: ctx.reactionIssues, + reactionIncludesPullRequests: ctx.reactionPullRequests, + reactionIncludesDiscussions: ctx.reactionDiscussions, + hasStatusComment: ctx.hasStatusComment, + statusCommentIncludesIssues: ctx.statusCommentIssues, + statusCommentIncludesPullRequests: ctx.statusCommentPRs, + statusCommentIncludesDiscussions: ctx.statusCommentDiscussions, + }, + ) + } // Keep this aligned with addActivationLabelPermissions: app-token scopes are // computed separately from GITHUB_TOKEN scopes because app-token permissions // only apply to steps using the minted app token, while label permissions in @@ -726,6 +741,7 @@ func (c *Compiler) addActivationArtifactUploadStep(ctx *activationJobBuildContex func (c *Compiler) buildActivationPermissions(ctx *activationJobBuildContext) (string, error) { permsMap := c.buildActivationBasePermissions(ctx) c.addCentralizedCommandActivationPermissions(permsMap, ctx) + c.addWorkflowCallActivationPermissions(permsMap, ctx) c.addActivationLabelPermissions(permsMap, ctx) if err := c.addActivationScriptPermissions(permsMap, ctx); err != nil { return "", err @@ -777,6 +793,37 @@ func (c *Compiler) addCentralizedCommandActivationPermissions(permsMap map[Permi } } +// addWorkflowCallActivationPermissions supplements the activation job's permission map when the +// workflow is triggered via workflow_call (i.e. it is used as a reusable workflow). +// +// At compile time it is impossible to know which GitHub event will fire in the *calling* workflow, +// so the compiler cannot restrict permissions to a specific event type (e.g. "issues" or +// "pull_request"). Instead it falls back to the broad permission set: all permission scopes that +// the configured reactions / status-comments could ever need are granted, respecting the per-type +// opt-out flags (reaction.issues, reaction.pull-requests, etc.). +// +// Because the caller event type is unknown at compile time, this path always uses the broad +// fallback (addBroadActivationInteractionPermissions) instead of event-aware trigger parsing. +func (c *Compiler) addWorkflowCallActivationPermissions(permsMap map[PermissionScope]PermissionLevel, ctx *activationJobBuildContext) { + if !hasWorkflowCallTrigger(ctx.data.On) { + return + } + if !ctx.hasReaction && !ctx.hasStatusComment { + return + } + compilerActivationJobLog.Print("workflow_call trigger detected; applying broad interaction permissions for reactions/status-comments") + addBroadActivationInteractionPermissions(permsMap, activationInteractionPermissionsOptions{ + hasReaction: ctx.hasReaction, + reactionIncludesIssues: ctx.reactionIssues, + reactionIncludesPullRequests: ctx.reactionPullRequests, + reactionIncludesDiscussions: ctx.reactionDiscussions, + hasStatusComment: ctx.hasStatusComment, + statusCommentIncludesIssues: ctx.statusCommentIssues, + statusCommentIncludesPullRequests: ctx.statusCommentPRs, + statusCommentIncludesDiscussions: ctx.statusCommentDiscussions, + }) +} + func (c *Compiler) addActivationLabelPermissions(permsMap map[PermissionScope]PermissionLevel, ctx *activationJobBuildContext) { if ctx.data.LockForAgent { permsMap[PermissionIssues] = PermissionWrite