Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
222 changes: 222 additions & 0 deletions pkg/workflow/activation_permissions_scope_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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.
Comment on lines +569 to +573

func TestActivationPermissionsWorkflowCallReaction(t *testing.T) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] These three scenarios cover the GITHUB_TOKEN path well, but there is no test for the GitHub App path (buildActivationAppTokenPermissions) — a workflow_call + reaction/status-comment workflow that also configures activation.github-app would silently produce an app token with no write scopes.

💡 Suggested test scenario

Add a fourth test with activation.github-app configured to verify that the minted app-token permissions section also includes issues: write, pull-requests: write, and discussions: write. The existing tests only exercise the GITHUB_TOKEN permissions: block in the activation job, not the gh-app-token-permissions: block.

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")
}
6 changes: 4 additions & 2 deletions pkg/workflow/compiler_activation_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
47 changes: 47 additions & 0 deletions pkg/workflow/compiler_activation_job_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnose] The app-token path (buildActivationAppTokenPermissions) has the same gap this PR is fixing for the GITHUB_TOKEN path — it still calls addActivationInteractionPermissions with the raw onSection, which will silently drop write scopes for a workflow_call-only trigger.

💡 Root-cause detail

buildActivationAppTokenPermissions (line 236) calls:

addActivationInteractionPermissions(appPerms, activationInteractionPermissionsOptions{
    onSection: ctx.data.On,  // workflow_call-only: no issues/pull_request events
    ...
})

addActivationInteractionPermissionsMap parses the event set and finds only workflow_call — none of the hasIssuesEvent / hasPullRequestEvent / hasDiscussionEvent flags fire, so no write permission is granted to the minted app token.

The fix should mirror what this new function does: detect hasWorkflowCallTrigger in buildActivationAppTokenPermissions and call addBroadActivationInteractionPermissions there too (or extract a shared helper both paths call).

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,
})
Comment on lines +820 to +824
}

func (c *Compiler) addActivationLabelPermissions(permsMap map[PermissionScope]PermissionLevel, ctx *activationJobBuildContext) {
if ctx.data.LockForAgent {
permsMap[PermissionIssues] = PermissionWrite
Expand Down
Loading