Skip to content

Conversation

fbartho
Copy link
Member

@fbartho fbartho commented Mar 12, 2025

Alternative PR for: #1477
Fixes: #1479

Fixes transitive CVE: GHSA-h5c3-5r3r-rr8q

Contents

  • This PR upgrades our dependency on @octokit/rest (to the highest version we run on without ESM support).
  • This PR upgrades our devDependency of release-it as that also depends on @octokit/rest
  • This PR upgrades our CI testing to node 20 (from 18) -- because release-it requires the node-engine to match "^20.9.0 || >=22.0.0". I could tweak CI to install with node-20, and then run it with node 18, but that seems messy/risky/low-value.

This kicks the can down the road on our migration to ESM.

chris-griffin and others added 6 commits February 19, 2025 22:06

- v19 dropped support for node 10/12 (https://github.com/octokit/rest.js/releases/tag/v19.0.0)
- v20 dropped support for node 14/16, removed preview support for the REST API, and removed the agent option (https://github.com/octokit/rest.js/releases/tag/v20.0.0)
- v21 updated the package to ESM (https://github.com/octokit/rest.js/releases/tag/v21.0.0)

None of these breaking changes should impact v12 of danger-js as it requires node >= 18.
This is a build-tool so doesn't affect our users, but it does prevent casual CI testing on Node 18.
@fbartho fbartho added the dependencies Pull requests that update a dependency file label Mar 12, 2025
@fbartho fbartho requested a review from orta March 12, 2025 19:16
@fbartho
Copy link
Member Author

fbartho commented Mar 12, 2025

@orta Appveyor appears to be failing the CI, even though all the tests passed, and there's no reported error messages

@orta
Copy link
Member

orta commented Mar 15, 2025

Eh, I can live with the appveyor builds being red

@mikehall314
Copy link

Hello folks -- how do we progress landing this fix?

@justinc324
Copy link

+1 to the above - any updates? Looks like the green light has been given

@fbartho fbartho merged commit 2f2cd43 into main Apr 3, 2025
2 of 4 checks passed
@fbartho fbartho deleted the fb/cve-octokit-rest-mitigation branch April 3, 2025 18:30
@fbartho
Copy link
Member Author

fbartho commented Apr 3, 2025

@justinc324 @mikehall314 I've merged it, but I don't have permissions to package and ship a release, so it'll go out when that next happens.

In the meantime, you could directly reference the SHA in your package version selectors, I think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SECURITY] npm audit report: @octokit dependencies contain some CVEs
5 participants