packit/rpm-build - Only collaborators can trigger Packit-as-a-Service

[yarda]

We want all PRs to trigger the rebuild. Why we are receiving this error on PRs from non-collaborators?
packit/rpm-build - Only collaborators can trigger Packit-as-a-Service

I have to trigger the rebuild manually. It happened e.g. in:
redhat-performance/tuned#210

[yarda]

Is it intended behaviour or is there a way how to configure it not to behave this way? We want all PRs to trigger the rebuild and unit tests (run during the build process) and beaker tests on the copr-builds.

[lachmanfrantisek]

It's intended behaviour, so no-one from the public can trigger the build with potentially vulnerable code inside.

[yarda]

IMHO copr should build in the sandbox without network and with watchdog configured timeouts, similarly for the beaker tests. Github users trying to abuse it should get ban. End users will not install temporal RPMs.

So it's not possible to use Packit as the CI for all contributors without manual intervention? This returns us to the point without Packit when we were triggering our scripts manually.

[lachmanfrantisek]

So it's not possible to use Packit as the CI for all contributors without manual intervention?

OK, so it's not a problem to use copr without singing the CLA for Fedora?

We are using sandboxing as well, so it should not be technically a problem to skip that check.

Some middle-way can be to:

• Wait for the first contributor trigger per-PR. (Needs one 'manual' built for each PR from the public.)
• Whitelist the user after one approval in that repo. (Needs one 'manual' built for each contributor from the public.)

[TomasTomecek]

The only reason we are doing this is legal and licensing: https://docs.pagure.org/copr.copr/user_documentation.html#what-i-can-build-in-copr

If forbidden code ever gets to Fedora servers (e.g. copr builders and copr repos afterwards), Fedora can easily be sued for distributing such code.

Obviously, there is a solution here: this process could be configurable - changes from any contributor would be built. In case the rules mentioned above get violated, we'd be forced to ban the upstream project from using the packit service.

We are actually working with our legal team to get proper terms of service so we can get rid of this check, but it's getting fairly slow.

Team mates, what's your opinion here?

[yarda]

@TomasTomecek thanks for info, looking forward to get it resolved soon.

[pemensik]

Obvious solution would be to stop redistributing the merged code until the review. But that should not affect testing the merge and presenting test results after the change.

I think it could be solved by hiding sources created by merge requests changes, just present results. Make such sources accessible only by development team and it should be fine.

This is to prevent redistribution, not execution. Please stop redistribution, not execution in this case.

[TomasTomecek]

Obvious solution would be to stop redistributing the merged code until the review. But that should not affect testing the merge and presenting test results after the change.

I think it could be solved by hiding sources created by merge requests changes, just present results. Make such sources accessible only by development team and it should be fine.

This is to prevent redistribution, not execution. Please stop redistribution, not execution in this case.

Building in copr means redistributing code.

[pemensik]

Building in copr means redistributing code.

Then different service not redistributing code should be used to test pull requests instead. Or copr might add restricted access to selected builds.

[TomasTomecek]

Building in copr means redistributing code.

Then different service not redistributing code should be used to test pull requests instead. Or copr might add restricted access to selected builds.

Yes, I agree with you. The thing is that changing such service (or creating our own) while keeping all the features and user experience (or even improving it) would take us weeks to implement, test, deploy and verify.

I just want to be perfectly clear what's the ask from you here and how to solution could look.

[TomasTomecek]

Design decision:

• Utilize check from Improve reporting for copr builds #237 whether p-s is allowed to be used
• If a PR by a non-contributor is created: the upstream project needs to trigger p-s for the first time
  • Create an issue in the notification repo to whitelist the person
• Once the check is green, keep on buildin'
• Whitelist checking: check if the person is whitelisted, not just the namespace of the repo

[lachmanfrantisek]

Output from the today's architecture meeting:

• \packit whitelist to whitelist author of the PR in the PR/project
  • more transparent, allows removing from the project whitelist
  • onetime action needed from the maintainers

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.
We are doing this to be sure that the issue is still relevant. Anyone can comment to remove the stale state. (The issues marked with pinned, security, bug or EPIC label
are not considered stale.)

[lachmanfrantisek]

I am adding EPIC so we don't receive messages from the stale bot. (Multiple things needs to be done here.)

[lachmanfrantisek]

The issue is here for some time so it's probably a time to make some conclusion. There were multiple solutions mentioned already:

• Globally whitelist users -- user needs to be approved on our side for the first time.
• Comment of the allowed user will unlock the PR for forever.
• Provide some config option and/or comment command to enable the user across the project.

Do we have anything else? Which one(s) do we prefer? (The solutions can be combined...)

[TomasTomecek]

The issue is here for some time so it's probably a time to make some conclusion. There were multiple solutions mentioned already:

* Globally whitelist users -- user needs to be approved on our side for the first time.

I really like this one - once a user is approved we can trust the person.

* Comment of the allowed user will unlock the PR for forever.

Sounds good as well, though you'd need to store this in DB probably which may be complicated to implement.

* Provide some config option and/or comment command to enable the user across the project.

Can be a great way in combination with 1)

[lachmanfrantisek]

Some update on this topic. Copr has reached to us that we don't need to require write access for the pull-request authors to start the build.

I am going to close this issue in favour of #920 since it contains the more up-to-date info and contains the solution for this problem.

The implementation is not hard and we want to start working on this soon.