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

governance: introduce a background check for Spoon integrators #4300

Open
monperrus opened this issue Nov 19, 2021 · 7 comments
Open

governance: introduce a background check for Spoon integrators #4300

monperrus opened this issue Nov 19, 2021 · 7 comments

Comments

@monperrus
Copy link
Collaborator

Dear all,

Recently, there have been a number of software supply chain attacks. Basically, malicious persons push malicious code in open-source software: https://github.blog/2020-09-02-secure-your-software-supply-chain-and-protect-against-supply-chain-threats-github-blog/

Spoon is concerned by this problem, because if somebody pushes a backdoor in Spoon, she would have access to lots of source code, incl. proprietary code.

Consequently, integrators have the great responsibility to avoid backdoors. But what happens if integrators themselves are the attack vector?

To remediate to this problem, one solution is to introduce some kind of background check before giving the integrator role.

WDYT?

@slarse
Copy link
Collaborator

slarse commented Nov 24, 2021

How would we go about doing that?

I feel like there are several things we can do before that. For example, we can have stronger branch protection rules, and require each PR to be approved before being merged (that's kind of a basic thing we really should have). Maintainers can't approve their own PRs, but that of course doesn't stop them from creating another account to create the PR to be approved.

To get around that we can require each PR be approved by two maintainers. That's of course extra overhead, but it's much safer. I think that beats any background check we can ever get going.

@MartinWitt
Copy link
Collaborator

I would love having a protected master, forbidding any direct commit.

@MartinWitt
Copy link
Collaborator

@monperrus as this is kinda related, how about marking the master branch as protected, forbidding direct pushes and forcing pull requests? Normally, we should never require direct pushes to master, and it reduces the chance of mistakes by integrators.

@raghav-deepsource
Copy link

@MartinWitt that is definitely the way to go. At DeepSource, for example, we make sure to keep master protected, and any PR needs approval from at least one code owner to proceed. (Of course, we use JenkinsX to make our CI checks more flexible). I believe it should be possible to add code owner checks in github itself.

@slarse
Copy link
Collaborator

slarse commented Aug 31, 2022

@monperrus Please consider the branch protection we suggested above. You're afaik the only one who can add branch protection rules. There is also an option to let administrators override branch protection rules (if you want to still have the ability to YOLO merge yourself), but mortal integrators will not be able to do so.

#4300 (comment)

As it stands right now, I can create a PR and merge it without any other integrator having any say about it. That feels a bit iffy.

@monperrus
Copy link
Collaborator Author

related work:
Trusting Trust: Humans in the Software Supply Chain Loop
https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=9888996

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants