Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump @octokit dependencies to newer versions #1960

Open
martincostello opened this issue Feb 18, 2025 · 10 comments · May be fixed by #1972
Open

Bump @octokit dependencies to newer versions #1960

martincostello opened this issue Feb 18, 2025 · 10 comments · May be fixed by #1972
Labels
enhancement New feature or request github

Comments

@martincostello
Copy link
Contributor

@actions/github depends on relatively old versions of @octokit packages that make it impossible to update to the latest versions without breaking changes.

This is particularly noticeable now that dependabot alerts are being opened in repositories using them due to GHSA-h5c3-5r3r-rr8q, GHSA-rmvr-2pp2-xj38 and GHSA-x4c5-c7rf-jjgv.

Please update the dependencies to newer versions that make it easier to keep up-to-date with the GitHub API's evolution while also resolving these vulnerabilities.

Image

@actions/[email protected] requires @octokit/plugin-paginate-rest@^9.0.0
No patched version available for @octokit/plugin-paginate-rest
@actions/[email protected] requires @octokit/request@^8.3.1 via @octokit/[email protected]
@actions/[email protected] requires @octokit/request@^8.3.0 via a transitive dependency on @octokit/[email protected]
No patched version available for @octokit/request
@martincostello martincostello added the enhancement New feature or request label Feb 18, 2025
@francosalcedo
Copy link

francosalcedo commented Feb 18, 2025

To temporarily resolve the issue, you can add the following to your package.json:

`
"resolutions": {

"@nx-tools/container-metadata/**/@octokit/plugin-paginate-rest": "11.4.1",

"@nx-tools/container-metadata/**/@octokit/core": "6.1.4"

}

`

@martincostello
Copy link
Contributor Author

Looks like there was some updates overnight that fixed most of them. Now only @octokit/plugin-paginate-rest with GHSA-h5c3-5r3r-rr8q is the only problematic one.

@cupofjoakim
Copy link

@martincostello According to the vuln page it seems that issue is patched in 9.2.2 so it should be fixed right? AFAIK this should be unblocked now.

@martincostello
Copy link
Contributor Author

Looks like dependabot security updates isn't able to do anything about it for some reason, but running npm audit fix manually seems to resolve the alerts.

@cupofjoakim
Copy link

cupofjoakim commented Feb 25, 2025

Had a quick go but it seemed to me that some of the packages have to be bumped several majors and I honestly don't have the time to make sure it works. Doesn't seem like a hard job though, just a time thing.

@crazymykl
Copy link

crazymykl commented Feb 25, 2025

npm audit on npm 11.1.0 doesn't register that @octokit/plugin-paginate-rest-9.2.2 and @octokit/request-8.4.1 are not vulnerable.

Minimal repro:

~/workspace ❯❯❯ mkdir npm-github-actions
~/workspace ❯❯❯ cd npm-github-actions
~/w/npm-github-actions ❯❯❯ npm i @actions/github

added 24 packages in 871ms
~/w/npm-github-actions ❯❯❯ npm audit
# npm audit report

@octokit/plugin-paginate-rest  <=11.4.0
Severity: moderate
Depends on vulnerable versions of @octokit/core
@octokit/plugin-paginate-rest has a Regular Expression in iterator Leads to ReDoS Vulnerability Due to Catastrophic Backtracking - https://github.com/advisories/GHSA-h5c3-5r3r-rr8q
fix available via `npm audit fix --force`
Will install @actions/[email protected], which is a breaking change
node_modules/@octokit/plugin-paginate-rest
  @actions/github  >=3.0.0
  Depends on vulnerable versions of @octokit/core
  Depends on vulnerable versions of @octokit/plugin-paginate-rest
  node_modules/@actions/github

@octokit/request  <=9.2.0
Severity: moderate
@octokit/request has a Regular Expression in fetchWrapper that Leads to ReDoS Vulnerability Due to Catastrophic Backtracking - https://github.com/advisories/GHSA-rmvr-2pp2-xj38
fix available via `npm audit fix --force`
Will install @actions/[email protected], which is a breaking change
node_modules/@octokit/request
  @octokit/core  <=5.2.0
  Depends on vulnerable versions of @octokit/graphql
  Depends on vulnerable versions of @octokit/request
  node_modules/@octokit/core
    @octokit/plugin-rest-endpoint-methods  10.4.1 || 13.2.2
    Depends on vulnerable versions of @octokit/core
    node_modules/@octokit/plugin-rest-endpoint-methods
  @octokit/graphql  <=2.1.3 || 3.0.0 - 7.1.1
  Depends on vulnerable versions of @octokit/request
  node_modules/@octokit/graphql

6 moderate severity vulnerabilities

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

package-lock.json confirms that it installs the above mentioned versions.

Edit:

This looks like it's caused by npm/cli#8125.

@mislav
Copy link

mislav commented Mar 4, 2025

Thanks everyone for the info so far, especially @crazymykl who pointed out the npm audit bug which is something that really threw me off initially. I've contributed a PR that bumps @octokit/plugin-paginate-rest, hopefully eliminating the last of Dependabot notices of moderate severity for the @actions/github package.

I've also explored a short spike on what it would take to migrate to the newest Octokit version, but it was a whole can of worms that I gave up on shortly after. This project uses @octokit/core v5, and it seems that the breaking change in Octokit v6 is that it now publishes ES modules. Without changing any project configuration, this migration breaks Jest-based tests, which only has experimental support for ESM, but once I made the suggested changes to test configuration, tests started failing with cryptic import errors and mismatched TS types. It looks like some heavier lifting might be required across this whole repo and all its packages just to migrate to @octokit/core v6, unless I'm missing some easy compatibility layer.

@wolfy1339
Copy link

Jest is a pain for ESM, and TypeScript.

If you bump all Octokit packages to their latest version there shouldn't be any issues. It should be mostly compatible with existing code.

If you need help migrating, feel free to reach out

(I am a community maintainer of the JS Octokit packages)

@trystan2k
Copy link

What if Jest tests are migrate to vitest, would it solve this ESM problem, no ? We could try to use codemod (like this one https://codemod.com/registry/jest-to-vitest) to make it faster ?

@cupofjoakim
Copy link

I second a swap to vitest. Apart from some imports and the test setup, it's basically a drop in replacement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request github
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants