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

BUG - fail-build behavior takes on severity-cutoff unintentionally #378

Open
RLI-Rdeaton opened this issue Oct 2, 2024 · 10 comments
Open
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@RLI-Rdeaton
Copy link

Consider the following action:

  grype:
    needs: build
    runs-on: ubuntu-latest
    steps:
      -
        name: Scan image
        uses: anchore/[email protected]
        id: grypescan
        with:
          registry-username: ${{ vars.DOCKERHUB_USERNAME }}
          registry-password: ${{ secrets.DOCKERHUB_TOKEN }}
          image: ${{ env.IMAGE_NAME }}
          fail-build: false
      -
        name: upload Anchore scan SARIF report
        uses: github/codeql-action/upload-sarif@v2
        with:
          sarif_file: ${{ steps.grypescan.outputs.sarif }}

This action runs, however it does not appear to respect the fact that I have set fail-build to false. Per discussion with @spiffcs this appears to be a few different bugs (https://anchorecommunity.discourse.group/t/seeing-a-real-weird-issue-with-github-actions-for-scan-action/156/2). Here's the output:

Warning: Unexpected input(s) 'registry-username', 'registry-password', valid inputs are ['image', 'path', 'sbom', 'fail-build', 'output-format', 'severity-cutoff', 'only-fixed', 'add-cpes-if-none', 'by-cve', 'grype-version', 'vex']
Run anchore/[email protected]
  with:
    registry-username: modusmundi
    registry-password: ***
    image: modusmundi/testingscap:latest
    fail-build: false
    output-format: sarif
    severity-cutoff: medium
    only-fixed: false
    add-cpes-if-none: false
    by-cve: false
  env:
    REGISTRY: docker.io
    IMAGE_NAME: modusmundi/testingscap:latest
/usr/bin/sh /home/runner/work/_temp/30bc3b5b-920a-4713-8c7c-7286ced65a11 -d -b /home/runner/work/_temp/30bc3b5b-920a-4713-8c7c-7286ced65a11_grype v0.80.0
[info] checking github for release tag='v0.80.0' 
[debug] http_download(url=https://github.com/anchore/grype/releases/v0.80.0) 
[info] fetching release script for tag='v0.80.0' 
[debug] http_download(url=https://raw.githubusercontent.com/anchore/grype/v0.80.0/install.sh) 
[info] checking github for release tag='v0.80.0' 
[debug] http_download(url=https://github.com/anchore/grype/releases/v0.80.0) 
[info] using release tag='v0.80.0' version='0.80.0' os='linux' arch='amd64' 
[debug] downloading files into /tmp/tmp.8dFny5tNmB 
[debug] http_download(url=https://github.com/anchore/grype/releases/download/v0.80.0/grype_0.80.0_checksums.txt) 
[debug] http_download(url=https://github.com/anchore/grype/releases/download/v0.80.0/grype_0.80.0_linux_amd64.tar.gz) 
[info] installed /home/runner/work/_temp/30bc3b5b-920a-4713-8c7c-7286ced65a11_grype/grype 
grype output...
  Executing: grype -o sarif --fail-on medium modusmundi/testingscap:latest
  discovered vulnerabilities at or above the severity threshold

I'm reporting it so it does not get lost- what I would expect here is to be able to not fail a build and get a full outlay of current vulns in the build.

@RLI-Rdeaton RLI-Rdeaton changed the title BUG - BUG - fail-build behavior takes on severity-cutoff unintentionally Oct 2, 2024
@RLI-Rdeaton
Copy link
Author

@spiffcs Hey there, is there any plan to fix this in the near future? I'm getting back to this part of my set of actions and I'm looking to understand if I should just be building out my own workflow to run grype to meet my needs this point. It's totally valid if this is low priority for you all, just trying to figure out my path forward.

@willmurphyscode willmurphyscode added the bug Something isn't working label Oct 16, 2024
@willmurphyscode
Copy link
Contributor

willmurphyscode commented Oct 16, 2024

Hi @RLI-Rdeaton I just had a quick look here and I am not able to reproduce, so I feel like I must be missing something. Would you mind taking a look and seeing what's different in the way I'm using the action?

Here's what I did to test:

  1. I made a repo where I can just run scan-action on demand for different images: https://github.com/willmurphyscode/action-troubleshooting/blob/main/.github/workflows/test-scan-action.yml
  2. I ran with fail-build: true, and you can see this job is marked as failed: https://github.com/willmurphyscode/action-troubleshooting/actions/runs/11371180917/job/31632656903#step:2:4
  3. I ran with fail-build: false, and you can see this job is NOT marked as failed: https://github.com/willmurphyscode/action-troubleshooting/actions/runs/11371168527/job/31632612484#step:2:4

When I pass fail-build: false, the workflow passes (gets the green checkmark from GitHub) but still warns that the scan failed. Are you seeing different behavior than this?

Can you help me understand what I'm missing? @spiffcs what testing did you do that I might have missed?

If it turns out I'm using the action differently than you all were, I'll update my workflow and try to get a public run that reproduces the bug to link here.

Thanks!

@willmurphyscode
Copy link
Contributor

I realized in your sample you were on an older version than I am, so I rolled back to check that:

So I still don't understand what I'm missing.

@willmurphyscode
Copy link
Contributor

To answer your question directly @RLI-Rdeaton I'm happy to fix this fairly promptly if I can reproduce it and understand it 😄

@spiffcs
Copy link
Contributor

spiffcs commented Oct 17, 2024

@willmurphyscode I set up the action locally on a local private/personal repository

  • Running it again today the fail-build option works as expected.
  • The registry-username and registry-password are correctly working, but need to be added as expected inputs in the next release. There are warnings that might make a user think there is something wrong
  • Finally here is what my action says when trying to upload the sarif report. I think we're missing adding the fingerprint unless that was updated in latest? Let me double check that a grype update didn't fix this
Run github/codeql-action/upload-sarif@v3
Warning: Caught an exception while gathering information for telemetry: HttpError: Resource not accessible by integration. Will skip sending status report.
Uploading results
  Processing sarif files: ["./results.sarif"]
  Validating ./results.sarif
  Combining SARIF files using the CodeQL CLI
  Adding fingerprints to SARIF file. See https://docs.github.com/en/enterprise-cloud@latest/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#providing-data-to-track-code-scanning-alerts-across-runs for more information.
  Error: Resource not accessible by integration
  Warning: Caught an exception while gathering information for telemetry: HttpError: Resource not accessible by integration. Will skip sending status report.

@spiffcs
Copy link
Contributor

spiffcs commented Oct 17, 2024

Just commenting back here I do still see this issue, but don't have the bandwidth at the moment to try and dig in and fix:

Run github/codeql-action/upload-sarif@v2
Warning: CodeQL Action v2 will be deprecated on December 5th, 2024. Please update all occurrences of the CodeQL Action in your workflow files to v3. For more information, see https://github.blog/changelog/2024-01-12-code-scanning-deprecation-of-codeql-action-v2/
Warning: Caught an exception while gathering information for telemetry: HttpError: Resource not accessible by integration. Will skip sending status report.
Uploading results
  Processing sarif files: ["./results.sarif"]
  Validating ./results.sarif
  Combining SARIF files using the CodeQL CLI
  Adding fingerprints to SARIF file. See https://docs.github.com/en/enterprise-cloud@latest/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#providing-data-to-track-code-scanning-alerts-across-runs for more information.
  Error: Resource not accessible by integration
  Warning: Caught an exception while gathering information for telemetry: HttpError: Resource not accessible by integration. Will skip sending status report.

Here is my config which was originally configured to check the fail-build and registry-* fields.

name: "scan action test ground"

on:
  # allow running on-demand
  workflow_dispatch:

jobs:
  dogfood:
    runs-on: ubuntu-20.04
    steps:
      - name: Scan image
        id: scan
        uses: anchore/[email protected]
        with:
          image: "caphill4/test:latest"
          fail-build: false
          registry-username: ${{ secrets.REGISTRY_USERNAME }}
          registry-password: ${{ secrets.REGISTRY_PASSWORD }}
      - name: upload the sarif report
        uses: github/codeql-action/upload-sarif@v2
        with:
          sarif_file: ${{ steps.scan.outputs.sarif }}

@RLI-Rdeaton
Copy link
Author

So looking at this again I think the crux of my issue was 'fail-build: false' still triggers warnings because 'severity-cutoff: medium' is set, and there's no way to disable severity-cutoff. I ended up just writing something to pull down grype and scan for me so this is moot at this point, but I found the behavior confusing for my use case. If that's just the way the action is expected to work, that's fine, and I guess it's on me. This can be closed if the behavior for severity-cutoff won't change.

@willmurphyscode willmurphyscode self-assigned this Oct 17, 2024
@willmurphyscode willmurphyscode moved this to In Progress in OSS Oct 17, 2024
@willmurphyscode
Copy link
Contributor

@RLI-Rdeaton I agree that this is confusing behavior. severity-cutoff: medium really maps to grype's --fail-on medium flag, so we're telling grype "fail on medium and also don't fail," which is confusing for everyone. Thanks for the detailed bug report and the feedback here!

I think we should really have 2 different inputs:

  1. min-severity: medium meaning, "don't bother me about CVEs with severity less than medium; filter them from the output` and
  2. fail-on-severity: medium meaning, "fail the workflow step if there's a CVE at medium or higher severity", and not have fail-build: as an input at all.

I think what's really going on here is that you expected ( quite reasonably! ) that passing severity-cutoff: medium and fail-build: false to mean, "complain about CVEs that are medium and higher, but don't fail the build either way." The action technically behaves this way, but the output is super confusing, both from scan-action and from codeql-action/upload-sarif.

We have some discussion over on discourse about making Grype's output less noisy with regard to severity cutoffs and the like.

I'll keep this issue open to track cleaning up the logging behavior at least, so Grype doesn't log like it's failing the build. Maybe something like Detected severity above <severity-cutoff>, but continuing because fail-build is false would have been enough.

@RLI-Rdeaton
Copy link
Author

Also having a mechanism to disable the severity cutoff (As none exists, looking in the code real quick) would be good too.

E.G.: If fail-on-severity isn't set, or is set to "none", don't fail and just report.

This isn't pressing anymore, but that way someone who may just be running grype for visibility instead of a gate isn't hit with warnings unnecessarily.

@willmurphyscode
Copy link
Contributor

Developer notes: this issue now tracks two changes to scan-action:

  1. If severity-cutoff and fail-build: false are both passed, clearly state in logs that the min severity failed, but that the action is continuing because of fail-build: false
  2. There should be a value you can set for severity-cutoff that means "don't pass --fail-on to grype at all" (right now it defaults to "medium" and validates that it's a valid severity, so there's no way to pass nothing).

@willmurphyscode willmurphyscode moved this from In Progress to Ready in OSS Oct 17, 2024
@willmurphyscode willmurphyscode added the good first issue Good for newcomers label Oct 18, 2024
@willmurphyscode willmurphyscode removed their assignment Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
Status: Ready
Development

No branches or pull requests

3 participants