Unify some false positive logic (#4720)

The (wordlist) false positive check used to happen in two places: Within filterResults (a catchall function that dropped findings for various configuration-dependent reasons), and again in processResult, which didn't drop findings, but instead annotated them with a boolean flag to indicate false positivity. Both of these two functions are themselves called in two separate places.

This redundancy is confusing and has led to at least one known bug. This commit moves the filtration from filterResults to the end of the engine, where results are now dropped based on the false positive flag previously set in processResult. The net effect is that false positive filtration is only executed in one place (instead of two) and the actual false positive checking is only executed in two places (instead of four).

There are additional improvements along this line that could be made (e.g. moving entropy filtering as well) but I want to do things very slowly and incrementally.

pkg/detectors/falsepositives.go

@@ -2,7 +2,6 @@ package detectors
 
 import (
 	_ "embed"
-	"fmt"
 	"math"
 	"strings"
 	"unicode"
@@ -172,30 +171,3 @@ func FilterResultsWithEntropy(ctx context.Context, results []Result, entropy flo
 	}
 	return filteredResults
 }
-
-// FilterKnownFalsePositives filters out known false positives from the results.
-func FilterKnownFalsePositives(ctx context.Context, detector Detector, results []Result) []Result {
-	var filteredResults []Result
-
-	isFalsePositive := GetFalsePositiveCheck(detector)
-
-	for _, result := range results {
-		if len(result.Raw) == 0 {
-			ctx.Logger().Error(fmt.Errorf("empty raw"), "Skipping result: invalid")
-			continue
-		}
-
-		if result.Verified {
-			filteredResults = append(filteredResults, result)
-			continue
-		}
-
-		if isFp, reason := isFalsePositive(result); isFp {
-			ctx.Logger().V(4).Info("Skipping result: false positive", "result", string(result.Raw), "reason", reason)
-			continue
-		}
-		filteredResults = append(filteredResults, result)
-	}
-
-	return filteredResults
-}

pkg/detectors/falsepositives_test.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
-	logContext "github.com/trufflesecurity/trufflehog/v3/pkg/context"
 	"github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb"
 )
 
@@ -76,41 +75,6 @@ func TestGetFalsePositiveCheck_CustomLogic(t *testing.T) {
 	}
 }
 
-func TestFilterKnownFalsePositives_DefaultLogic(t *testing.T) {
-	results := []Result{
-		{Raw: []byte("00000")},  // "default" false positive list
-		{Raw: []byte("number")}, // from wordlist
-		// from uuid list
-		{Raw: []byte("00000000-0000-0000-0000-000000000000")},
-		{Raw: []byte("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx")},
-		// real secrets
-		{Raw: []byte("hga8adshla3434g")},
-		{Raw: []byte("f795f7db-2dfe-4095-96f3-8f8370c735f9")},
-	}
-	expected := []Result{
-		{Raw: []byte("hga8adshla3434g")},
-		{Raw: []byte("f795f7db-2dfe-4095-96f3-8f8370c735f9")},
-	}
-	filtered := FilterKnownFalsePositives(logContext.Background(), fakeDetector{}, results)
-	assert.ElementsMatch(t, expected, filtered)
-}
-
-func TestFilterKnownFalsePositives_CustomLogic(t *testing.T) {
-	results := []Result{
-		{Raw: []byte("a specific magic string")}, // specific target
-		{Raw: []byte("00000")},                   // "default" false positive list
-		{Raw: []byte("number")},                  // from wordlist
-		{Raw: []byte("hga8adshla3434g")},         // real secret
-	}
-	expected := []Result{
-		{Raw: []byte("00000")},
-		{Raw: []byte("number")},
-		{Raw: []byte("hga8adshla3434g")},
-	}
-	filtered := FilterKnownFalsePositives(logContext.Background(), customFalsePositiveChecker{}, results)
-	assert.ElementsMatch(t, expected, filtered)
-}
-
 func TestIsFalsePositive(t *testing.T) {
 	type args struct {
 		match          string

pkg/engine/engine.go

@@ -1156,10 +1156,6 @@ func (e *Engine) filterResults(
 		results = clean(results)
 	}
 
-	if !e.retainFalsePositives {
-		results = detectors.FilterKnownFalsePositives(ctx, detector.Detector, results)
-	}
-
 	if e.filterEntropy != 0 {
 		results = detectors.FilterResultsWithEntropy(ctx, results, e.filterEntropy, e.retainFalsePositives)
 	}
@@ -1213,7 +1209,10 @@ func (e *Engine) notifierWorker(ctx context.Context) {
 		startTime := time.Now()
 		// Filter unwanted results, based on `--results`.
 		if !result.Verified {
-			if result.VerificationError() != nil {
+			if result.IsWordlistFalsePositive && !e.retainFalsePositives {
+				// Skip false positives
+				continue
+			} else if result.VerificationError() != nil {
 				if !e.notifyUnknownResults {
 					// Skip results with verification errors.
 					continue