From 7ff6b93eb5e3e5f672fdab2b7390dc4dd01946bc Mon Sep 17 00:00:00 2001
From: Christopher Allen <cpcallen+git@google.com>
Date: Tue, 2 Nov 2021 15:55:02 +0000
Subject: [PATCH] chore: Fix permissions for Assign requested reviewers
 workflow (#5666)

* refactor: Inline assign_reviewers script to avoid checkout

Per https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
it is not safe to do a checkout of the submitter-supplied code
AND THEN RUN IT (via require).  This is pretty bad.

We want to give this script more permissions by running it
`on: [pull_request_target]` (instead of `pull_request`); this would
give it permission to modify the PR (e.g. add comments, change
assignment).  While it would be OK to do a checkout with default
parameters (which in `pull_request_target` would check out *our*
branch rather than the submitted one) it simplest just to inline
this small script and thereby obviate the need to do a checkout at all.

* chore: Give assign_reviewers action required permissions

Changing it from `on: [pull_request]` to `on: [pull_request_review]`
will give the action write access to our repository, allowing it to
change the assignment of the PR.

This is now safe as the script does not ever check out any
submitter-supplied code.

* docs: Comment tweaks for assign_reviewers.yml
---
 .github/workflows/assign_reviewers.yml        | 34 +++++++++++++++----
 .github/workflows/scripts/assign_reviewers.js | 27 ---------------
 2 files changed, 28 insertions(+), 33 deletions(-)
 delete mode 100644 .github/workflows/scripts/assign_reviewers.js

diff --git a/.github/workflows/assign_reviewers.yml b/.github/workflows/assign_reviewers.yml
index d4ae7daa6..2395a085d 100644
--- a/.github/workflows/assign_reviewers.yml
+++ b/.github/workflows/assign_reviewers.yml
@@ -1,19 +1,41 @@
 name: Assign requested reviewers
 
+# This workflow adds requested reviewers as assignees. If you remove a
+# requested reviewer, it will not remove them as an assignee.
+#
+# See https://github.com/google/blockly/issues/5643 for more
+# information on why this was added.
+#
+# N.B.: Runs with a read-write repo token.  Do not check out the
+# submitted branch!
 on:
-  pull_request:
+  pull_request_target:
     types: [review_requested]
 
 jobs:
   requested-reviewer:
     runs-on: ubuntu-latest
     steps:
-    - name: Check Out Blockly
-      uses: actions/checkout@v2
-
     - name: Assign requested reviewer
       uses: actions/github-script@v5
       with:
         script: |
-          const {assign} = require('./.github/workflows/scripts/assign_reviewers.js')
-          assign(github, context, core)
+          try {
+            if (context.payload.pull_request === undefined) {
+            throw new Error("Can't get pull_request payload. " +
+                            'Check a request reviewer event was triggered.');
+            }
+            const reviewers = context.payload.pull_request.requested_reviewers;
+            // Assignees takes in a list of logins rather than the
+            // reviewer object.
+            const reviewerNames = reviewers.map(reviewer => reviewer.login);
+            const {number:issue_number} = context.payload.pull_request;
+            github.rest.issues.addAssignees({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              issue_number: issue_number,
+              assignees: reviewerNames
+            });
+          } catch (error) {
+            core.setFailed(error.message);
+          }
diff --git a/.github/workflows/scripts/assign_reviewers.js b/.github/workflows/scripts/assign_reviewers.js
deleted file mode 100644
index be9980660..000000000
--- a/.github/workflows/scripts/assign_reviewers.js
+++ /dev/null
@@ -1,27 +0,0 @@
-/**
- * This script adds requested reviewers as assignees. If you remove a requested
- * reviewer, it will not remove them as an assignee.
- *
- * See https://github.com/google/blockly/issues/5643 for more information on
- * why this was added.
- */
-exports.assign = function(github, context, core) {
-  try {
-    if (context.payload.pull_request === undefined) {
-      throw new Error("Can't get pull_request payload. Check a request reviewer event was triggered.");
-    }
-    const reviewers = context.payload.pull_request.requested_reviewers;
-    // Assignees takes in a list of logins rather than the reviewer object.
-    const reviewerNames = reviewers.map(reviewer => reviewer.login);
-    const {number:issue_number} = context.payload.pull_request;
-
-    github.rest.issues.addAssignees({
-      owner: context.repo.owner,
-      repo: context.repo.repo,
-      issue_number: issue_number,
-      assignees: reviewerNames
-    });
-  } catch (error) {
-    core.setFailed(error.message);
-  }
-}