Verify author of a PR status

[justinwyer]

Bulldozer can be configured using required_statuses to validate that a PR has the specified status set before merging, ostensibly this may be paired with policy-bot to provide assurance that a PR can be merged; however any collaborator to a repository can set a status on a PR, essentially impersonating policy-bot, and bulldozer will happily merge the PR.

I'd like to propose, and if agreed, implement an enhancement to required_statuses where it continues to behave as a list of statuses set by any user or a list of statuses set by a set of users.

# keeps working as is
merge:
  required_statuses:
    - "policy-bot: main"
 
# validates the user that set the status
merge:
  required_statuses:
    "policy-bot: main":
      - policy-bot[bot]

Policy-bot could be changed to use checks, which can only be set by GitHub Apps, in place of statuses; however this would be a significant change and has already being attempted and abandoned once before palantir/policy-bot#46.

[asvoboda]

While I think this is friendly, Github repo settings can also prevent users from merging PRs unless they match coming from a specific app. One alternative solution could be just to use those repo settings instead of encoding policy style options into bulldozer.

[justinwyer]

Thanks for the feedback @asvoboda, I guess that makes sense.

[dominykas]

👋 I'd like to see if this can be revisited, please...

Github repo settings can also prevent users from merging PRs

Yes, the GH repo settings can enforce that only Bulldozer can merge PRs - but in this use case we want to verify who has set the status check.

While it is also possible to enforce the status check via GH settings to only be set by an app, that also makes the status check mandatory, which might not be desirable, as some status checks might only be necessary for automerging, but not necessary for human merging (and we want to avoid the yellow dot for human use cases).

Example use case (this does require a separate feature in policybot to not set the status when all rules are skipped, but lets assume the feature exists, for the sake of the example - it may well be some other app):

• Policy: set policybot: only docs status check when PR contains changes only in the generated documentation folder
• Automerge when policybot: only docs status is green and it was set by the policybot app

In the above case, we do not want to turn policybot: only docs into a required check via GH repo setting. Since an Evil Person could just set that status check and trigger an automerge, we need to validate who created the check inside Bulldozer.