Blog
|
DEVOPS

GitHub CODEOWNERS: A Developer’s Code Review Guide

By
Nir Valtman
July 23, 2022
7 Mins

How we tested GitHub CODEOWNERS scenarios?

We decided to test GitHub CODEOWNERS scenarios by combining branch protection rules, direct and indirect git permission assignments, and other misconfigurations. The common settings across all scenarios are:

GitHub CODEOWNERS Example #0a: Expected Behavior with “Require review from Code Owners”

Scenario Settings

  • A GitHub CODEOWNERS file is configured with paths and the assigned reviewers are GitHub users or Teams with effective Write permissions.
CODEOWNER File configuration
  • The Branch Protection policy setting “Require review from Code Owners” is enabled.
Branch protection policy setting

Results

If the “Require review from Code Owners” setting is enabled, after the user or Team with effective Write permission approved the pull request, merging is allowed.

Showing changes approved in GitHub

Each of the assigned reviewers gets an email notification when assigned to review the Pull Request.  

If the reviewer is not in the CODEOWNERS file or does not have the effective Write permission, the merge is blocked.

Showing "review required" in GitHub

GitHub CODEOWNERS Example #0b: Expected Behavior without “Require review from Code Owners”

Scenario Settings

  • A CODEOWNERS is configured with paths and the assigned reviewers are users or Teams with effective Write permissions.
CODEOWNERS file configuration
  • The Branch Protection policy setting “Require review from Code Owners” is disabled.
Setting your branch protection policy

Results

Any approval by any non-author user with a Write permission passes the required check to merge the code into the protected branch.

Changes approved in GitHub

Everyone in the relevant paths in the CODEOWNERS file will get a notification to approve the Pull Request, but the action can be taken by a user not in the CODEOWNERS file.

GitHub CODEOWNERS Example #1: Empty CODEOWNERS file

Scenario Settings

  • Attempted CODEOWNERS settings as in the screenshots below – one with no paths and another with a path but no user/Team assignment.
Showing the Codeowners file as valid
Showing the Codeowners file as valid

Results

Regardless of the “Require review from Code Owners” setting, any approval by any non-author user with a Write access will allow the code to be merged.  

GitHub CODEOWNERS Example #2: Empty team in CODEOWNERS  

Scenario Settings

  • Created a Team with Write access and no members.
Managing access in GitHub
  • CODEOWNERS settings as in the screenshots below.
Showing the Codeowners file as valid

Results

If “Require review from Code Owners” is enabled, the merging is blocked unless bypassed by an admin or a user is added to the empty Team.

Showing that review is required in GitHub
Tip:

This approach is a suitable alternative to archiving a stale repository. The considerations are highlighted in the considerations to protect stale code repositories.

GitHub CODEOWNERS Example #3: Empty path in CODEOWNERS

Scenario Settings

  • CODEOWNERS settings with a specific path, but without any users assigned to it.
CODEOWNERS settings with a specific path, but without any users assigned to it
  • A file is created under the path in row #3 above.

Results

Regardless of the “Require review from Code Owners” setting, any approval by any non-author user with a Write access will allow the code to be merged.  

Tip:

This approach is good in order to allow merges in paths where the file changes are less sensitive. For example, allow everyone to modify a folder with a markdown content that explains developers how to setup the service on their local machine.

GitHub CODEOWNERS Example #4: Team in CODEOWNERS without Write permission

Scenario Settings

  • Created a Team with Read permission. None of the team members have effective Write permission either through other Teams or directly.
Showing a team with read permissions
  • CODEOWNERS settings as in the screenshots below. It has an error deliberately.
CODEOWNERS file with an error

Results

Regardless of the “Require review from Code Owners” setting, even after the user in the Team “arnica-codeowners” approved the pull request, merging is blocked.

GitHub showing that review is required

Additionally, since the Team is misconfigured, the members will not receive an email notification with the ask to review the Pull Request.

Tip:

To fix this error, simply add the Write permission for this Team.  

GitHub CODEOWNERS Example #5: Team in CODEOWNERS without Write permission, but user has Write permission

Scenario Settings

  • Created a Team with Read access. One of the team members has Write access directly.
Managing access in GitHub
  • CODEOWNERS settings as in the screenshots below. It has an error deliberately.
CODEOWNERS file showing errors

Results

Same as Scenario #0a.

This is interesting as the Team is misconfigured, but the approval of the member with the Write access is counted as a valid check.  

The downside of this misconfiguration is that the user with Write access did not receive an email notification to review the Pull Request.  

Tip:

To fix this error, simply add the permission Write for this Team.  

GitHub CODEOWNERS Example #6: User without Write permission

Scenario Settings

  • Granted a user the Read permission. The user does not have any higher permissions through Teams.
Granting a user read permissions in GitHub
  • CODEOWNERS settings as in the screenshot below. It has an error deliberately.
CODEOWNERS file showing an error

Results

Regardless of the “Require review from Code Owners” setting, even after the user in the Team “arnica-codeowners” approved the pull request, merging is blocked. It seems to be like scenario #4 above, where the owner is identified but has insufficient access to the repository.  

Review Required in GitHub

Since the user is misconfigured, an email notification will not be sent. Managing developer permissions effectively, such as with Arnica's Developer Access Management, can prevent such issues.

Tip:

To fix this error, simply add the permission Write for this user.  

GitHub CODEOWNERS Example #7: User without direct Write permission, but with indirect Write permission

Scenario Settings

  • Granted a user the Read permission. The user has higher permissions through the Team “arnica-codeowners”.
Granting a user read permissions in GitHub
  • CODEOWNERS settings as in the screenshot below.
CODEOWNERS file shown as valid

Results

Similar to Scenario #0a.  

Note that in this case the user received an email notification with the ask to review the Pull Request.  

email notification with the ask to review the Pull Request
Tip:

When a “mixed role” is configured, the more permissive permission takes place. There might be a misconfiguration in the way permissions are defined, and therefore, worth reviewing the root cause of this configuration.  

GitHub CODEOWNERS Example #8: Invalid user

Scenario Settings

  • CODEOWNERS settings as in the screenshots below – one user is no longer in the organization and the other user does not exist on GitHub. Both users have the same error.
CODEOWNERS settings

Results

Similar to Scenario #1.

Tip:

Assign Teams instead users in CODEOWNERS files, so that if a reviewer leaves the organization, the file remains valid and enforced.  

Fix GitHub CODEOWNERS with one-click in Arnica

This part is a bit “sales-ish”, but if you got here, wouldn’t it be great if you could see all mentioned misconfigurations and fix them with a single click?  

At Arnica, we identify misconfigured CODEOWNERS across all organizations for free forever, regardless how many users and repositories are scanned.  The easy-button solution also fixes misconfigured CODEOWNERS. Here is a snapshot:

Fixing mismanaged CODEOWNERS in Arnica

Summary: CODEOWNERS Scenarios We Tested

Here are the scenarios and test results in a single table:

Scenario Can merge when “Require review from Code Owners” is enabled Can merge when “Require review from Code Owners” is disabled Email Notification
#1: Empty CODEOWNERS file Upon approval by any non-author user with a Write permission Upon approval by any non-author user with a Write permission N/A
#2: Empty team in CODEOWNERS No Upon approval by any non-author user with a Write permission N/A
#3: Empty path in CODEOWNERS Upon approval by any non-author user with a Write permission Upon approval by any non-author user with a Write permission N/A
#4: Team in CODEOWNERS without Write permission No Upon approval by any non-author user with a Write permission No
#5: Team in CODEOWNERS without Write permission, but user has Write permission Upon approval by the code owner(s) Upon approval by any non-author user with a Write permission No
#6: User without Write permission No Upon approval by any non-author user with a Write permission No
#7: User without direct Write permission, but with indirect Write permission Upon approval by the code owner(s) Upon approval by any non-author user with a Write permission Yes
#8: Invalid user Upon approval by any non-author user with a Write permission Upon approval by any non-author user with a Write permission N/A

We can summarize this even further using the following logic

  • If “Require review from Code Owners” is enabled and there is a CODEOWNERS file, and it has at least one path that is associated with an existing user or a team:
    - CODEOWNERS is enforced - The required review check will only pass (excluding overrides) if the approval was made by any user (other than the author) that is either directly or indirectly associated with the CODEOWNERS path, and has effective Write permission or higher (either directly or via a Team)
  • Else (If the branch protection policy setting of “Require review from Code Owners” is disabled, or enabled but with no CODEOWNERS files or empty CODEOWNERS file, or has empty paths, or has paths with invalid users / teams):
    - CODEOWNERS is not enforced - Any approval by any non-author user with a Write permission (or higher) will allow the code to be merged.

Reduce Risk and Accelerate Velocity

Integrate Arnica ChatOps with your development workflow to eliminate risks before they ever reach production.  

Try Arnica