-
Notifications
You must be signed in to change notification settings - Fork 247
Description
🤖 AI Assisted Bug Report
Problem
PR #15518 (fixing #15516) introduced computePermissionsForSafeOutputs to compute minimal permissions for the safe_outputs and conclusion jobs. However, the add-comment and hide-comment entries still use NewPermissionsContentsReadIssuesWritePRWriteDiscussionsWrite(), which unconditionally includes discussions: write — even when no discussion-related safe outputs (create-discussion, close-discussion, update-discussion) are configured.
This causes the same 422 error that #15516 reported:
RequestError [HttpError]: The permissions requested are not granted to this installation.
…when the GitHub App installation doesn't have the Discussions permission granted.
Root Cause
In pkg/workflow/safe_outputs_permissions.go:
if safeOutputs.AddComments != nil {
permissions.Merge(NewPermissionsContentsReadIssuesWritePRWriteDiscussionsWrite())
// ^^^^^^^^^^^^^^^^
// discussions: write always included
}if safeOutputs.HideComment != nil {
permissions.Merge(NewPermissionsContentsReadIssuesWritePRWriteDiscussionsWrite())
// same problem
}The rationale is that add_comment.cjs can comment on discussions, not just issues/PRs. But if no discussion-related safe-output is configured, the agent will never target a discussion, so discussions: write is unnecessary.
The same pattern also exists in pkg/workflow/add_comment.go L124 (standalone handler permissions).
Impact
Any workflow using add-comment or hide-comment with a GitHub App that doesn't have Discussions permission will fail at the token generation step in the safe_outputs and conclusion jobs. The agent job completes successfully, but its output (comments, labels, assignments) is never applied.
Affected compiled output (lock.yml):
- Job-level
permissions:block includesdiscussions: write create-github-app-tokenstep includespermission-discussions: write
Workaround: Manually remove discussions: write and permission-discussions: write from the compiled lock file after every gh aw compile.
Suggested Fix
In safe_outputs_permissions.go, use NewPermissionsContentsReadIssuesWritePRWrite() (already exists at permissions_factory.go L83) instead:
if safeOutputs.AddComments != nil {
permissions.Merge(NewPermissionsContentsReadIssuesWritePRWrite())
}if safeOutputs.HideComment != nil {
permissions.Merge(NewPermissionsContentsReadIssuesWritePRWrite())
}discussions: write is already independently added when discussion-related safe outputs are configured:
if safeOutputs.CreateDiscussions != nil {
permissions.Merge(NewPermissionsContentsReadIssuesWriteDiscussionsWrite())
}
if safeOutputs.CloseDiscussions != nil {
permissions.Merge(NewPermissionsContentsReadDiscussionsWrite())
}
if safeOutputs.UpdateDiscussions != nil {
permissions.Merge(NewPermissionsContentsReadDiscussionsWrite())
}So workflows that actually use discussions will still get the permission via those entries.
Same fix should be applied to add_comment.go L124 for the standalone handler path.
Reproduction
- Create a workflow with
add-commentbut no discussion safe-outputs:
safe-outputs:
app:
app-id: ${{ secrets.APP_ID }}
private-key: ${{ secrets.APP_PRIVATE_KEY }}
owner: 'myorg'
repositories: ['myrepo']
add-comment:
target: "*"
max: 1- Use a GitHub App that does NOT have the Discussions permission
gh aw compile- Observe
discussions: writein the lock file'ssafe_outputsandconclusionjobs - Run the workflow — token generation fails with 422
Environment
- gh-aw: v0.45.7 (also affected: v0.43.9, v0.43.22, all versions since Fix: Compute minimal permissions for conclusion/safe_outputs jobs based on configured safe-outputs #15518)
- actions/create-github-app-token: v2.2.1
Related
- Conclusion job should not request
discussions: writewhen workflow has no discussion-related safe-outputs #15516 — original report (closed by Fix: Compute minimal permissions for conclusion/safe_outputs jobs based on configured safe-outputs #15518, but the fix was incomplete) - Fix: Compute minimal permissions for conclusion/safe_outputs jobs based on configured safe-outputs #15518 — partial fix (introduced
computePermissionsForSafeOutputsbut didn't fixadd-comment/hide-comment)