sonatype-nexus-community / nancy

A tool to check for vulnerabilities in your Golang dependencies, powered by Sonatype OSS Index
Apache License 2.0
549 stars 76 forks source link

Feature request: Disable exit code on vulnerability count #204

Open PPACI opened 3 years ago

PPACI commented 3 years ago

Hi Sonatype. I would like to introduce a feature request to ease some use case in CI/CD

  1. Use Nancy in a CI/CD Pipeline
  2. Read the output
  3. If vulnerabilities are found, Nancy exit with the number of vulnerabilities founds, immediately crashing the CI/CD

this is a very interesting feature, but in some case, I would like to be able to disable this.

Being able to do something like Nancy sleuth --no-exit

https://github.com/sonatype-nexus-community/nancy/blob/ab916e3855a4e147ffd50f18b777cc21cc7f9b2a/internal/cmd/root.go#L449

Maybe we could do something like count > 0 && exitFlag?

Thank you for your hard work!

cc @bhamail / @DarthHater

zendern commented 3 years ago

@PPACI Thanks for the suggestion but I think you might be able to do that without modifying Nancy at all. Examples below are for linux but there might be a way to do it if on windows if that is your ci environment.

Something like so

set -e
EXIT_CODE=0
./nancy --sleuth || EXIT_CODE=$?
echo $EXIT_CODE

That assumes you want to keep the exit code around for later. If you want to bail on the exit code altogether you can just do this.

./nancy --sleuth || true

Also I'm curious can you describe what you are trying to do a little more?? Nancy is setup that way so that the feedback loop of when there is a vulnerability discovered you know about it immediately and can address it by fixing it or adding a temporary/permanent exclusion.

PPACI commented 3 years ago

Thank you for the answer. Your suggestion (the first one), is exactly what I do.

We're also using different analyser such as Trivy, which has the following option :https://github.com/aquasecurity/trivy#image-1

   --severity value    severities of vulnerabilities to be displayed (comma separated) (default: "UNKNOWN,LOW,MEDIUM,HIGH,CRITICAL") [$TRIVY_SEVERITY]
   --exit-code value   Exit code when vulnerabilities were found (default: 0) [$TRIVY_EXIT_CODE]

This feature proposal was more to be on par feature wise with other security scanner than a real need (which can be addressed as you said by bash directly).

DarthHater commented 3 years ago

My preference here would be to not exit with the vulnerability count as the code (because it is a misuse of POSIX standards anyways), and have it just exit with 1 if it finds vulns. I think that's more reliable to people to begin with.

The --exit-code thing we never really did because it's fairly trivial to do the ./nancy sleuth || true, but I wouldn't be against it if you got a good example of how it's more helpful than the bash version. In my mind the only way it's really helpful is that you'd see legit errors with Nancy (which can happen if OSS Index, etc... went down). Ignoring vulns is an anti-practice though so we've been hesitant to do it (promotes bad behavior), and more so recommend using a .nancy-ignore etc... type file if you truly want to ignore stuff.

DarthHater commented 3 years ago

Also, thanks for filing an issue, always nice to get to chat with someone in the community! Feel free to send us a PR if you want, as well, we love contributions!

zendern commented 3 years ago

Sorry about the accidental close 🤦‍♂️

I'm with @DarthHater on how it feels like it could promote bad behavior. You just put this little exit-code flag in place in your build pipeline and tada no more vulns ever again (if even not intentional). But maybe if you use the flag the intention is more that you plan on using the output and doing more with it later anyways.

deadlysyn commented 2 years ago

in summary i gather:

  1. need to be mindful not to promote bad behavior
  2. errors should exit 1 vs vuln count (values >1 and <=255 have special meaning)
  3. ensure useful output when exiting on error

so the smallest thing we have agreement to do is adjust behavior per 2, but may also need (have not looked closely) to think about what if anything we can improve for 3.

mention of crashing CI/CD/halting build such as not to allow reading logs made me think we may provide less output when exiting at this point, but that may also be simple build system behavior (e.g. reap container w/ no external logging).

since this is pretty stale, is there enough consensus for me to generate a PR for 2?