-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
ci: Refactor screenshot workflow (#7773)
This workflow, triggerable only by maintainers, had some potential security issues. This is a big refactor, and makes several changes: - Clean up description text (non-critical) - Add granular permissions to set status (without this, the workflow was broken since we changed default permissions) - Split the update-pr job into commit-new-screenshots (unprivileged) and update-pr (privileged as @shaka-bot) The commit-new-screenshots job runs code that the PR author controls, such as "npm ci" (controlled through package.json and package-lock.json), and "./build/updateScreenshots.py" (easily edited to do whatever). These steps could be used to do literally anything, including modify tools in /usr/bin on the workflow VM that are needed by the privileged steps. By moving the privileged steps into a completely separate job, we can ensure a clean slate without worrying about the VM's state. We only transfer the .git/ folder between the two jobs. So the commit-new-screenshots job will create the commit, and the update-pr job will actually push that commit from a clean VM. The job is once again functional, and for the first time, actually safe.
- Loading branch information
1 parent
3d742fe
commit de0f33c
Showing
1 changed file
with
79 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ on: | |
# avoid malicious code executing in the lab. | ||
inputs: | ||
pr: | ||
description: "A PR number to build and test in the lab. Required even if \"pr\" is given." | ||
description: "A PR number to build and test in the lab." | ||
required: true | ||
|
||
jobs: | ||
|
@@ -29,6 +29,11 @@ jobs: | |
name: Set Pending Status | ||
needs: compute-sha | ||
runs-on: ubuntu-latest | ||
|
||
permissions: | ||
# "Write" to statuses to update commit status | ||
statuses: write | ||
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
|
@@ -46,6 +51,11 @@ jobs: | |
name: Get Selenium Lab Screenshots | ||
needs: [set-pending-status] | ||
uses: ./.github/workflows/selenium-lab-tests.yaml | ||
|
||
permissions: | ||
# "Write" to statuses to update commit status, needed by nested jobs. | ||
statuses: write | ||
|
||
with: | ||
# Pass the pre-computed SHA directly to the nested workflow. | ||
# Do NOT pass "pr" and reinterpret it into a SHA in the nested workflow. | ||
|
@@ -54,14 +64,18 @@ jobs: | |
ignore_test_status: true | ||
job_name_prefix: "Get Selenium Lab Screenshots / " | ||
|
||
update-pr: | ||
name: Update PR | ||
commit-new-screenshots: | ||
name: Commit New Screenshots | ||
runs-on: ubuntu-latest | ||
needs: [compute-sha, run-lab-tests] | ||
# NOTE: NO PERMISSIONS ON THIS JOB. It runs PR-author-controlled code from | ||
# the PR, and so must be untrusted! | ||
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
ref: ${{ needs.compute-sha.outputs.SHA }} | ||
fetch-depth: 0 | ||
persist-credentials: false | ||
|
||
- name: Get artifacts | ||
|
@@ -73,30 +87,82 @@ jobs: | |
|
||
- name: Update screenshots | ||
run: | | ||
# NOTE: Steps of this could be influenced by the PR author, which is | ||
# why we run this job without any accessible tokens or special | ||
# permissions. | ||
# Install prerequisites. | ||
npm ci | ||
# Update the official screenshots for any that have visibly changed. | ||
# This is not a byte-for-byte comparison, but based on pixel diffs. | ||
./build/updateScreenshots.py | ||
# Emulate the actions bot. | ||
git config user.name "github-actions[bot]" | ||
git config user.email "41898282+github-actions[bot]@users.noreply.github.com" | ||
# Act as Shaka Bot. | ||
git config user.name "shaka-bot" | ||
git config user.email "shaka-[email protected]" | ||
# Commit the changes. Ignore failure, in case there are no changes. | ||
# Commit the changes to the screenshots only. Ignore failure, in | ||
# case there are no changes. | ||
git add test/test/assets/screenshots/*/*.png || true | ||
git commit -m ':robot: Update all screenshots' || true | ||
- name: Cache Commits | ||
# Here we cache commits, made above in an untrusted job, to pull into a | ||
# separate, trusted job with permission to push to the repo. The | ||
# untrusted job can't pollute the environment of the trusted job by, | ||
# say, modifying /usr/bin/gh. | ||
uses: actions/cache/save@v4 | ||
with: | ||
path: .git/ | ||
key: screenshot-commits-${{ needs.compute-sha.outputs.SHA }} | ||
|
||
- name: Debug | ||
uses: mxschmitt/[email protected] | ||
with: | ||
limit-access-to-actor: true | ||
if: failure() | ||
|
||
update-pr: | ||
name: Update PR | ||
runs-on: ubuntu-latest | ||
needs: [compute-sha, commit-new-screenshots] | ||
|
||
# NOTE: No granular permissions here, because we use SHAKA_BOT_TOKEN | ||
# instead of the default token. The action to push to the PR must be done | ||
# by an actor with permission, and the default GitHub token doesn't work. | ||
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
ref: ${{ needs.compute-sha.outputs.SHA }} | ||
fetch-depth: 0 | ||
persist-credentials: false | ||
|
||
- name: Restore Commits | ||
# Here we restore commits, made above in the above untrusted job, to | ||
# pull into this trusted job. See comments above on "Cache Commits". | ||
uses: actions/cache/restore@v4 | ||
with: | ||
path: .git/ | ||
key: screenshot-commits-${{ needs.compute-sha.outputs.SHA }} | ||
|
||
- name: Update PR | ||
env: | ||
GH_TOKEN: ${{ github.token }} | ||
GH_TOKEN: ${{ secrets.SHAKA_BOT_TOKEN }} | ||
run: | | ||
# Update the PR. | ||
# Compute the destination for the push. This uses the GitHub API | ||
# because this workflow is not triggered directly by a PR, so there | ||
# is no context variable that supplies these details. | ||
PR_API_URL="/repos/${{ github.repository }}/pulls/${{ inputs.pr }}" | ||
REMOTE=$(gh api $PR_API_URL | jq -r .head.repo.html_url) | ||
BRANCH=$(gh api $PR_API_URL | jq -r .head.ref) | ||
# Lean on $GH_TOKEN to authenticate the push. | ||
gh auth setup-git | ||
# If there were no changes, this will do nothing, but succeed. | ||
git push "$REMOTE" HEAD:"$BRANCH" | ||
|
@@ -109,6 +175,11 @@ jobs: | |
set-final-status: | ||
name: Set Final Status | ||
runs-on: ubuntu-latest | ||
|
||
permissions: | ||
# "Write" to statuses to update commit status | ||
statuses: write | ||
|
||
needs: [compute-sha, run-lab-tests, update-pr] | ||
# Will run on success or failure, but not if the workflow is cancelled. | ||
if: ${{ success() || failure() }} | ||
|