aquasecurity / trivy-action

Runs Trivy as GitHub action to scan your Docker container image for vulnerabilities
Apache License 2.0
806 stars 234 forks source link

severity set to CRITICAL but action returning all vulnerabilities when using Sarif format #95

Open larryclaman opened 2 years ago

larryclaman commented 2 years ago

I'm using the gh action to scan my container, and I have the severity field set to critical, but the scan seems to be returning ALL vulnerabilities. My code looks like:

    - name: Run Trivy vulnerability scanner
      uses: aquasecurity/trivy-action@2a2157eb22c08c9a1fac99263430307b8d1bc7a2
      with:
        image-ref: ${{ env.REGISTRYNAME }}.azurecr.io/${{ env.IMAGENAME }}:${{ github.sha }}
        format: 'template'
        template: '@/contrib/sarif.tpl'
        output: 'trivy-results.sarif'
        ignore-unfixed: true
        vuln-type: 'os,library'
        severity: 'CRITICAL'

    - name: Upload Trivy scan results to GitHub Security tab
      uses: github/codeql-action/upload-sarif@v1
      with:
        sarif_file: 'trivy-results.sarif'
simar7 commented 2 years ago

thanks @larryclaman for reporting this - we'll take a look

@krol3 could you check if your unit tests are able to reproduce this issue locally?

kgeorgiou commented 2 years ago

I believe this is because of the SARIF output format. According to the following lines, this is by design: https://github.com/aquasecurity/trivy-action/blob/a7a829a4345428ddd92ca57b18257440f6a18c90/entrypoint.sh#L138-L144

larryclaman commented 2 years ago

@kgeorgiou Thanks for pointing this out. Couple of thoughts:

  1. Why is this not documented? (eg why is this 'design choice' only buried in the code?) Note: I'd be happy to submit the simple PR to the README file pointing this out.
  2. As to the design choice, I need to ask: 'Why'? The comment says "SARIF is special. We output all vulnerabilities". Again, why? Could you point me to some background as to why this is the case? Why not let the use decide which vulnerabilities they want to receive? Git blame points me to: https://github.com/aquasecurity/trivy-action/commit/1ccef265f594a7555a720f623a461a3d69b45bf7 but it doesn't say why this choice was made.
larryclaman commented 2 years ago

cc @simar7 -- could you comment on the above? I'm trying to understand why sarif reports have special case to return ALL vulnerabilities rather than respecting the user's filter settings.

simar7 commented 2 years ago

@kgeorgiou Thanks for pointing this out. Couple of thoughts:

  1. Why is this not documented? (eg why is this 'design choice' only buried in the code?) Note: I'd be happy to submit the simple PR to the README file pointing this out.

Sorry we missed documenting it. Please feel free to send a PR.

  1. As to the design choice, I need to ask: 'Why'? The comment says "SARIF is special. We output all vulnerabilities". Again, why? Could you point me to some background as to why this is the case? Why not let the use decide which vulnerabilities they want to receive? Git blame points me to: https://github.com/aquasecurity/trivy-action/commit/1ccef265f594a7555a720f623a461a3d69b45bf7 but it doesn't say why this choice was made.

GitHub allows the user to filter vulnerabilities. The idea is to report all, irrespective of the level and only fail (return non zero exit code) if a vulnerability was over the user specified threshold.