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

Updated socks package to a version that doesn't use the IP package #2177

Merged
merged 5 commits into from
Feb 15, 2024

Conversation

shrshindeMSFT
Copy link
Contributor

For more information about how to contribute to this repo, visit this page.

Description

This repo has a CG violation for the ip project. The ip project has a published security vulnerability:
indutny/node-ip#136
GHSA-78xj-cgh5-2h22

Unfortunately, there's no fixed version of the ip project. Moreover, there are questions in the "Issues" section of that project asking whether there's even a maintainer for it.

We indirectly use ip because we use lerna, which has a whole chain of dependencies which eventually leads to ip
The package in that chain which depends on ip is socks. The socks package HAS been updated to stop depending on "ip" given the uncertainty of getting a fix in ip.

This change forces us to use the latest version of socks that removes the ip dependency. Eventually, over probably a while, the whole lerna dependency change will update to use this newer version of socks, but who knows how long that will take. In the meantime, this override ensures that with our current set of dependencies we no longer consume the vulnerable version of ip.

I also added a section to our package.json file that provides room to explain what each override is for, as a way of hopefully helping our future selves to know when we might be able to remove any of them. As the explanation the file says, it's a hacky way of putting comments in the file since comments are not allowed in JSON.

If this Pull Request should close/resolve any issues when merged, use the special syntax for that here.

Main changes in the PR:

Override socks package in package.json file.

Unit Tests added:

Unit tests are required for all changes. If no unit tests were added as part of this change, please explain why they aren't necessary.

N/A

End-to-end tests added:

N/A

Additional Requirements

Change file added:

Ensure the change file meets the formatting requirements.

N/A

@shrshindeMSFT shrshindeMSFT requested a review from a team as a code owner February 15, 2024 19:00
jadahiya-MSFT
jadahiya-MSFT previously approved these changes Feb 15, 2024
pnpm-lock.yaml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
Copy link
Contributor

@AE-MS AE-MS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few questions

jadahiya-MSFT
jadahiya-MSFT previously approved these changes Feb 15, 2024
@shrshindeMSFT shrshindeMSFT enabled auto-merge (squash) February 15, 2024 21:38
@shrshindeMSFT shrshindeMSFT merged commit 720b43f into main Feb 15, 2024
19 checks passed
@shrshindeMSFT shrshindeMSFT deleted the shrshinde/fixIPVulnerability branch February 15, 2024 21:46
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

Successfully merging this pull request may close these issues.

3 participants