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

Unexpected behaviour when scanning with SARIF formatting #142

Open
jonpulsifer opened this issue Jul 5, 2022 · 3 comments
Open

Unexpected behaviour when scanning with SARIF formatting #142

jonpulsifer opened this issue Jul 5, 2022 · 3 comments

Comments

@jonpulsifer
Copy link

jonpulsifer commented Jul 5, 2022

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?

Similar #95 and #153

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.

trivy $GLOBAL_ARGS ${scanType} $ARGS ${artifactRef}

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

trivy-action/entrypoint.sh

Lines 165 to 171 in 7b7aa26

# SARIF is special. We output all vulnerabilities,
# regardless of severity level specified in this report.
# This is a feature, not a bug :)
if [[ "${format}" == "sarif" ]]; then
echo "Building SARIF report with options: ${SARIF_ARGS}" "${artifactRef}"
trivy --quiet ${scanType} --format sarif --output ${output} $SARIF_ARGS ${artifactRef}
fi

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

@simar7
Copy link
Member

simar7 commented Jul 7, 2022

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

👋 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.

@simar7
Copy link
Member

simar7 commented Aug 2, 2022

👋 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 #156

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

2 participants