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 remove optional deps if necessary #672

Open
isaacs opened this issue Jan 25, 2023 · 4 comments
Open

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

isaacs opened this issue Jan 25, 2023 · 4 comments

Comments

@isaacs
Copy link
Contributor

isaacs commented Jan 25, 2023

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 [email protected]

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

  • n/a
@ljharb
Copy link
Contributor

ljharb commented Jan 25, 2023

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

isaacs commented Jan 29, 2023

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

ljharb commented Jan 29, 2023

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

khalyomede commented May 14, 2023

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.

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

3 participants