General improvements on security - Suggestion of Scorecards GitHub Action

[diogoteles08]

Hello! Given the current scenario of increasing attacks on supply chain projects, Google (in partnership with the Open Source Security Foundation) has hired me to work around important open-source projects to help increase security, in any aspect or concern that might be relevant.

I'm talking to you because mesonbuild/meson has reached a significant importance and impact, so I would like to make myself available to help with any security concern or discussion you might have.

Reading a bit through the project history, I see that you are already concerned about security, and that is amazing! So in case you don't have any immediate pendencies that I could help with, I would suggest you to add the GitHub Action of Scorecards to your project. This would help you to easily track possible vulnerabilities and security pendencies over your code. I'm also available to create a PR adding it, if you wish.

Let me know if you have any further questions.

Thanks for the attention =)

[eli-schwartz]

What exactly does this action do? From a quick glance it mostly seems to check github settings on the repository itself?

Note that for example, we don't have external dependencies aside for python itself, but it looks like it would still grade the project based on whether dependabot is used. I think the only thing that is relevant to be updated is the versions of github actions.

• There are various testsuite dependencies, but those are installed from distro repositories inside docker images inheriting :latest (plus one explicit Ubuntu Bionic image) in ci/ciimage/*/image.json and rebuilt weekly
• I do not like dependabot, though I'd be willing to consider Renovate which I understand is supposed to be more flexible and also NOT spam forks.

We don't use branch protection, and don't enforce reviews in github. We do culturally insist on them, but there are exceptions, including pushing doc-only updates directly to git master or trivial CI fixes merged without review. Personally, I would be happy to see consistent or enforced PGP-signing of commits (I always do this without exception), as I think this generally a better indicator of provenance.

Does the scorecard detect when PRs are merged by someone other than the submitter, or have a "lgtm" comment instead of an approving review? Seemingly not -- I attempted to run it, and it said:

|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 1 / 10  | Code-Review            | 2 out of last 13 changesets    | https://github.com/ossf/scorecard/blob/main/docs/checks.md#code-review            |
|         |                        | reviewed before merge -- score |                                                                                   |
|         |                        | normalized to 1                |                                                                                   |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|

but as far as I can tell, most of those are authored by one person and merged by another with what that link says is "implicit review" despite not being successfully detected that way?

[diogoteles08]

Hi Eli,

These are great questions and observations! Happy to answer and clarify the related security checks. At that point I'll indicate some directions to enhance security and fill the gaps pointed by scorecards. But also I want to emphasize that I’m able to help you with any security improvements you might be interested in—we don’t have to focus on using Scorecards or necessarily achieving a high score. Scorecards is great and can be really useful for many projects, but as an automatic tool it does have limitations. So I’m happy to hear your thoughts on other improvements or discuss solutions that work for you. Basically, my job is to help important projects like yours however I can.

As for your points, I'll split into different answers:

What exactly does this action do? From a quick glance it mostly seems to check GitHub settings on the repository itself?

The action inspects different aspects of the repo looking for possible vulnerabilities and propose ways to minimize them. So it does look into your GitHub settings, but also runs security-minded inspections over the source-code, build, dependencies, testing and maintenance. You can quickly see all the checks evaluated here, where you can also find a link to read in details how each check score is evaluated, and why the checks are important for security.

Note that for example, we don't have external dependencies aside for python itself, but it looks like it would still grade the project based on whether dependabot is used.

First, it's great that your code don't use external dependencies aside from python, that already brings more security. But unfortunately dependencies inside GitHub actions and docker images also can bring vulnerabilities. Two different Scorecards inspections relate to this matter:

• The Dependency Update Tool evaluates the repo commitment to update out-of-date dependencies, that might have known flaws or be prone to attacks. Unfortunatelly, as an automatic tool, it's challenging to consider all forms of dependency updates. So it indeed verifies only if the project uses dependency update tools, but it also considers the renovatebot (I personally also avoid using dependabot haha).

• The Pinned-Dependencies checks how the dependencies especify their versions. And this is an interesting discussion here because you said you use the :latest tag on your docker images. This method definetely works in the sense of keeping the dependencies updated, but it still leaves your code vulnerable to updates that might break your code, or to updates made by a malicious agent. And this same issue can happen when using tag-pinned dependencies (i.e., foobar@X.Y.Z), as a malicious agent with access to the repo could change the code that this version points to. That is why this check suggests the usage of pinned-depencies, that refer directly to the last commit used on the dependency. Although it may seem more difficult to update, they can be easily maintained using renovatebot.

Does the scorecard detect when PRs are merged by someone other than the submitter, or have a "lgtm" comment instead of an approving review?

So first repling the easy question: no, it does not consider comments such as "lgtm" as approving reviews, it only recognizes "Github-approved review". I suppose that's because it might be hard to detect all variants of those comments, and maybe because that's quite exacly the function of the GitHub approves.

About the implicit reviews, this feature is currently in discussion between Scorecard team, but the purpose is to make it mark a commit as reviewed if the author of the PR is different than the committer of the PR as long as both these are verified (i.e signed entities). So the flow that you are currently using on meson should be enough to get a good score on Branch Protection, maybe only needing to adapt to the author also signing their commits.

The interesting/weird thing is that I ran scorecards over your project and it actually detected 15 code reviews out of the last 30 commits. So seems it is recognizing your implicit reviews. Anyway, I have already contacted the Scorecards team to understand what is going on in this case.