Bug: SAST is calculated incorrectly

[eustas]

Context: https://github.com/google/brotli/security/code-scanning/4 Quick view on actions panel reveals that report is not true: https://github.com/google/brotli/actions/workflows/codeql.yml?query=branch%3Amaster

[spencerschrock]

Scorecard does its analysis by downloading the repo's tarball. brotli uses a .gitattributes file which doesn't include the github workflows in its download, so it has trouble seeing your codeql workflow file.

This is unfortunately a known issue (https://github.com/ossf/scorecard/issues/2489#issuecomment-1331377334), and some of the alternatives (e.g git clone) have proved slow enough that we haven't switched over. It may be worth having a Scorecard CLI argument (and a corresponding GitHub action argument) which does a git clone.

[eustas]

Thanks for pointing me to that. But for me it looks like a different problem. Scorecard does not say that SAST is not detected, it says: "SAST tool detected but not run on all commmits".

[spencerschrock]

Thanks for pointing me to that. But for me it looks like a different problem. Scorecard does not say that SAST is not detected, it says: "SAST tool detected but not run on all commmits".

My mistake, we recently changed how we look for the workflow file, so I had assumed that was it.

In terms of detecting it on all commits, I see this when running Scorecard

go run main.go --repo google/brotli --checks SAST --show-details --verbosity debug --show-details --format json | jq

    {
      "details": [
        "Debug: tool detected: github-code-scanning",
        "Debug: tool detected: github-code-scanning",
        "Warn: 2 commits out of 5 are checked with a SAST tool",
        "Warn: CodeQL tool not detected"
      ],
      "score": 4,
      "reason": "SAST tool is not run on all commits -- score normalized to 4",
      "name": "SAST",
      "documentation": {
        "url": "https://github.com/ossf/scorecard/blob/main/docs/checks.md#sast",
        "short": "Determines if the project uses static code analysis."
      }
    }

The offending PRs seem to be empty PRs from Copybara:

List of pull requests without CI test: 1039, 1035, 1021

For each of the last 30 commits, Scorecard looks at the associated PR and then the last commit in that PR to look for the SAST tool. So the current logic doesn't count direct pushes to main, but rather that a SAST tool is run on a PR before merging. There has been some discussion on that here https://github.com/ossf/scorecard/issues/1580