spoon icon indicating copy to clipboard operation
spoon copied to clipboard

governance: introduce a background check for Spoon integrators

Open monperrus opened this issue 3 years ago • 7 comments

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?

monperrus avatar Nov 19 '21 17:11 monperrus

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.

slarse avatar Nov 24 '21 21:11 slarse

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

MartinWitt avatar Nov 26 '21 02:11 MartinWitt

@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.

MartinWitt avatar Dec 02 '21 04:12 MartinWitt

@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.

raghav-deepsource avatar Jan 21 '22 07:01 raghav-deepsource

@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.

https://github.com/INRIA/spoon/issues/4300#issuecomment-978263023

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.

slarse avatar Aug 31 '22 15:08 slarse

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

monperrus avatar Sep 14 '22 09:09 monperrus