Code Review

ID

code_review

Severity

high

Family

SCM

Tags

code-reviews, non-reachable, security, source-code, supply-chain

Description

Does the project require code review before code is merged?

This check determines whether the project requires code review before pull requests (merge requests) are merged.

Security

Reviews detect various unintentional problems, including vulnerabilities that can be fixed immediately before they are merged, which improves the quality of the code.

Reviews may also detect or deter an attacker trying to insert malicious code (either as a malicious contributor or as an attacker who has subverted a contributor’s account), because a reviewer might either detect the subversion, including any attempts made by the attacker to obfuscate malicious code or implant an evil dependency.

Lack of code review increases the risk of unintentional vulnerabilities or possible injection of malicious code.

Requiring review does not eliminate all risks. The other reviewers might fail to notice unintentional vulnerabilities or malicious code, be colluding with a malicious developer, or even be the same person (using a "sock puppet" account).

Mitigation / Fix

  • If the project has only one contributor, or does not have enough reviewers to practically require that all contributions be reviewed, try to recruit more maintainers to the project who will be willing to review others' work. Ideally at least some of these people will be from different organizations. If the project has very limited utility, consider expanding its intended utility so more people will be interested in improving it, and make that larger scope clear to potential contributors.

  • Follow security best practices by performing strict code reviews for every new pull request / merge request.

  • Make "code reviews" mandatory in your repository configuration. (Instructions for GitHub.)

  • Enforce the rule for administrators / code owners as well. (instructions for github.)