Merge pull request #25 from cultureamp/add-ignore-file

feat: allow findings to be ignored

.golangci.yml

@@ -39,6 +39,7 @@ linters:
   - revive
   - tagalign
   - whitespace
+  - musttag
   # deprecated linters
   - interfacer
   - golint
@@ -62,5 +63,11 @@ linters-settings:
         audit: disabled
       G101: # "Look for hard-coded credentials"
         mode: strict
+
   cyclop:
     max-complexity: 20
+
+  exhaustive:
+    # Presence of "default" case in switch statements satisfies exhaustiveness,
+    # even if all enum members are not listed.
+    default-signifies-exhaustive: true

README.md

@@ -5,12 +5,14 @@ service. By default the plugin will cause the step to fail if there are critical
 or high vulnerabilities reported, but there are configurable thresholds on this
 behaviour.
 
-> ℹ️ TIP: if you want the build to continue when vulnerabilities are found, be
+> ℹ️ **TIP**: if you want the build to continue when vulnerabilities are found, be
 > sure to supply values for `max-criticals` and `max-highs` parameters. If these
 > are set to high values your build will never fail, but details will be
 > supplied in the annotation.
 >
-> Check out the FAQs below for more information
+> If a finding is irrelevant, or you're waiting on an upstream fix, use an
+> "ignore" configuration file instead: see the [ignore
+> findings](./docs/ignore-findings.md) documentation.
 
 ## Example
 
@@ -76,12 +78,20 @@ If the number of critical vulnerabilities in the image exceeds this threshold
 the build is failed. Defaults to 0. Use a sufficiently large number (e.g. 999)
 to allow the build to always pass.
 
+> [!IMPORTANT]
+> Prefer an [ignore file](./docs/ignore-findings.md) over setting thresholds if
+> a finding is irrelevant or time to respond is required.
+
 ### `max-highs` (Optional, string)
 
 If the number of high vulnerabilities in the image exceeds this threshold the
 build is failed. Defaults to 0. Use a sufficiently large number (e.g. 999) to
 allow the build to always pass.
 
+> [!IMPORTANT]
+> Prefer an [ignore file](./docs/ignore-findings.md) over setting thresholds if
+> a finding is irrelevant or time to respond is required.
+
 ### `image-label` (Optional, string)
 
 When supplied, this is used to title the report annotation in place of the
@@ -118,18 +128,67 @@ for more information.
 
 ## FAQ
 
