Skip to content

fix: remove invalid permission-discussions from GitHub App token fields#25508

Merged
pelikhan merged 1 commit intomainfrom
copilot/review-permission-manager
Apr 9, 2026
Merged

fix: remove invalid permission-discussions from GitHub App token fields#25508
pelikhan merged 1 commit intomainfrom
copilot/review-permission-manager

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 9, 2026

Summary

The convertPermissionsToAppTokenFields function was incorrectly generating permission-discussions: write in the actions/create-github-app-token token minting step. This field is not a declared input in the action's action.yml.

Research

Deep-dive into the GitHub docs confirmed:

Permission Context Valid?
discussions: write GITHUB_TOKEN job-level permissions ✅ Valid (documented GitHub Actions scope)
permission-discussions actions/create-github-app-token input Not declared in action.yml

The actions/create-github-app-token action's # GENERATED inputs section includes permission-team-discussions (org-level team discussions) but does not include permission-discussions (repository discussions). Passing an undeclared input is silently ignored by the GitHub Actions runner, so the field had zero practical effect while producing misleading YAML.

GitHub App installation tokens inherit the full set of app-installation permissions by default. When explicit permission-* fields are set, the token is scoped to only those. Since permission-discussions was silently ignored, the token already inherited the app's discussions permission from the installation — the explicit (and invalid) field was redundant.

Changes

  • pkg/workflow/safe_outputs_app_config.go: Remove the permission-discussions mapping from convertPermissionsToAppTokenFields; add a comment explaining why discussions is intentionally omitted.
  • pkg/workflow/safe_outputs_app_test.go: Update TestSafeOutputsAppTokenDiscussionsPermission to assert that permission-discussions is absent (not a valid action input), and verify that other permissions (permission-contents, permission-issues) remain present.
  • pkg/workflow/github_app_permissions_validation_test.go: Add a test case to TestConvertPermissionsToAppTokenFields_GitHubAppOnly documenting that PermissionDiscussions must not produce a permission-discussions field.

Testing

All workflow package tests pass.

The actions/create-github-app-token action does not declare
permission-discussions as a supported input in its action.yml
(the generated permissions section has permission-team-discussions
for org-level team discussions, but not permission-discussions for
repository discussions). Passing an unsupported input is silently
ignored, so the field had no effect. GitHub App installation tokens
inherit the full set of app-installation permissions by default,
so discussions access works through the app's own permissions.

Update the test to verify permission-discussions is absent and add
a new test case in TestConvertPermissionsToAppTokenFields_GitHubAppOnly
to document this constraint.

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3b926256-5b9e-401d-9567-b6fab58fd992

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review April 9, 2026 17:38
Copilot AI review requested due to automatic review settings April 9, 2026 17:38
@pelikhan pelikhan merged commit 8a9fe35 into main Apr 9, 2026
52 of 53 checks passed
@pelikhan pelikhan deleted the copilot/review-permission-manager branch April 9, 2026 17:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes an invalid permission-discussions input from the actions/create-github-app-token minting step generation, and updates tests to ensure the unsupported field is not emitted.

Changes:

  • Remove the PermissionDiscussionspermission-discussions mapping when generating actions/create-github-app-token inputs.
  • Update the safe-outputs app workflow test to assert permission-discussions is absent while other permission inputs remain.
  • Add a permissions-validation test case documenting that discussions must not map to permission-discussions.
Show a summary per file
File Description
pkg/workflow/safe_outputs_app_config.go Stops emitting an unsupported permission-discussions field and documents the omission rationale.
pkg/workflow/safe_outputs_app_test.go Updates assertions to ensure permission-discussions is not present in the generated token mint step.
pkg/workflow/github_app_permissions_validation_test.go Adds coverage that PermissionDiscussions must not generate permission-discussions input fields.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 3

Comment on lines +264 to +271
// Note: PermissionDiscussions ("discussions") is intentionally NOT mapped to "permission-discussions"
// here. The actions/create-github-app-token action does NOT declare "permission-discussions" as a
// supported input (see the generated inputs in its action.yml). Passing an unsupported input would
// be silently ignored, meaning the discussions scope would never be explicitly set. GitHub App
// installation tokens inherit the full set of app-installation permissions by default, so the token
// will have discussions access whenever the GitHub App installation itself was granted that permission.
// Repository-level discussions operations should therefore work without an explicit permission-discussions
// field.
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

The function-level comment above states this maps all permissions to actions/create-github-app-token permission-* inputs, but PermissionDiscussions is now intentionally excluded. Please update that header comment to reflect this exception so future readers don’t assume discussions will be forwarded like other GitHub Actions scopes.

