Add IgnoreUpdateMerges option (#39)

When this option is true, a commit on the PR branch is ignored if it
meets these conditions:

  1. Has two parents (i.e. is a simple merge commit)
  2. Was created via the UI or the API
  3. Has one parent in the last 100 commits on the target branch

An ignored commit does not invalidate approvals and the author/committer
do not count as contributors. Also refactor the approval test code to
avoid destructively modifying the default pull.Context data.

README.md

@@ -29,6 +29,7 @@ UI to view the detailed approval status of any pull request.
     - [Disapproval is Disabled by Default](#disapproval-is-disabled-by-default)
     - [`or`, `and`, and `if` (Rule Predicates)](#or-and-and-if-rule-predicates)
     - [Cross-organization Membership Tests](#cross-organization-membership-tests)
+    - [Update Merges](#update-merges)
 * [Deployment](#deployment)
 * [Development](#development)
 * [Contributing](#contributing)
@@ -163,6 +164,13 @@ options:
   # approvals for this rule. False by default.
   invalidate_on_push: false
 
+  # If true, "update merges" do not invalidate approval (if invalidate_on_push
+  # is enabled) and their authors/committers do not count as contributors. An
+  # "update merge" is a merge commit that was created in the UI or via the API
+  # and merges the target branch into the pull request branch. These are
+  # commonly created by using the "Update branch" button in the UI.
+  ignore_update_merges: false
+
   # "methods" defines how users may express approval. The defaults are below.
   methods:
     comments:
@@ -294,6 +302,24 @@ Effectively, skipped rules are treated as if they don't exist.
 not in the organization that owns the repository where the rules appear. In
 this case, `policy-bot` must be installed on all referenced organizations.
 
+#### Update Merges
+
+For a commit on a branch to count as an "update merge" for the purpose of the
+`ignore_update_merges` option, the following must be true:
+
+1. The commit must have exactly two parents
+2. The commit must have the `committedViaWeb` property set to `true`
+3. One parent must exist in the last 100 commits on the target branch of the
+   pull request
+
+These will all be true after updating a branch using the UI, but historic
+merges on long-running branches or merges created with the API may not be
+ignored. If this happens, you will need to reapprove the pull request.
+
+Note that `policy-bot` cannot detect if an update merge contains any merge
+conflict resolutions. If you enable this option, users _may_ be able to merge
+unapproved code by exploiting the conflict editor.
+
 ## Deployment
 
 `policy-bot` is easy to deploy in your own environment as it has no dependencies

policy/approval/approve.go

@@ -45,9 +45,10 @@ type Rule struct {
 }
 
 type Options struct {
-	AllowAuthor      bool `yaml:"allow_author"`
-	AllowContributor bool `yaml:"allow_contributor"`
-	InvalidateOnPush bool `yaml:"invalidate_on_push"`
+	AllowAuthor        bool `yaml:"allow_author"`
+	AllowContributor   bool `yaml:"allow_contributor"`
+	InvalidateOnPush   bool `yaml:"invalidate_on_push"`
+	IgnoreUpdateMerges bool `yaml:"ignore_update_merges"`
 
 	Methods *common.Methods `yaml:"methods"`
 }
@@ -123,9 +124,9 @@ func (r *Rule) IsApproved(ctx context.Context, prctx pull.Context) (bool, string
 	sort.Stable(common.CandidatesByCreationTime(candidates))
 
 	if r.Options.InvalidateOnPush {
-		commits, err := prctx.Commits()
+		commits, err := r.filteredCommits(prctx)
 		if err != nil {
-			return false, "", errors.Wrap(err, "failed to get commits")
+			return false, "", err
 		}
 
 		lastCommitTime := commits[len(commits)-1].CreatedAt
@@ -157,7 +158,7 @@ func (r *Rule) IsApproved(ctx context.Context, prctx pull.Context) (bool, string
 
 	// "contributor" is any user who added a commit to the PR
 	if !r.Options.AllowContributor {
-		commits, err := prctx.Commits()
+		commits, err := r.filteredCommits(prctx)
 		if err != nil {
 			return false, "", err
 		}
@@ -211,10 +212,61 @@ func (r *Rule) IsApproved(ctx context.Context, prctx pull.Context) (bool, string
 	return false, msg, nil
 }
 
+func (r *Rule) filteredCommits(prctx pull.Context) ([]*pull.Commit, error) {
+	commits, err := prctx.Commits()
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to list commits")
+	}
+
+	needsFiltering := r.Options.IgnoreUpdateMerges
+	if !needsFiltering {
+		return commits, nil
+	}
+
+	var filtered []*pull.Commit
+	for _, c := range commits {
+		isUpdate, err := isUpdateMerge(prctx, c)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to detemine update merge status")
+		}
+
+		switch {
+		case isUpdate:
+		default:
+			filtered = append(filtered, c)
+		}
+	}
+	return filtered, nil
+}
+
+func isUpdateMerge(prctx pull.Context, c *pull.Commit) (bool, error) {
+	// must be a simple merge commit (exactly 2 parents)
+	if len(c.Parents) != 2 {
+		return false, nil
+	}
+
+	// must be created via the UI or the API (no local merges)
+	if !c.CommittedViaWeb {
+		return false, nil
+	}
+
+	// one parent must exist in recent history on the target branch
+	targets, err := prctx.TargetCommits()
+	if err != nil {
+		return false, err
+	}
+	for _, target := range targets {
+		if c.Parents[0] == target.SHA || c.Parents[1] == target.SHA {
+			return true, nil
+		}
+	}
+
+	return false, nil
+}
+
 func numberOfApprovals(count int) string {
 	if count == 1 {
 		return "1 approval"
 	}
-
 	return fmt.Sprintf("%d approvals", count)
 }