INRIA / spoon

Spoon is a metaprogramming library to analyze and transform Java source code. :spoon: is made with :heart:, :beers: and :sparkles:. It parses source files to build a well-designed AST with powerful analysis and transformation API.
http://spoon.gforge.inria.fr/
Other
1.76k stars 352 forks source link

governance: introduce a background check for Spoon integrators #4300

Open monperrus opened 3 years ago

monperrus commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

MartinWitt commented 2 years ago

@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 commented 2 years ago

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

monperrus commented 2 years ago

FTR On the feasibility of stealthily introducing vulnerabilities in open-source software via hypocrite commits,” Proc. Oakland, 2021

slarse commented 2 years ago

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

monperrus commented 2 years ago

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