npm audit for a not yet installed package

[Christian24]

Describes functionality for the CLI that allows users to request an advisory (aka npm audit report) on a package before installing it.

References

Closes #223

[ruyadorno]

I believe it should be an opt-out, so everytime an user runs npm view <pkg> it should notify you about any vulns by default, so in order to opt-out you'd need to npm view <pkg> --no-audit and any users that already chose opt-out from audits in their .npmrc files/configs (using audit=false) will already be opting-out here.

[ljharb]

@ruyadorno how would that affect npm view --json? Would it risk slowing down this often-used command?

[Christian24]

So, even though there have been no new comments here, I was pretty convinced last week that putting this information into the npm audit command makes the most sense (as was also the plan of the #223), since that is the context most users are familar with audit results.

However, @ruyadorno I agree with you that this is package data and it should possibly be available as output by npm view by default. So, I added the output to npm view too.

@ljharb I added a --no-audit parameter for you, which makes this more in line with install.

I have updated the RFC accordingly.

[darcyclarke]

@Christian24 I think we only discussed making this optional information for npm view (potentially behind a flag like --with-audit); I recall us not wanting to bloat/slow down npm view since all of the information shown in that command can be fetched from the registry Packument (ie. package + document) today.

I'd probably revert & keep this RFC scoped to npm audit x for now & update it to include references into how we would implement this.

As I think I also noted in the last call, we're going to probably need to create a temp directory to install into & resolve the package's dependencies. How the information will look when it's logged from npm audit x is probably subtly different from how npm audit looks today as well (ie. we'd probably want to remove references to addressing/resolving issues w/ "npm audit fix --force" or "Will install <x>, which is a breaking change" etc.). Good starting points to look for where we'd update/add support are in - lib/utils/audit.js, lib/utils/reify-output.js & npm-audit-reporter).

I think we can circle back on this once you've updated/added detail on how the implementation/output would work/look (as noted above).

P.S. For fun, I wrote a quick bash script to do this/walk through the steps in the interim https://gist.github.com/darcyclarke/6d9e9de555997e9aa9fe828fe1fdef7d

[ljharb]

Do note, because of file deps, creating a temp directory for this will be a bit trickier than it appears: ljharb/aud#2

[Christian24]

@darcyclarke thanks for the feedback.

@Christian24 I think we only discussed making this optional information for npm view (potentially behind a flag like --with-audit); I recall us not wanting to bloat/slow down npm view since all of the information shown in that command can be fetched from the registry Packument (ie. package + document) today.

Not that I am against this, but I remember @ruyadorno pointing out it should be opt-out and @ljharb has a use case for view, where he uses parameters anyway and so another parameter seemed fine to me, especially since this makes the parameter in line with install.

I'd probably revert & keep this RFC scoped to npm audit x for now & update it to include references into how we would implement this.

Okay, fine by me. I think adding it to audit is definitely enough for now. It just seemed to me some people wanted to add this to view too.

As I think I also noted in the last call, we're going to probably need to create a temp directory to install into & resolve the package's dependencies.

How the information will look when it's logged from npm audit x is probably subtly different from how npm audit looks today as well (ie. we'd probably want to remove references to addressing/resolving issues w/ "npm audit fix --force" or "Will install <x>, which is a breaking change" etc.). Good starting points to look for where we'd update/add support are in - lib/utils/audit.js, lib/utils/reify-output.js & npm-audit-reporter).

There is some info in the RFC already, but I will check it against v7 and will try to update with some implementation details.

I think we can circle back on this once you've updated/added detail on how the implementation/output would work/look (as noted above).

P.S. For fun, I wrote a quick bash script to do this/walk through the steps in the interim https://gist.github.com/darcyclarke/6d9e9de555997e9aa9fe828fe1fdef7d

I will try the script tomorrow at work and see if it works in our strict environment. :)

[ruyadorno]

Okay, fine by me. I think adding it to audit is definitely enough for now. It just seemed to me some people wanted to add this to view too.

I was on the camp of moving it to npm view when first proposed but the more I think about it the more I think it should really live in npm audit instead as initially proposed (really sorry about the back and forth @Christian24 😬).

Making the link with a previous discussion we had about validating license files, it was proposed that the license validation should also live along with audit and the main reason being that auditing was from its inception always intended to be more than just vulnerabilities check and it should encompass multiple kinds of validation for your project.

With that in mind it seems to me that having individual auditing of packages live in npm audit is a better long-term solution and it could also potentially provide these different validations (like license scan) if they ever get implemented.

[Christian24]

@darcyclarke I tried your script. With npm v6 at least I get a 403 here. Couldn't try npm v7, because corporate restrictions. install --package-lock-only seems to get the 403.

@ruyadorno Understood. I will remove all references to view from the RFC.

[Christian24]

Sorry folks, this took quite a while. Since @darcyclarke's script does not work for me, I tried to go the route of adding a new endpoint for the CLI to query. I added some ideas of how this could be implemented. This might potentially be wrong, so it would be cool if @ruyadorno, @isaacs or someone from the CLI team could take a look. Additionally, I added some information on the fix part of the npm audit report. If possible it would be cool to keep this output, but only change it slightly to give users information on whetever issues can be fixed.