search

lirantal / is-website-vulnerable

finds publicly known security vulnerabilities in a website's frontend JavaScript libraries
Apache License 2.0
1.94k stars 111 forks source link

installing extraneous dependencies #54

Closed travi closed 4 years ago

travi commented 4 years ago

i noticed some peer dependency warnings about semantic-release and got curious why my non-package project had semantic-release installed at all. it looks like your dev dependencies are getting installed for some reason.

Expected Behavior

installing is-website-vulnerable should only install it and its production dependencies

Current Behavior

even though semantic-release and its plugins are only listed as dev-dependencies, at least some of the dependencies are getting installed in my project:

$ npm ls @semantic-release/github
matt.travi.org@ /path/to/my/project/matt.travi.org
└─┬ is-website-vulnerable@1.9.3
  └── @semantic-release/github@5.5.5  extraneous

Possible Solution

i havent used shrinkwrap much, but that is my best guess as to why this is happening. is it possible to only shrinkwrap prod dependencies?

Steps to Reproduce (for bugs)

  1. npm install is-website-vulnerable --save-dev
  2. npm ls @semantic-release/github (or another dev-dependency)

Context

i'm seeing peer-dependency warnings for dev-dependencies of this project that shouldnt have an impact on my project. i normally also enforce that npm ls exits with zero, which this would make fail. this particular does not fully enforce that yet, so this issue slipped through unnoticed originally

Your Environment

lirantal commented 4 years ago

ahh, good catch, it is due to the shrinkwrap. will take a look at fixing this, thanks!

lirantal commented 4 years ago

@travi which npm cli version are you running?

travi commented 4 years ago

I'm just using the version installed with the version of node mentioned above. I'm afk atm, but can confirm in a bit

travi commented 4 years ago

Regardless of whether dev dependencies should be installed or not, the warnings i see after they do get installed in my project highlight that your peer dependencies aren't compatible ;)

It looks like you're using the beta of semantic-release, but latest of the plugins. You should be using the pre-releases of those too. npm ls should help identify a compatible combination

travi commented 4 years ago

So if those were resolved, it would at least prevent warnings on my side. While i agree that it's odd that shrinkwrap installs the dev dependencies, i don't necessarily mind if it doesn't cause errors

lirantal commented 4 years ago

yep, I see those now too. let's figure out first the shrinkwrap issue as I'd need to update that with any change regardless.

lirantal commented 4 years ago

@travi can you try with the latest stable Node.js, or npm version 6.12.1 ? I don't get peer dependency issues with it

lirantal commented 4 years ago

Nevermind, I do see it. The npm CLI is weird as it skipped those in the first install but running another install after everything is installed they will show up 🤷‍♂

github-actions[bot] commented 4 years ago

:tada: This issue has been resolved in version 1.9.4 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

lirantal commented 4 years ago

@travi can you check now if you're still getting peer deps issues?

travi commented 4 years ago

so, unfortunately, it doesn't seem to be available yet:

$ npm dist-tags ls is-website-vulnerable
latest: 1.9.3

i had a package publish earlier today that is still not available either. i sent a note to support earlier today, but the status page doesnt note any issues.

travi commented 4 years ago

at least i wasnt crazy: https://status.npmjs.org/incidents/lg9535dl2ff3

i'll keep an eye on the incident and test once it gets resolved

travi commented 4 years ago

now that i could install the new version, i was able to confirm that the peer-dependency issues look good, but npm ls does still complain about the extraneous dev-dependencies, and therefore exits with 1 instead of 0

lirantal commented 4 years ago

Yep, will see about fixing that.

travi commented 4 years ago

been a while since i've looked into this, so trying to remember where this landed. did we end up figuring out a way to sort out the shrinkwrap issues, or are we stuck in the state where devDependencies are getting wrapped until npm v7 is available?

travi commented 4 years ago

in case it helps us with context, figured it could help keeping this thread tied to this issue

lirantal commented 4 years ago

Indeed still need to work on this. Let me get to review this again.

github-actions[bot] commented 4 years ago

:tada: This issue has been resolved in version 1.14.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

lirantal commented 4 years ago

@travi is-website-vulnerable@1.14.1 fixes the issue now thanks to lockfile-prune

travi commented 4 years ago

Great work! Looks like that did it. Thanks a lot!