Skip to content
This repository has been archived by the owner on Sep 5, 2019. It is now read-only.

Is the check recursive? #16

Open
plynchnlm opened this issue Nov 12, 2015 · 4 comments
Open

Is the check recursive? #16

plynchnlm opened this issue Nov 12, 2015 · 4 comments

Comments

@plynchnlm
Copy link

I think the README file should say something about whether, in addition to your own package.json file, it will also recursively check the package.json files for the packages you include (say, by looking under node_modules). If recursive checking is not needed, then the README should explain why not.

@evilpacket
Copy link

@plynchnlm the check is recursive at the moment. Currently for each dependency we resolve all the dependencies and go all the way down. The entire tree. We don't use the node_modules folder, we only use the package.json and npm-shrinkwrap.json if provided. (shrinkwrap is much faster as versions are already resolved for us).

We can probably update the docs a bit to better reflect it's behavior.

@plynchnlm
Copy link
Author

@evilpacket Without checking node_modules, you don't really know precisely what version of a package is installed, do you? If package.json lists "~1.0.0" and there is a security issue in 1.0.1, then you don't know whether the user has 1.0.0 (safe), 1.0.1 (not safe), or 1.0.2 (safe). Probably you handle that by reporting the possibility of the danger, which would be fine-- but that too could use some documentation in the README. I can see that using the shrinkwrap option would avoid this issue.

@latentflip
Copy link
Contributor

@plynchnlm that is correct yes. Currently check without a shrinkwrap will report: "what dependencies would you install if you were to run a clean npm install right now based on your package.json, and what vulnerabilities would they have. At the moment we don't check what is currently installed on whatever machine you are running check on.

As you suggest, in the future we could potentially list: "vulnerabilities in any versions that would match the semvers in your package.json", but we don't right now afaik. And yes, shrinkwrap is the alternative to ensure that wherever your application is installed, it will get consistent versions which have all been checked for vulns (vulns that were known at the point of you last running nsp check that is).

@evilpacket
Copy link

@plynchnlm you do have a point, there is a race condition with certain types of semver ranges. We do resolve the latest matching version in the registry and use that for the test so it's fairly accurate.

We highly recommend shrinkwrap as this is the way to get repeatable builds for production and really know what you are getting.

Thank you for your comments. Some of this is spurring new ideas and ways to report that we might be able to add in the future. We'll start with the docks and go from there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants