Skip to content

Bump @octokit dependencies to newer versions #1960

Open
@martincostello

Description

@martincostello

@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/github@6.0.0 requires @octokit/plugin-paginate-rest@^9.0.0
No patched version available for @octokit/plugin-paginate-rest
@actions/github@6.0.0 requires @octokit/request@^8.3.1 via @octokit/core@5.2.0
@actions/github@6.0.0 requires @octokit/request@^8.3.0 via a transitive dependency on @octokit/graphql@7.1.0
No patched version available for @octokit/request

Activity

francosalcedo

francosalcedo commented on Feb 18, 2025

@francosalcedo

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

martincostello commented on Feb 19, 2025

@martincostello
ContributorAuthor

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

cupofjoakim commented on Feb 20, 2025

@cupofjoakim

@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

martincostello commented on Feb 20, 2025

@martincostello
ContributorAuthor

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

cupofjoakim commented on Feb 25, 2025

@cupofjoakim

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

crazymykl commented on Feb 25, 2025

@crazymykl

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/github@2.2.0, 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/github@2.2.0, 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

mislav commented on Mar 4, 2025

@mislav

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

wolfy1339 commented on Mar 4, 2025

@wolfy1339

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

trystan2k commented on Mar 4, 2025

@trystan2k

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

cupofjoakim commented on Mar 5, 2025

@cupofjoakim

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

timbru31

timbru31 commented on Mar 10, 2025

@timbru31

4 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @mislav@crazymykl@trystan2k@timbru31@martincostello

      Issue actions

        Bump @octokit dependencies to newer versions · Issue #1960 · actions/toolkit