rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

[RRFC] npm audit fix should consider package source in alert delivery

Open darakian opened this issue 1 year ago • 6 comments

Motivation ("The Why")

At the moment npm audit does not seem to take into account the origin source of a package when delivering alerts. This results in alerts which are written specifically for packages hosted on npm to be delivered erroneously to users who pull packages of matching names from other locations.

Example

https://github.com/github/advisory-database/issues/2701#issuecomment-1772006702

How

Current Behaviour

Consider the package source when resolving npm audit alerts

Desired Behaviour

Alerts do not get erroneously delivered.

References

GitHub advisory database ecosystem definitions: https://github.com/github/advisory-database/#supported-ecosystems

darakian avatar Oct 20 '23 21:10 darakian

Non-scoped packages from other locations are generally inadvisable; is there a reason they're not scoped (with a scope you own on the public registry, to avoid a supply chain attack vector)?

ljharb avatar Oct 20 '23 21:10 ljharb

Non-scoped packages from other locations are generally inadvisable

Agreed.

is there a reason they're not scoped

In the example provided it seems to be that the project simply doesn't publish to npm. Not my code, not my project, so I can't speak to the motivation ¯\_(ツ)_/¯

darakian avatar Oct 20 '23 21:10 darakian

Even if "they should be scoped" I think it is common enough that it is worth discussing a fix. Unfortunately a fix here will be hard. I think the main issue is that most companies should be pointing all of their installs through an internal proxy, so the tarball urls in the lockfiles will all point internally even if they are just proxied from the public registry.

And my guess is that most people are not implementing their own internal audit endpoints. I could be wrong, but likely most folks just proxy right on to public npm for that. I think the audit would need the some of the local config to be able to make correct decisions. And honestly it might even need the out of band information from the internal registry proxy.

wesleytodd avatar Oct 20 '23 22:10 wesleytodd

I just took a look at the linked report, and in the case of a git dependency I think the lockfile does have enough information to disambiguate from the registry. I am guessing that the audit needs to only report on tag, version, and range specs?

Still though, the problem is larger than just the git refs.

wesleytodd avatar Oct 20 '23 22:10 wesleytodd

Hey @wesleytodd just checking in on this issue. Anything we should report back to the user who posted it with?

KateCatlin avatar Nov 16 '23 17:11 KateCatlin

Hm, not sure. I would think that @lukekarrys or @wraithgar might need to answer that. I am guessing the GH advisory db or whatever reporting systems are involved have not been updated to take into account the spec type like I mentioned above?

wesleytodd avatar Nov 16 '23 17:11 wesleytodd