chore(ci): no more pull_request_target#8992
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Lighthouse and the new Compare Bundle Size job in Bundle size reporting now uses Next/Turbopack Chromatic triggers on Reviewed by Cursor Bugbot for commit 6a95094. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8992 +/- ##
=======================================
Coverage 75.41% 75.41%
=======================================
Files 98 98
Lines 8636 8636
Branches 318 318
=======================================
Hits 6513 6513
Misses 2119 2119
Partials 4 4 ☔ View full report in Codecov by Harness. |
There was a problem hiding this comment.
Pull request overview
This PR removes usage of the privileged pull_request_target trigger in CI workflows and replaces direct PR-commenting with a safer two-workflow pattern: untrusted pull_request workflows serialize comment data into an artifact, and a trusted workflow_run workflow posts the comment after completion.
Changes:
- Switch Lighthouse and Chromatic workflows from
pull_request_targettopull_requestand adjust permissions/commenting behavior accordingly. - Add a new
Leave Commentworkflow that downloads apr-commentartifact onworkflow_runcompletion and posts it to the PR. - Replace the standalone bundle-compare
workflow_runworkflow by integrating bundle comparison into the main Build workflow and emitting apr-commentartifact.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/lighthouse.yml | Runs Lighthouse in pull_request context and uploads a pr-comment artifact instead of commenting directly. |
| .github/workflows/leave-comment.yml | New trusted workflow_run workflow intended to post PR comments based on downloaded artifacts. |
| .github/workflows/chromatic.yml | Moves Chromatic to pull_request and changes how the Chromatic token is sourced. |
| .github/workflows/bundle-compare.yml | Removes the old standalone bundle compare workflow_run workflow. |
| .github/workflows/build.yml | Adds an in-workflow bundle size comparison job and uploads pr-comment artifacts for trusted commenting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if: | | ||
| github.event_name != 'pull_request_target' || | ||
| github.event_name != 'pull_request' || | ||
| ( | ||
| github.event.label.name == 'github_actions:pull-request' && | ||
| github.actor != 'dependabot[bot]' && | ||
| github.event.pull_request.head.ref != 'chore/crowdin' |
| workingDir: packages/ui-components | ||
| buildScriptName: storybook:build | ||
| projectToken: ${{ secrets.CHROMATIC_PROJECT_TOKEN }} | ||
| projectToken: ${{ vars.CHROMATIC_PROJECT_TOKEN }} |
There was a problem hiding this comment.
can we make sure we talk about this in writing?
(edit so it's near the review: https://www.chromatic.com/docs/github-actions/#project-token-secret)
There was a problem hiding this comment.
yeah, per https://www.chromatic.com/docs/github-actions/#project-token-secret isnt this a secet?
There was a problem hiding this comment.
A few sections later in that same file, https://www.chromatic.com/docs/github-actions/#run-chromatic-on-external-forks-of-open-source-projects:
Sharing project tokens allows contributors and others to run Chromatic builds on your project, which can use snapshots. They cannot access your account, settings, or accept baselines. This can be an acceptable tradeoff for open source projects that value community contributions.
|
Not sure why Next.js didn't generate the stats file... maybe something changed? |
|
Looks like |
Signed-off-by: Aviv Keller <me@aviv.sh>
Signed-off-by: Aviv Keller <me@aviv.sh>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 827ce1d. Configure here.
Signed-off-by: Aviv Keller <me@aviv.sh>
|
@nodejs/nodejs-website for approval |
| workingDir: packages/ui-components | ||
| buildScriptName: storybook:build | ||
| projectToken: ${{ secrets.CHROMATIC_PROJECT_TOKEN }} | ||
| projectToken: ${{ vars.CHROMATIC_PROJECT_TOKEN }} |
There was a problem hiding this comment.
can we make sure we talk about this in writing?
(edit so it's near the review: https://www.chromatic.com/docs/github-actions/#project-token-secret)
There was a problem hiding this comment.
want to make sure we all look at this slower.
https://www.chromatic.com/docs/github-actions/#forked-repositories does suggest this pattern (plaintext vs vars), with it as a tradeoff for just this use case. are we accepting of the possible misuse of quota from forks?
|
(Copying my review comment for visibility) A few sections later in that same file, https://www.chromatic.com/docs/github-actions/#run-chromatic-on-external-forks-of-open-source-projects:
|
|
IMHO the risk of a misused quota is substantially lower than the risk of a misconfigured/vulnerable The worst case scenario is that we get some spam Storybook builds that affect our (very high) quota of which we use very little. The worst case scenario of a compromised workflow, on the other hand, is far more dangerous |

No description provided.