Excessive Pull Request Reviewers / CODEOWNERS

Definitions

Pull request reviewers have permissions to comment, approve, or request changes on code changes in a pull request, ensuring code quality and adherence to standards. CODEOWNERS are assigned automatic reviewers for specific files or directories, granting them authority to approve or request changes in those areas, ensuring expertise-based oversight of code modifications.

The permission of a pull request reviewers or CODEOWNERS in the context of Arnica means that the pull request approval is counted towards the ability to merge code to the target branch.

Risks

Having excessive pull request reviewers or CODEOWNERS permissions in a source code repository can pose several operational and security risks:

  1. Code Integrity and Quality: If there are too many reviewers or CODEOWNERS with excessive permissions, it can lead to a lack of accountability and oversight. Multiple reviewers may result in conflicting feedback or inconsistent code quality standards, potentially compromising the integrity of the codebase.

  2. Delayed Review Process: Excessive reviewers can slow down the pull request review process. With numerous reviewers, coordination and consensus among them can become challenging, leading to delays in merging changes and impacting development timelines.

  3. Increased Complexity: More reviewers introduce additional complexity to the review process. It becomes harder to manage and track feedback, comments, and suggested changes, making it more likely that important issues or vulnerabilities are overlooked.

  4. Security Risks: If there are too many reviewers or CODEOWNERS, it increases the likelihood of oversight or negligence in identifying security vulnerabilities. This can lead to the introduction of insecure code into the repository, potentially exposing the system to attacks or breaches.

  5. Limited Ownership and Accountability: Excessive reviewers may dilute individual ownership and accountability for code changes. With multiple reviewers involved, it can be challenging to attribute responsibility for specific issues or bugs, hindering effective issue resolution and debugging.

How excessive permissions are determined?

Excessive pull request reviewers and CODEOWNERS are identities that their approval is counted towards the ability to merge code to the target branch, but they did not review any code changes in the applicable source code repository in a given time frame.

How to change the excessive permissions logic?

To configure the time frame for excessive pull request reviewers or CODEOWNERS permissions, go to the Policies page under the Admin section and adjust the permissions definitions settings for Code Contributors.

How to mitigate excessive permissions?

Navigate to the Git Posture page and click on the Permissions widget.

If you have multiple source code management (SCM) tools, select the relevant SCM type at the top right corner menu.

Github: Modify existing CODEOWNERS file

Arnica creates a Pull Request with the modified CODEOWNERS based on the Pull Request activities in each of the branches.

Instead of removing members from Teams, Arnica adjusts the CODEOWNERS file by removing the original Team slugs (e.g. @org_name/team_slug) that have members with excessive permissions and replacing them with Arnica-managed Teams with the naming convention @org_name/[arnica]-{repo}-{branch}-{path]-CODEOWNERS.

For example, the screenshot below demonstrates the changes between a manually configured CODEOWNERS file and the Arnica-managed Teams:

To ensure that CODEOWNERS are the only ones who can approve Pull Requests before merging, Arnica will ensure that the branch protection policies require at least 1 approver from the CODEOWNERS file is enabled, as in the screenshot in the "Create and enforce CODEOWNERS" above.

Azure DevOps: Modify existing branch protection policy

When mitigating excessive reviewer permissions, Arnica treats the policy for each branch and distinct path individually. For each reviewer list, Arnica removes any user present that has not completed a review, leaving only those users who have exercised their reviewer privileges. In the event that a group, and not users are listed in the reviewer list, the group's members will be assessed individually, and a new group will be created by Arnica. This group will include only those members from the original group that were not excessively privileged and will replace the old group on the policy list.

Last updated