[INS-285] Fix custom detectors line number reporting to match the full regex instead of capture group (#4697)

* set the full match as primary secret for correct line number reporting

* add test case for multiline, update documentation

* update comment

* add test case to engine

* sort regexes before selecting first for deterministic behaviour

* remove leftover todo comment

---------

Co-authored-by: Kashif Khan <70996046+kashifkhan0771@users.noreply.github.com>

Author: mustansir14

pkg/custom_detectors/CUSTOM_DETECTORS.md

@@ -38,7 +38,7 @@ This guide will walk you through setting up a custom detector in TruffleHog to i
    - **`verify`**: An optional section to validate detected secrets. If you want to verify or unverify detected secrets, this section needs to be configured. If not configured, all detected secrets will be marked as unverified. Read [verification server examples](#verification-server-examples)
 
    **Other allowed parameters:**
-   - **`primary_regex_name`**: This parameter allows you designate the primary regex pattern when multiple regex patterns are defined in the regex section. If a match is found, the match for the designated primary regex will be used to determine the line number. The value must be one of the names specified in the regex section.
+   - **`primary_regex_name`**: This parameter allows you designate the primary regex pattern when multiple regex patterns are defined in the regex section. If a match is found, the match for the designated primary regex will be used to determine the line number. The value must be one of the names specified in the regex section. If not provided, the first regex name in sorted order will be used as the primary regex by default.
    - **`exclude_regexes_capture`**: This parameter allows you to define regex patterns to exclude specific parts of a detected secret. If a match is found within the detected secret, the portion matching this regex is excluded from the result.
    - **`exclude_regexes_match`**: This parameter enables you to define regex patterns to exclude entire matches from being reported as secrets. This applies to the entire matched string, not just the token.
    - **`entropy`**: This parameter is used to assess the randomness of detected strings. High entropy often indicates that a string is a potential secret, such as an API key or password, due to its complexity and unpredictability. It helps in filtering false-positives. While an entropy threshold of `3` can be a starting point, it's essential to adjust this value based on your project's specific requirements and the nature of the data you have.

pkg/custom_detectors/custom_detectors.go

@@ -66,6 +66,9 @@ func NewWebhookCustomRegex(pb *custom_detectorspb.CustomRegex) (*CustomRegexWebh
 		}
 	}
 
+	// Ensure primary regex name is set.
+	ensurePrimaryRegexNameSet(pb)
+
 	// TODO: Copy only necessary data out of pb.
 	return &CustomRegexWebhook{pb}, nil
 }
@@ -229,14 +232,27 @@ func (c *CustomRegexWebhook) createResults(ctx context.Context, match map[string
 		values := match[key]
 		// values[0] contains the entire regex match.
 		secret := values[0]
+		fullMatch := values[0]
 		if len(values) > 1 {
 			secret = values[1]
 		}
 		raw += secret
 
-		// if the match is of the primary regex, set it's value as primary secret value in result
+		// We set the full regex match as the primary secret value.
+		// Reasoning:
+		// The engine calculates the line number using the match. When a primary secret is set, it uses that value instead of the raw secret.
+		// While the secret match itself is sufficient to calculate the line number, the same group match could appear elsewhere in the data.
+		// To avoid ambiguity, we store the full regex match as the primary secret value.
+		// This primary secret value is used only for identifying the exact line number and is not used anywhere else.
+
+		// Example:
+		// Full regex match: secret = ABC123
+		// Secret (raw): ABC123
+
+		// In this case, the primary secret value stores the full string `secret = ABC123`,
+		// allowing the engine to pinpoint the exact location and avoid matching redundant occurrences of `ABC123` in the data.
 		if c.PrimaryRegexName == key {
-			result.SetPrimarySecretValue(secret)
+			result.SetPrimarySecretValue(fullMatch)
 		}
 	}
 
@@ -394,3 +410,15 @@ func (c *CustomRegexWebhook) Description() string {
 	}
 	return c.GetDescription()
 }
