workflows: Unsplit new-prs

[tstellar]

This is essentially a revert of 91fdb20. It is safe to use the pull_request_target event for new-prs, because it does not checkout any code from the pull request branch.

[llvmbot]

@llvm/pr-subscribers-github-workflow

Author: Tom Stellard (tstellar)

Changes

This is essentially a revert of 91fdb20. It is safe to use the pull_request_target event for new-prs, because it does not checkout any code from the pull request branch.

---

Full diff: https://github.com/llvm/llvm-project/pull/69560.diff

2 Files Affected:

• (modified) .github/workflows/new-prs.yml (+15-38)
• (removed) .github/workflows/pr-receive.yml (-34)

diff --git a/.github/workflows/new-prs.yml b/.github/workflows/new-prs.yml
index c1952ddab83f78b..052ae39654028fe 100644
--- a/.github/workflows/new-prs.yml
+++ b/.github/workflows/new-prs.yml
@@ -1,7 +1,14 @@
 name: "Labelling new pull requests"
 on:
-  workflow_run:
-    workflows: ["PR Receive"]
+  # It's safe to use pull_request_target here, because we aren't checking out
+  # code from the pull request branch.
+  # See https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
+  pull_request_target:
+    types:
+      - opened
+      - reopened
+      - ready_for_review
+      - synchronize
 
 jobs:
   automate-prs-labels:
@@ -9,48 +16,18 @@ jobs:
       contents: read
       pull-requests: write
     runs-on: ubuntu-latest
+    # Ignore PRs with more than 10 commits.  Pull requests with a lot of
+    # commits tend to be accidents usually when someone made a mistake while trying
+    # to rebase.  We want to ignore these pull requests to avoid excessive
+    # notifications.
     if: >
       github.repository == 'llvm/llvm-project' &&
-      github.event.workflow_run.event == 'pull_request_target' &&
-      github.event.workflow_run.conclusion == 'success'
+      github.event.pull_request.draft == false &&
+      github.event.pull_request.commits < 10
     steps:
-      # From: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
-      # Updated version here: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#using-data-from-the-triggering-workflow
-      - name: Debug
-        run: |
-          echo "Event: ${{ github.event.workflow_run.event }} Conclusion: ${{ github.event.workflow_run.conclusion }}"
-      - name: 'Download artifact'
-        uses: actions/github-script@v6
-        with:
-          script: |
-            const artifacts = await github.rest.actions.listWorkflowRunArtifacts({
-               owner: context.repo.owner,
-               repo: context.repo.repo,
-               run_id: context.payload.workflow_run.id
-            });
-            const matchArtifact = artifacts.data.artifacts.find((artifact) =>
-              artifact.name === 'pr'
-            );
-            const download = await github.rest.actions.downloadArtifact({
-               owner: context.repo.owner,
-               repo: context.repo.repo,
-               artifact_id: matchArtifact.id,
-               archive_format: 'zip'
-            });
-            const { writeFileSync } = require('node:fs');
-            writeFileSync('${{ github.workspace }}/pr.zip', Buffer.from(download.data));
-
-      - run: unzip pr.zip
-
-      - name: "Get PR Number"
-        id: vars
-        run:
-          echo "pr-number=$(cat NR)" >> "$GITHUB_OUTPUT"
-
       - uses: actions/labeler@v4
         with:
           configuration-path: .github/new-prs-labeler.yml
           # workaround for https://github.com/actions/labeler/issues/112
           sync-labels: ''
           repo-token: ${{ secrets.ISSUE_SUBSCRIBER_TOKEN }}
-          pr-number: ${{ steps.vars.outputs.pr-number }}
diff --git a/.github/workflows/pr-receive.yml b/.github/workflows/pr-receive.yml
deleted file mode 100644
index 13f1a883cf8ff67..000000000000000
--- a/.github/workflows/pr-receive.yml
+++ /dev/null
@@ -1,34 +0,0 @@
-# See https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
-
-name: PR Receive
-on:
-  pull_request_target:
-    types:
-      - opened
-      - reopened
-      - ready_for_review
-      - synchronize
-
-permissions:
-  contents: read
-
-jobs:
-  pr-target:
-    runs-on: ubuntu-latest
-    # Ignore PRs with more than 10 commits.  Pull requests with a lot of
-    # commits tend to be accidents usually when someone made a mistake while trying
-    # to rebase.  We want to ignore these pull requests to avoid excessive
-    # notifications.
-    if: github.repository == 'llvm/llvm-project' &&
-        github.event.pull_request.draft == false &&
-        github.event.pull_request.commits < 10
-    steps:
-      - name: Store PR Information
-        run: |
-          mkdir -p ./pr
-          echo ${{ github.event.number }} > ./pr/NR
-
-      - uses: actions/upload-artifact@v3
-        with:
-          name: pr
-          path: pr/

[boomanaiden154]

LGTM. This definitely makes things look quite a bit cleaner.

From what I understand, it's not that easy to misuse pull_request_target as you have to explicitly run the checkout action with a different ref.

Should probably be safe, but take that with a grain of salt as I am definitely not a CI/CD security expert.

[boomanaiden154]

Sorry, one more comment.

[boomanaiden154]

LGTM

[boomanaiden154]

@tstellar Any chance this can get merged soon? This looks to be eliminating quite a few Github API requests per PR, which would be really good to have right now given that we're hitting the 1000 requests/hr limit.

https://github.com/llvm/llvm-project/actions/runs/6723750547