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

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

Open
darakian opened this issue Oct 20, 2023 · 6 comments
Open

Comments

@darakian
Copy link

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

github/advisory-database#2701 (comment)

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

@ljharb
Copy link
Contributor

ljharb commented Oct 20, 2023

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)?

@darakian
Copy link
Author

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 ¯\_(ツ)_/¯

@wesleytodd
Copy link

wesleytodd commented Oct 20, 2023

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
Copy link

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.

@KateCatlin
Copy link

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

@wesleytodd
Copy link

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?

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

No branches or pull requests

4 participants