§ whyisthisdown_
← Back to posts

Reviewed-by: Nobody

Open a pull request against your own repository. Attach a label called approved. Click merge. Nobody else looks at it — not a maintainer, not a bot, not a passing stranger.

Run OpenSSF Scorecard against that repository and ask for the Code-Review check:

| 10 / 10 | Code-Review | all changesets reviewed |

Code-Review is one of the checks Scorecard classifies as High risk, the tier reserved for “unintentional vulnerabilities or possible injection of malicious code”. Its documented purpose is to determine whether a project requires human code review before merge. The number above is the top score, and the repository that earned it has never had a review of any kind.

This piece is about how that happens, how long it has been known, and what the documentation says instead. All of it is against a pinned commit — d1fab88, 15 August 2026, the tip of main when I looked — and all of it is reproducible from five public lab repositories linked at the bottom.

Five doors, one lock

Scorecard groups the last thirty commits on the default branch into changesets and asks whether each one was approved. Before it can ask, it has to decide which review system the changeset came through. That decision lives in checks/raw/code_review.go, and it is a fixed sequence:

if revisionID := getProwRevisionID(c); revisionID != "" { … ReviewPlatformProw }
if revisionID := getGithubRevisionID(c); revisionID != "" { … ReviewPlatformGitHub }
if revisionID := getPhabricatorRevisionID(c); revisionID != "" { … ReviewPlatformPhabricator }
if revisionID := getGerritRevisionID(c); revisionID != "" { … ReviewPlatformGerrit }
if revisionID := getPiperRevisionID(c); revisionID != "" { … ReviewPlatformPiper }

Here is what each detector actually tests:

Platform Evidence required Who supplies it
Prow merged PR carries a label named lgtm or approved anyone with triage on the repo
GitHub commit has an associated merged PR the PR itself
Phabricator commit message matches Differential Revision:[^\r\n]*(D\d+) whoever wrote the commit
Gerrit commit message contains Reviewed-on: and Reviewed-by: whoever wrote the commit
Piper commit message matches PiperOrigin-RevId:\s*(\d{3,}) whoever wrote the commit

The Gerrit detector, in full, because there is nothing to elide:

func getGerritRevisionID(c *clients.Commit) string {
  m := c.Message
  if strings.Contains(m, "Reviewed-on:") &&
    strings.Contains(m, "Reviewed-by:") {
    return c.SHA
  }
  return ""
}

Two substring searches. The value after Reviewed-by: is never parsed, never compared to the author, never checked against anything. Reviewed-by: Nobody <nobody@example.invalid> satisfies it. So does Reviewed-by: followed by nothing at all.

Detection is only half of it. The scoring probe, codeApproved, is what turns a platform label into a verdict:

func approved(c *checker.Changeset) (bool, error) {
  switch c.ReviewPlatform {
  case checker.ReviewPlatformProw,
    checker.ReviewPlatformGerrit,
    checker.ReviewPlatformPhabricator,
    checker.ReviewPlatformPiper:
    return true, nil
  }
  for _, review := range c.Reviews {
    if review.State == "APPROVED" && review.Author.Login != c.Author.Login {
      return true, nil
    }
  }
  return false, nil
}

Four of the five platforms return true before the loop. Only the GitHub path ever reaches the line that compares a reviewer to an author. And because the raw layer populates Reviews and Author only for GitHub changesets, the other four arrive at this function with an empty review list and a zero-valued author — which is fine, since nothing is going to look at them.

The probe’s own definition file describes its job as checking that changes were “approved by someone who is not the author of the changeset”. For four of the five paths, that sentence describes code that does not exist.

The lab

I did not want to argue this from source. Five public repositories, one scenario each, Scorecard built from d1fab88, run on 29 August 2026:

Repo What is in it Score Reason
rbn-a-control 3 commits straight to main, plain messages 0/10 Found 0/3 approved changesets
rbn-b-gerrit 3 commits straight to main, each with fake Reviewed-on: / Reviewed-by: trailers 10/10 all changesets reviewed
rbn-c-prow-label 2 PRs, author = merger, zero reviews, labels approved and lgtm 10/10 all changesets reviewed
rbn-d-selfmerge 1 PR, author = merger, zero reviews, no label 5/10 Found 1/2 approved changesets
rbn-e-phab-piper commits with Differential Revision: https://example.invalid/D1 and PiperOrigin-RevId: 100 10/10 all changesets reviewed

The control does what it should: no review, no points. Every other row is a repository with the same amount of review — none — and three of them score full marks.

Row D is the one that shows the GitHub path is not the problem. Self-merge without a label falls through to the login comparison, the merger equals the author, and the changeset is correctly rejected. Row C is the same repository shape with one label added. The label routes the changeset to Prow, Prow short-circuits, and the comparison that would have caught it is never executed. Detection order matters: Prow is tested before GitHub, so a label wins over the evidence that was sitting right there.

A small detail from the lab that turns out to matter: a commit hook on my machine appended a Co-authored-by: trailer to every message. The detectors did not care. strings.Contains does not require the marker to be a trailer, to be in the footer, or to be alone. It has to be somewhere in the text.

One more thing the lab surfaced. --show-details adds nothing to this check. The DETAILS column is a link to the docs page; the JSON output has "details": null. A High-weight check hands you a ten and cannot tell you which changesets it counted, which platform it thought each one came through, or what it took as evidence. The number is the entire output. If you have spent time on alerting you will recognise the shape: a signal with no trace behind it is indistinguishable from a signal that was never computed.

Known since 2021, closed as irrelevant

None of this is a discovery. In April 2021, issue #370, opened by a maintainer, asked the question directly: the Gerrit check looks for Reviewed-on: in the message — “Is there a better way?” A follow-up in the same thread worked through what a real answer would need: for a repository that might be hostile, the only trustworthy signal would be an attestation from the hosting platform that Gerrit was actually running and enforcing, and no such thing existed.

That is the correct analysis. The correct conclusion from it is that Scorecard cannot verify Gerrit review from a GitHub mirror, and should say so — inconclusive, the outcome the check already uses when it sees only bot activity. Instead the issue sat for nearly three years, a stale-bot asked whether it still mattered, and in February 2024 it was closed after community backlog refinement as no longer relevant. The substring check it asked about is unchanged.

The commit history around it moved in the other direction. A timeline, from git log:

Date PR What changed
2020-11 #76 Gerrit detection introduced: \nReviewed-on: and \nReviewed-by:
2021-04 #370 Maintainer asks whether the Gerrit string check can be replaced
2022-03 #1783 Issue: PR created and merged by the same person counts as reviewed. Still open.
2022-05 #1884, #1889 Phabricator (required Reviewed By: as well) and Piper added
2022-09 #2260 All three message heuristics loosened: newlines dropped, Phabricator’s reviewer requirement removed, Piper moved to a bare regex. Prow label path added.
2022-11 #2413 Merger recorded as an implicit APPROVED review
2024-01 #3302 codeReviewOneReviewers moved to the “not included by any checks” list
2024-02 #370 closed: no longer relevant
2024-04 #3979 Check migrated to the codeApproved probe; the four-platform short-circuit takes its current form

Read that middle stretch again. Six months after a maintainer asked whether the heuristics were too weak, one PR made all of them weaker. Two months after that, “someone other than the author clicked merge” became a form of approval. Issue #1783, which described exactly that as a bypass, is still open.

And #3302 is the one that stings. codeReviewOneReviewers is a probe that lives in the repository today. It parses the review list, deduplicates reviewer logins, and explicitly excludes the author from the count — the thing the scoring probe does not do. Since January 2024 it sits in a list literally labelled Uncategorized with the comment “Probes which aren’t included by any checks”. Scorecard has the correct implementation. It ships the other one. The OSPS Baseline coverage document still cites the unused probe as evidence for the code-review control.

