Increase scanner pool safety; fix panics and data races (#760)

* Increase scanner pool safety; fix panics

Signed-off-by: egibs <[email protected]>

* Run a quick scan to validate the scanner

Signed-off-by: egibs <[email protected]>

* More optimizations; run tests with -race; address data races

Signed-off-by: egibs <[email protected]>

---------

Signed-off-by: egibs <[email protected]>

Makefile

@@ -103,12 +103,12 @@ out/$(SAMPLES_REPO)/.decompressed-$(SAMPLES_COMMIT): out/${SAMPLES_REPO}/.git/co
 # unit tests only
 .PHONY: test
 test:
-	go test ./pkg/...
+	go test -race ./pkg/...
 
 # integration tests only
 .PHONY: integration
 integration: out/$(SAMPLES_REPO)/.decompressed-$(SAMPLES_COMMIT)
-	go test -timeout 0 ./tests/...
+	go test -race -timeout 0 ./tests/...
 
 .PHONY: bench
 bench: out/$(SAMPLES_REPO)/.decompressed-$(SAMPLES_COMMIT)

pkg/action/diff.go

@@ -148,10 +148,16 @@ func Diff(ctx context.Context, c malcontent.Config) (*malcontent.Report, error)
 	destPath := c.ScanPaths[1]
 
 	var g errgroup.Group
-	var src, dest map[string]*malcontent.FileReport
-	var srcBase, destBase string
-	srcCh := make(chan map[string]*malcontent.FileReport, 1)
-	destCh := make(chan map[string]*malcontent.FileReport, 1)
+
+	type scanResult struct {
+		files map[string]*malcontent.FileReport
+		base  string
+		err   error
+	}
+
+	srcCh := make(chan scanResult, 1)
+	destCh := make(chan scanResult, 1)
+
 	srcIsArchive := programkind.IsSupportedArchive(srcPath)
 	destIsArchive := programkind.IsSupportedArchive(destPath)
 
@@ -166,29 +172,30 @@ func Diff(ctx context.Context, c malcontent.Config) (*malcontent.Report, error)
 	}
 
 	g.Go(func() error {
-		src, srcBase, err = relFileReport(ctx, c, srcPath)
-		if err != nil {
-			return err
-		}
-		srcCh <- src
+		files, base, err := relFileReport(ctx, c, srcPath)
+		srcCh <- scanResult{files: files, base: base, err: err}
 		return nil
 	})
 
 	g.Go(func() error {
-		dest, destBase, err = relFileReport(ctx, c, destPath)
-		if err != nil {
-			return err
-		}
-		destCh <- dest
+		files, base, err := relFileReport(ctx, c, destPath)
+		destCh <- scanResult{files: files, base: base, err: err}
 		return nil
 	})
 
 	if err := g.Wait(); err != nil {
 		return nil, err
 	}
 
-	src = <-srcCh
-	dest = <-destCh
+	srcResult := <-srcCh
+	if srcResult.err != nil {
+		return nil, fmt.Errorf("source scan error: %w", srcResult.err)
+	}
+
+	destResult := <-destCh
+	if destResult.err != nil {
+		return nil, fmt.Errorf("destination scan error: %w", destResult.err)
+	}
 
 	close(srcCh)
 	close(destCh)
@@ -204,20 +211,20 @@ func Diff(ctx context.Context, c malcontent.Config) (*malcontent.Report, error)
 	// When scanning two files, do a 1:1 comparison and
 	// consider the source -> destination as a change rather than an add/delete
 	if (srcInfo.IsDir() && destInfo.IsDir()) || (srcIsArchive && destIsArchive) {
-		handleDir(ctx, c, src, dest, d)
+		handleDir(ctx, c, srcResult.files, destResult.files, d)
 	} else {
 		var srcFile, destFile *malcontent.FileReport
-		for _, fr := range src {
+		for _, fr := range srcResult.files {
 			srcFile = fr
 			break
 		}
-		for _, fr := range dest {
+		for _, fr := range destResult.files {
 			destFile = fr
 			break
 		}
 		if srcFile != nil && destFile != nil {
-			formatSrc := displayPath(srcBase, srcFile.Path)
-			formatDest := displayPath(destBase, destFile.Path)
+			formatSrc := displayPath(srcResult.base, srcFile.Path)
+			formatDest := displayPath(destResult.base, destFile.Path)
 			if scoreFile(srcFile, destFile) {
 				d.Removed.Set(srcFile.Path, srcFile)
 				d.Added.Set(destFile.Path, destFile)

pkg/action/scan.go

@@ -15,6 +15,7 @@ import (
 	"sort"
 	"strings"
 	"sync"
+	"sync/atomic"
 
 	"github.com/chainguard-dev/clog"
 	"github.com/chainguard-dev/malcontent/pkg/archive"
@@ -32,7 +33,7 @@ const interactive string = "Interactive"
 
 var (
 	// compiledRuleCache are a cache of previously compiled rules.
-	compiledRuleCache *yarax.Rules
+	compiledRuleCache atomic.Pointer[yarax.Rules]
 	// compileOnce ensures that we compile rules only once even across threads.
 	compileOnce         sync.Once
 	ErrMatchedCondition = errors.New("matched exit criteria")
@@ -205,21 +206,26 @@ func exitIfHitOrMiss(frs *sync.Map, scanPath string, errIfHit bool, errIfMiss bo
 }
 
 func CachedRules(ctx context.Context, fss []fs.FS) (*yarax.Rules, error) {
-	if compiledRuleCache != nil {
-		return compiledRuleCache, nil
+	if rules := compiledRuleCache.Load(); rules != nil {
+		return rules, nil
 	}
 
-	var yrs *yarax.Rules
 	var err error
 	compileOnce.Do(func() {
+		var yrs *yarax.Rules
 		yrs, err = compile.Recursive(ctx, fss)
 		if err != nil {
 			err = fmt.Errorf("compile: %w", err)
+			return
 		}
-
-		compiledRuleCache = yrs
+		compiledRuleCache.Store(yrs)
 	})
-	return compiledRuleCache, err
+
+	if err != nil {
+		return nil, err
+	}
+
+	return compiledRuleCache.Load(), nil
 }
 
 // matchResult represents the outcome of a match operation.