search

tnobody / lerna-audit

Micro util to run npm audit in lerna monorepos
9 stars 11 forks source link

This module does not return the right status-codes. #21

Open IgnorantSapient opened 3 years ago

IgnorantSapient commented 3 years ago

Let's say I run lerna-audit from the root of my directory with the --no-fix option, and vulnerabilities are found, running echo $? to check the execution status does not return a non zero status code.

For instance npm audit --audit-level=high returns a status code of 1 if high | critical vulnerabilities are found.

Would you be open to adding the right status code outputs to this CLI tool along with the --audit-level flag? This would come in handy when using with a build / CI system, I am happy to send a PR, please let me know.....

svettwer commented 3 years ago

Hiho :wave:

In my opinion, passing additional parameters to npm audit is never a problem. Concerning the return code change, I think this would be a breaking change. Given you use lerna-audit in a CI/CD pipeline: A non zero return code would cancel the pipeline. Why would you like to change the return code? What's your use case?

Nevertheless, it might make sense to rethink the return codes when we put out a new major version.

alexmcmanus commented 3 years ago

I have the same requirement. Given that lerna-audit is a wrapper around npm audit, I'd expect it to match exit codes. In my case, I want the CI pipeline to fail if there are vulnerabilities - it's too easy to ignore automated checks that don't fail the build.

svettwer commented 3 years ago

As there are not too much requirements atm, I think putting out a major version would be okay. The only limiting factor is time, as always. :hourglass: :smile:

hiro5id commented 3 years ago

I also have the same requirement. We use npm audit in gating tests part of the CI pipeline to check for vulnerabilities. The CI pipeline needs to be blocked if vulnerabilities show up. We don't want to deploy apps with vulnerabilities.