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

Enable Scorecard Github Action and Badge #25054

Merged
merged 8 commits into from
Oct 19, 2022
Merged

Conversation

joycebrum
Copy link
Contributor

Issue

Closes #25042

Description

I've created the PR to show the files involved, but feel free to question or request any changes, I'll be happy to fix it.

It is related to the chore feature described at #25042 .

@yuwata yuwata added ci github_actions Pull requests that update Github_actions code labels Oct 18, 2022
@yuwata yuwata requested a review from mrc0mmand October 18, 2022 15:30
@yuwata
Copy link
Member

yuwata commented Oct 18, 2022

@joycebrum Could you drop the Signed-off-by tags in the commits, unless your company really requests that? We do not use the tag.

@joycebrum
Copy link
Contributor Author

@joycebrum Could you drop the Signed-off-by tags in the commits, unless your company really requests that? We do not use the tag.

Sure, I'll do it

@mrc0mmand
Copy link
Member

 Enter the verification code SBLF-PRPH in your browser at: https://oauth2.sigstore.dev/auth/device?user_code=SBLF-PRPH
Code will be valid for 300 seconds
2022/10/18 16:17:09 error signing scorecard json results: error signing payload: getting key from Fulcio: retrieving cert: error obtaining token: expired_token

Is this caused by the fact that the run-in-PR-context support for the Scorecard action is not yet officially supported?

@keszybz
Copy link
Member

keszybz commented Oct 18, 2022

"12 out of last 17 changesets reviewed before merge -- score normalized to 7"

That's actually wrong. All changesets were reviewed, just not annotated via the github mechanism.

"no releases found"

Hmmm.

"no published package detected"

Hmmm.

"Warn: no GitHub publishing workflow detected"

That should count as a plus, no? :----]

"Info: Dependabot detected: .github/dependabot.yml:1"

Right now that thing is on, but it generates so much noise, that I'd consider killing it again.

"dependency not pinned by hash detected -- score normalized to 5"

Bleh. Dependency pinning is a bad idea in general.

Would it be possible to disable (or hide?) those checks? They are either wrong or they suggest bad project management practices.

@joycebrum
Copy link
Contributor Author

@keszybz it is not possible to disable any test, but regarding each one of them:

"12 out of last 17 changesets reviewed before merge -- score normalized to 7"

About this, which mechanism do you use? Right now the Scorecard only checks for github approvals, indeed it assumes any change that was not approved through github, was not reviewed

"no releases found"

It is just a warning to "Signed Release Check not applicable". Thus, it is not calculated any score for it.

"no published package detected"

Same about the previous topic, as there is no published package in the repo, the packaging check is not applicable.

"Warn: no GitHub publishing workflow detected"

It is not a problem, it is just warning that, because of that, some check may not be applicable, so the score will not be calculated or considered.

Right now that thing is on, but it generates so much noise, that I'd consider killing it again.

There is also the renovatebot, you could check it it is an option for you, many maintainers prefer it instead of the dependabot.

"dependency not pinned by hash detected -- score normalized to 5"

Hash pinning dependencies reduces several security risks, please take a look at Pinned Dependencies Check

@joycebrum
Copy link
Contributor Author

 Enter the verification code SBLF-PRPH in your browser at: https://oauth2.sigstore.dev/auth/device?user_code=SBLF-PRPH
Code will be valid for 300 seconds
2022/10/18 16:17:09 error signing scorecard json results: error signing payload: getting key from Fulcio: retrieving cert: error obtaining token: expired_token

Is this caused by the fact that the run-in-PR-context support for the Scorecard action is not yet officially supported?

Problably, but I'll take a look

@keszybz
Copy link
Member

keszybz commented Oct 18, 2022

@keszybz it is not possible to disable any test, but regarding each one of them:

"12 out of last 17 changesets reviewed before merge -- score normalized to 7"

About this, which mechanism do you use? Right now the Scorecard only checks for github approvals, indeed it assumes any change that was not approved through github, was not reviewed

We make a comment in the pull request.

Right now that thing is on, but it generates so much noise, that I'd consider killing it again.

There is also the renovatebot, you could check it it is an option for you, many maintainers prefer it instead of the dependabot.

Thanks. I hope somebody will take a look at some point ;)

"dependency not pinned by hash detected -- score normalized to 5"

Hash pinning dependencies reduces several security risks, please take a look at Pinned Dependencies Check

