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

Patch for XLSX vulnerability #1371

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TylerSelden
Copy link

Pull Request Template

PR Checklist

  • I have run npm test locally and all tests are passing.
  • I have added/updated tests for any new behavior.
  • If this is a significant change, an issue has already been created where the problem / solution was discussed: N/A

PR Description

A patch for an underlying vulnerability in XLSX, which is a dependency of the @node-nlp/xtables package. This PR uses a patched version of XLSX, thus removing the vulnerability.

@Apollon77
Copy link
Contributor

LGTM

@TylerSelden
Copy link
Author

For anybody interested, here's some context on why this PR needs to exist in the first place:

A while back, the maintainers of XSLX (a dependency of this package) decided to stop publishing to NPM because NPM started requiring 2-Factor Authentication. Nobody really quite understands why this is such a big issue, but regardless, the maintainers of the package are essentially boycotting NPM and distributing the package themselves.

Many people are very unhappy about this, as can be expected, for a host of reasons, one of which being a critical vulnerability (a Denial of Service, if I'm not mistaken) in the XLSX package, which will now never get patched.

Thus, various people in the NPM community took it upon themselves to publish up-to-date versions of XSLX (like the one used in this PR). This PR will get rid of the critical vulnerabilities you see when you install node-nlp.

@Apollon77
Copy link
Contributor

Thats kind of what I also found in short check . Thank you for providing this background

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.

None yet

2 participants