branches: can't remove restrictions

[anderssonjohan]

Problem Description

When you remove a push restriction from a branch protection rule, the change can't be applied.

Test scenario A:

1. Create a branch protection rule with two users or teams in the push restrictions. Example:

branches:
  - name: default
    protection:
      restrictions:
        apps: []
        users: []
        teams: 
          - developers
          - marketing

2. Remove one user or team and push to admin repo
3. The app fails to apply the change

Test scenario B:

1. Create a branch protection rule with one user or team in the push restrictions. Example:

branches:
  - name: default
    protection:
      restrictions:
        apps: []
        users: []
        teams: 
          - developers

2. Manually add another user or team to the restriction using the GitHub UI
3. The app fails to apply the change (remove the manually added user/team) when trying to enforce the policy stated in the yaml

What is actually happening

"Error applying branch protection" in the log, with a 422 response with the following error from GitHub.

What is the expected behavior

"Branch protection applied successfully" in the log

Error output, if available

"data":{"message":"Invalid request.\n\nNo subschema in \"anyOf\" matched.\nFor 'items', {} is not a string.\nNot all subschemas of \"allOf\" matched.\nFor 'anyOf/1', {\"apps\"=>[], \"users\"=>[], \"teams\"=>[\"developers\", {}]} is not a null."

Context

Are you using the hosted instance of probot/settings or running your own?

My own

If running your own instance, are you using it with github.com or GitHub Enterprise?

github.com

Version of probot/settings

5.0.20-alpha.4

Version of GitHub Enterprise

N/A

Source

TL;DR;

1. compareDeep mutates its inputs
2. compareDeep produce unexpected results for lists

The reason for this bug is that mergeDeep.compareDeep mutates the given input that later on is sent to the GitHub API.
In my example, the function adds an empty object to the array branch.protection.restrictions.teams on this line:

safe-settings/lib/plugins/branches.js

Line 50 in 15518fe

const changes = mergeDeep.compareDeep(this.reformatAndReturnBranchProtection(result.data), branch.protection)

When I set a breakpoint on that line, the branch.protection.restrictions.teams array is ["developers"], but after the call the it has been changed to ["developers", {}].

The return value from compareDeep also contains unexpected data.
When the yaml only has the team "developers" and current repo settings has one or more extra users/teams, the following result is generated:

DEBUG (event): Result of compareDeep = {
  "additions": {},
  "modifications": {
    "restrictions": {
      "teams": [
        "developers",
        {
          "0": "d",
          "1": "e",
          "2": "v",
          "3": "e",
          "4": "l",
          "5": "o",
          "6": "p",
          "7": "e",
          "8": "r",
          "9": "s",
          "name": "developers"
        }
      ]
    }
  },
  "hasChanges": true
}

If there are two teams in the yaml, and one or more extra in the rule on GitHub, the invalid payload will be teams:["developers", "marketing", {}].

Also, the result of compareDeep is even more strange when having two teams in the yaml:

DEBUG (event): Result of compareDeep = {
  "additions": {},
  "modifications": {
    "restrictions": {
      "teams": [
        "developers",
        "marketing",
        {
          "0": "d",
          "1": "e",
          "2": "v",
          "3": "e",
          "4": "l",
          "5": "o",
          "6": "p",
          "7": "e",
          "8": "r",
          "9": "s",
          "name": "developers"
        },
        {
          "0": "d",
          "1": "e",
          "2": "v",
          "3": "e",
          "4": "l",
          "5": "o",
          "6": "p",
          "7": "e",
          "8": "r",
          "9": "s",
          "name": "developers"
        },
        {
          "0": "m",
          "1": "a",
          "2": "r",
          "3": "k",
          "4": "e",
          "5": "t",
          "6": "i",
          "7": "n",
          "8": "g",
          "name": "marketing"
        }
      ]
    }
  },
  "hasChanges": true
}

If you have three teams, the first and second teams will be repeated once more for that iteration. This is obviously caused by the recursive call to compareDeep.

[anderssonjohan]

Seems to originate from 0dac1a6 related to #276. The source object ref is used as list of modifications and later on mutated.