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

Bumping versions to fix vulnerabilities #7333

Open
joycebrum opened this issue Sep 8, 2023 · 5 comments
Open

Bumping versions to fix vulnerabilities #7333

joycebrum opened this issue Sep 8, 2023 · 5 comments

Comments

@joycebrum
Copy link
Contributor

Describe the bug

Hi again, I'd like to suggest a PR bumping vulnerable dependency versions to avoid the known vulnerabilities in them.

Example: I'd like to bump decode-uri-component to the 0.2.1 version in order to avoid GHSA-w573-4hg7-7wgq.

There are currently 68 vulnerabilities to be fixed (according to Scorecard analysys), let me know whether a PR would be welcome and I'll start to work on them right away.

Thanks!

Expected behavior

None

Reproduction code

No response

Reproduction URL

No response

Version

main

Environment

No response

Additional context

No response

@kwonoj
Copy link
Member

kwonoj commented Sep 8, 2023

If it's a casual bump should be fine to accept PR, however meanwhile would like to understand how it actually exposes a vulnerabilities as rxjs does not have any dependencies.

@joycebrum
Copy link
Contributor Author

The one I've mentioned (GHSA-w573-4hg7-7wgq) is a dependency of the "decision-tree-generator" tool.

But for each one I solve I will try explain in the PR how the vulnerable dependency would impact users or the project.

@joycebrum
Copy link
Contributor Author

Hi @kwonoj I've explored more the finding vulnerabilities (I've started with the package.json and package-lock.json on the root folder) by starting with handling the vulnerabilities reported by npm audit:

image

The changes related to the npm audit fix --force run can be seen in the following PR https://github.com/joycebrum/rxjs/pull/2/files --- I've created the PR to wait for #7341 resolution and use the ci to validate each upgrade I've made (the local run of npm test and npm run compile did run with success, but I'd like to double check).

In the meantime, I've analyzed the vulnerable dependencies and they were all either dev dependencies or docs_app dependencies, so there won't be any breaking change for the end users. Although the attack surfaces are a bit different for dev and runtime dependencies, it is still important to try to use not vulnerable dependencies whenever possible, but let me know what you think.

Besides, I may take some time to work on all the vulnerabilities.

@jakovljevic-mladen
Copy link
Member

jakovljevic-mladen commented Dec 5, 2023

Hi @joycebrum. Thank you a lot for bringing this to our attention.

A lot of dependencies (if not all of them) that RxJS has come from docs app. Currently, there are multiple PRs waiting for my or other team members' review, but due to lack of our personal time to do the review, I'm afraid we can't accept those PR immediately.

Also, current "decision-tree-generator" app has another PR open which seems like a huge rewrite, so making a PR from the current main branch would certainly bring the conflicts.

I suggest we wait for now until at least the mentioned PR is merged.

@joycebrum
Copy link
Contributor Author

Ah thanks for bringing that up. I was struggling too to be able to correctly bumping the right versions since there are lots of vulnerabilities found in indirect dependencies 😓 (sometimes deep in the dependency tree).

Let's wait this changes and hopefully the work here will be simpler. Feel free to ping me when that got merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants