Fix size comparison workflow on fork PRs (#1214)

This PR moves the size comparison action back to a separate workflow
which now uses the `pull_request_target` event. This event is triggered
at all the same times as the `pull_request` action, except that the
workflow file content comes from the target branch of the PR, and it is
run in the context of the repo owning the target branch. Practically,
this means that it will still have access to post a comment even if the
PR comes from a fork.

We don’t want the build actions to be run in a `pull_request_target`
workflow because they would get access to the secrets and be able to
perform arbitrary actions on the repository, even from fork PRs.

See the current version failing here:
https://github.com/actualbudget/actual/actions/runs/5395184895/jobs/9797388016?pr=1122

.github/workflows/build.yml

@@ -64,58 +64,3 @@ jobs:
         with:
           name: build-stats
           path: packages/desktop-client/build-stats
-
-  size-compare:
-    runs-on: ubuntu-latest
-    needs: [web]
-    if: github.event_name == 'pull_request'
-    permissions:
-      pull-requests: write
-    steps:
-      - name: Wait for ${{github.base_ref}} build to succeed
-        uses: fountainhead/action-wait-for-check@v1.1.0
-        id: master-build
-        with:
-          token: ${{ secrets.GITHUB_TOKEN }}
-          checkName: web
-          ref: ${{github.base_ref}}
-
-      - name: Report build failure
-        if: steps.master-build.outputs.conclusion == 'failure'
-        run: |
-          echo "Build failed on ${{github.base_ref}}"
-          exit 1
-
-      - name: Download build artifact from ${{github.base_ref}}
-        uses: dawidd6/action-download-artifact@v2
-        id: pr-build
-        with:
-          branch: ${{github.base_ref}}
-          workflow: build.yml
-          name: build-stats
-          path: base
-
-      - name: Download build artifact from PR
-        uses: actions/download-artifact@v2
-        with:
-          name: build-stats
-          path: head
-
-      - name: Strip content hashes from stats files
-        run: |
-          sed -i -E 's/\.[0-9a-f]{8,}\././g' ./head/*.json
-          sed -i -E 's/\.[0-9a-f]{8,}\././g' ./base/*.json
-
-      - uses: github/webpack-bundlesize-compare-action@v1.8.1
-        with:
-          github-token: ${{ secrets.GITHUB_TOKEN }}
-          current-stats-json-path: ./head/desktop-client-stats.json
-          base-stats-json-path: ./base/desktop-client-stats.json
-          title: desktop-client
-
-      - uses: github/webpack-bundlesize-compare-action@v1.8.1
-        with:
-          github-token: ${{ secrets.GITHUB_TOKEN }}
-          current-stats-json-path: ./head/loot-core-stats.json
-          base-stats-json-path: ./base/loot-core-stats.json
-          title: loot-core

.github/workflows/size-compare.yml

+name: Compare Sizes
+
+##########################################################################################
+# WARNING! This workflow uses the 'pull_request_target' event. That mans that it will    #
+# always run in the context of the main actualbudget/actual repo, even if the PR is from #
+# a fork. This is necessary to get access to a GitHub token that can post a comment on   #
+# the PR. Be VERY CAREFUL about adding things to this workflow, since forks can inject   #
+# arbitrary code into their branch, and can pollute the artifacts we download. Arbitrary #
+# code execution in this workflow could lead to a compromise of the main repo.           #
+##########################################################################################
+# See: https://securitylab.github.com/research/github-actions-preventing-pwn-requests    #
+##########################################################################################
+
+on:
+  pull_request_target:
+
+jobs:
+  compare:
+    runs-on: ubuntu-latest
+    permissions:
+      pull-requests: write
+    steps:
+      - name: Wait for ${{github.base_ref}} build to succeed
+        uses: fountainhead/action-wait-for-check@v1.1.0
+        id: master-build
+        with:
+          token: ${{ secrets.GITHUB_TOKEN }}
+          checkName: web
+          ref: ${{github.base_ref}}
+
+      - name: Wait for PR build to succeed
+        uses: fountainhead/action-wait-for-check@v1.1.0
+        id: wait-for-build
+        with:
+          token: ${{ secrets.GITHUB_TOKEN }}
+          checkName: web
+          ref: ${{github.event.pull_request.head.sha}}
+
+      - name: Report build failure
+        if: steps.wait-for-build.outputs.conclusion == 'failure'
+        run: |
+          echo "Build failed on PR branch or ${{github.base_ref}}"
+          exit 1
+      - name: Download build artifact from ${{github.base_ref}}
+        uses: dawidd6/action-download-artifact@v2
+        id: pr-build
+        with:
+          branch: ${{github.base_ref}}
+          workflow: build.yml
+          name: build-stats
+          path: base
+
+      - name: Download build artifact from PR
+        uses: dawidd6/action-download-artifact@v2
+        with:
+          pr: ${{github.event.pull_request.number}}
+          workflow: build.yml
+          name: build-stats
+          path: head
+
+      - name: Strip content hashes from stats files
+        run: |
+          sed -i -E 's/\.[0-9a-f]{8,}\././g' ./head/*.json
+          sed -i -E 's/\.[0-9a-f]{8,}\././g' ./base/*.json
+      - uses: github/webpack-bundlesize-compare-action@v1.8.1
+        with:
+          github-token: ${{ secrets.GITHUB_TOKEN }}
+          current-stats-json-path: ./head/desktop-client-stats.json
+          base-stats-json-path: ./base/desktop-client-stats.json
+          title: desktop-client
+
+      - uses: github/webpack-bundlesize-compare-action@v1.8.1
+        with:
+          github-token: ${{ secrets.GITHUB_TOKEN }}
+          current-stats-json-path: ./head/loot-core-stats.json
+          base-stats-json-path: ./base/loot-core-stats.json
+          title: loot-core

upcoming-release-notes/1214.md

+---
+category: Maintenance
+authors: [j-f1]
+---
+
+Fix the bundle size comparison workflow on fork PRs