From 2108712f2dda986cc48e0fa37558b877905a7879 Mon Sep 17 00:00:00 2001
From: Jed Fox <git@jedfox.com>
Date: Wed, 28 Jun 2023 12:04:15 -0400
Subject: [PATCH] Fix size comparison workflow on fork PRs (#1214)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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 | 55 ---------------------
.github/workflows/size-compare.yml | 77 ++++++++++++++++++++++++++++++
upcoming-release-notes/1214.md | 6 +++
3 files changed, 83 insertions(+), 55 deletions(-)
create mode 100644 .github/workflows/size-compare.yml
create mode 100644 upcoming-release-notes/1214.md
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 61d4c46aa..3acb45246 100644
--- a/.github/workflows/build.yml
+++ b/.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
diff --git a/.github/workflows/size-compare.yml b/.github/workflows/size-compare.yml
new file mode 100644
index 000000000..a21c7ba00
--- /dev/null
+++ b/.github/workflows/size-compare.yml
@@ -0,0 +1,77 @@
+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
diff --git a/upcoming-release-notes/1214.md b/upcoming-release-notes/1214.md
new file mode 100644
index 000000000..9955584ac
--- /dev/null
+++ b/upcoming-release-notes/1214.md
@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [j-f1]
+---
+
+Fix the bundle size comparison workflow on fork PRs
--
GitLab