Try out scorecard action

[laurentsimon]

once #1073 is landed.
For each push, scorecard will run and the results will be available in https://github.com/ossf/scorecard/security/code-scanning

I would like some feedback about the results. I've already noticed some improvements needed:

1. Short description should be shorter
2. Long description: shall we keep it long os just give users a URL?
3. Line numbers needed when referring to policy file violation
4. Successful results still appear in the GH security dashboard, because we mark them as note. Do we want to keep as-is or remove the notes entirely? Note that it may be useful to keep it in SARIF format in general, but remove it for GitHub actions only.

[laurentsimon]

FYI @azeemsgoogle

[asraa]

Successful results still appear in the GH security dashboard, because we mark them as note. Do we want to keep as-is or remove the notes entirely?

I would personally remove the notes entirely -- I don't think they have much value to the user (who could maybe go see a list of checks anyway on Scorecard), and I think it makes the important stuff less visible.

Long description: shall we keep it long os just give users a URL?

I like it long, don't like link chasing.

Short description should be shorter

Is this the title? If so is it canonical to have the title be the action of the check, or something indicating it's wrong, e.g. difference between "Determines if..." and "Check failed: Project workflow does not follow"

[azeemshaikh38]

This looks great. Some comments:

1. The titles Determines if ... should be shorter and easier to understand. Something like Dependencies are unpinned.
2. The short description should be shorter. Also, it has some HTML elements which don't get rendered well.
3. Remove alerts which pop up as Notes.
4. Can we also make use of the Severity level (Low, Medium etc.)?

[laurentsimon]

Thanks both of you for the feedback!

This looks great. Some comments:

1. The titles Determines if ... should be shorter and easier to understand. Something like Dependencies are unpinned.
2. The short description should be shorter. Also, it has some HTML elements which don't get rendered well.

the description is set to the long description atm. It does not support links. Maybe we should remove it? Note that the description is also used by GH next to the line with the rule ID https://github.com/ossf/scorecard/security/code-scanning/2865?query=ref%3Arefs%2Fheads%2Fmain so it's redundant. I have not tested what happens when there are multiple file locations reported.

3. Remove alerts which pop up as Notes.

Looks like nobody likes it :-)
Do we want a GitHub version without and a general SARIF version with it?

4. Can we also make use of the Severity level (Low, Medium etc.)?

in the making, yes.

[laurentsimon]

note: if a check has multiple findings (e.g. multiple binaries found in repo), GH only shows the first finding... which is not ideal. There's not much I can do about it though...:(

[azeemshaikh38]

note: if a check has multiple findings (e.g. multiple binaries found in repo), GH only shows the first finding... which is not ideal. There's not much I can do about it though...:(

Interesting. Any reason there can't be multiple findings from a single check?

[laurentsimon]

Unless I've broken something again, you should see severity levels in the scanning alert tab for our GitHub action https://github.com/ossf/scorecard/security/code-scanning

[naveensrinivasan]

Unless I've broken something again, you should see severity levels in the scanning alert tab for our GitHub action https://github.com/ossf/scorecard/security/code-scanning

It is lot better 👏

[tom--pollard]

Are specific permissions needed to see https://github.com/ossf/scorecard/security/code-scanning ? I receive a 404 @laurentsimon

[naveensrinivasan]

Are specific permissions needed to see https://github.com/ossf/scorecard/security/code-scanning ? I receive a 404 @laurentsimon

You need write permission to view a summary of all the alerts for a repository on the Security tab.
https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/managing-code-scanning-alerts-for-your-repository#viewing-the-alerts-for-a-repository

[laurentsimon]

@tom--pollard thanks for the interest in trying it out! You can easily test it out on a repo you own:

1. Create scorecard-policy.yml under .github/, as in https://github.com/ossf/scorecard/blob/main/.github/scorecard-policy.yml
2. Create a scorecard-analysis.yml workflow under github/workflows/ - just copy paste from https://github.com/ossf/scorecard/blob/main/.github/workflows/scorecard-analysis.yml

The workflow currently supports cron triggers and push triggers for the main/master branch only. I suggest you try the push trigger on the main branch as configured in our workflow yaml file. As a simple verification test, I created a few dummy .whl files in https://github.com/laurentsimon/scorecard-action-test. You should see them under https://github.com/<owner>/<repo>/security/code-scanning.