+
+// ensurePrimaryRegexNameSet sets the PrimaryRegexName field to the
+// first regex name in sorted order if it is not already set.
+// We're sorting to ensure deterministic behavior.
+func ensurePrimaryRegexNameSet(pb *custom_detectorspb.CustomRegex) {
+	if pb.PrimaryRegexName == "" {
+		for _, name := range slices.Sorted(maps.Keys(pb.Regex)) {
+			pb.PrimaryRegexName = name
+			return
+		}
+	}
+}

pkg/custom_detectors/custom_detectors_test.go

@@ -232,6 +232,60 @@ func TestDetectorPrimarySecret(t *testing.T) {
 	assert.Equal(t, "secret_YI7C90ACY1_yy", results[0].GetPrimarySecretValue())
 }
 
+func TestDetectorPrimarySecretFullMatch(t *testing.T) {
+	tests := []struct {
+		name  string
+		input *custom_detectorspb.CustomRegex
+		chunk []byte
+		want  string
+	}{
+		{
+			name: "primary regex full match",
+			input: &custom_detectorspb.CustomRegex{
+				Name:             "test",
+				Keywords:         []string{"secret"},
+				Regex:            map[string]string{"secret": `secret *= *"([^"\r\n]+)"`},
+				PrimaryRegexName: "secret",
+			},
+			chunk: []byte(`
+			// some code
+			secret="mysecret"
+			// some code
+			`),
+			want: `secret="mysecret"`,
+		},
+		{
+			name: "primary regex full match multiline",
+			input: &custom_detectorspb.CustomRegex{
+				Name:             "test",
+				Keywords:         []string{"secret"},
+				Regex:            map[string]string{"secret": `secret *= *"([^"]+)"`},
+				PrimaryRegexName: "secret",
+			},
+			chunk: []byte(`
+			// some code
+			secret="mysecret
+			thatspansmultiplelines"
+			// some code
+			`),
+			want: `secret="mysecret
+			thatspansmultiplelines"`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			detector, err := NewWebhookCustomRegex(tt.input)
+			assert.NoError(t, err)
+			results, err := detector.FromData(context.Background(), false, tt.chunk)
+			assert.NoError(t, err)
+			assert.Equal(t, 1, len(results))
+			assert.Equal(t, tt.want, results[0].GetPrimarySecretValue())
+		})
+	}
+
+}
+
 func TestDetectorValidations(t *testing.T) {
 	type args struct {
 		CustomRegex *custom_detectorspb.CustomRegex
@@ -707,6 +761,24 @@ func TestNewWebhookCustomRegex_Validation(t *testing.T) {
 	}
 }
 
+func TestNewWebhookCustomRegex_EnsurePrimaryRegexNameSet(t *testing.T) {
+	t.Parallel()
+
+	pb := &custom_detectorspb.CustomRegex{
+		Name:     "test",
+		Keywords: []string{"kw"},
+		Regex: map[string]string{
+			"regex_a": `regex_a`,
+			"regex_b": `regex_b`,
+		},
+		// PrimaryRegexName is not set.
+	}
+
+	detector, err := NewWebhookCustomRegex(pb)
+	assert.NoError(t, err)
+	assert.Equal(t, "regex_a", detector.GetPrimaryRegexName(), "expected PrimaryRegexName to be set to regex_a")
+}
+
 func BenchmarkProductIndices(b *testing.B) {
 	for i := 0; i < b.N; i++ {
 		_ = productIndices(3, 2, 6)

pkg/engine/engine_test.go

@@ -215,6 +215,21 @@ func TestFragmentLineOffsetWithPrimarySecret(t *testing.T) {
 	}
 }
 
+func TestFragmentLineOffsetWithPrimarySecretMultiline(t *testing.T) {
+	result := &detectors.Result{
+		Raw: []byte("secret here"),
+	}
+	result.SetPrimarySecretValue("secret:\nsecret here")
+
+	chunk := &sources.Chunk{
+		Data: []byte("line1\nline2\nsecret:\nsecret here\nline5"),
+	}
+	lineOffset, isIgnored := FragmentLineOffset(chunk, result)
+	assert.False(t, isIgnored)
+	// offset 2 means line 3
+	assert.Equal(t, int64(2), lineOffset)
+}
+
 func setupFragmentLineOffsetBench(totalLines, needleLine int) (*sources.Chunk, *detectors.Result) {
 	data := make([]byte, 0, 4096)
 	needle := []byte("needle")