Fix security vulnerability in workflows#1804
Merged
Conversation
"The workflow .github/workflows/eslint-check.yml contained a critical "pwn request" vulnerability that allows any GitHub user to execute arbitrary code with access to repository secrets by opening a pull request." See preactjs/compressed-size-action#54 for why that action shouldn't be used with pull_request_target This change in this PR drops compressed-size-action in favour of executing the steps ourselves in two workflows, one which produces the size artifact, and the other which reads the artifact has the permissions to write the message back to the original PR (which is in a third party repo)
…s failing run 'ESLint Annotation')
- Add `.github/scripts/measure-bundle-sizes.js` and `render-bundle-size-comment.js` to replace inline node scripts embedded in workflow YAML, improving readability and reusability - Refactor `eslint-check.yml` to use the new script files and fix checkout steps to handle both PR and non-PR triggers correctly - Refactor `pr-checks-privileged.yml` to replace the large `github-script` block with `render-bundle-size-comment.js` and the `marocchino/sticky-pull-request-comment` action; remove the now-unnecessary `pr_number.txt` artifact by reading the PR number directly from the workflow_run event - Pin `ataylorme/eslint-annotate-action` to a specific commit SHA - Add `actions: read` permission where needed for artifact downloads
|
Contributor
|
Size Change: 0 B Total Size: 10.1 MB ℹ️ View Unchanged
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a two-stage PR CI flow where unprivileged pull_request runs produce lint + bundle-size artifacts, and a follow-up privileged workflow_run consumes those artifacts to post PR annotations/comments without checking out or executing fork code.
Changes:
- Switch ESLint workflow trigger from
pull_request_targettopull_requestand upload lint/bundle-size artifacts. - Add base-branch build job to compute “base” bundle sizes for comparison.
- Add privileged
workflow_runworkflow plus helper scripts to render and post bundle-size comments and ESLint annotations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| .github/workflows/pr-checks-privileged.yml | New privileged workflow_run workflow to post PR comments/annotations from artifacts. |
| .github/workflows/eslint-check.yml | Reworks ESLint CI to run on pull_request, upload artifacts, and build base branch for size comparison. |
| .github/scripts/render-bundle-size-comment.js | Renders a markdown PR comment from PR/base size JSON inputs. |
| .github/scripts/measure-bundle-sizes.js | Collects bundle file sizes under dist/ and writes them to JSON. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: ${{ github.event.pull_request.head.repo.full_name }} | ||
| ref: ${{ github.head_ref }} |
| ref: ${{ github.head_ref }} | ||
| - name: Setup Node | ||
| uses: actions/setup-node@v3 | ||
| ref: ${{ github.base_ref }} |
| } | ||
|
|
||
| const rootDir = process.cwd(); | ||
| const sizes = {}; |
Comment on lines
+114
to
+122
| const rows = files | ||
| .map((filePath) => { | ||
| const fileDiff = (prSizes[filePath] ?? 0) - (baseSizes[filePath] ?? 0); | ||
| return `| \`${getFileLabel(filePath, packageName)}\` | ${formatSize( | ||
| baseSizes[filePath], | ||
| )} | ${formatSize(prSizes[filePath])} | ${formatDiff( | ||
| fileDiff, | ||
| baseSizes[filePath] ?? 0, | ||
| )} |`; |
- Look up PR number via API when workflow_run.pull_requests is empty (GitHub leaves it empty for fork PRs), falling back gracefully - Use head SHA instead of branch name for PR checkout to avoid TOCTOU - Fix formatSignedSize to produce +0 instead of -0 for zero values - Gate comment steps on successful PR number lookup Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a split GitHub Actions setup where unprivileged PR runs build/lint and upload artifacts, and a privileged workflow_run workflow consumes those artifacts to post PR comments and ESLint annotations without executing fork code.
Changes:
- Switch
eslint-check.ymlfrompull_request_targettopull_request, add bundle-size measurement + artifacts, and add a base-branch build job for bundle-size comparison. - Add a privileged
workflow_runworkflow to download artifacts and (a) post a sticky bundle-size PR comment and (b) publish ESLint annotations. - Add scripts to measure dist bundle sizes and render a markdown comment from PR vs base JSON size data.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/workflows/pr-checks-privileged.yml |
New privileged workflow_run workflow that downloads artifacts and posts PR comment + annotations. |
.github/workflows/eslint-check.yml |
Runs on pull_request/push, uploads ESLint report artifact, measures PR bundle sizes, builds base branch for comparison, uploads combined size artifacts. |
.github/scripts/render-bundle-size-comment.js |
Renders a markdown bundle size diff report grouped by package. |
.github/scripts/measure-bundle-sizes.js |
Walks the workspace to collect dist/*.{js,cjs,mjs,css} file sizes into JSON. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { data: prs } = await github.rest.pulls.list({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| head: `${run.head_repository.full_name}:${run.head_branch}`, |
|
|
||
| function walk(dirPath) { | ||
| for (const entry of fs.readdirSync(dirPath, { withFileTypes: true })) { | ||
| if (entry.name === 'node_modules') { |
Comment on lines
+69
to
75
| - name: Checkout workflow ref | ||
| uses: actions/checkout@v4 | ||
| - name: Prepare bundle size helper | ||
| run: | | ||
| cp .github/scripts/measure-bundle-sizes.js /tmp/measure-bundle-sizes.js | ||
| # --- Base branch --- | ||
| - uses: actions/checkout@v4 |
eoghanmurray
approved these changes
Mar 17, 2026
Contributor
eoghanmurray
left a comment
eoghanmurray
left a comment
There was a problem hiding this comment.
Happy to approve, pity it drops the emojis!
Member
Author
Oh that is a shame indeed! |
billyvg
pushed a commit
to getsentry/rrweb
that referenced
this pull request
Mar 23, 2026
* Fix a security hole in rrweb-io#1787 found by Arun Murugesan: "The workflow .github/workflows/eslint-check.yml contained a critical "pwn request" vulnerability that allows any GitHub user to execute arbitrary code with access to repository secrets by opening a pull request." See preactjs/compressed-size-action#54 for why that action shouldn't be used with pull_request_target This change in this PR drops compressed-size-action in favour of executing the steps ourselves in two workflows, one which produces the size artifact, and the other which reads the artifact has the permissions to write the message back to the original PR (which is in a third party repo) * The annotate action also needed pull-requests: write permission (fixes failing run 'ESLint Annotation') * ci(bundle-size): extract bundle size scripts and simplify workflow - Add `.github/scripts/measure-bundle-sizes.js` and `render-bundle-size-comment.js` to replace inline node scripts embedded in workflow YAML, improving readability and reusability - Refactor `eslint-check.yml` to use the new script files and fix checkout steps to handle both PR and non-PR triggers correctly - Refactor `pr-checks-privileged.yml` to replace the large `github-script` block with `render-bundle-size-comment.js` and the `marocchino/sticky-pull-request-comment` action; remove the now-unnecessary `pr_number.txt` artifact by reading the PR number directly from the workflow_run event - Pin `ataylorme/eslint-annotate-action` to a specific commit SHA - Add `actions: read` permission where needed for artifact downloads * ci: add fork PR support and harden workflow - Look up PR number via API when workflow_run.pull_requests is empty (GitHub leaves it empty for fork PRs), falling back gracefully - Use head SHA instead of branch name for PR checkout to avoid TOCTOU - Fix formatSignedSize to produce +0 instead of -0 for zero values - Gate comment steps on successful PR number lookup Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Eoghan Murray <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
billyvg
pushed a commit
to getsentry/rrweb
that referenced
this pull request
Mar 23, 2026
* Fix a security hole in rrweb-io#1787 found by Arun Murugesan: "The workflow .github/workflows/eslint-check.yml contained a critical "pwn request" vulnerability that allows any GitHub user to execute arbitrary code with access to repository secrets by opening a pull request." See preactjs/compressed-size-action#54 for why that action shouldn't be used with pull_request_target This change in this PR drops compressed-size-action in favour of executing the steps ourselves in two workflows, one which produces the size artifact, and the other which reads the artifact has the permissions to write the message back to the original PR (which is in a third party repo) * The annotate action also needed pull-requests: write permission (fixes failing run 'ESLint Annotation') * ci(bundle-size): extract bundle size scripts and simplify workflow - Add `.github/scripts/measure-bundle-sizes.js` and `render-bundle-size-comment.js` to replace inline node scripts embedded in workflow YAML, improving readability and reusability - Refactor `eslint-check.yml` to use the new script files and fix checkout steps to handle both PR and non-PR triggers correctly - Refactor `pr-checks-privileged.yml` to replace the large `github-script` block with `render-bundle-size-comment.js` and the `marocchino/sticky-pull-request-comment` action; remove the now-unnecessary `pr_number.txt` artifact by reading the PR number directly from the workflow_run event - Pin `ataylorme/eslint-annotate-action` to a specific commit SHA - Add `actions: read` permission where needed for artifact downloads * ci: add fork PR support and harden workflow - Look up PR number via API when workflow_run.pull_requests is empty (GitHub leaves it empty for fork PRs), falling back gracefully - Use head SHA instead of branch name for PR checkout to avoid TOCTOU - Fix formatSignedSize to produce +0 instead of -0 for zero values - Gate comment steps on successful PR number lookup Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Eoghan Murray <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
billyvg
added a commit
to getsentry/rrweb
that referenced
this pull request
Mar 23, 2026
Cherry picks the following PRs that fix security vulns from GHA: - rrweb-io#1787 - rrweb-io#1804 This also completely removes the privileged workflows added in rrweb-io#1804 as they are not necessary for us. --------- Co-authored-by: Eoghan Murray <[email protected]> Co-authored-by: Justin Halsall <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Simplificaiton of #1803, fixes security issue introduced in #1787
Address a critical security vulnerability in the GitHub workflows by replacing the
compressed-size-actionwith custom scripts to measure bundle sizes. This change enhances security by preventing arbitrary code execution through pull requests. Additionally, the workflows have been refactored for improved readability and maintainability. The necessary permissions for actions have been adjusted to ensure proper functionality.