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

[Feature] Ignore specific path of vulnerability #85

Closed
clement-escolano opened this issue May 17, 2019 · 5 comments
Closed

[Feature] Ignore specific path of vulnerability #85

clement-escolano opened this issue May 17, 2019 · 5 comments
Labels
enhancement New feature or request

Comments

@clement-escolano
Copy link
Contributor

clement-escolano commented May 17, 2019

Hello again :-)

I am using this project regularly and one thing bothers me a bit. When I have a vulnerability alert from a nested dependency that does not affect me and there is no upgrade possible, I have to ignore the advisory.
For instance, the braces dependency with advisory 786 is a dependency of jest so I am OK to ignore it. However, I will not be warned in the future if a dependency used in the production build uses braces and I will be exposed to a Regular Expression Denial of Service.

I would like to ignore path only and not global advisories. The package npm-audit-resolver (similar to audit-ci) does this for instance.

Thank you again for this package

What do you think about it? I am willing to write a pull request for this issue if you agree.

@quinnturner
Copy link
Member

I agree that this is an issue, 100%. I would definitely be open to a PR for it!

Sorry for the late response, am on vacation now, just got Wi-Fi 😀

@quinnturner quinnturner added the enhancement New feature or request label May 22, 2019
@clement-escolano
Copy link
Contributor Author

No problem 😉

Before starting a PR, how do you think we should handle this? I was thinking an option like whitelisted-paths which will include a list of the whitelisted paths with the related advisory.

The content of the option would be 476|dependency>nested-vulnerable-dependency (with 476 the advisory number). What do you think?

@quinnturner
Copy link
Member

whitelisted-paths is a clear option, I like it!

I also like the <advisory #>|dep1>dep2>...>depn approach, works well for CLI as well as config.

One consideration I would look for in this PR is for audit-ci to tell the user if there's a whitelisted-path that is not actually vulnerable at or above their level; either due to a typo or a vulnerability that has been addressed by an update. You can see an example of this in common.js:

const found = summary.whitelistedAdvisoriesNotFound.join(', ');
const msg = `Vulnerable whitelisted advisories not found: ${found}.\nConsider not whitelisting them.`;
console.warn('\x1b[33m%s\x1b[0m', msg);

This can be revised to be whitelisted paths.

@clement-escolano
Copy link
Contributor Author

OK thank you for the feedback. I will look into it next week and open a PR as soon as I have something.

@quinnturner
Copy link
Member

Introduced with #104 in release v2.3.0 😄 This makes the package significantly more secure and useful, thank you!

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

No branches or pull requests

2 participants