Yeah, I'm aware why people do this. Nevertheless, it's a solution that causes more problems than it solves. I don't think users should be getting their software straight from github, but from distributions. And distributions use versioned dependencies. Also, dependency pinning only works as long as all projects are active and developed concurrently. As soon as somebody steps away for a few months, which is just fine and causes no problems without dependency pinning, everything falls apart with dependency pinning, because suddenly the mechanism is introducing vulnerabilities and preventing upgrades, not solving them. Even in the upstream world, strict semantic versioning à la Rust is a better mechanism. So in general dependency pinning is a poor solution that generates unneeded and counterproductive work.

@bluca
Copy link
Member

bluca commented Oct 18, 2022

Yeah, I'm aware why people do this. Nevertheless, it's a solution that causes more problems than it solves. I don't think users should be getting their software straight from github, but from distributions. And distributions use versioned dependencies. Also, dependency pinning only works as long as all projects are active and developed concurrently. As soon as somebody steps away for a few months, which is just fine and causes no problems without dependency pinning, everything falls apart with dependency pinning, because suddenly the mechanism is introducing vulnerabilities and preventing upgrades, not solving them. Even in the upstream world, strict semantic versioning à la Rust is a better mechanism. So in general dependency pinning is a poor solution that generates unneeded and counterproductive work.

Also it's completely moot here, as we don't ship binaries or runnables, we just publish release tags that include no dependency whatsoever. Distributions consume the sources and do their own dependency handling. This is by design.

The only thing we use dependencies in GH for is the CI, but that's just running tests and throwing away the result, the CI will never publish anything that is consumed downstream.

@joycebrum
Copy link
Contributor Author

I've fixed, it won't try to upload in the security dashboard if it is a PR

Copy link
Member

@mrc0mmand mrc0mmand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @joycebrum! Let's merge this and see how it looks in action.

@mrc0mmand mrc0mmand merged commit b7a279f into systemd:main Oct 19, 2022
@@ -14,6 +14,7 @@ System and Service Manager
[![Fossies codespell report](https://fossies.org/linux/test/systemd-main.tar.gz/codespell.svg)](https://fossies.org/linux/test/systemd-main.tar.gz/codespell.html)</br>
[![Coverage Status](https://coveralls.io/repos/github/systemd/systemd/badge.svg?branch=main)](https://coveralls.io/github/systemd/systemd?branch=main)</br>
[![Packaging status](https://repology.org/badge/tiny-repos/systemd.svg)](https://repology.org/project/systemd/versions)
[![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/systemd/systemd/badge)](https://api.securityscorecards.dev/projects/github.com/systemd/systemd)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe one more question - should the badge point to the JSON endpoint? Or is there some "human-friendly" dashboard with all the alerts this should point to instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Scorecard team is working on improving this interface to be more "human-friendly"

@mrc0mmand
Copy link
Member

There is also the renovatebot, you could check it it is an option for you, many maintainers prefer it instead of the dependabot.

Thanks. I hope somebody will take a look at some point ;)

Ah, interesting, I was wondering if there's any suitable alternative for Dependabot, because the experience with it has been so far very... subpar, and all the issue we have with it haven't been addressed for year now, even though they're reported by many people. I'll check the Renovatebot once I have a bit of spare time, thanks!

@poettering
Copy link
Member

Hmm, if I click on the new badge I get thrown in some JSON document. That's not really helpful, is it? Am I supposed to read this data like that? isn't there some web thing that translates this for me into something html i actually can read?

I mean it's fine if this available in some JSON format, but this is not for the human eye then...

also, what consumers of this JSON data actually exist? exporting this data in a machine-readable way is only useful if someone imports that data. who does?

@bluca
Copy link
Member

bluca commented Oct 19, 2022

The warnings are added to the repo's code scanning scoreboard: https://github.com/systemd/systemd/security/code-scanning
There were a few raised, but all false positives, so I've already marked them as such

@mrc0mmand
Copy link
Member

The warnings are added to the repo's code scanning scoreboard: https://github.com/systemd/systemd/security/code-scanning There were a few raised, but all false positives, so I've already marked them as such

Yeah, but the code-scanning stuff is just one part of Scorecard, it tracks much more, at least according to that JSON, hence having some human-friendly dashboard for this would be great.

@joycebrum
Copy link
Contributor Author

About the JSON with the run results, this is the issue that the Scorecard team is working on: ossf/scorecard-webapp#206, which wants to improve this JSON to be more readable and meaninful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci github_actions Pull requests that update Github_actions code squash-on-merge
Development

Successfully merging this pull request may close these issues.

Enable OpenSSF Scorecard Github Action and Badge
6 participants