palantir / policy-bot

A GitHub App that enforces approval policies on pull requests
Apache License 2.0
780 stars 108 forks source link

Don't look at check run results in other repositories #807

Closed iainlane closed 4 months ago

iainlane commented 4 months ago

We are notified about check run completions via the check_run webhook event. This event contains a pull_requests field, which is a list of PRs that contain the SHA which just got checked:

      "pull_requests": [
        {
          "url": "https://api.github.com/repos/myorg/myrepo/pulls/1337",
          "id": 333333333,
          "number": 1337,
          "head": {
            "ref": "some-branch",
            "sha": "abcdef0123456789abcdef0123456789abcdef01",
            "repo": {
              "id": 11111111,
              "url": "https://api.github.com/repos/myorg/myrepo",
              "name": "myrepo"
            }
          },
          "base": {
            "ref": "main",
            "sha": "0101010101010101010101010101010101010101",
            "repo": {
              "id": 111111111,
              "url": "https://api.github.com/repos/myorg/myrepo",
              "name": "myrepo"
            }
          }
        }
      ],

However (mainly but not exclusively), in the case of public repositories, this list can contain PRs in other repositories:

    "pull_requests": [
      {
        "url": "https://api.github.com/repos/forkowner/myrepo/pulls/3",
        "id": 1234567890,
        "number": 3,
        "head": {
          "ref": "main",
          "sha": "6666666666666666666666666666666666666666",
          "repo": {
            "id": 111111111,
            "url": "https://api.github.com/repos/myorg/myrepo",
            "name": "myrepo"
          }
        },
        "base": {
          "ref": "main",
          "sha": "8888888888888888888888888888888888888888",
          "repo": {
            "id": 555555555,
            "url": "https://api.github.com/repos/forkowner/myrepo",
            "name": "myrepo"
          }
        }
      }
    ]

...this is because people can fork the repository and then create PRs from our repo into theirs. These PRs will contain SHAs from our repository and this causes them to appear in the pull_requests list of the check run, since they will share the same check run status in any repo: the check run is attached to the commit itself.

These PRs should be ignored for our purposes. We aren't evaluating a policy on the remote repo. When a check_run completes, we are only interested in PRs for our own repo.

iainlane commented 4 months ago

I was reviewing our logs when working on #794 earlier and noticed a lot of instances of this message

failed to create evaluation context: failed to load pull request details: Could not resolve to a PullRequest with the number of <implausibly low number>

this turned out to be the reason!