diff --git a/.changeset/patch-fix-add-comment-perms.md b/.changeset/patch-fix-add-comment-perms.md new file mode 100644 index 0000000000..f9c305860a --- /dev/null +++ b/.changeset/patch-fix-add-comment-perms.md @@ -0,0 +1,4 @@ +--- +"gh-aw": patch +--- +Fix add-comment and hide-comment safe outputs to request issues:write instead of pull_requests:write. diff --git a/.github/workflows/agent-performance-analyzer.lock.yml b/.github/workflows/agent-performance-analyzer.lock.yml index a8a2fac12f..28afebad43 100644 --- a/.github/workflows/agent-performance-analyzer.lock.yml +++ b/.github/workflows/agent-performance-analyzer.lock.yml @@ -1049,7 +1049,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1358,7 +1357,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/archie.lock.yml b/.github/workflows/archie.lock.yml index 096bc67df3..33423c6ea2 100644 --- a/.github/workflows/archie.lock.yml +++ b/.github/workflows/archie.lock.yml @@ -858,7 +858,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1129,7 +1128,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/brave.lock.yml b/.github/workflows/brave.lock.yml index db6c3216e1..9e599d3fa4 100644 --- a/.github/workflows/brave.lock.yml +++ b/.github/workflows/brave.lock.yml @@ -850,7 +850,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1118,7 +1117,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/ci-doctor.lock.yml b/.github/workflows/ci-doctor.lock.yml index 1ae30697a7..1ec2996f55 100644 --- a/.github/workflows/ci-doctor.lock.yml +++ b/.github/workflows/ci-doctor.lock.yml @@ -1026,7 +1026,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1285,7 +1284,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/daily-assign-issue-to-user.lock.yml b/.github/workflows/daily-assign-issue-to-user.lock.yml index 91445497dd..05f8cd2b45 100644 --- a/.github/workflows/daily-assign-issue-to-user.lock.yml +++ b/.github/workflows/daily-assign-issue-to-user.lock.yml @@ -847,7 +847,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1055,7 +1054,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/daily-cli-performance.lock.yml b/.github/workflows/daily-cli-performance.lock.yml index 51269265ff..135184cbb0 100644 --- a/.github/workflows/daily-cli-performance.lock.yml +++ b/.github/workflows/daily-cli-performance.lock.yml @@ -1080,7 +1080,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1361,7 +1360,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/daily-fact.lock.yml b/.github/workflows/daily-fact.lock.yml index 587e6cf7fd..fc6af1ad18 100644 --- a/.github/workflows/daily-fact.lock.yml +++ b/.github/workflows/daily-fact.lock.yml @@ -789,7 +789,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -980,7 +979,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "codex" diff --git a/.github/workflows/dev-hawk.lock.yml b/.github/workflows/dev-hawk.lock.yml index 62230e0fc2..077565cafd 100644 --- a/.github/workflows/dev-hawk.lock.yml +++ b/.github/workflows/dev-hawk.lock.yml @@ -917,7 +917,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1157,7 +1156,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/discussion-task-miner.lock.yml b/.github/workflows/discussion-task-miner.lock.yml index 8e5de29cb8..3a1577a648 100644 --- a/.github/workflows/discussion-task-miner.lock.yml +++ b/.github/workflows/discussion-task-miner.lock.yml @@ -955,7 +955,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1237,7 +1236,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/issue-monster.lock.yml b/.github/workflows/issue-monster.lock.yml index ad4c987e5f..eb3be1c35c 100644 --- a/.github/workflows/issue-monster.lock.yml +++ b/.github/workflows/issue-monster.lock.yml @@ -872,7 +872,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1138,7 +1137,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/pdf-summary.lock.yml b/.github/workflows/pdf-summary.lock.yml index 50b59e13c2..d1e2860c44 100644 --- a/.github/workflows/pdf-summary.lock.yml +++ b/.github/workflows/pdf-summary.lock.yml @@ -949,7 +949,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1222,7 +1221,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/scout.lock.yml b/.github/workflows/scout.lock.yml index fac3a01fd9..d80298f133 100644 --- a/.github/workflows/scout.lock.yml +++ b/.github/workflows/scout.lock.yml @@ -1025,7 +1025,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1320,7 +1319,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "claude" diff --git a/.github/workflows/smoke-temporary-id.lock.yml b/.github/workflows/smoke-temporary-id.lock.yml index 3c8bdd109d..22508102d7 100644 --- a/.github/workflows/smoke-temporary-id.lock.yml +++ b/.github/workflows/smoke-temporary-id.lock.yml @@ -942,7 +942,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1217,7 +1216,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/smoke-test-tools.lock.yml b/.github/workflows/smoke-test-tools.lock.yml index e3af3d10d2..137c494675 100644 --- a/.github/workflows/smoke-test-tools.lock.yml +++ b/.github/workflows/smoke-test-tools.lock.yml @@ -836,7 +836,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1076,7 +1075,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/sub-issue-closer.lock.yml b/.github/workflows/sub-issue-closer.lock.yml index 6054cee9c4..40bcb2e077 100644 --- a/.github/workflows/sub-issue-closer.lock.yml +++ b/.github/workflows/sub-issue-closer.lock.yml @@ -889,7 +889,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1097,7 +1096,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/.github/workflows/workflow-health-manager.lock.yml b/.github/workflows/workflow-health-manager.lock.yml index f81629ca3b..76d9f2fbec 100644 --- a/.github/workflows/workflow-health-manager.lock.yml +++ b/.github/workflows/workflow-health-manager.lock.yml @@ -1019,7 +1019,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: noop_message: ${{ steps.noop.outputs.noop_message }} tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} @@ -1326,7 +1325,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write timeout-minutes: 15 env: GH_AW_ENGINE_ID: "copilot" diff --git a/pkg/workflow/add_comment.go b/pkg/workflow/add_comment.go index 05630953bb..32aef2b101 100644 --- a/pkg/workflow/add_comment.go +++ b/pkg/workflow/add_comment.go @@ -116,11 +116,12 @@ func (c *Compiler) buildCreateOutputAddCommentJob(data *WorkflowData, mainJobNam // Determine permissions based on discussions field // Default (nil or true) includes discussions:write for GitHub Apps with Discussions permission + // Note: PR comments are issue comments, so only issues:write is needed, not pull_requests:write var permissions *Permissions if data.SafeOutputs.AddComments.Discussions != nil && !*data.SafeOutputs.AddComments.Discussions { - permissions = NewPermissionsContentsReadIssuesWritePRWrite() + permissions = NewPermissionsContentsReadIssuesWrite() } else { - permissions = NewPermissionsContentsReadIssuesWritePRWriteDiscussionsWrite() + permissions = NewPermissionsContentsReadIssuesWriteDiscussionsWrite() } // Use the shared builder function to create the job diff --git a/pkg/workflow/compiler_safe_outputs_job_test.go b/pkg/workflow/compiler_safe_outputs_job_test.go index a51c1f3204..7fe3b8f6cd 100644 --- a/pkg/workflow/compiler_safe_outputs_job_test.go +++ b/pkg/workflow/compiler_safe_outputs_job_test.go @@ -52,7 +52,7 @@ func TestBuildConsolidatedSafeOutputsJob(t *testing.T) { }, expectedJobName: "safe_outputs", checkPermissions: true, - expectedPerms: []string{"contents: read", "issues: write", "pull-requests: write", "discussions: write"}, + expectedPerms: []string{"contents: read", "issues: write", "discussions: write"}, }, { name: "create pull requests with patch", @@ -83,7 +83,7 @@ func TestBuildConsolidatedSafeOutputsJob(t *testing.T) { }, expectedJobName: "safe_outputs", checkPermissions: true, - expectedPerms: []string{"contents: read", "issues: write", "pull-requests: write", "discussions: write"}, + expectedPerms: []string{"contents: read", "issues: write", "discussions: write"}, }, { name: "with threat detection enabled", diff --git a/pkg/workflow/notify_comment_test.go b/pkg/workflow/notify_comment_test.go index 892379204a..9f84759778 100644 --- a/pkg/workflow/notify_comment_test.go +++ b/pkg/workflow/notify_comment_test.go @@ -216,16 +216,14 @@ func TestConclusionJob(t *testing.T) { } // Check permissions based on what safe-outputs are configured - // When add-comment is configured, it requires issues, pull-requests, and discussions permissions + // When add-comment is configured, it requires issues and discussions permissions + // (PR comments are issue comments, so only issues: write is needed, not pull-requests: write) // When only missing_tool/noop is configured, minimal permissions are needed if tt.addCommentConfig { - // add-comment requires full write permissions + // add-comment requires issues and discussions write permissions if !strings.Contains(job.Permissions, "issues: write") { t.Error("Expected 'issues: write' permission when add-comment is configured") } - if !strings.Contains(job.Permissions, "pull-requests: write") { - t.Error("Expected 'pull-requests: write' permission when add-comment is configured") - } if !strings.Contains(job.Permissions, "discussions: write") { t.Error("Expected 'discussions: write' permission when add-comment is configured") } diff --git a/pkg/workflow/safe_outputs_permissions.go b/pkg/workflow/safe_outputs_permissions.go index 57979abaf7..fc65b3e18c 100644 --- a/pkg/workflow/safe_outputs_permissions.go +++ b/pkg/workflow/safe_outputs_permissions.go @@ -32,10 +32,11 @@ func computePermissionsForSafeOutputs(safeOutputs *SafeOutputsConfig) *Permissio safeOutputsPermissionsLog.Print("Adding permissions for add-comment") // Check if discussions permission should be excluded (discussions: false) // Default (nil or true) includes discussions:write for GitHub Apps with Discussions permission + // Note: PR comments are issue comments, so only issues:write is needed, not pull_requests:write if safeOutputs.AddComments.Discussions != nil && !*safeOutputs.AddComments.Discussions { - permissions.Merge(NewPermissionsContentsReadIssuesWritePRWrite()) + permissions.Merge(NewPermissionsContentsReadIssuesWrite()) } else { - permissions.Merge(NewPermissionsContentsReadIssuesWritePRWriteDiscussionsWrite()) + permissions.Merge(NewPermissionsContentsReadIssuesWriteDiscussionsWrite()) } } if safeOutputs.CloseIssues != nil { @@ -105,10 +106,11 @@ func computePermissionsForSafeOutputs(safeOutputs *SafeOutputsConfig) *Permissio safeOutputsPermissionsLog.Print("Adding permissions for hide-comment") // Check if discussions permission should be excluded (discussions: false) // Default (nil or true) includes discussions:write for GitHub Apps with Discussions permission + // Note: Hiding comments (issue/PR/discussion) only needs issues:write, not pull_requests:write if safeOutputs.HideComment.Discussions != nil && !*safeOutputs.HideComment.Discussions { - permissions.Merge(NewPermissionsContentsReadIssuesWritePRWrite()) + permissions.Merge(NewPermissionsContentsReadIssuesWrite()) } else { - permissions.Merge(NewPermissionsContentsReadIssuesWritePRWriteDiscussionsWrite()) + permissions.Merge(NewPermissionsContentsReadIssuesWriteDiscussionsWrite()) } } if safeOutputs.DispatchWorkflow != nil { diff --git a/pkg/workflow/safe_outputs_permissions_test.go b/pkg/workflow/safe_outputs_permissions_test.go index 3827f47ba8..22ec452e9b 100644 --- a/pkg/workflow/safe_outputs_permissions_test.go +++ b/pkg/workflow/safe_outputs_permissions_test.go @@ -79,10 +79,9 @@ func TestComputePermissionsForSafeOutputs(t *testing.T) { }, }, expected: map[PermissionScope]PermissionLevel{ - PermissionContents: PermissionRead, - PermissionIssues: PermissionWrite, - PermissionPullRequests: PermissionWrite, - PermissionDiscussions: PermissionWrite, + PermissionContents: PermissionRead, + PermissionIssues: PermissionWrite, + PermissionDiscussions: PermissionWrite, }, }, { @@ -94,9 +93,8 @@ func TestComputePermissionsForSafeOutputs(t *testing.T) { }, }, expected: map[PermissionScope]PermissionLevel{ - PermissionContents: PermissionRead, - PermissionIssues: PermissionWrite, - PermissionPullRequests: PermissionWrite, + PermissionContents: PermissionRead, + PermissionIssues: PermissionWrite, }, }, { @@ -107,10 +105,9 @@ func TestComputePermissionsForSafeOutputs(t *testing.T) { }, }, expected: map[PermissionScope]PermissionLevel{ - PermissionContents: PermissionRead, - PermissionIssues: PermissionWrite, - PermissionPullRequests: PermissionWrite, - PermissionDiscussions: PermissionWrite, + PermissionContents: PermissionRead, + PermissionIssues: PermissionWrite, + PermissionDiscussions: PermissionWrite, }, }, { @@ -122,9 +119,8 @@ func TestComputePermissionsForSafeOutputs(t *testing.T) { }, }, expected: map[PermissionScope]PermissionLevel{ - PermissionContents: PermissionRead, - PermissionIssues: PermissionWrite, - PermissionPullRequests: PermissionWrite, + PermissionContents: PermissionRead, + PermissionIssues: PermissionWrite, }, }, { diff --git a/pkg/workflow/schemas/github-workflow.json b/pkg/workflow/schemas/github-workflow.json index f43bac7de3..4587e33cca 100644 --- a/pkg/workflow/schemas/github-workflow.json +++ b/pkg/workflow/schemas/github-workflow.json @@ -181,8 +181,7 @@ "$ref": "#/definitions/permissions-level" }, "id-token": { - "type": "string", - "enum": ["write", "none"] + "$ref": "#/definitions/permissions-level" }, "issues": { "$ref": "#/definitions/permissions-level"