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

Bump download vendor package version #79

Closed
wants to merge 1 commit into from
Closed

Bump download vendor package version #79

wants to merge 1 commit into from

Conversation

zsoltpuskas
Copy link

The download vendor package has dependency for minimist which contains a security vulnerability. This commit bumps version to the latest in which the bug is fixed.

@TheJaredWilcurt
Copy link
Member

TheJaredWilcurt commented Jul 29, 2020

Unfortunately [email protected] requires Node 10+. It is very common for NW.js users to globally install a version of Node.js that matches the version built in to NW.js. This is because many Node Modules will detect the globally installed Node version and either do a custom build for that version or download files specific to that version of Node during the npm install.

If, for example, I have Node v14.0.0 installed globally on my machine, and I npm install --save node-sass, then node-sass will download a Node 14 binding for itself. But if my desktop app is written using NW.js LTS (0.14.7), which comes with Node v5.11.1 built-in. Then the app would not be able to require('node-sass'), it would fail because the copy of node-sass that was installed was for the wrong version (14.0.0 instead of 5.11.1). The easiest way to fix this is to switch your global install of Node to match the same that is built in to NW.js. This means that all of your dependencies and devDependencies will build or download any custom files to match your global, which is the same as what is built in to NW.js. However, if you have "nw" as a devDep and it cannot run on anything below Node 10, this will cause MANY headaches and lots of troubleshooting across all of your dependencies. Basically, I don't want an easy change done in this repo to force many hard changes to all NW.js users.

Because of this we have a few options:

  1. Leave download at its current version (ignore the minimist warning). Justification: The nw Node Module is only used to download a local copy of NW.js into your repo for easier development. It should only ever be a devDependency. It will not be shipped to an end user, and it will not run server-side. Therfore any security vulnerabilities associated to its dependencies should, in almost all cases, be irrelevant.
  2. Depending on the dependency associations, it may be possible to add minimist as a direct dependency to nw, thus allowing this repo to control the minimist version and download will use this instead of installing the older version of it. Not sure if that will work, but someone can try if they want.
  3. Find a replacement for download that offers similar functionality, but is not reliant on stupid minimist which I swear to god has a security issue every 3 days. Then submit a PR adopting the new dependency and updating any code needed for the switch.
  4. Ask the maintainers of download very very nicely to do a new release of a previous version that still supported older Node versions, but with the new minimist.

Edit: Added a 4th option

@zsoltpuskas
Copy link
Author

My opinion is that only the third option is acceptable. However choosing a less used library tend to end up with no update from the maintainer after a few years. Doing an implementation inside this project requires effort and have to be maintained also. Feel free to close the pull request if you think this solution is not acceptable to you.

@TheJaredWilcurt TheJaredWilcurt changed the title Bump download vendor package version Bump download vendor package version Aug 4, 2020
@zsoltpuskas zsoltpuskas closed this Nov 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants