Skip reverification results during deduplication (#5069)

Author: mcastorina

pkg/engine/engine.go

@@ -1294,12 +1294,18 @@ func (e *Engine) notifierWorker(ctx context.Context) {
 		// custom detectors which have the same type.
 		// MD5 hash of the key is used to reduce memory usage of the dedupe cache,
 		// since the raw result and source metadata can be large.
-		h := md5.Sum([]byte(fmt.Sprintf("%s%s%s%s%+v", result.DetectorName, result.DetectorType.String(), result.Raw, result.RawV2, result.SourceMetadata)))
-		key := string(h[:])
-		if _, ok := e.dedupeCache.Get(key); ok {
-			continue
+		//
+		// This deduplication only applies to results that are *not*
+		// from reverification, since we are expected to see the same
+		// result from reverification and want to Dispatch it below.
+		if result.SecretID == 0 {
+			h := md5.Sum([]byte(fmt.Sprintf("%s%s%s%s%+v", result.DetectorName, result.DetectorType.String(), result.Raw, result.RawV2, result.SourceMetadata)))
+			key := string(h[:])
+			if _, ok := e.dedupeCache.Get(key); ok {
+				continue
+			}
+			e.dedupeCache.Add(key, struct{}{})
 		}
-		e.dedupeCache.Add(key, struct{}{})
 
 		if result.Verified {
 			atomic.AddUint64(&e.metrics.VerifiedSecretsFound, 1)

pkg/engine/engine_test.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
+	lru "github.com/hashicorp/golang-lru/v2"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/testing/protocmp"
@@ -1990,3 +1991,77 @@ func TestEngine_IterativeDecoding(t *testing.T) {
 		})
 	}
 }
+
+// captureDispatcher records every dispatched result for assertion in tests.
+type captureDispatcher struct {
+	results []detectors.ResultWithMetadata
+}
+
+func (d *captureDispatcher) Dispatch(_ context.Context, result detectors.ResultWithMetadata) error {
+	d.results = append(d.results, result)
+	return nil
+}
+
+// TestNotifierWorker_ReverifiedResultsBypassDedupe verifies that the notifier's
+// dedupe cache is skipped when a result carries a non-zero SecretID — i.e., when
+// it originated from reverification — so that the dispatcher sees every
+// reverification result even when the underlying secret has not changed.
+func TestNotifierWorker_ReverifiedResultsBypassDedupe(t *testing.T) {
+	tests := []struct {
+		name         string
+		secretID     int64
+		wantDispatch int
+	}{
+		{
+			name:         "non-reverified duplicates are deduplicated",
+			secretID:     0,
+			wantDispatch: 1,
+		},
+		{
+			name:         "reverified duplicates bypass the dedupe cache",
+			secretID:     42,
+			wantDispatch: 2,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cache, err := lru.New[string, struct{}](16)
+			require.NoError(t, err)
+
+			disp := &captureDispatcher{}
+			e := &Engine{
+				results:                 make(chan detectors.ResultWithMetadata, 4),
+				dedupeCache:             cache,
+				dispatcher:              disp,
+				notifyVerifiedResults:   true,
+				notifyUnverifiedResults: true,
+				notifyUnknownResults:    true,
+			}
+
+			result := detectors.ResultWithMetadata{
+				SourceMetadata: &source_metadatapb.MetaData{
+					Data: &source_metadatapb.MetaData_Git{
+						Git: &source_metadatapb.Git{Line: 1},
+					},
+				},
+				SourceType: sourcespb.SourceType_SOURCE_TYPE_GIT,
+				SecretID:   tt.secretID,
+				Result: detectors.Result{
+					DetectorType: detector_typepb.DetectorType(-1),
+					Raw:          []byte("a-secret"),
+					Verified:     true,
+				},
+			}
+
+			// Push the same result twice — identical hash inputs.
+			e.results <- result
+			e.results <- result
+			close(e.results)
+
+			e.notifierWorker(context.Background())
+
+			assert.Equal(t, tt.wantDispatch, len(disp.results))
+		})
+	}
+}