Require approval for PR trigerred jobs

[LecrisUT]

As far as I can see in the documentation, there is no prior authorization of the PR author to approve to run the CI jobs or not. It was pointed out that there was an exploit attempt, e.g. here, which can affect the jobs that enable enable_net option for copr_build. We should patch these vulnerabilities and check a few common security aspects:

• Pull request triggers should not be triggered for non-contributors, or at least get the configuration from the repository setting of github actions
• Jobs are not triggered on commits on fork repositories that did not install the app. Maybe this one is already present
• Jobs are not triggered when using /packit build by non-authorized users. Maybe also already implemented

[TomasTomecek]

Related:
#1965
#1964

[lachmanfrantisek]

Hi @LecrisUT ! Thanks for the detailed description. We will discuss this with a team tomorrow, but a bit of context in the meantime:

• We now require a FAS account with a signed Fedora Contribution Agreement for each GitHub installation.
• Originally, we allowed only people with push rights to trigger the Packit actions (either a PR push or comment command), but it was really painful for any external contribution. (Unlike all other CI systems that run automatically.) So, after approval from Copr and Testing Farm (that run the builds/tests), we allowed anyone to trigger the Packit action (with an exception of internal tests that have multiple levels of approval).
• As part of Permission checks are done per-job, not per-installation #1850, we are working on various tasks to simplify the permission schema:
  • We want to get rid of installation checks (that does not make much if we allow jobs to be run by external contributors).
  • We want to (re)introduce the blocking of users.

But, for this specific issue, I would try to find a solution to make it harder to misuse Packit without bringing complications to the real contributors. A few ideas I have in mind or we've discussed:

• Have per-user/project/namespace limits and/or priority. And enhance them either manually or with time.
• We can look at first contributors more strictly, but projects with a lot of new contributors don't like this..;)
• Both Copr and TestingFarm are ok with being open (allow anyone to trigger actions) if we have an easy and quick way how to block such users.
• We can try to detect malicious behaviour (number of jobs in a queue, number of projects in a namespace, number of pull requests opened, account signup date), but I am not sure if it's worth spending the time on that.

Jobs are not triggered on commits on fork repositories that did not install the app. Maybe this one is already present

We don't get any messages for such events so we don't react to those.

Pull request triggers should not be triggered for non-contributors, or at least get the configuration from the repository setting of github actions

Not 100% sure what you mean by that, but we are GitHub application (not GitHub action) and don't have any UI settings. But we can think about loading some info from the already-merged state of the Packit config file.

[lachmanfrantisek]

Technical note: I am going to transfer this issue to the service repo where the permissions are handled...

[lachmanfrantisek]

And one last thing. @LecrisUT Are you more afraid of misusing/spamming your repository or about the waste of resources on our side (with spam/DOS/mining possibility)?

[LecrisUT]

Are you more afraid of misusing/spamming your repository or about the waste of resources on our side (with spam/DOS/mining possibility)?

The latter, but also security practices in general. In the last attack that was briefly reported on the matrix room, they basically ran a curl -s ... | bash which who knows what they were trying/able to do. In theory they could expose github credentials and all. So the main line of defense is to wait for approval of a job before running it.

Pull request triggers should not be triggered for non-contributors, or at least get the configuration from the repository setting of github actions

Not 100% sure what you mean by that

If you navigate to Repository -> Settings -> Actions -> General, you can see there a configuration of the security that they have set for Github Action. If possible to get these information from the API, then we could mirror these options to the packit service configuration. E.g.:

We could synchronize these permissions to the \pakit help and other such actions. But I think the minimum that we should have are for PR jobs

[lachmanfrantisek]

In theory they could expose github credentials and all. So the main line of defense is to wait for approval of a job before running it.

Luckily, we run actions/builds inside a sandboxed environment (no secrets available) so it's more about the waste of resources. The Testing Farm should have some auto-detection of malicious execution but I don't know the details... But I agree that we should do something about that!

If you navigate to Repository -> Settings -> Actions -> General, you can see there a configuration of the security that they have set for Github Action. If possible to get these information from the API, then we could mirror these options to the packit service configuration. E.g.:

Since this is just for GitHub actions (!= GitHub applications), we are not affected by that and I am unable to find it in the API documentation... So, if we want to have it configurable, we should find another way to set this (e.g. merged state of the config). But still, this will not stop people from creating a new project and running the jobs there.

[lachmanfrantisek]

Quick update after today's discussion at team architecture meeting:

• We are thinking about implementing user-based limits. (That can cover both spamming legitimate projects and creating spammy installations.)
• We want to align the rules with the implementation of blocklist: Implement blocklisting mechanism #1964
• We realised that we need more time to discuss this and are having a separate design meeting next week. (But any suggestions are welcome!)

[lachmanfrantisek]

The outcome of today's discussion is to fix this problem by these two actions items:

• Implement user/namespace/project usage metrics #1990 (newly created)
• Implement blocklisting mechanism #1964

With that, I am going to close this issue. Please, subscribe to the link issues to follow the progress...