BUG: Code-Review check inaccurate for at least one project

[crlorentzen]

Describe the bug Looking for some help to understand why OpenSSF Scorecard check for Code-Review on Openssl is marked as a zero https://securityscorecards.dev/viewer/?uri=github.com/openssl/openssl when I look at the recent commits I see associated PRs. I am not understanding the openssf code itself to figure this out. Has anyone else noticed an inconsistency like this and have thoughts on figuring out why?

Reproduction steps Steps to reproduce the behavior:

1. Run OpenSSF Scorecard on https://github.com/openssl/openssl
2. Manually review the recent commits to see if they have an approved PR.

Expected behavior I expect the score to be better since when I look at the latest 30 commits I find approved PRs associated with each commit

[gcanlin]

I guess scorecard run this GraphQL API to check whether commits associate any pull requests.

curl -H "Authorization: Bearer YOUR_TOKEN" \
     -H "Content-Type: application/json" \
     -X POST \
     -d '{"query":"{ repository(owner: \"openssl\", name: \"openssl\") { object(expression: \"HEAD\") { ... on Commit { history(first: 5) { nodes { oid message associatedPullRequests(first: 1) { nodes { number mergedAt reviews(last: 10) { nodes { state author { login } } } } } } } } } } }"}' \
     https://api.github.com/graphql | jq

And we get the result:

{
  "data": {
    "repository": {
      "object": {
        "history": {
          "nodes": [
            {
              "oid": "b20da2328018107414fe896e59e7d4d6c8af8174",
              "message": "Revert \"Pairwise check for DH keys import as part of FIPS\"\n\nNot needed anymore. It's handled in the 'ec (fips): add PCT for key import'\n\nThis reverts commit e08b83cbb3b853ae9dc364c32d927405172918ac.\n\nReviewed-by: Neil Horman <[email protected]>\nReviewed-by: Tomas Mraz <[email protected]>\n(Merged from https://github.com/openssl/openssl/pull/28122)",
              "associatedPullRequests": {
                "nodes": []
              }
            },
            {
              "oid": "1afc4e8baa3226ea6edb643180246201968d8958",
              "message": "dh: add extra argument to ossl_dh_check_pairwise\n\nReviewed-by: Neil Horman <[email protected]>\nReviewed-by: Tomas Mraz <[email protected]>\n(Merged from https://github.com/openssl/openssl/pull/28122)",
              "associatedPullRequests": {
                "nodes": []
              }
            },
            {
              "oid": "db969c3ab08240cf9652cb621fbf1936d056464c",
              "message": "dh: add FIPS 140-3 PCT on key import.\n\nThis is mandated by FIPS 140-3 IG 10.3.A additional comment 1\n\nReviewed-by: Neil Horman <[email protected]>\nReviewed-by: Tomas Mraz <[email protected]>\n(Merged from https://github.com/openssl/openssl/pull/28122)",
              "associatedPullRequests": {
                "nodes": []
              }
            },
            {
              "oid": "88a13095667228c2361361c97704ea992d837ade",
              "message": "fips: add DH PCT name\n\nReviewed-by: Neil Horman <[email protected]>\nReviewed-by: Tomas Mraz <[email protected]>\n(Merged from https://github.com/openssl/openssl/pull/28122)",
              "associatedPullRequests": {
                "nodes": []
              }
            },
            {
              "oid": "32ff539daf83cccc15a159fe214cac66acc80fec",
              "message": "changes: add note about PCT on key import to the FIPS provider\n\nThis is mandated by FIPS 140-3 IG 10.3.A additional comment 1.\n\nReviewed-by: Neil Horman <[email protected]>\nReviewed-by: Tomas Mraz <[email protected]>\n(Merged from https://github.com/openssl/openssl/pull/28122)",
              "associatedPullRequests": {
                "nodes": []
              }
            }
          ]
        }
      }
    }
  }
}

All commits weren't associated their pull requests. I found all PR will be closed by maintainers in the openssl repos of GitHub. It seems that they don't merge PRs from GitHub UI. The maintainers will pull the PR into their local directories and push the commits to remote instead of merging directly in GitHub. So scorecard miss these. I think that we can add a new detection to check whether message includes Reviewed-by string. If so, this commit will be marked as a reviewed commit.