The documentation describes a different program

docs/checks.md is what the score links to. It is also what every downstream tool quotes when it explains what a Code-Review score means. Three things in it are not true of the code.

Scoring. The docs say scoring is leveled, not proportional: seven points off if a single human change is unreviewed, three more if several are. That paragraph was written in January 2023 for PR #2542, which implemented exactly that. PR #2882 reverted the implementation to proportional in June 2023 — its diff touches one file, and that file is not the documentation. The current formula is min(10 × approved ÷ total, 10). Row D above is the live proof: one of two changesets approved, score 5. The docs say that repository should score 3.

Bots. The docs are emphatic that reviews by bots, including AI reviewers, do not count. In the code, IsBot is computed for exactly one identity: the PR author, from its GraphQL resource path. Reviewers have a login and nothing else; so does the merger. The only comparison on the GitHub path is reviewer login ≠ author login. An app that approves pull requests passes it. A human who merges an app’s pull request passes it.

Coverage. The docs describe GitHub, GitLab, Prow and Gerrit. Phabricator and Piper — two of the four unconditional-approval paths — are not mentioned. The docs also say the implicit-review rule fires when the merger differs from the committer; the code compares the merger to the PR author. That one is a point in the code’s favour, and it is the only place in this check where the code is stricter than its description.

The pattern is the same one I keep writing about. The docs page is a summary of a changelog, which is a summary of a diff, and each layer is easier to read than the artifact underneath it and therefore wins by default. Here the description drifted from the implementation three years ago and the drift is still the canonical explanation of what a High-risk score means.

It fails in both directions

If the check only over-scored, you could treat it as a floor and move on. It does not.

Issue #4730, from July 2025, is a user asking why OpenSSL scores zero on Code-Review when every one of its last thirty commits has an approved pull request attached. I have not traced the mechanism for that particular project and will not guess at it here. The point is narrower: the same check that awards a perfect score to a repository with a fake trailer and no reviewers can award nothing to a repository whose reviews are visible to anyone who opens the commit list. A metric that errs in both directions is not a conservative estimate. It is noise with a confident number attached.

What consumes the number

Nobody runs Scorecard for fun. The score is an input: to dependency dashboards, to security policies that gate which packages a build may pull, to procurement checklists, to the aggregate score that shows up as a badge in a README. Code-Review carries High weight in that aggregate. Every consumer inherits the check’s assumptions without seeing them, because the output has no details and the docs describe a different algorithm.

I want to be precise about what I am and am not claiming. I am not claiming Scorecard is useless, or that the maintainers are careless — the same repository has a probe that does this right, and an issue thread from 2021 that understood the problem better than most of the tools built on top of the score. I am claiming three specific things:

  1. A High-risk check accepts self-attested evidence on four of five paths, and the evidence is a substring in text the author controls.
  2. This was identified by the project in 2021, made weaker in 2022, and closed as irrelevant in 2024.
  3. The documentation for the check has described the wrong scoring algorithm since June 2023 and omits two of the four unconditional paths.

The fix is not exotic and does not require the platform attestation the 2021 thread wished for. A message-only signal cannot verify review; the honest outcome is inconclusive, which the check already knows how to emit. The Prow path has a merged PR in hand and should fall through to the login comparison instead of skipping it. The docs should say what the code does. And --show-details should show the details — which changesets, which platform, which evidence — so the next person who gets a surprising number can find out why without cloning the repository and reading Go.

Until then, when a dependency shows you Code-Review: 10, you know one of the following is true: it has human review, or someone typed Reviewed-by: into a commit message, or a PR had a label on it. The score does not distinguish between them, and nothing downstream of the score can either.


Lab repositories, all public and left in place for reproduction: rbn-a-control · rbn-b-gerrit · rbn-c-prow-label · rbn-d-selfmerge · rbn-e-phab-piper. Scorecard v5.5.1-0.20260815060127-d1fab88f5463, run 2026-08-29.

§ Sources & References