Suggested change
// Note: PermissionDiscussions ("discussions") is intentionally NOT mapped to "permission-discussions"
// here. The actions/create-github-app-token action does NOT declare "permission-discussions" as a
// supported input (see the generated inputs in its action.yml). Passing an unsupported input would
// be silently ignored, meaning the discussions scope would never be explicitly set. GitHub App
// installation tokens inherit the full set of app-installation permissions by default, so the token
// will have discussions access whenever the GitHub App installation itself was granted that permission.
// Repository-level discussions operations should therefore work without an explicit permission-discussions
// field.
// Note: this mapper forwards GitHub Actions-compatible permissions to the corresponding
// actions/create-github-app-token "permission-*" inputs, with one intentional exception:
// PermissionDiscussions ("discussions") is NOT mapped to "permission-discussions".
// The actions/create-github-app-token action does NOT declare "permission-discussions" as a
// supported input (see the generated inputs in its action.yml). Passing an unsupported input would
// be silently ignored, meaning the discussions scope would never be explicitly set. GitHub App
// installation tokens inherit the full set of app-installation permissions by default, so the token
// will have discussions access whenever the GitHub App installation itself was granted that permission.
// Repository-level discussions operations should therefore work without an explicit
// permission-discussions field.

Copilot uses AI. Check for mistakes.
Comment on lines +264 to +271
// Note: PermissionDiscussions ("discussions") is intentionally NOT mapped to "permission-discussions"
// here. The actions/create-github-app-token action does NOT declare "permission-discussions" as a
// supported input (see the generated inputs in its action.yml). Passing an unsupported input would
// be silently ignored, meaning the discussions scope would never be explicitly set. GitHub App
// installation tokens inherit the full set of app-installation permissions by default, so the token
// will have discussions access whenever the GitHub App installation itself was granted that permission.
// Repository-level discussions operations should therefore work without an explicit permission-discussions
// field.
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

This change drops PermissionDiscussions from the token-mint step while still emitting other permission-* fields (e.g., contents/issues). That creates a mismatch where safe_outputs_permissions.go computes discussions: write for create-discussion, but the minted GitHub App token may not actually be granted discussions access because it’s not part of the explicit permissions list. Consider adjusting the token-minting logic for discussion-requiring workflows (e.g., avoid scoping permissions when discussions is required, or use a supported input mechanism if one exists) to ensure create-discussion can still succeed without always relying on fallback-to-issue.

Copilot uses AI. Check for mistakes.
Comment on lines +114 to 121
// TestSafeOutputsAppTokenDiscussionsPermission tests that discussions permission is handled correctly
// in the GitHub App token minting step.
//
// The actions/create-github-app-token action does NOT declare "permission-discussions" as a supported
// input (its generated action.yml only has "permission-team-discussions" for org-level team discussions).
// Therefore "permission-discussions" must NOT be emitted in the token mint step — the GitHub App
// installation token inherits the app's discussion permission from the installation itself.
func TestSafeOutputsAppTokenDiscussionsPermission(t *testing.T) {
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

The test name TestSafeOutputsAppTokenDiscussionsPermission now primarily asserts that permission-discussions is absent (because it’s not a valid action input). Renaming the test to reflect what it actually verifies would prevent confusion about whether the minted token is expected to include discussions permissions.

Suggested change
// TestSafeOutputsAppTokenDiscussionsPermission tests that discussions permission is handled correctly
// in the GitHub App token minting step.
//
// The actions/create-github-app-token action does NOT declare "permission-discussions" as a supported
// input (its generated action.yml only has "permission-team-discussions" for org-level team discussions).
// Therefore "permission-discussions" must NOT be emitted in the token mint step — the GitHub App
// installation token inherits the app's discussion permission from the installation itself.
func TestSafeOutputsAppTokenDiscussionsPermission(t *testing.T) {
// TestSafeOutputsAppTokenOmitsUnsupportedDiscussionsPermissionInput tests that the unsupported
// "permission-discussions" input is not emitted in the GitHub App token minting step.
//
// The actions/create-github-app-token action does NOT declare "permission-discussions" as a supported
// input (its generated action.yml only has "permission-team-discussions" for org-level team discussions).
// Therefore "permission-discussions" must NOT be emitted in the token mint step — the GitHub App
// installation token inherits the app's discussion permission from the installation itself.
func TestSafeOutputsAppTokenOmitsUnsupportedDiscussionsPermissionInput(t *testing.T) {

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants