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

v3.x fix: remove deprecated lodash per-method packages for vulnerability fixes #80

Merged
merged 4 commits into from
Nov 21, 2024

Conversation

EmilianoSanchez
Copy link
Contributor

This PR applies a similar fix to #78, but specifically for version 3.0.3 of the library, which is compatible with Node.js v12+.

We are using this reliable library in the @splitsoftware/splitio package and need to maintain compatibility with v3 in order to support Node.js v14 and above. Since the upcoming v4 release will drop support for Node.js versions below v20, staying on v3 is important for us.

We would appreciate a new patch version (3.0.4) that includes this fix, to avoid some misleading vulnerability alerts related to deprecated lodash per-method packages, such as lodash.eq and lodash.indexof, which are sometimes flagged by security analysis tools.

If a patch release is not possible, please let us know, so we can consider an alternative approach, such as releasing a new temporary NPM package until we can migrate to v4. However, we believe this would be a valuable update for other users who might still be using older versions of Node.js.

Thanks in advance,

@folkvir
Copy link
Collaborator

folkvir commented Nov 21, 2024

✋ The patch is possible, it will also remove the diff from the 4.0 PR. Good point!
If you could also do:

  • a renaming of every lodash import with lodash/XXXX
  • update the lodash package to latest (if possible) and its associated types: @types/lodash

It will also be very appreciated 🙏

@folkvir folkvir self-assigned this Nov 21, 2024
@folkvir folkvir self-requested a review November 21, 2024 14:47
@folkvir folkvir added enhancement dependencies Pull requests that update a dependency file labels Nov 21, 2024
@EmilianoSanchez
Copy link
Contributor Author

Thank you @folkvir ,

I did the updates as you suggested, but the only concern was that the linter is reporting a wrong error on direct ES module imports (e.g., import eq from 'lodash/eq'), so I had to place the following comment to ignore the error:

// eslint-disable-next-line node/no-missing-import

It seems to be an issue with the older version of eslint used in v3, because v4, which has an updated version of eslint, doesn't report any error with those imports.

@folkvir
Copy link
Collaborator

folkvir commented Nov 21, 2024

Just add the .js extension for the node configuration in the package.json. I will commit the fix 👍

diff --git a/package.json b/package.json
index d772555..7544c4d 100644
--- a/package.json
+++ b/package.json
@@ -120,7 +120,8 @@
           "./src"
         ],
         "tryExtensions": [
-          ".ts"
+          ".ts",
+          ".js"
         ]
       }
     },

@EmilianoSanchez
Copy link
Contributor Author

Done. That fixed it. Thanks!

Copy link
Collaborator

@folkvir folkvir left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thank you for the fix, very appreciated!

@folkvir folkvir merged commit 4b67470 into Callidon:master Nov 21, 2024
6 checks passed
@folkvir
Copy link
Collaborator

folkvir commented Nov 21, 2024

Released: https://github.com/Callidon/bloom-filters/releases/tag/v3.0.4
Should soon be published on NPM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants