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

Allow auto-merge from PRs made on a pre-authorized fork of General (@JuliaRegistrator/General or @jlbuild/General #105216

Closed
nkottary opened this issue Apr 19, 2024 · 4 comments
Labels
CI continuous integration

Comments

@nkottary
Copy link
Member

Registrator now lets you specify a fork of the registry to make the PRs against. Its better to use a fork for security reasons since we can reduce the bot privilege on the main registry. Can we allow auto-merge on a recognized fork of General? @DilumAluthge

We need to create this fork repository so lets also discuss how that should be done.

@DilumAluthge
Copy link
Member

Can we allow auto-merge on a recognized fork of General?

There are going to be some challenges. I'll write up a few details.

@nkottary
Copy link
Member Author

@DilumAluthge bump

@giordano giordano added the CI continuous integration label Oct 6, 2024
@DilumAluthge
Copy link
Member

Short answer:

No, we cannot auto-merge PRs from a recognized/authorized fork of the General registry.

To increase security of the General registry with regards to the JuliaRegistrator bot, I have blocked the JuliaRegistrator bot from merging or pushing to the master branch of the General registry.


Long answer:

The question in this issue is specifically about the JuliaRegistrator app. The issue is specifically asking two questions:

  1. Can the JuliaRegistrator app push to a fork (e.g. JuliaRegistrator/General) instead of to the main repo (JuliaRegistries/General), and still have JuliaRegistrator PRs be auto-merged?
  2. Would doing so increase the security of the General registry?

This issue does not concern PRs made from other forks to the General registry; the issue is specifically about JuliaRegistrator pushing to an authorized fork (JuliaRegistrator/General).

The answer to the first question is that there is no easy way to securely auto-merge PRs that are made from a fork of the General registry. GitHub has a variety of security mechanisms in place, and these mechanisms essentially prevent us from using AutoMerge (at least, with the current architecture of AutoMerge) to auto-merge PRs from a fork of the General registry. Is it possible to bypass these security mechanisms? Yes, but:

  1. GitHub would discourage you from doing so.
  2. At some point, the effort to bypass the security mechanisms means that you open yourself up to more potentially security risks than compared to the previous situation (JuliaRegistrator pushing to the General registry), and I think your security situation ends up net worse off.

A lot of this has to do with the fact that AutoMerge is a GitHub Action, not a GitHub App. If AutoMerge were completely rearchitected and rewritten as a GitHub App, then the situation becomes very different, and some things that were previously not possible now become possible. However:

  1. This represents a huge amount of additional work, and it's not even clear to me that the payoff is worth it.
  2. If AutoMerge becomes a GitHub App, then some trustworthy entity has to setup and maintain the server (and surrounding infrastructure) that the GitHub App will run on, and they have to deploy and maintain the App on that server and infrastructure, and they have to keep the App up-to-date, and they have to keep the whole thing secure. This is a nontrivial ask. Also, it will become difficult (if not impossible) for Julia community members outside of that trustworthy entity to work on deploying and updating AutoMerge. Compare that to the current situation, where AutoMerge is developed and deployed entirely on GitHub and GitHub Actions, with every step of the deployment publicly visible, and it is much easier for Julia community members to participate in the process.

The second question is a more general question about security on the General registry with respect to the JuliaRegistrator app. In response to this, I'll point out that we can restrict which users and apps can merge to the master branch of the General registry. I have applied such restrictions. The JuliaRegistrator app has been blocked from merging to the master branch; it can push to other branches, but not to the master branch.

@DilumAluthge DilumAluthge closed this as not planned Won't fix, can't repro, duplicate, stale Oct 21, 2024
@DilumAluthge
Copy link
Member

DilumAluthge commented Oct 21, 2024

Here's an alternate way to frame the same information that I wrote above.

Consider this sentence from the OP:

It's better to use a fork for security reasons since we can reduce the bot privilege on the main registry.

With the current architecture of AutoMerge as a GitHub Action, this sentence is false.

For AutoMerge (in its current GitHub Actions architecture) to function, the registrator bots (@JuliaRegistrator and @jlbuild) are required to have commit/push privileges on the main JuliaRegistries/General repository.

We can restrict the permissions of the bots, by blocking the @JuliaRegistrator and @jlbuild bots from merging or pushing to the master branch. I have already implemented this restriction.

But we cannot remove commit/push access from @JuliaRegistrator or @jlbuild as long as AutoMerge exists in its current GitHub Actions architecture.

@DilumAluthge DilumAluthge changed the title Allow auto-merge from PRs made on fork of General Allow auto-merge from PRs made on an authorized fork of General (@JuliaRegistrator/General or @jlbuild/General Oct 21, 2024
@DilumAluthge DilumAluthge changed the title Allow auto-merge from PRs made on an authorized fork of General (@JuliaRegistrator/General or @jlbuild/General Allow auto-merge from PRs made on a pre-authorized fork of General (@JuliaRegistrator/General or @jlbuild/General Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI continuous integration
Projects
None yet
Development

No branches or pull requests

3 participants