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:
- A High-risk check accepts self-attested evidence on four of five paths, and the evidence is a substring in text the author controls.
- This was identified by the project in 2021, made weaker in 2022, and closed as irrelevant in 2024.
- 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.