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 request: Disable exit code on vulnerability count #204

Open
PPACI opened this issue Nov 27, 2020 · 6 comments
Open

Feature request: Disable exit code on vulnerability count #204

PPACI opened this issue Nov 27, 2020 · 6 comments

Comments

@PPACI
Copy link

PPACI commented Nov 27, 2020

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

  • What are you trying to do?
  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.

  • What feature or behavior is this required for?

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

  • How could we solve this issue? (Not knowing is okay!)

if count := audit.LogResults(configOssi.Formatter, packageCount, coordinates, invalidCoordinates, configOssi.CveList.Cves); count > 0 {

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

  • Anything else?

Thank you for your hard work!

cc @bhamail / @DarthHater

@zendern
Copy link
Contributor

zendern commented Nov 27, 2020

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

PPACI commented Dec 3, 2020

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

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

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 zendern closed this as completed Dec 3, 2020
@zendern zendern reopened this Dec 3, 2020
@zendern
Copy link
Contributor

zendern commented Dec 3, 2020

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

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?

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

4 participants