Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

grant GITHUB_TOKEN write permissions #11

Closed

Conversation

engelmi
Copy link
Member

@engelmi engelmi commented Jan 8, 2024

In order to enable GH workflows to add comments in PRs, set the default_workflow_permissions: "write". One example can be seen in this PR: eclipse-bluechi/bluechi#690 - to add the coverage results as a comment to the PR. Currently, it gets a 403 response.
@netomi Is this the default_workflow_permission the right option in order to achieve this?

@engelmi engelmi requested review from a team as code owners January 8, 2024 08:22
@engelmi engelmi requested a review from mwperina January 8, 2024 08:23

This comment has been minimized.

@netomi
Copy link
Contributor

netomi commented Jan 8, 2024

Setting the default permissions to write would solve the issue however, using "read" as default is something that we would like to have as a default for security reasons. You can easily fix that by adding the following snippet to your workflow to explicitly request write tokens for accessing pull requests:

jobs:
  rpmbuild:
    runs-on: ubuntu-latest
    permissions:
      pull-requests: write

See also https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs

On a side note: if you run this workflow for pull requests that originate from forks, the workflow will fail as no write token will be granted to such pull requests for security reasons. There is a blog post explaining the reasoning and workarounds:

https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

Copy link

github-actions bot commented Jan 8, 2024

Diff for 51c3a04:
Printing local diff:

Actions are indicated with the following symbols:
+   create
!   modify
!   forced update
-   delete

Organization eclipse-bluechi[id=eclipse-bluechi]
  there have been 4 validation infos, enable verbose output with '-v' to to display them.

  
!   org_workflow_settings {
!     default_workflow_permissions                             = "read" -> "write"
!   }
  
  Plan: 0 to add, 1 to change, 0 to delete.
Canonical Diff for 51c3a04:
Showing canonical diff:

Organization eclipse-bluechi[id=eclipse-bluechi]

--- canonical
+++ original
@@ -84,16 +84,12 @@
           secret: "********"
         }
       ]
-      workflows+: {
-        actions_can_approve_pull_request_reviews: false
-      }
     }
     orgs.newRepo('bluechi-ansible-collection') {
       allow_update_branch: false
       branch_protection_rules: [
         orgs.newBranchProtectionRule('main') {
           required_approving_review_count: 1
-          required_status_checks+: []
           requires_conversation_resolution: true
           requires_linear_history: true
           requires_strict_status_checks: true
@@ -109,16 +105,12 @@
         "ansible"
         "bluechi"
       ]
-      workflows+: {
-        actions_can_approve_pull_request_reviews: false
-      }
     }
     orgs.newRepo('bluechi-on-yocto') {
       allow_update_branch: false
       branch_protection_rules: [
         orgs.newBranchProtectionRule('main') {
           required_approving_review_count: 1
-          required_status_checks+: []
           requires_conversation_resolution: true
           requires_linear_history: true
           requires_strict_status_checks: true
@@ -134,9 +126,6 @@
         "bluechi"
         "yocto"
       ]
-      workflows+: {
-        actions_can_approve_pull_request_reviews: false
-      }
     }
     orgs.newRepo('hashmap.c') {
       allow_merge_commit: true
@@ -147,16 +136,12 @@
       has_projects: false
       has_wiki: false
       web_commit_signoff_required: false
-      workflows+: {
-        actions_can_approve_pull_request_reviews: false
-      }
     }
     orgs.newRepo('terraform-provider-bluechi') {
       allow_update_branch: false
       branch_protection_rules: [
         orgs.newBranchProtectionRule('main') {
           required_approving_review_count: 1
-          required_status_checks+: []
           requires_conversation_resolution: true
           requires_linear_history: true
           requires_strict_status_checks: true
@@ -173,9 +158,6 @@
         "terraform"
         "terraform-provider"
       ]
-      workflows+: {
-        actions_can_approve_pull_request_reviews: false
-      }
     }
   ]
   settings+: {

@engelmi
Copy link
Member Author

engelmi commented Jan 8, 2024

Thanks for the quick reply! @netomi

Setting the default permissions to write would solve the issue however, using "read" as default is something that we would like to have as a default for security reasons. You can easily fix that by adding the following snippet to your workflow to explicitly request write tokens for accessing pull requests:

jobs:
  rpmbuild:
    runs-on: ubuntu-latest
    permissions:
      pull-requests: write

See also https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs
On a side note: if you run this workflow for pull requests that originate from forks, the workflow will fail as no write token will be granted to such pull requests for security reasons. There is a blog post explaining the reasoning and workarounds:

https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

Ah ok, I didn't know about that option. But after reading the blog post you linked, I think that is the approach I have to use - so I have to tweak the BlueChi PR a bit. It was an interesting read. Thanks again!
I'll close this PR.

@engelmi engelmi closed this Jan 8, 2024
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.

2 participants