Let me know if you encounter problems.

[laurentsimon]

@asraa @priyawadhwa would you be interested in trying out the action on sigstore, envoy and tekton repos?
It's not ready yet, but sometimes in 1 month from now.

[jhutchings1]

@laurentsimon What's the latest on this? Love seeing the scorecard integrate with Actions and the code-scanning tab. Let me know if you need any connection to get that into the code scanning actions list in the security tab.

[naveensrinivasan]

Thanks, Also what is the process to get it verified https://docs.github.com/en/developers/github-marketplace/github-marketplace-overview/about-marketplace-badges?

[laurentsimon]

@laurentsimon What's the latest on this? Love seeing the scorecard integrate with Actions and the code-scanning tab. Let me know if you need any connection to get that into the code scanning actions list in the security tab.

Thanks Justin. Is this the same as being published on the market place? (We'd love to do that). What's a typical timeline to have an action published?

[jhutchings1]

@laurentsimon There are a couple of places you can get this listed including the marketplace and the code scanning tab (eg https://github.com/ossf/scorecard/security/code-scanning/setup). We'd be happy to work with you on this. Let me tag my colleague @josepalafox who can follow-up on the process bits?

[laurentsimon]

@laurentsimon There are a couple of places you can get this listed including the marketplace and the code scanning tab (eg https://github.com/ossf/scorecard/security/code-scanning/setup). We'd be happy to work with you on this. Let me tag my colleague @josepalafox who can follow-up on the process bits?

Thanks!

[josepalafox]

When you make a release of the action there's a checkbox for admins to "publish to marketplace" assuming you have the right permissions you should just see this, else TO-DO is get the permissions or find someone with them that can publish to marketplace:

https://docs.github.com/en/actions/creating-actions/publishing-actions-in-github-marketplace

Next please make a PR using the security starter workflow template here:
https://github.com/actions/starter-workflows

Ignore parts of the prompt that are not applicable like "are you a part of the parter program?". The major pieces will be the full SHA, logo, 140 char discription, and the workflow.yml file.

I would like to float some additional development ideas for this action that I think could be really helpful if there's a committee meeting I can attend to share a few ideas let me know.

[laurentsimon]

Thanks for the info. We'll start looking into this.
Feel free to attend our bi-weekly meeting, we'd love to hear you suggestions. The next by-weekly meeting is next Monday at 330pm PST. It's an OSSF public meeting, see https://github.com/ossf/scorecard#connect-with-the-scorecards-community
You can see the meeting notes and add items to it by joining the [email protected] mailing list, as pointed in the link I pasted above.

[evverx]

@laurentsimon I wonder if it would be possible to run it on PRs to analyze diffs to make sure new alerts or regressions aren't introduced? While running it on push events is helpful I think it would be even better if PRs with new alerts didn't make it to repositories in the first place.

[laurentsimon]

yes, we have a working prototype that runs on PR already. GitHub takes care of de-deduplication using the SARIF format, so we (scorecard) need not worry about diff ourselves, which is really helpful.

[evverx]

we have a working prototype that runs on PR already

Good to know. Thanks! Just to be sure, does that mean that I can in theory start using it in my experimental fork?

I took a look at the config at https://github.com/ossf/scorecard/pull/1073/files#diff-4341708db212df7d9bb86a51df68dc97ff9a262aec1eacddff2552a363b08c9f. Is my understanding correct that to turn on just the "token-permission" check I should add something like

version: 1
policies:
  Token-Permissions:
      score: 10
      mode: enforced

?

What happens if I turn on another check where the score is, say, 5 and it stays the same when a PR with new issues is opened?

[evverx]

@laurentsimon I ran the action and noticed that the "binary artifacts" alerts I complained a lot about were classified as "(Test)":

so it seems #1270 could be put on hold in a way.

Anyway, is that a GitHub feature? If so, I wonder how it's implemented? It would help to solve #1256

[laurentsimon]

I think it's a GitHub feature, yes. I suspect they look for test in path. @josepalafox do you know?

[josepalafox]

I think this is the relevant docs.

https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/managing-code-scanning-alerts-for-your-repository#about-labels-for-alerts-that-are-not-found-in-application-code

[evverx]

@josepalafox thank you for the link! Unfortunately it isn't mentioned there how it's implemented. My guess would be that the code assigning labels to alerts came from LGTM and based on my experience with it it's pretty tuned so it would be great if it could be borrowed somehow. I opened #1263 a few days ago where I filtered out all the files containing test but I'm concerned that it's too broad.

@laurentsimon looks like some checks like "Fuzzing" and "CII-Best-Practices" shouldn't pop up when the action is run on forks (to be honest, I don't think "CII-Best-Practices" should pop up anywhere at all even though all the projects I more or less contribute to have it). Anyway, would it be possible for the action to avoid reporting some issues depending on where it's run. I of course can always add github.repository == 'systemd/systemd' to block the action but that would prevent external contributors from getting relevant alerts easily

[laurentsimon]

@laurentsimon looks like some checks like "Fuzzing" and "CII-Best-Practices" shouldn't pop up when the action is run on forks (to be honest, I don't think "CII-Best-Practices" should pop up anywhere at all even though all the projects I'm more or less contribute to have it). Anyway, would it be possible for the action to avoid reporting some issues depending on where it's run. I of course can always add github.repository == 'systemd/systemd' to block the action but that would prevent external contributors from getting relevant alerts easily

Interesting idea. Which checks would you like to keep in a fork?

[evverx]

@laurentsimon initially I thought that "Fuzzing", "CII-Best-Practices", "Signed-Releases", "Branch-Protection" and "Packaging" should be excluded there and then I realized that some forks are used to keep "stable" branches of upstream projects that are used by downstream projects like various linux distributons and there "Signed-Releases", "Branch-Protection" and "Packaging" probably shouldn't be skipped. so it seems only "Fuzzing" and "CII-Best-Practices" can be safely turned off when forks are analyzed. The other checks I think should probably be configured via the config file and if: github.repository and that's already possible as far as I can tell

[evverx]

Regarding "CII-Best-Practices" I think it shouldn't affect the score at all and if it should be promoted for whatever reason it would make sense to make it informational. The problem is that all that check shows is that at some point in the past some projects followed the practices CII considers "best practices" and generally they aren't kept up to date so looking at a badge I can't be sure that whatever it says is up to date. For example, at some point the systemd project used only Coverity Scan and when it was down for almost half a year the badge kept saying a static analysis tool was used. When projects migrate from one hosting service to another they tend to lose a couple of jobs where code is covered with sanitizers for example and their badges don't reflect that. This static nature of the CII badges I think isn't exactly in line with scorecard being able to show the actual state.

[evverx]

@laurentsimon trying to figure out why I got

I noticed that the action uses 2.2.8-103-g12ef070 (which as far as I can tell is kind of old). I wonder if it would be possible to point the action to a version of scorecard with all the changes to the "Binary-Artifact", "Fuzzing", "SAST", "CI-Test", "Protected-Branches" checks that have been merged this week included?

[evverx]

Also it doesn't seem to be possible to pass "--verbosity Debug" to scorecard via the action, which makes it hard to figure out for example why scorecard thinks that PRs weren't reviewed even though were or where SASTs were lost. I'm not sure where those debug message should end up in the end though (maybe, right where short descriptions are?)

[laurentsimon]

@laurentsimon trying to figure out why I got I noticed that the action uses 2.2.8-103-g12ef070 (which as far as I can tell is kind of old). I wonder if it would be possible to point the action to a version of scorecard with all the changes to the "Binary-Artifact", "Fuzzing", "SAST", "CI-Test", "Protected-Branches" checks that have been merged this week included?

We've seen this error before. Thanks for sharing.
@azeemshaikh38 FYI
@@josepalafox do you know why this happens sometimes only? It looks like a permission problem, but happens very rarely so very hard to reproduce and understand the root cause.

[laurentsimon]

Also it doesn't seem to be possible to pass "--verbosity Debug" to scorecard via the action, which makes it hard to figure out for example why scorecard thinks that PRs weren't reviewed even though were or where SASTs were lost. I'm not sure where those debug message should end up in the end though (maybe, right where short descriptions are?)

great point. I could do a debug build. Currently scorecard ignores Debug messages when creating SARIF output. I can hack something up temporarily to help you debug. I could add it to the description maybe for debugging?

[laurentsimon]

@laurentsimon initially I thought that "Fuzzing", "CII-Best-Practices", "Signed-Releases", "Branch-Protection" and "Packaging" should be excluded there and then I realized that some forks are used to keep "stable" branches of upstream projects that are used by downstream projects like various linux distributons and there "Signed-Releases", "Branch-Protection" and "Packaging" probably shouldn't be skipped. so it seems only "Fuzzing" and "CII-Best-Practices" can be safely turned off when forks are analyzed. The other checks I think should probably be configured via the config file and if: github.repository and that's already possible as far as I can tell

can you create a tracking issue for this?

[evverx]

I can hack something up temporarily to help you debug. I could add it to the description maybe for debugging?

I think this feature should be permanent to make it easier for maintainers to dispute scorecard findings :-) It should be turned off by default probably though

can you create a tracking issue for this?

Done: #1304

[evverx]

@laurentsimon I'm not sure the action can be used when PRs are opened until issues like #1268 (comment) keep affecting the checks unpredictably. Diffs will most likely trigger bogus alerts unrelated to PRs due to checks like SAST and CI-Test producing different scores depending on when they are run.

[laurentsimon]

these checks are not enabled on pull requests by default. The reasoning is that a pull request cannot fix them, so a developer would need to disable scorecard or change its config until the checks passes. We considered this a usability issue. In a nutshell, we only enable checks that a PR may have an effect on, i.e., those that only use source code and not GitHub APIs. It's indicated in the https://github.com/ossf/scorecard/blob/main/docs/checks/internal/checks.yaml#L21: GitHub means it uses GitHub APIs and cannot be run on pull request; local means it can run on locally downloaded repo and will be run on pull requests. Thoughts?

[evverx]

Good to know. Thanks! I think that makes sense. I saw --local in https://github.com/ossf/scorecard-actions/blob/main/analyze/entrypoint.sh but wasn't sure what that is :-)

[laurentsimon]

FYI, I'm going to be updating the repo of the scorecard action according to https://docs.github.com/en/actions/creating-actions/publishing-actions-in-github-marketplace#about-publishing-actions, so the scorecard action will break sometimes in the next few days. I'll post a message here when the transition is done.

[laurentsimon]

I'm doing some dogfooding. Given the issue #1415, I've created a second action to see if folks prefer the original one or the v2 one. This dogfoodv2 creates different result entries in the dashboard for all checks, including the Branch-Protection settings, Dependency-Update-Tool and SAST (those had a single entry in the dasboard in the previous version).

For Dependency-Update-Tool, this mean one entry is created if dependabot is disabled and another one if renovabot is disabled. If at least one of the tools is configured, both dashboard results will be marked as "fixed" and will no longer show up. I wonder if you find this good or confusing. Essentially, every Detail present in the scorecard's JSON results are used to create a dashboard entry.

For SAST, it means you'd see one entry for X commits out of Y are checked with a SAST tool and one entry for CodeQL tool not detected. This seems OK since a user may configure CodeQL and it will take time for the tool to catch up and be run on all commits after config is enabled. So long as at least 1 commit is not checked by a SAST tool, the X commits out of Y are checked with a SAST tool entry will remain.

For Branch-Protection, it's pretty simple: one entry for each (settings,branchname) pair.

Here are the steps if you'd like to help me compare it to the previous version:

0. If you tried the previous dogfood action, go to the scanning results on GitHub and delete all results from scorecard. (There's a selection box on the right side you can use to select scorecard as tool)
1. Create a read-only PAT as described in https://github.com/ossf/scorecard-action/blob/test/dogfoodv2/starter-workflows/code-scanning/scorecards.yml#L33 (the workflow secret does not work for graphQl API for branch protection - tracked in BUG: githubv4.Query: Resource not accessible by integration in Branch-Protection #1097)
2. Use this workflow https://github.com/laurentsimon/scorecard-action-test-3/blob/test/dogfoodv2/.github/workflows/scorecard-analysis.yml. The config file is not longer needed.

[naveensrinivasan]

Closing this as we have been already using it.