On PRs from external users, dismiss stale reviews when a new commit is pushed. (#572)

jonsimantov · web-flow · commit 8f6bb51314f2 · 2021-07-23T12:06:05.000-07:00
With our new project settings, when a PR is approved, that approval will stay valid even if further commits are pushed into that PR's branch.

For external users, though, we want to invalidate those approvals if any additional commits are made subsequent to the original approval. In this case, we consider anyone without "admin" access to be an "external user", and any PR from a forked repo to be an "external PR". In either of those scenarios, stale approvals will be dismissed.

This PR adds a workflow that runs on pull_request_target, which runs inside the `main` branch and has enough access to dismiss reviews. It also adds a script that allows you to dismiss reviews on a PR.
diff --git a/.github/workflows/checks_secure.yml b/.github/workflows/checks_secure.yml
@@ -0,0 +1,38 @@
+name: Checks (secure)
+# These are run on base branch with read/write access.
+
+on:
+  pull_request_target:
+    types: [synchronize]
+
+jobs:
+  dismiss_stale_approvals:
+    # Dismiss stale approvals for non-admins or if this PR comes from a fork.
+    runs-on: ubuntu-latest
+    # Only if another commit was added to the PR.
+    steps:
+      - name: Check user permission
+        id: check
+        uses: scherermichael-oss/action-has-permission@1.0.6
+        # This action sets outputs.has-permission to '1' or ''
+        with:
+          required-permission: admin
+        env:
+          GITHUB_TOKEN: ${{ github.token }}
+      - uses: actions/checkout@v2
+        if: steps.check.outputs.has-permission != 1 || github.event.pull_request.head.repo.full_name != github.repository
+        with:
+          submodules: false
+      - name: Setup python
+        if: steps.check.outputs.has-permission != 1 || github.event.pull_request.head.repo.full_name != github.repository
+        uses: actions/setup-python@v2
+        with:
+          python-version: 3.7
+      - name: Install prerequisites
+        if: steps.check.outputs.has-permission != 1 || github.event.pull_request.head.repo.full_name != github.repository
+        run: pip install -r scripts/gha/requirements.txt
+      - name: Dismiss reviews
+        if: steps.check.outputs.has-permission != 1 || github.event.pull_request.head.repo.full_name != github.repository
+        shell: bash
+        run: |
+          python scripts/gha/dismiss_reviews.py --token ${{github.token}} --pull_number ${{github.event.pull_request.number}} --review_state=APPROVED --message "🍞 Dismissed stale approval on external PR."
diff --git a/scripts/gha/dismiss_reviews.py b/scripts/gha/dismiss_reviews.py
@@ -0,0 +1,85 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""A utility to dismiss PR reviews.
+
+USAGE:
+  python scripts/gha/dismiss_reviews.py \
+    --token ${{github.token}} \
+    --pull_number ${{needs.check_trigger.outputs.pr_number}} \
+    [--message 'Message to be posted on the dismissal.'] \
+    [--reviewer github_username] \
+    [--review_state ANY|APPROVED|CHANGES_REQUESTED|COMMENTED|PENDING]
+"""
+
+import datetime
+import shutil
+
+from absl import app
+from absl import flags
+from absl import logging
+
+import github
+
+FLAGS = flags.FLAGS
+_DEFAULT_MESSAGE = "Dismissing stale review."
+
+flags.DEFINE_string(
+    "token", None,
+    "github.token: A token to authenticate on your repository.")
+
+flags.DEFINE_string(
+    "pull_number", None,
+    "Github's pull request #.")
+
+flags.DEFINE_string(
+    "message", _DEFAULT_MESSAGE,
+    "Message to post on dismissed reviews")
+
+flags.DEFINE_string(
+    "reviewer", None,
+    "Reviewer to dismiss (by username). If unspecified, dismiss all reviews.")
+
+flags.DEFINE_enum(
+    "review_state", "ANY", ["ANY", "APPROVED", "CHANGES_REQUESTED", "COMMENTED",
+                            "PENDING"],
+    "Only dismiss reviews in this state. Specify ANY for any state.")
+
+def main(argv):
+  if len(argv) > 1:
+    raise app.UsageError("Too many command-line arguments.")
+  # Get list of reviews from PR.
+  reviews = github.get_reviews(FLAGS.token, FLAGS.pull_number)
+  logging.debug("Found %d reviews", len(reviews))
+
+  # Filter out already-dismissed reviews.
+  reviews = [r for r in reviews if r['state'] != 'DISMISSED']
+  # Filter by review_state if specified.
+  if FLAGS.review_state != 'ANY':
+    reviews = [r for r in reviews if r['state'] == FLAGS.review_state]
+  # Filter by reviewer's username, if specified.
+  if FLAGS.reviewer:
+    reviews = [r for r in reviews if r['user']['login'] == FLAGS.reviewer]
+
+  if reviews:
+    review_ids = [r['id'] for r in reviews]
+    logging.debug("Dismissing reviews: %s", review_ids)
+    for review_id in review_ids:
+      github.dismiss_review(FLAGS.token, FLAGS.pull_number,
+                            review_id, FLAGS.message)
+
+if __name__ == "__main__":
+  flags.mark_flag_as_required("token")
+  flags.mark_flag_as_required("pull_number")
+  app.run(main)
diff --git a/scripts/gha/github.py b/scripts/gha/github.py
@@ -38,8 +38,8 @@
 FIREBASE_URL = '%s/repos/%s/%s' % (BASE_URL, OWNER, REPO)
 logging.set_verbosity(logging.INFO)
 
-def requests_retry_session(retries=RETRIES, 
-                           backoff_factor=BACKOFF, 
+def requests_retry_session(retries=RETRIES,
+                           backoff_factor=BACKOFF,
                            status_forcelist=RETRY_STATUS):
     session = requests.Session()
     retry = Retry(total=retries,
@@ -84,7 +84,7 @@ def update_issue_comment(token, issue_number, comment):
 
 def search_issues_by_label(label):
   """https://docs.github.com/en/rest/reference/search#search-issues-and-pull-requests"""
-  url = f'{BASE_URL}/search/issues?q=repo:{OWNER}/{REPO}+label:"{label}"+is:issue' 
+  url = f'{BASE_URL}/search/issues?q=repo:{OWNER}/{REPO}+label:"{label}"+is:issue'
   headers = {'Accept': 'application/vnd.github.v3+json'}
   with requests_retry_session().get(url, headers=headers, timeout=TIMEOUT) as response:
     logging.info("search_issues_by_label: %s response: %s", url, response)
@@ -93,7 +93,7 @@ def search_issues_by_label(label):
 
 def list_comments(token, issue_number):
   """https://docs.github.com/en/rest/reference/issues#list-issue-comments"""
-  url = f'{FIREBASE_URL}/issues/{issue_number}/comments' 
+  url = f'{FIREBASE_URL}/issues/{issue_number}/comments'
   headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
   with requests_retry_session().get(url, headers=headers, timeout=TIMEOUT) as response:
     logging.info("list_comments: %s response: %s", url, response)
@@ -102,7 +102,7 @@ def list_comments(token, issue_number):
 
 def add_comment(token, issue_number, comment):
   """https://docs.github.com/en/rest/reference/issues#create-an-issue-comment"""
-  url = f'{FIREBASE_URL}/issues/{issue_number}/comments' 
+  url = f'{FIREBASE_URL}/issues/{issue_number}/comments'
   headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
   data = {'body': comment}
   with requests.post(url, headers=headers, data=json.dumps(data), timeout=TIMEOUT) as response:
@@ -111,7 +111,7 @@ def add_comment(token, issue_number, comment):
 
 def update_comment(token, comment_id, comment):
   """https://docs.github.com/en/rest/reference/issues#update-an-issue-comment"""
-  url = f'{FIREBASE_URL}/issues/comments/{comment_id}' 
+  url = f'{FIREBASE_URL}/issues/comments/{comment_id}'
   headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
   data = {'body': comment}
   with requests_retry_session().patch(url, headers=headers, data=json.dumps(data), timeout=TIMEOUT) as response:
@@ -120,15 +120,15 @@ def update_comment(token, comment_id, comment):
 
 def delete_comment(token, comment_id):
   """https://docs.github.com/en/rest/reference/issues#delete-an-issue-comment"""
-  url = f'{FIREBASE_URL}/issues/comments/{comment_id}' 
+  url = f'{FIREBASE_URL}/issues/comments/{comment_id}'
   headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
   with requests.delete(url, headers=headers, timeout=TIMEOUT) as response:
     logging.info("delete_comment: %s response: %s", url, response)
 
 
 def add_label(token, issue_number, label):
   """https://docs.github.com/en/rest/reference/issues#add-labels-to-an-issue"""
-  url = f'{FIREBASE_URL}/issues/{issue_number}/labels' 
+  url = f'{FIREBASE_URL}/issues/{issue_number}/labels'
   headers={}
   headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
   data = [label]
@@ -138,15 +138,15 @@ def add_label(token, issue_number, label):
 
 def delete_label(token, issue_number, label):
   """https://docs.github.com/en/rest/reference/issues#delete-a-label"""
-  url = f'{FIREBASE_URL}/issues/{issue_number}/labels/{label}' 
+  url = f'{FIREBASE_URL}/issues/{issue_number}/labels/{label}'
   headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
   with requests.delete(url, headers=headers, timeout=TIMEOUT) as response:
     logging.info("delete_label: %s response: %s", url, response)
 
 
 def list_artifacts(token, run_id):
   """https://docs.github.com/en/rest/reference/actions#list-workflow-run-artifacts"""
-  url = f'{FIREBASE_URL}/actions/runs/{run_id}/artifacts' 
+  url = f'{FIREBASE_URL}/actions/runs/{run_id}/artifacts'
   headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
   with requests_retry_session().get(url, headers=headers, timeout=TIMEOUT) as response:
     logging.info("list_artifacts: %s response: %s", url, response)
@@ -155,9 +155,40 @@ def list_artifacts(token, run_id):
 
 def download_artifact(token, artifact_id, output_path):
   """https://docs.github.com/en/rest/reference/actions#download-an-artifact"""
-  url = f'{FIREBASE_URL}/actions/artifacts/{artifact_id}/zip' 
+  url = f'{FIREBASE_URL}/actions/artifacts/{artifact_id}/zip'
   headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
   with requests.get(url, headers=headers, stream=True, timeout=TIMEOUT) as response:
     logging.info("download_artifact: %s response: %s", url, response)
     with open(output_path, 'wb') as file:
         shutil.copyfileobj(response.raw, file)
+
+
+def dismiss_review(token, pull_number, review_id, message):
+  """https://docs.github.com/en/rest/reference/pulls#dismiss-a-review-for-a-pull-request"""
+  url = f'{FIREBASE_URL}/pulls/{pull_number}/reviews/{review_id}/dismissals'
+  headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
+  data = {'message': message}
+  with requests.put(url, headers=headers, data=json.dumps(data),
+                    stream=True, timeout=TIMEOUT) as response:
+    logging.info("dismiss_review: %s response: %s", url, response)
+    return response.json()
+
+def get_reviews(token, pull_number):
+  """https://docs.github.com/en/rest/reference/pulls#list-reviews-for-a-pull-request"""
+  url = f'{FIREBASE_URL}/pulls/{pull_number}/reviews'
+  headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
+  page = 1
+  per_page = 100
+  results = []
+  keep_going = True
+  while keep_going:
+    params = {'per_page': per_page, 'page': page}
+    page = page + 1
+    keep_going = False
+    with requests.get(url, headers=headers, params=params,
+                      stream=True, timeout=TIMEOUT) as response:
+      logging.info("get_reviews: %s response: %s", url, response)
+      results = results + response.json()
+      # If exactly per_page results were retrieved, read the next page.
+      keep_going = (len(response.json()) == per_page)
+  return results
