npm / rfcs

Public change requests/proposals & ideation
Other
730 stars 240 forks source link

[RRFC] `npm audit fix` should remove optional deps if necessary #672

Open isaacs opened 1 year ago

isaacs commented 1 year ago

Motivation ("The Why")

Sometimes an optional transitive dependency may have a security advisory against it, and there may be no way to fix it.

For example:

$ npm i express-prometheus-middleware@1.2.0

added 69 packages, and audited 136 packages in 3s

7 packages are looking for funding
  run `npm fund` for details

5 vulnerabilities (3 high, 2 critical)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

$ npm audit fix

up to date, audited 136 packages in 565ms

7 packages are looking for funding
  run `npm fund` for details

5 vulnerabilities (3 high, 2 critical)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

# oof

# but! check this out!

$ npm i express-prometheus-middleware --omit=optional

added 65 packages, and audited 66 packages in 460ms

7 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

Example

How

Current Behaviour

Desired Behavior

References

ljharb commented 1 year ago

I don't think that's a safe option to take - vulns are quite often false positives, and while we can assume that the thing for which it's optional can work without it, we can't assume that the behavior of the application will remain unbroken if the optional dep isn't present.

isaacs commented 1 year ago

If the application is depending on the optional dep, they should install it as a dependency, I'd think?

But yes, it would be a semver major change to npm for sure.

ljharb commented 1 year ago

What i mean is, a package could have it as an optional dep, and removing that optional dep is a breaking change for the package, because it might break consumers of the package.

khalyomede commented 1 year ago

I am for this feature as well and would like to provide another use case.

A package made a minor upgrade, and introduced a security issue. Yu fill in a new issue on the package (or you send an email to keep it private), and still want your CI to pass, and you could ask npm audit to just ignore the package until either the issue is fixed/you remove the package/you use another package/you stay on the version that have no security issue.