ossf / scorecard

OpenSSF Scorecard - Security health metrics for Open Source
https://scorecard.dev
Apache License 2.0
4.3k stars 470 forks source link

Feature: SAST tool run on PR should count more than those run after merge #1268

Open laurentsimon opened 2 years ago

laurentsimon commented 2 years ago

See https://github.com/ossf/scorecard/issues/1031#issuecomment-969117938 (Additional long-term improvements are in https://github.com/ossf/scorecard/issues/966#issuecomment-915598041)

We would like to give more points to repos that run SAST before merging code, i.e. on pull_request event.

FYI, cron-scheduled runs are automatically disabled after 60 days of inactivity https://docs.github.com/en/actions/managing-workflow-runs/disabling-and-enabling-a-workflow

laurentsimon commented 2 years ago

@evverx FYI

evverx commented 2 years ago

I'm not sure whether it's the right place to mention this issue with the checks analyzing PRs but since I've run into it mostly using the SAST check I think it would probably make sense to switch from the "n out of 30 PRs have something" approach to something that would account for glitches preventing status checks from being registered properly or weird results that GH API returns once in a while. Otherwise it seems even if projects where SASTs are run on PRs were rated higher they would most likely loose some points anyway due to what looks like transient GH issues.

evverx commented 2 years ago

For example, LGTM was run in https://github.com/libbpf/libbpf/pull/353 but GH API says that it wasn't.

evverx commented 2 years ago

One option would be to figure out whether LGTM is listed among the apps that can be triggered on pull requests with https://docs.github.com/en/rest/reference/checks#get-a-check-suite and if it's there and it is run in the majority of pull requests it should be safe to say that projects are analyzed with LTGM. I'm not sure about other apps though

laurentsimon commented 2 years ago

I'm not sure whether it's the right place to mention this issue with the checks analyzing PRs but since I've run into it mostly using the SAST check I think it would probably make sense to switch from the "n out of 30 PRs have something" approach to something that would account for glitches preventing status checks from being registered properly or weird results that GH API returns once in a while. Otherwise it seems even if projects where SASTs are run on PRs were rated higher they would most likely loose some points anyway due to what looks like transient GH issues.

@josepalafox is this a known issue with GitHub APIs?

laurentsimon commented 2 years ago

One option would be to figure out whether LGTM is listed among the apps that can be triggered on pull requests with https://docs.github.com/en/rest/reference/checks#get-a-check-suite and if it's there and it is run in the majority of pull requests it should be safe to say that projects are analyzed with LTGM. I'm not sure about other apps though

with this API, looks like we could get the last N pushes that have used it; and we'd need to map them to the last PRs. Is my understanding correct? That sound doable.

Are there limitations in terms of whom can use this API: GitHub Apps must have the checks:read permission on a private repository or pull access to a public repository to get check suites. OAuth Apps and authenticated users must have the repo scope to get check suites in a private repository. Is it accessible by non-apps?

laurentsimon commented 2 years ago

summary:

  1. tool configured (codeQl) and runs as cron => give some point (we already do this )
  2. tool configured on each push => increase the score (we don't do this)
  3. tool configured on pull request => increase score (we do this)
evverx commented 2 years ago

tool configured on each push => increase the score (we don't do this)

It should probably be "on every PR". systemd for example doesn't run SASTs on push events. It runs LGTM on PRs and CodeQL and Coverity once a day due their various limitations.

laurentsimon commented 2 years ago

ah, right. Agreed.

github-actions[bot] commented 8 months ago

This issue is stale because it has been open for 60 days with no activity.