Unexpected behaviour when scanning with SARIF formatting

jonpulsifer commented 2 years ago

Wat

The action runs trivy twice when the format is set to sarif, so some options are not respected (including timeouts)

imo we should remove the second trivy run altogether, since it's mostly duplication, but thought I should open an issue before a PR 😄 .. any concerns?

Expectation

Given the following step:

- name: Run Trivy image scanner
  uses: aquasecurity/trivy-action@0.5.1
  timeout-minutes: 10
  with:
    scan-type: image
    vuln-type: os
    image-ref: ${{inputs.repository}}/${{ inputs.image }}
    format: sarif
    output: trivy-image.sarif
    ignore-unfixed: true
    hide-progress: false
    security-checks: vuln
    timeout: 10m0s

I was expecting the step to succeed in under 10 minutes.

Reality

It fails with a timeout after 8 minutes.

2022-07-04T20:38:08.520Z    FATAL   image scan error: scan error: image scan failed: failed analysis: analyze error: timeout: context deadline exceeded

Cause

It runs trivy twice. Once with a timeout, and one without.

Additionally, the timeouts in my case are caused by secret scanning on a large image, which would not be disabled when trivy runs the second time.

https://github.com/aquasecurity/trivy-action/blob/7b7aa264d83dc58691451798b4d117d53d21edfe/entrypoint.sh#L162

Resolves to: trivy image --format sarif --ignore-unfixed --vuln-type os --security-checks vuln --severity UNKNOWN,LOW,MEDIUM,HIGH,CRITICAL --output trivy-image.sarif --timeout 10m library/alpine

https://github.com/aquasecurity/trivy-action/blob/7b7aa264d83dc58691451798b4d117d53d21edfe/entrypoint.sh#L165-L171

Resolves to: trivy --quiet image --format sarif --output trivy-image.sarif --ignore-unfixed --vuln-type os library/alpine

simar7 commented 2 years ago

hi @jonpulsifer - as mentioned in the issue #95, this is currently by design. We want all vulnerabilities that exist in a repository to be submitted as SARIF results to GitHub. If you think this isn't desired and would rather not have it, we'll take your feedback.

If you have any other ideas on how we can implement this better (without running the scan twice), we're happy to hear from you.

jonpulsifer commented 2 years ago

:wave: pardon the delay and thanks for the response!

In our case:

we do not need trivy to perform a secret scan because we have another tool that does this (and it also dramatically increases the action runtime for large containers)
we do not need trivy to execute twice, as the first scan is using --format sarif with the customizations we choose

A specific fix for us would be to append timeout and security-checks to SARIF_ARGS and adjust the entrypoint to look like this:

if [ $trivyConfig ]; then
   echo "Running Trivy with trivy.yaml config from: " $trivyConfig
   trivy --config $trivyConfig ${scanType} ${artifactRef}
   returnCode=$?
elif [[ "${format}" == "sarif" ]]; then
   # SARIF is special. We output all vulnerabilities,
   # regardless of severity level specified in this report.
   # This is a feature, not a bug :)
   echo "Building SARIF report with options: ${SARIF_ARGS}" "${artifactRef}"
   trivy --quiet ${scanType} --format sarif --output ${output} $SARIF_ARGS ${artifactRef}
   returnCode=$?
else
   echo "Running trivy with options: ${ARGS}" "${artifactRef}"
   echo "Global options: " "${GLOBAL_ARGS}"
   trivy $GLOBAL_ARGS ${scanType} $ARGS ${artifactRef}
   returnCode=$?
fi

In any case, it sounds like adding timeout and security-checks to SARIF_ARGS would provide some relief. https://github.com/aquasecurity/trivy-action/issues/153 is essentially a duplicate.

simar7 commented 2 years ago

👋 pardon the delay and thanks for the response!

In our case:

we do not need trivy to perform a secret scan because we have another tool that does this (and it also dramatically increases the action runtime for large containers)

we do not need trivy to execute twice, as the first scan is using --format sarif with the customizations we choose

A specific fix for us would be to append timeout and security-checks to SARIF_ARGS and adjust the entrypoint to look like this:
if [ $trivyConfig ]; then
   echo "Running Trivy with trivy.yaml config from: " $trivyConfig
   trivy --config $trivyConfig ${scanType} ${artifactRef}
   returnCode=$?
elif [[ "${format}" == "sarif" ]]; then
   # SARIF is special. We output all vulnerabilities,
   # regardless of severity level specified in this report.
   # This is a feature, not a bug :)
   echo "Building SARIF report with options: ${SARIF_ARGS}" "${artifactRef}"
   trivy --quiet ${scanType} --format sarif --output ${output} $SARIF_ARGS ${artifactRef}
   returnCode=$?
else
   echo "Running trivy with options: ${ARGS}" "${artifactRef}"
   echo "Global options: " "${GLOBAL_ARGS}"
   trivy $GLOBAL_ARGS ${scanType} $ARGS ${artifactRef}
   returnCode=$?
fi
In any case, it sounds like adding timeout and security-checks to SARIF_ARGS would provide some relief. #153 is essentially a duplicate.

hi @jonpulsifer - thanks for explaining, that makes sense. I've cut a PR for it https://github.com/aquasecurity/trivy-action/pull/156

aquasecurity / trivy-action