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

High-severity vulnerability due to outdated 'Rollup' dependency #13767

Closed
3 tasks done
its-anas opened this issue Sep 24, 2024 · 11 comments
Closed
3 tasks done

High-severity vulnerability due to outdated 'Rollup' dependency #13767

its-anas opened this issue Sep 24, 2024 · 11 comments
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK

Comments

@its-anas
Copy link

its-anas commented Sep 24, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/nextjs

SDK Version

8.30.0

Framework Version

sentry/nextjs: 8.30.0, next: 14.2.12

Link to Sentry event

No response

Reproduction Example/SDK Setup

No response

Steps to Reproduce

  1. Install @sentry/nextjs@8.30.0.
  2. An npm warning appears post-installation regarding the rollup dependency vulnerability flagged in the advisory GHSA-gcx4-mw62-g8wm.
  3. Run npm ls rollup to check dependency versions.
  4. Notice that @sentry/nextjs is pulling in rollup@2.78.0.

Expected Result

The latest version of @sentry/nextjs should use a non-vulnerable version of rollup, preferably >=3.29.5 or later.

Actual Result

@sentry/nextjs depends on rollup@2.78.0 through sub-dependencies, which is flagged by npm audit for a high-severity XSS vulnerability.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Sep 24, 2024
@github-actions github-actions bot added the Package: nextjs Issues related to the Sentry Nextjs SDK label Sep 24, 2024
@its-anas
Copy link
Author

By adding the latest 'rollup' version (4.22.4) to "overrides" section of my package.json and resetting node_modules, I was able to resolve the npm audit warning related to the outdated rollup version:
However, I'm unsure if this approach might have any unintended side effects. It would be great to know if this is a safe interim fix or if a more comprehensive solution is needed.

@mydea
Copy link
Member

mydea commented Sep 24, 2024

This should be fixed by #13761!

@tomasz-sodzawiczny
Copy link

tomasz-sodzawiczny commented Sep 25, 2024

#13761 updates the version on main but does not actually release @sentry/next, correct?

Btw - why is rollup in @sentry/next dependencies, rather than devDependencies? Any reason to ship it to consumers?

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Sep 25, 2024
@its-anas
Copy link
Author

its-anas commented Sep 25, 2024

#13761 updates the version on main but does not actually release @sentry/next, correct?

Btw - why is rollup in your dependencies, rather than devDependencies? Any reason to ship it to consumers?

The npm vulnerability warning appears once @sentry/nextjs is installed or when you run npm audit in the project. I didn't have rollup installed before this; I only installed it temporarily and added it to overrides in order to temporarily resolve the vulnerability.

@w3nl
Copy link

w3nl commented Sep 25, 2024

Why did Sentry use fixed dependency versions.
If you had used ^, we don't have to wait on Sentry released a version with the updated dependency or overrule a dependency.
My Sentry used rollup 3.29.4, and the fix was in 3.29.5.
With ^3.29.4 we had already a fix.
Now I add this ugly temporary fix in my project:

  "overrides": {
    "@sentry/nextjs": {
      "rollup": "^3.29.5"
    }
  }

@tomasz-sodzawiczny
Copy link

I didn't have rollup installed before this; I only installed it temporarily and added it to overrides in order to temporarily resolve the vulnerability.

@its-anas Whoops, my message was unclear - "your" referred to Sentry in this case - I'm a user, in the same boat as you. Edited the comment, sorry for the confusion.

@its-anas
Copy link
Author

its-anas commented Sep 25, 2024

Issue is fixed, rollup is upgraded to the minimum required version. Thanks guys.

https://github.com/getsentry/sentry-javascript/blob/develop/packages/nextjs/package.json

@CopyJosh
Copy link

Is this something that could get bumped in v7? Not a big deal, seems safe to set the resolution for us at the time being.

@JacobArrow
Copy link

It would be nice to get it bumped in v7 also

@aldenquimby
Copy link
Contributor

+1 for back-porting to v7 please. I am unable to use v8 for a variety of reasons

@aldenquimby
Copy link
Contributor

aldenquimby commented Sep 26, 2024

Btw - why is rollup in @sentry/next dependencies, rather than devDependencies? Any reason to ship it to consumers?

@tomasz-sodzawiczny it's needed by the runtime code, see

* Use Rollup to process the proxy module code, in order to split its `export * from '<wrapped file>'` call into
* individual exports (which nextjs seems to need).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK
Projects
Archived in project
Development

No branches or pull requests

7 participants