fix(misconf): move disabled checks filtering after analyzer scan

[nikpivkin]

Description

We recently changed the way the Rego-scanner is initialized: it is now created once before starting a scan. This caused the filtering of disabled checks to stop working for the image history analyzer.

We need to reconsider the approach to disabling checks for specific analyzers. Two options are possible:

• Also pass a list of disabled checks to the Rego-scanner for each analyzer type. However, this complicates the implementation, since we need to distinguish between docker configuration analyzer and dockerfile analyzer, which call the same dockerfile parser and throw additional options through several call levels - just for the filtering at the Rego-scanning stage.

• Filter disabled checks after the scan is complete, at the Trivy level, outside of the iac package.

This PR implements the second approach - it's simpler and requires fewer changes.

Related issues

• Close bug(misconf): Trivy does not skip AVD-DS-0016 when scanning docker image history #8903

Checklist

• I've read the guidelines for contributing to this repository.
• I've followed the conventions in the PR title.
• I've added tests that prove my fix is effective or that my feature works.
• I've updated the documentation with the relevant information (if needed).
• I've added usage information (if the PR introduces new options)
• I've included a "before" and "after" example to the description (if the PR is a user interface change).

[DmitriyLewen]

Perhaps i am missing something, so fill free to correct me.
I think we can:

1. Add new type for Dockerfile built from history (It looks like we just use this value in report, so we can separate dockerfile and dockerfile from history).
2. Use this type to find result for this dockerfile and filter misconfigs using postScan hooks

@nikpivkin @knqyf263 wdyt?

[nikpivkin]

Interesting solution. I thought about the new type too, but didn't add it so as not to change the reports. Hooks were added for extensions, should we use them inside Trivy? Using hooks would allow fewer changes to the scanning logic.

[DmitriyLewen]

This is the logic for post-processing the obtained result.
Just like we remove system files received from OS packages, that handler works on the layer level and doesn’t fit this case — whereas postScan is exactly what we need.
However, I thought about it again:
Why we can't handle result in analyzer?
instead of share disabled rules into docker scanner, we will remove these rules from result.

UPD:
something like that:

diff --git a/pkg/fanal/analyzer/imgconf/dockerfile/dockerfile.go b/pkg/fanal/analyzer/imgconf/dockerfile/dockerfile.go
index ae8850137..bbc82d29a 100644
--- a/pkg/fanal/analyzer/imgconf/dockerfile/dockerfile.go
+++ b/pkg/fanal/analyzer/imgconf/dockerfile/dockerfile.go
@@ -5,8 +5,10 @@ import (
 	"context"
 	"fmt"
 	"regexp"
+	"slices"
 	"strings"
 
+	"github.com/aquasecurity/trivy/pkg/log"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"golang.org/x/xerrors"
 
@@ -19,16 +21,14 @@ import (
 	"github.com/aquasecurity/trivy/pkg/version/doc"
 )
 
-var disabledChecks = []misconf.DisabledCheck{
-	{
-		ID: "DS007", Scanner: string(analyzer.TypeHistoryDockerfile),
-		Reason: "See " + doc.URL("docs/target/container_image", "disabled-checks"),
-	},
-	{
-		ID: "DS016", Scanner: string(analyzer.TypeHistoryDockerfile),
-		Reason: "See " + doc.URL("docs/target/container_image", "disabled-checks"),
-	},
-}
+var (
+	// TODO use set
+	disabledChecks = []string{
+		"DS007",
+		"DS016",
+	}
+	reason = "See " + doc.URL("docs/target/container_image", "disabled-checks")
+)
 
 const analyzerVersion = 1
 
@@ -72,8 +71,10 @@ func (a *historyAnalyzer) Analyze(ctx context.Context, input analyzer.ConfigAnal
 		return nil, nil
 	}
 
+	misconfig := removeDisabledRules(misconfs[0])
+
 	return &analyzer.ConfigAnalysisResult{
-		Misconfiguration: &misconfs[0],
+		Misconfiguration: &misconfig,
 	}, nil
 }
 
@@ -163,6 +164,20 @@ func normalizeCopyCreatedBy(input string) string {
 	return copyInRe.ReplaceAllString(input, `$1 `)
 }
 
+func removeDisabledRules(misconfig types.Misconfiguration) types.Misconfiguration {
+	var failures []types.MisconfResult
+	for _, failure := range misconfig.Failures {
+		if slices.Contains(disabledChecks, failure.ID) {
+			log.WithPrefix("image history analyzer").Info("Skipping disabled check",
+				log.String("ID", failure.ID), log.String("reason", reason))
+			continue
+		}
+		failures = append(failures, failure)
+	}
+	misconfig.Failures = failures
+	return misconfig
+}
+
 func (a *historyAnalyzer) Required(_ types.OS) bool {
 	return true
 }

[nikpivkin]

@DmitriyLewen I think you're right. This is a special case and there is no need to overcomplicate it. I put the filtering in the analyzer.

[simar7]

@nikpivkin what if we don't evaluate the disabled checks to begin with? Then there won't be anything to filter out right? Plus it will be less checks to evaluate rather than to evaluate and filter (discard) the results.

[nikpivkin]

what if we don't evaluate the disabled checks to begin with?

This would require sufficient changes for such a request. Is the performance issue a cause for concern in the current approach? Performance will not be affected in any way. In addition, post-scan filtering is common in Trivy, which includes ignore rules and Rego policies.

[simar7]

what if we don't evaluate the disabled checks to begin with?

This would require sufficient changes for such a request. Is the performance issue a cause for concern in the current approach? Performance will not be affected in any way. In addition, post-scan filtering is common in Trivy, which includes ignore rules and Rego policies.

Yes I understand, we should think about it and certainly not needed in this PR. It was just an idea I had for improving status-quo.

Is the performance issue a cause for concern in the current approach?

Not in trivy standalone but it can certainly shave some cycles in the operator world since the workloads are much bigger.