Skip to content

Conversation

slorber
Copy link
Collaborator

@slorber slorber commented Sep 1, 2025

Motivation

The recent Nx critical supply chain attack (https://socket.dev/blog/nx-packages-compromised) is due to the usage of pull_request_target: https://x.com/adnanthekhan/status/1958722939534417989

We also use pull_request_target on this action, but:

  • this is very risky, and the code below can introduce critical vulnerabilities
  • it doesn't even work because we run Lighthouse on our main branch (see Consolidate Lighthouse CI report #9449 (comment)), and checkout out the base branch would introduce the vulnerability

So, I'm moving the event to the much safer pull_request. This:

  • fixes the security risk
  • fixes the Lighthouse job for internal PRs
  • unfortunately, external PRs won't be able to post the GitHub lighthouse comment.

Posting a comment to an external PR should be fixable. If someone wants to improve our workflow, please submit a PR, but I'll only accept the usage of pull_request_target on tiny workflows that doesn't execute any code outside of the Yaml workflow itself.

See also https://github.blog/security/application-security/how-to-secure-your-github-actions-workflows-with-codeql/

Test Plan

CI

@slorber slorber requested a review from Josh-Cena as a code owner September 1, 2025 13:07
@slorber slorber added the pr: ignore This PR is not meaningful enough to appear in the changelog. label Sep 1, 2025
@meta-cla meta-cla bot added the CLA Signed Signed Facebook CLA label Sep 1, 2025
Copy link

netlify bot commented Sep 1, 2025

[V2]

Name Link
🔨 Latest commit e8afdd5
🔍 Latest deploy log https://app.netlify.com/projects/docusaurus-2/deploys/68b59a8944f6fb00086b364f
😎 Deploy Preview https://deploy-preview-11393--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

github-actions bot commented Sep 1, 2025

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO Report
/ 🟠 52 🟢 98 🟢 100 🟢 100 Report
/docs/installation 🟠 61 🟢 97 🟢 100 🟢 100 Report
/docs/category/getting-started 🟠 71 🟢 100 🟢 100 🟠 86 Report
/blog 🟠 59 🟢 96 🟢 100 🟠 86 Report
/blog/preparing-your-site-for-docusaurus-v3 🟠 58 🟢 92 🟢 100 🟢 100 Report
/blog/tags/release 🟠 61 🟢 96 🟢 100 🟠 86 Report
/blog/tags 🟠 68 🟢 100 🟢 100 🟠 86 Report

@slorber slorber merged commit 72c48b5 into main Sep 1, 2025
15 of 16 checks passed
@slorber slorber deleted the slorber/fix-pull_request_target-risk branch September 1, 2025 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: ignore This PR is not meaningful enough to appear in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant