feat: adds support for enhanced continuous scanning

[jdew89]

Adding support for enhanced continuous scanning in ECR. This setting returns a different JSON structure which breaks the action.

This PR resolves #45

The changes are as follows:

• A status of COMPLETE and ACTIVE are now acceptable. ACTIVE is the status given for enhanced continuous scans.
• Will now handle both basic and enhanced scans by looking for the property: imageScanFindings.enhancedFindings or imageScanFindings.findings
• Can find the CVE names for both basic and enhanced by looking at imageScanFindings.enhancedFindings.[].packageVulnerabilityDetails.vulnerabilityId for enhanced scans.
• Updates the findings details output section with new properties imageScanFindings.enhancedFindings.[].packageVulnerabilityDetails.vulnerablePackages and imageScanFindings.enhancedFindings.[].packageVulnerabilityDetails.cvss

[alexjurkiewicz]

Thanks for this PR! It's a long-overdue enhancement.

One concern I have is that this PR duplicates key code paths to handle differences in basic & enhanced scan message formats.

Why don't we drop basic scan support?

This action has history of breaking backwards compatibility on the master branch, when v2.0.0 was released last year. So I think it's fine to once again break backwards compat and drop basic scanning support.

[jdew89]

Yeah, I'm disappointed AWS made them have different JSON structures. Makes this more complicated than it really needs to be. I could remove the basic scanning support and you could bump to version 3.0. People looking for basic scanning support could still use v2.0.1 and enhanced scanners can use v3.0. Is that what you would like to do?

[alexjurkiewicz]

Yes please!

[pzi]

My first thought was to make it a setting and assume "Basic" until a "useEnhancedScan" feature flag is set to true. Having said that, I don't use this workflow any more... whatever works best for the people using it :)

[alexjurkiewicz]

I saw this AWS announcement, basic scanning will get some new responses soon anyway. Seems like a good time to drop support:

Starting May 1, 2024, you will receive the following error when using describe-image-scan-findings API [2] or describe-images API [3] to retrieve the scan findings for images running an operating system version that is not in our supported version list [1]:

    "imageScanStatus": {
        "status": "FAILED",
        "description": "UnsupportedImageError: The operating system '<OS>' version '<version>' is not supported."
    }

You will receive the following error message when attempting to view scan findings for those images via the AWS console:
"UnsupportedImageError: The operating system '<OS>' version '<version>' is not supported."

In addition, you will see a ‘FAILED’ status in the ‘detail’ field of the EventBridge event [4] that is sent when the image scan is completed.
{
    "version":"0",
    "id":"7c19afad-7203-e3ae-61c9-5a905e6963d2",
    "detail-type":"ECR Image Scan",
    "source":"aws.ecr",
    "account":"<accountId>",
    "time":"<timestamp>",
    "region":"<region>",
    "resources":["<resource_arn>"],
    "detail": {
        "scan-status":"FAILED",
        "repository-name":"<repository_name>",
        "image-digest":"<image_digest>",
        "image-tags":["<image_tags>"]
    }
}

For continued coverage, please use supported operating system versions and make the necessary updates prior to May 1, 2024. If you have any questions or concerns, please contact AWS Support [5]

[1] https://docs.aws.amazon.com/AmazonECR/latest/userguide/image-scanning-basic.html
[2] https://docs.aws.amazon.com/cli/latest/reference/ecr/describe-image-scan-findings.html
[3] https://docs.aws.amazon.com/cli/latest/reference/ecr/describe-images.html
[4] https://docs.aws.amazon.com/AmazonECR/latest/userguide/ecr-eventbridge.html
[5] https://aws.amazon.com/support

[jdew89]

Makes sense. I'll try and finish up the PR this week. I've been pretty busy.

[jdew89]

@alexjurkiewicz I found some time today to update it. Basic scanning dependent code was removed. Let me know what you think.

[alexjurkiewicz]

Great stuff!

I see that you've put a friendly message for basic scanning results further into the call stack. Good idea, especially since this is a breaking change 👍

To simplify, we can pull the error check up, and only pass in enhanced findings to the action's "real logic".

Something like:

    // ...existing code
    }).promise().catch(
      (err) => {
        core.debug(`Error: ${err}`);
        if (err.code === 'ScanNotFoundException') { return null }
        throw err
      })

    // new code
    if (!findings.imageScanFindings.enhancedFindings) {
          throw new Error(`Basic scan not supported. Please enable enhanced scanning in ECR.`)
    }

    return {
      marker: findings.nextToken,
      results: findings.imageScanFindings.enhancedFindings,
// ...

Then you can simplify further down, like removing isEnhancedScan and inlining getEnhancedScanFindings.

[jdew89]

Let me know if the changes I made are what you're looking for. I moved the code to getFindings because we have 2 functions and getFindings is called first so that's where the enhancedScan check should happen. The getAllFindings is paginated which makes changing that one a little more challenging.

[alexjurkiewicz]

Thank you! This looks great. If you comments from @pzi , I'll merge and cut a release ASAP

[pzi]

Thank you! This looks great. If you comments from @pzi , I'll merge and cut a release ASAP

No need to wait for me. Get cutting :)

[pzi]

Thank you! This looks great. If you comments from @pzi , I'll merge and cut a release ASAP

No need to wait for me. Get cutting :)

Having said that... does the README need updating at all to make it clear it now relies on the enhanced scan and it will fail if that's not used? Might as well be upfront and not wait until people complain :)

[jdew89]

I've added a README update. Let me know if you think this is sufficient.

[pzi]

I've added a README update. Let me know if you think this is sufficient.

LGTM, thanks for that!

[alexjurkiewicz]

Sorry for the big delays. You know... life. This is merged and let's wait for bug reports from people pinned to @master :)