Skip to content

Commit 12d0672

Browse files
authored
fix(pr-test): do not use pull_request_target (#33003)
* fix(pr-test): do not use pull_request_target See https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
1 parent 4f7e16b commit 12d0672

File tree

2 files changed

+24
-25
lines changed

2 files changed

+24
-25
lines changed

.github/workflows/pr-review-companion.yml

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,31 +5,47 @@
55

66
name: PR review companion
77

8-
on: workflow_call
8+
on:
9+
workflow_run:
10+
workflows: ["PR Test"]
11+
types:
12+
- completed
913

1014
jobs:
1115
review:
1216
runs-on: ubuntu-latest
17+
if: ${{ github.event.workflow_run.conclusion == 'success' }}
1318
steps:
1419
- name: "Download artifact"
1520
uses: actions/download-artifact@v4
1621
with:
17-
name: build
22+
pattern: build
1823
path: build
24+
merge-multiple: true
25+
github-token: ${{ secrets.GITHUB_TOKEN }}
26+
run-id: ${{ github.event.workflow_run.id }}
27+
28+
- name: Check for artifacts
29+
if: ${{ hashFiles('build/') != '' }}
30+
run: |
31+
echo "HAS_ARTIFACT=true" >> "$GITHUB_ENV"
1932
2033
- uses: actions/checkout@v4
34+
if: ${{ env.HAS_ARTIFACT }}
2135
with:
2236
repository: mdn/yari
2337
path: yari
2438

2539
- name: Install Python
40+
if: ${{ env.HAS_ARTIFACT }}
2641
id: setup-python
2742
uses: actions/setup-python@v5
2843
with:
2944
python-version: "3.10"
3045

3146
# See https://www.peterbe.com/plog/install-python-poetry-github-actions-faster
3247
- name: Load cached ~/.local
48+
if: ${{ env.HAS_ARTIFACT }}
3349
uses: actions/cache@v4
3450
with:
3551
path: ~/.local
@@ -38,12 +54,14 @@ jobs:
3854
key: dotlocal-${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-0
3955

4056
- name: Install Python poetry
57+
if: ${{ env.HAS_ARTIFACT }}
4158
uses: snok/[email protected]
4259
with:
4360
virtualenvs-create: true
4461
virtualenvs-in-project: true
4562

4663
- name: Load cached venv
64+
if: ${{ env.HAS_ARTIFACT }}
4765
id: cached-poetry-dependencies
4866
uses: actions/cache@v4
4967
with:
@@ -53,17 +71,19 @@ jobs:
5371
key: venv-${{ runner.os }}-${{ hashFiles('**/poetry.lock') }}-${{ steps.setup-python.outputs.python-version }}-0
5472

5573
- name: Install poetry dependencies
56-
if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true'
74+
if: ${{ env.HAS_ARTIFACT && steps.cached-poetry-dependencies.outputs.cache-hit != 'true' }}
5775
run: |
5876
cd yari/deployer
5977
poetry install --no-interaction --no-root
6078
6179
- name: Install Deployer
80+
if: ${{ env.HAS_ARTIFACT }}
6281
run: |
6382
cd yari/deployer
6483
poetry install --no-interaction
6584
6685
- name: Deploy and analyze built content
86+
if: ${{ env.HAS_ARTIFACT }}
6787
env:
6888
BUILD_OUT_ROOT: ${{ github.workspace }}/build
6989

.github/workflows/pr-test.yml

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,7 @@
77
name: PR Test
88

99
on:
10-
# The `GITHUB_TOKEN` in workflows triggered by the `pull_request_target` event
11-
# is granted read/write repository access.
12-
# Please pay attention to limit the permissions of each job!
13-
# https://docs.github.com/actions/using-jobs/assigning-permissions-to-jobs
14-
pull_request_target:
10+
pull_request:
1511
branches:
1612
- main
1713

@@ -22,8 +18,6 @@ jobs:
2218
# Set the permissions to `read-all`, preventing the workflow from
2319
# any accidental write access to the repository.
2420
permissions: read-all
25-
outputs:
26-
has_assets: ${{ steps.build-content.outputs.has_assets }}
2721
env:
2822
BASE_SHA: ${{ github.event.pull_request.base.sha }}
2923
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
@@ -35,8 +29,6 @@ jobs:
3529

3630
steps:
3731
- uses: actions/checkout@v4
38-
with:
39-
ref: "${{ env.HEAD_SHA }}"
4032

4133
- name: Get changed files
4234
run: |
@@ -119,9 +111,6 @@ jobs:
119111
# be able to use this raw diff file for the benefit of analyzing.
120112
wget https://github.com/${{ github.repository }}/compare/${{ env.BASE_SHA }}...${{ env.HEAD_SHA }}.diff -O ${{ env.BUILD_OUT_ROOT }}/DIFF
121113
122-
# Set the output variable so the next job could skip if there are no assets
123-
echo "has_assets=true" >> "$GITHUB_OUTPUT"
124-
125114
- name: Merge static assets with built documents
126115
if: ${{ env.GIT_DIFF_CONTENT }}
127116
run: |
@@ -143,13 +132,3 @@ jobs:
143132
144133
export CONTENT_ROOT=$(pwd)/files
145134
yarn filecheck ${{ env.GIT_DIFF_FILES }}
146-
147-
review:
148-
needs: tests
149-
if: ${{ needs.tests.outputs.has_assets }}
150-
# write permissions are required to create a comment in the corresponding PR
151-
permissions: write-all
152-
uses: ./.github/workflows/pr-review-companion.yml
153-
# inherit the secrets from the parent workflow
154-
# https://docs.github.com/actions/using-workflows/reusing-workflows#using-inputs-and-secrets-in-a-reusable-workflow
155-
secrets: inherit

0 commit comments

Comments
 (0)