ossf / scorecard

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

BUG: `RepoClient` data format #1242

Open azeemshaikh38 opened 2 years ago

azeemshaikh38 commented 2 years ago

Currently, the RepoClient data format uses primitive Golang types. This means that when certain values are not returned, the result struct will have default values set. This can lead to incorrect results.

We should update the RepoClient data to use *bool, *string etc. And the convention will be that if the value is non-nil, only then analyze the resut.

evverx commented 2 years ago

Could it be that this causes https://github.com/ossf/scorecard/issues/1247?

azeemshaikh38 commented 2 years ago

Most likely yes. I have a PR out to fix this for Branch-Protection: #1243. With this PR, this is the result I get:

scorecard  --format json --show-details --verbosity Debug --checks Branch-Protection  --repo=https://github.com/evverx/systemd | jq
{
  "date": "2021-11-11",
  "repo": {
    "name": "github.com/evverx/systemd",
    "commit": "1b1e6249a0987820cde646a11c1b559dcb02d62f"
  },
  "scorecard": {
    "version": "3.0.1",
    "commit": "6c1c789dc5b05cde492334f57b53807c786b038a"
  },
  "score": 2,
  "checks": [
    {
      "details": [
        "Info: 'force pushes' disabled on branch 'main'",
        "Info: 'allow deletion' disabled on branch 'main'",
        "Warn: linear history disabled on branch 'main'",
        "Warn: status checks for merging disabled on branch 'main'",
        "Info: number of required reviewers is 2 on branch 'main'",
        "Warn: Stale review dismissal disabled on branch 'main'",
        "Warn: Owner review not required on branch 'main'",
        "Warn: 'admininistrator' PRs are exempt from reviews on branch 'main'"
      ],
      "score": 2,
      "reason": "branch protection is not maximal on development and all release branches",
      "name": "Branch-Protection",
      "documentation": {
        "url": "https://github.com/ossf/scorecard/blob/6c1c789dc5b05cde492334f57b53807c786b038a/docs/checks.md#branch-protection",
        "short": "Determines if the default and release branches are protected with GitHub's branch protection settings."
      }
    }
  ],
  "metadata": null
}

Does that align with your expected Scorecard output?

evverx commented 2 years ago

It does but with #1243 applied I got

{
  "date": "2021-11-11",
  "repo": {
    "name": "github.com/evverx/systemd",
    "commit": "1b1e6249a0987820cde646a11c1b559dcb02d62f"
  },
  "scorecard": {
    "version": "3.1.1-46-ga4afb71",
    "commit": "a4afb711e30eb368d3ecfa8b38d4c4efb62ca076"
  },
  "score": 4.0,
  "checks": [
    {
      "details": [
        "Info: 'force pushes' disabled on branch 'main'",
        "Info: 'allow deletion' disabled on branch 'main'",
        "Warn: linear history disabled on branch 'main'",
        "Warn: 'administrator' PRs are exempt from reviews on branch 'main'",
        "Info: strict status check enabled on branch 'main'",
        "Warn: status checks for merging have no specific status to check on branch 'main'",
        "Warn: number of required reviewers is only 824638264608 on branch 'main'",
        "Info: Stale review dismissal enabled on branch 'main'",
        "Warn: Owner review not required on branch 'main'"
      ],
      "score": 4,
      "reason": "branch protection is not maximal on development and all release branches",
      "name": "Branch-Protection",
      "documentation": {
        "url": "https://github.com/ossf/scorecard/blob/a4afb711e30eb368d3ecfa8b38d4c4efb62ca076/docs/checks.md#branch-protection",
        "short": "Determines if the default and release branches are protected with GitHub's branch protection settings."
      }
    }
  ],
  "metadata": null
}

Warn: number of required reviewers is only 824638264608 on branch 'main' and Info: strict status check enabled on branch 'main' look weird I'd say :-) Let me try to rebuild scorecard anew with that PR

evverx commented 2 years ago

Looks like in your version of scorecard both https://github.com/ossf/scorecard/issues/1247 and https://github.com/ossf/scorecard/issues/1246 are fixed (but it somehow lost the "Stale review dismissal" subcheck). In my version of scorecard the two issues are somehow still present but manifest themselves a bit differently.

evverx commented 2 years ago

I'm not sure if that helps but just in case

$ go version
go version go1.17.3 linux/amd64

$ cat /proc/version
Linux version 5.14.16-101.fc33.x86_64 (mockbuild@bkernel01.iad2.fedoraproject.org) (gcc (GCC) 10.3.1 20210422 (Red Hat 10.3.1-1), GNU ld version 2.35-18.fc33) #1 SMP Thu Nov 4 01:35:22 UTC 2021
azeemshaikh38 commented 2 years ago

Ok, another great catch @evverx!

Hope that helps.

azeemshaikh38 commented 2 years ago

Closing this.

cpanato commented 9 months ago

Is this still relevant?