Eliminate AWS credential exposure on fork PRs in REPL artefact workflow#6376
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Eliminates AWS credential exposure to fork PRs by splitting the REPL artefact pipeline into an untrusted build workflow (pull_request, no secrets) and a trusted upload workflow (workflow_run) that authenticates to AWS via OIDC and posts the PR comment.
Changes:
- Convert
repl-artefacts.ymlfrompull_request_targettopull_request; it now only builds and uploads a GitHub Actions artifact, with no AWS access. - Add
repl-artefacts-upload.ymltriggered viaworkflow_run, which downloads the artefact, removes the build label, authenticates to AWS via OIDC (AWS_ROLE_ARN), uploads to S3, and posts/updates the PR comment. - Move label removal and Vercel preview commenting into the trusted upload workflow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/repl-artefacts.yml | Switched to untrusted pull_request build that only produces a GitHub Actions artifact; removed all AWS/secret usage and PR commenting. |
| .github/workflows/repl-artefacts-upload.yml | New trusted workflow_run job that downloads the artefact, OIDC-auths to AWS, uploads to S3, removes label, and posts the PR comment. |
Comments suppressed due to low confidence (2)
.github/workflows/repl-artefacts-upload.yml:102
patrickedqvist/wait-for-vercel-previewneeds the PR head commit SHA to find the matching Vercel deployment. In aworkflow_run-triggered job,github.shadoes not reliably correspond to the PR head SHA, and noshainput is being passed here. As a result, the action may time out or never find the preview URL, and the comment with the Vercel preview link may stop being posted.
Consider explicitly forwarding the originating PR head SHA via the action's sha input, e.g. using ${{ github.event.workflow_run.head_sha }}.
- name: Find Vercel preview URL
uses: patrickedqvist/wait-for-vercel-preview@d7982701e6fcd3ae073bff929e408e004404d38d # v1.3.3
id: waitForVercel
with:
token: ${{ secrets.GITHUB_TOKEN }}
.github/workflows/repl-artefacts-upload.yml:34
- Deriving the PR number from the artifact directory name with
ls | sedis fragile: ifactions/download-artifactever places additional files (e.g. a README, hidden file) or its naming convention changes, the capturedPR_NUMBERwill silently become incorrect — which could lead to commenting on the wrong PR and/or removing the label from the wrong PR.
A more robust approach is to have the build workflow write the PR number into the artifact itself (or upload it as a separate small artifact), and then read that file here, rather than parsing it from the directory name.
- name: Get PR number
id: pr-number
run: |
# The artifact is named repl-artefacts-pr-<number>
PR_NUMBER=$(ls repl-artefacts | sed 's/repl-artefacts-pr-//')
echo "pr-number=$PR_NUMBER" >> "$GITHUB_OUTPUT"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#resolve-aws-issueNotice: Ensure you have installed the latest nightly Rust toolchain. If you haven't installed it yet, please see https://www.rust-lang.org/tools/install to learn how to download Rustup and install Rust. or load it into the REPL: |
Performance report
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6376 +/- ##
=======================================
Coverage 98.78% 98.78%
=======================================
Files 274 274
Lines 10795 10795
Branches 2883 2883
=======================================
Hits 10664 10664
Misses 89 89
Partials 42 42 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This should reduce the attack surface and also avoid social engineering attacks.
5390c0d to
45f5f44
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
.github/workflows/repl-artefacts-upload.yml:38
- Removing the label here runs on every successful build, including same-repo PRs that were never labeled (the build job triggers for any synchronize/opened event when
head.repo.full_name == 'rollup/rollup').gh pr edit --remove-labelreturns a non-zero exit code when the label is not present on the PR, which will fail the upload job and prevent the artefacts from ever being uploaded for ordinary internal PRs. Consider gating this step on the original event being alabeledaction (e.g., by passing that info via the artifact) or by checking first whether the label is present.
- name: Remove 'x⁸ ⚙️ build repl artefacts' label
run: gh pr edit ${{ steps.pr-number.outputs.pr-number }} --remove-label 'x⁸ ⚙️ build repl artefacts'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
.github/workflows/repl-artefacts-upload.yml:102
patrickedqvist/wait-for-vercel-previewpolls the GitHub deployments API for the SHA of the workflow event's checkout. Underworkflow_run, the defaultGITHUB_REF/SHA is the default branch's HEAD, not the PR commit, so this action will not find the Vercel preview for the PR and the step will time out (or theif: steps.waitForVercel.outputs.urlguard will silently skip the comment update). The SHA needs to be passed explicitly, e.g. via the action'sshainput set to${{ github.event.workflow_run.head_sha }}.
- name: Find Vercel preview URL
uses: patrickedqvist/wait-for-vercel-preview@d7982701e6fcd3ae073bff929e408e004404d38d # v1.3.3
id: waitForVercel
with:
token: ${{ secrets.GITHUB_TOKEN }}
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
This PR has been released as part of rollup@4.61.0. You can test it via |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
The existing
repl-artefacts.ymlworkflow usedpull_request_targetwithref: ${{ github.event.pull_request.head.sha }}, which checks out and executes code from fork pull requests in a context that has access to repository secrets (AWS_ACCESS_KEY_ID,AWS_SECRET_ACCESS_KEY). A maintainer adding thex⁸ ⚙️ build repl artefactslabel to a malicious fork PR would trigger a build of that untrusted code with full access to the AWS credentials.This is fixed by splitting the workflow into two parts:
repl-artefacts.yml— runs onpull_request(untrusted, no secrets):repl-artefacts-upload.yml— runs onworkflow_run(trusted, never executes fork code):aws-actions/configure-aws-credentials) instead of long-lived static credentials — requires addingAWS_ROLE_ARNas a repository secret and configuring a corresponding IAM role that truststoken.actions.githubusercontent.comx⁸ ⚙️ build repl artefactslabel (moved here from the build workflow, since fork PRs onpull_requestevents have read-onlyGITHUB_TOKENregardless of declared permissions)After merging, the
AWS_ACCESS_KEY_IDandAWS_SECRET_ACCESS_KEYsecrets can be removed from the repository. See the AWS OIDC setup steps in the commit description.