-### I have a vulnerability that isn't resolved yet, but I can wait on fixing. How do I do configure this plugin so I can unblock my builds?
-
-Refer to how to set your [max-criticals](https://github.com/cultureamp/ecr-scan-results-buildkite-plugin#max-criticals-optional-string), and [max-highs](https://github.com/cultureamp/ecr-scan-results-buildkite-plugin#max-highs-optional-string).
-
-### Are there guidelines on using up?
-
-Yes. Changing the `max-criticals` and `max-high` settings should not be taken lightly.
-
-This option is effectively a deferral of fixing the vulnerability. **Assess the situation first**. If the CVE describes a scenario that aligns with how your project is used, then you should be working to fix it rather than defer it. For help on this, check out the following the steps outlined [here](https://cultureamp.atlassian.net/wiki/spaces/PST/pages/2960916852/Central+SRE+Support+FAQs#I-have-high%2Fcritical-vulnerabilities-for-my-ECR-image%2C-and-its-blocking-my-builds.-What%E2%80%99s-going-on%3F).
-
-Below are some recommendations if you choose to exercise this option:
-
-1. Set the thresholds to the number of identified high or critical vulnerabilities. This is so you’re not permitting more vulnerabilities than you should. Especially for those you can fix by updating dependencies or packages.
-
-2. Set a scheduled reminder for your team to check if a fix is available for the CVE. If a fix is available, address it, and then lower your threshold for the respective vulnerability severity.
+### The build is failing for an unresolved vulnerability. How do I do configure this plugin so I can unblock my builds?
+
+There are two options here:
+
+1. configure an [ignore file](docs/ignore-findings.md) to tell the plugin to
+  skip this vulnerability. Set an `until` date to ensure that it is dealt with
+  in the future.
+2. refer to how to set your [max-criticals](#max-criticals-optional-string), and
+  [max-highs](#max-highs-optional-string) thresholds.
+
+Out of the two options, the first is far more preferable as it targets the
+finding that is causing the problem. Altering a threshold is unlikely to be
+rolled back and is indiscriminate about the findings that it will ignore.
+
+### How do I set my thresholds? _OR_ My build often breaks because of this plugin. Should I change the thresholds?
+
+Setting the `max-criticals` and `max-high` thresholds requires some thought, and
+needs to take into account the risk profile of the company and the service
+itself.
+
+Allowing a container with a critical or high vulnerability to be deployed is not
+necessarily wrong, it really depends on the environment of the deployed
+application and the nature of the vulnerabilities themselves.
+
+Consult with your company's internal security teams about the acceptable risk
+level for a service if a non-zero threshold is desired, and consider making use
+of the [ignore file](docs/ignore-findings.md) configuration to avoid temporary
+blockages. Using an ignore entry with an expiry is strongly recommended over
+increasing the threshold.
+
+### What should I think about when ignoring a finding for a period of time?
+
+Consider the following:
+
+- Follow the link to read the details of the CVE. Does it impact a component
+  that is directly (or indirectly) used by your application?
+- Reference the CVSS score associated with the vulnerability and look carefully
+  at the vector via the link. The vector will help inform about how this
+  vulnerability can affect your system.
+- Consider using the CVSS calculator (reached via the vector link) to fill out
+  the "Environmental" part of the score. The environmental score helps judge the
+  risks posed in the context of your application deployment environment. This is
+  likely adjust the base score and assist in your decision making.
+- Is a patch or update already available?
+- Is this update likely to be made available in an update to the current base
+  image, or does it exist in a later version of the base image?
+
+Possible actions:
+
+1. Update the base image that incorporates the fix. This may be the simplest
+   option.
+2. Remove the dependency from the image, or choose a different base image
+   without the issue.
+3. Add an ignore with an expiry to allow for an update to be published within a
+   period of time. This is only valid if your corporate standards allow it, and
+   there is a plan to follow up on this issue in the given time frame.
+4. Update the image definition to update the package with the vulnerability.
+   This can negatively impact image size, and create a future maintenance issue
+   if not done carefully. Take care to keep the number of packages updated
+   small, and avoid adding hard-coded versions unless a process exists to keep
+   them up-to-date.
+5. Ignore the finding indefinitely. This will generally only be valid if
+   dependency both cannot be removed and is not used in a vulnerable fashion.
+   Your working environment may require special exceptions for this.

docs/ignore-findings.md

@@ -0,0 +1,44 @@
+# Ignoring findings
+
+Findings can be ignored using a YAML file with the following structure:
+
+```yaml
+ignores:
+  - id: CVE-2023-100
+  - id: CVE-2023-200
+    until: 2023-12-31
+    reason: allowing 2 weeks for base image to update
+  - id: CVE-2023-300
+```
+
+- each element must have at least the `id` field
+- the `until` field defines the expiry of this ignore entry. This allows a team time to respond while temporarily allowing builds to continue.
+- the `reason` field gives a justification that is rendered in the annotation for greater visibility. Including the "why" in this field is highly recommended.
+
+Ignore configuration can be specified in a number of places. If a listing for a finding with the same CVE name appears in multiple files, the most local wins: central configuration can be overridden by the repository.
+
+From least to most important:
+
+- `/etc/ecr-scan-results-buildkite-plugin/ignore.y[a]ml` (part of the agent, not modifiable by builds)
+- `buildkite/ecr-scan-results-ignore.y[a]ml` (specified alongside the pipeline)
+- `.buildkite/ecr-scan-results-ignore.y[a]ml`
+- `.ecr-scan-results-ignore.y[a]ml` (local repository configuration)
+
+Configuration in the `/etc/ecr-scan-results-buildkite-plugin` directory allows for organizations to ship agents with plugin configuration that centrally manages findings that can be ignored.
+
+> [!IMPORTANT]
+> When a finding is ignored, it is removed from consideration for threshold checks, but it's not discarded. The annotation created by the plugin adds details to the results, giving high visibility on the configured behaviour.
+
+## Rendering
+
+The summary counts at the top show the number of ignored findings:
+
+<img src="img/summary-counts.png" alt="summary counts" width="60%">
+
+Ignored findings are separated from the main list and shown at the bottom:
+
+<img src="img/ignore-finding-list.png" alt="ignored finding list" width="60%">
+
+If a reason for ignoring a finding is provided, it's made available by expanding the Until date:
+
+<img src="img/ignore-reason.png" alt="ignored reason" width="60%">

src/finding/summary.go

@@ -0,0 +1,125 @@
+package finding
+
+import (
+	"slices"
+	"time"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/ecr/types"
+	"github.com/cultureamp/ecrscanresults/findingconfig"
+)
+
+type Detail struct {
+	// The name associated with the finding, usually a CVE number.
+	Name        string
+	URI         string
+	Description string
+	Severity    types.FindingSeverity
+
+	PackageName    string
+	PackageVersion string
+	CVSS2Score     string
+	CVSS2Vector    string
+
+	Ignore *findingconfig.Ignore
+}
+
+type SeverityCount struct {
+	// Included is the number of findings that count towards the threshold for this severity.
+	Included int32
+
+	// Ignored is the number of findings that were ignored for the purposes of the threshold.
+	Ignored int32
+}
+
+type Summary struct {
+	// the counts by threshold, taking ignore configuration into account
+	Counts map[types.FindingSeverity]SeverityCount
+
+	Details []Detail
+
+	// the set of finding IDs that have been ignored by configuration
+	Ignored []Detail
+
+	// The time of the last completed image scan.
+	ImageScanCompletedAt *time.Time
+
+	// The time when the vulnerability data was last scanned.
+	VulnerabilitySourceUpdatedAt *time.Time
+}
+
+func (s *Summary) addDetail(d Detail) {
+	s.Details = append(s.Details, d)
+	s.updateCount(d.Severity, SeverityCount{Included: 1})
+}
+
+func (s *Summary) addIgnored(d Detail) {
+	s.Ignored = append(s.Ignored, d)
+	s.updateCount(d.Severity, SeverityCount{Ignored: 1})
+}
+
+func (s *Summary) updateCount(severity types.FindingSeverity, updateBy SeverityCount) {
+	counts := s.Counts[severity]
+
+	counts.Ignored += updateBy.Ignored
+	counts.Included += updateBy.Included
+
+	s.Counts[severity] = counts
+}
+
+func newSummary() Summary {
+	return Summary{
+		Counts: map[types.FindingSeverity]SeverityCount{
+			"CRITICAL": {},
+			"HIGH":     {},
+		},
+		Details: []Detail{},
+		Ignored: []Detail{},
+	}
+}
+
+func Summarize(findings *types.ImageScanFindings, ignoreConfig []findingconfig.Ignore) Summary {
+	summary := newSummary()
+
+	summary.ImageScanCompletedAt = findings.ImageScanCompletedAt
+	summary.VulnerabilitySourceUpdatedAt = findings.VulnerabilitySourceUpdatedAt
+
+	for _, f := range findings.Findings {
+		detail := findingToDetail(f)
+
+		index := slices.IndexFunc(ignoreConfig, func(ignore findingconfig.Ignore) bool {
+			return ignore.ID == detail.Name
+		})
+
+		if index >= 0 {
+			detail.Ignore = &ignoreConfig[index]
+			summary.addIgnored(detail)
+		} else {
+			summary.addDetail(detail)
+		}
+	}
+
+	return summary
+}
+
+func findingToDetail(finding types.ImageScanFinding) Detail {
+	return Detail{
+		Name:           aws.ToString(finding.Name),
+		URI:            aws.ToString(finding.Uri),
+		Description:    aws.ToString(finding.Description),
+		Severity:       finding.Severity,
+		PackageName:    findingAttributeValue(finding, "package_name"),
+		PackageVersion: findingAttributeValue(finding, "package_version"),
+		CVSS2Score:     findingAttributeValue(finding, "CVSS2_SCORE"),
+		CVSS2Vector:    findingAttributeValue(finding, "CVSS2_VECTOR"),
+	}
+}
+
+func findingAttributeValue(finding types.ImageScanFinding, name string) string {
+	for _, a := range finding.Attributes {
+		if aws.ToString(a.Key) == name {
+			return aws.ToString(a.Value)
+		}
+	}
+	return ""
+}