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

Add optional auto merge job #40

Closed
wants to merge 1 commit into from
Closed

Conversation

joroKr21
Copy link
Contributor

@joroKr21 joroKr21 commented Nov 29, 2020

@djspiewak
Copy link
Collaborator

I'm relatively certain this isn't going to work. ☹️ The problem here is the fact that Scala Steward opens its PRs from a fork, meaning the GITHUB_TOKEN you have access to in the PR only has read permissions on the host repository. This is a pretty fundamental restriction, since without it, any repo using GH Actions could be vandalized at will.

@joroKr21
Copy link
Contributor Author

Hmm maybe it would work if you give scala-steward write permissions to the repo but I'm not sure if that's a good idea 🤔

@djspiewak
Copy link
Collaborator

Hmm maybe it would work if you give scala-steward write permissions to the repo

It doesn't. :-( That would actually be very intuitive, but I've done a lot of experimenting with the Actions permissions model in the past (attempting to enable a use-case almost exactly like this one), and it just doesn't work here. The problem, more specifically, is that the GITHUB_TOKEN granted to jobs is an application token, not a user token, and the application is bound to the repository and has repository-level permissions. There is no mechanism in place for conditionally granting that token extra permissions depending on the user who opened the PR, though I really would like for there to be.

Basically, what I want github to do here is grant the write token (as well as full secrets access) to any PRs from people with write access or above, regardless of the PR origin. Such users would be able to push to the upstream anyway, so it wouldn't affect security, but it would make a lot of automation and use-cases possible (especially on private repositories) which are now today impossible. This has been suggested to them as an official feature request, but no word on implementation.

@djspiewak djspiewak closed this Nov 30, 2020
@joroKr21
Copy link
Contributor Author

It looks like there is a workaround: https://github.com/ridedott/merge-me-action#github-actions

          # Depending on branch protection rules, a  manually populated
          # `GITHUB_TOKEN_WORKAROUND` environment variable with permissions to
          # push to a protected branch must be used. This variable can have an
          # arbitrary name, as an example, this repository uses
          # `GITHUB_TOKEN_DOTTBOTT`.
          #
          # When using a custom token, it is recommended to leave the following
          # comment for other developers to be aware of the reasoning behind it:
          #
          # This must be used as GitHub Actions token does not support
          # pushing to protected branches.
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

The comment refers to https://github.com/ridedott/merge-me-action/blob/master/.github/workflows/continuous-integration.yaml#L178

@djspiewak
Copy link
Collaborator

That token would need to be unencrypted though, opening up branch pushing to the entire world. The only way to hide the token from global eyes would be to tuck it into secrets, which in turn means it isn't available to PRs from forks.

@joroKr21
Copy link
Contributor Author

Hmm, this is a bit of a rabbit hole. It looks like there were already some improvements:
https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/
For private repositories you can share secrets to forks whereas for public repositories there are different events like pull_request_target and workflow_run but I'm not sure how that would look like yet.

@joroKr21
Copy link
Contributor Author

E.g. this action has listed pull_request_target as supported event: https://github.com/pascalgn/automerge-action
But it's based on labels. I wonder if the merge-me action would work with pull_request_target.

@djspiewak
Copy link
Collaborator

Ooooooh! This is very nice and yes, I think it could be used to solve this. I'll give this some thought…

@djspiewak djspiewak reopened this Nov 30, 2020
@djspiewak djspiewak changed the base branch from master to main January 17, 2021 18:00
@987Nabil
Copy link

987Nabil commented Jun 3, 2021

I want to mention that scala steward also has a --do-not-fork option. If set, the github token would have write access. That would be useful for private repos.

@joroKr21
Copy link
Contributor Author

joroKr21 commented Jun 3, 2021

I need to update the PR to v2 of the action - it also shows how to configure it to:

  1. Trigger not on PR but on successful completion of the validation step (I think that runs in a different context)
  2. Add a custom token to work around the problem

@joroKr21
Copy link
Contributor Author

joroKr21 commented Jun 3, 2021

@joroKr21
Copy link
Contributor Author

joroKr21 commented Feb 12, 2022

Might try this again in sbt-typelevel. But it should also involve adding scala-steward as a GitHub action.

@joroKr21 joroKr21 closed this Feb 12, 2022
@joroKr21 joroKr21 deleted the auto-merge branch February 12, 2022 07:24
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.

Optional automerge action for scala-steward PRs
3 participants