Closes #147 — the finding from the security sweep, and the one where the current behaviour is wrong on the live instance.
mr review resolves with CanRead and applies no further check, and
reviewGates counted every fresh verdict. On a public repository that let
anyone with an account:
- satisfy
require_approvals— defeating four-eyes review by holding two accounts, which withregistration = "open"means defeating it outright; and - block a merge the owner wanted, with no override and only a force-push to clear it.
Both were confirmed against a running instance before this was written,
and TestReviewsCountOnlyFromWriters fails on the first of them if the
filter is removed — I checked.
Reviewing stays open to every reader. An outside opinion on a public change is worth having and discarding it would be the wrong fix; it just does not decide the gate. What does is write access, because that is the same question the gates already answer: someone who could push the change themselves is whose approval means the repository accepts it.
A CODEOWNERS entry naming someone without write is a misconfiguration for an owner to fix rather than a case to special-case here — they could not merge what they approved.
mr show and the merge request page mark an uncounted review advisory.
A page showing an approval the gate ignores differs from the gate, and
that difference would surface only when a merge was refused. Both read the
same ReviewersWhoCount, so they cannot drift.
Closes #147