fix(security): vulnerabilities in GHA workflows#270
Conversation
…rweb-io#1787) * Update filesize badges (might need further evolution before 2.0.0) * Don't run full CI/CD when only .md docs have changed in the PR - move eslint checks into their own file so they can also ignore .md changes - prettier checks don't need the same perms as eslint, so we can demote pull_request_target -> pull_request * Add empty changeset * Implement the bundle size change originally originally added in rrweb-io#1784 - adding here also to show how the conflicts would resolve * Update .github/workflows/eslint-check.yml --------- Co-authored-by: Justin Halsall <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
* 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
changed the title
|
cc @chargome -- I'm going to merge this to get rid of |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| - master | ||
| - release/** | ||
| paths-ignore: | ||
| - '**/*.md' |
There was a problem hiding this comment.
Push trigger lost branch filter, now runs on all branches
Medium Severity
The push trigger's branches filter (master and release/**) was removed and replaced solely with paths-ignore. This causes the full test suite (including Puppeteer/xvfb) to run on pushes to every branch, not just master and release/**. GitHub Actions supports both branches and paths-ignore together, so adding paths-ignore didn't require removing the branch filter. This results in duplicate CI runs for same-repo PRs and wasted resources on feature-branch pushes.
| name: bundle-size-data | ||
| path: | | ||
| pr-sizes.json | ||
| base-sizes.json |
There was a problem hiding this comment.
Dead bundle size job wastes CI with no consumer
Medium Severity
The bundle_size_build job runs a full yarn install + yarn build:all on the base branch for every PR and uploads a bundle-size-data artifact. However, no workflow in the repository downloads or uses this artifact. The PR description states the privileged bundle-size-comment workflow was intentionally removed, but the expensive jobs producing data for it were left behind. The PR bundle size measurement steps in eslint_check_upload are similarly orphaned.
Additional Locations (1)
| '', | ||
| ].join('\n'); | ||
|
|
||
| process.stdout.write(body); |
There was a problem hiding this comment.
Unused render-bundle-size-comment.js script added to repository
Low Severity
This 155-line script is newly added but never referenced by any workflow in the repository. It was part of the cherry-picked upstream bundle-size-comment workflow, which was intentionally removed per the PR description. The script is dead code.
5 participants
Cherry picks the following PRs that fix security vulns from GHA:
This also completely removes the privileged workflows added in rrweb-io#1804 as they are not necessary for us.