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

Npm Audit Failure #208

Closed
Shimin-Zhang opened this issue Apr 4, 2019 · 8 comments
Closed

Npm Audit Failure #208

Shimin-Zhang opened this issue Apr 4, 2019 · 8 comments

Comments

@Shimin-Zhang
Copy link

2 of the dependencies of api spec converter 2.7.32, google-discovery-to-swagger and raml-to-swagger, both require an out-dated version of jsonpath/static-eval that has security vulnerabilities. (see https://www.npmjs.com/advisories/758 and https://www.npmjs.com/advisories/548).

Can you update your dependencies so they are no longer vulnerable?

@kgrubb
Copy link

kgrubb commented Apr 8, 2019

The npm audit review:
Screen Shot 2019-04-08 at 11 01 56 AM

Looks like google-discovery-to-swagger and raml-to-swagger aren't really maintained anymore. Perhaps there are other modules that can be used instead?

@rbren
Copy link
Collaborator

rbren commented Apr 12, 2019

Thanks all. I updated some deps using npm audit fix --force, and opened issues in google-discovery-to-swagger and raml-to-swagger. Those packages are still maintained (they were built by the same folks working on api-spec-converter), just quiet.

@RCBiczok
Copy link

Feel free to ask for help 👍

@cjolif
Copy link
Contributor

cjolif commented Jul 18, 2019

Per company rules I can't use this module because of this. If you can't get the dependencies fixed, wouldn't an easy fix on your side be to make those dependencies optional? Right now I don't convert from RAML or Google Discovery so I don't even need those packages but there are not optional and raise a red security flag :(

@rbren
Copy link
Collaborator

rbren commented Jul 18, 2019

That's a great idea! Looks like we'd just have to edit the code a bit to handle missing optionalDependencies.

I won't have time in the near future to get to this, but will happily accept a PR.

@cjolif
Copy link
Contributor

cjolif commented Jul 18, 2019

@bobby-brennan you're all set if you want to review see #218

@edmundo096
Copy link

They also could be on peerDependencies instead.

Based on the reading of the peer dependencies https://npm.github.io/using-pkgs-docs/package-json/types/peerdependencies.html, the formaters could be considered as plugins that the user should install manually as per requirement.

Furthermore, npm audit doesn't audits peers dependencies: https://docs.npmjs.com/auditing-package-dependencies-for-security-vulnerabilities#running-a-security-audit-with-npm-audit.
This would (in theory, haven't tried) avoid installing them and showing in audits until the user adds the required formaters on their dependency (which then it would be audited as far I understand).

The only downside is console spamming of WARNs for each of the peer dependencies.

@rbren
Copy link
Collaborator

rbren commented Aug 3, 2020

Dependencies are optional now. npm audit fix has also been run. Happy to reopen if there's anything here I missed.

@rbren rbren closed this as completed Aug 3, 2020
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

6 participants