diff --git a/pkg/custom_detectors/custom_detectors_test.go b/pkg/custom_detectors/custom_detectors_test.go index a8b435e54..760971b68 100644 --- a/pkg/custom_detectors/custom_detectors_test.go +++ b/pkg/custom_detectors/custom_detectors_test.go @@ -632,7 +632,10 @@ func TestDetectorValidations(t *testing.T) { results, err := detector.FromData(context.Background(), false, []byte(tt.input.Data)) assert.NoError(t, err) - ignoreOpts := cmpopts.IgnoreFields(detectors.Result{}, "ExtraData", "verificationError", "primarySecret") + ignoreOpts := cmp.Options{ + cmpopts.IgnoreUnexported(detectors.Result{}), + cmpopts.IgnoreFields(detectors.Result{}, "ExtraData"), + } if diff := cmp.Diff(results, tt.want, ignoreOpts); diff != "" { t.Errorf("CustomDetector.FromData() %s diff: (-got +want)\n%s", tt.name, diff) } diff --git a/pkg/detectors/datadogapikey/datadogapikey_test.go b/pkg/detectors/datadogapikey/datadogapikey_test.go index f5918a4d1..a6495bcad 100644 --- a/pkg/detectors/datadogapikey/datadogapikey_test.go +++ b/pkg/detectors/datadogapikey/datadogapikey_test.go @@ -41,7 +41,7 @@ func TestDataDogApiKey_Pattern_WithValidAPIKey(t *testing.T) { return } - if diff := cmp.Diff(wantedResult, results, cmpopts.IgnoreFields(detectors.Result{}, "verificationError", "primarySecret")); diff != "" { + if diff := cmp.Diff(wantedResult, results, cmpopts.IgnoreUnexported(detectors.Result{})); diff != "" { t.Errorf("%s diff: (-want +got)\n%s", "TestDataDogApiKey_Pattern_WithValidAPIKeyOnly", diff) } } diff --git a/pkg/detectors/detectors.go b/pkg/detectors/detectors.go index fc781af1d..9efb88d64 100644 --- a/pkg/detectors/detectors.go +++ b/pkg/detectors/detectors.go @@ -134,6 +134,11 @@ type Result struct { Value string Line int64 } + + // chunkOffset stores the byte position of this result's secret within chunk data. + // Used to disambiguate line numbers when the same secret appears multiple times. + chunkOffset int64 + chunkOffsetSet bool } // CopyVerificationInfo clones verification info (status and error) from another Result struct. This is used when @@ -176,6 +181,22 @@ func (r *Result) GetPrimarySecretValue() string { return r.primarySecret.Value } +// SetChunkOffset records the byte position of this result's secret within the chunk data. +func (r *Result) SetChunkOffset(offset int64) { + r.chunkOffset = offset + r.chunkOffsetSet = true +} + +// ChunkOffset returns the byte position of this result's secret within the chunk data. +func (r *Result) ChunkOffset() int64 { + return r.chunkOffset +} + +// HasChunkOffset reports whether a chunk offset has been explicitly set on this result. +func (r *Result) HasChunkOffset() bool { + return r.chunkOffsetSet +} + // redactSecrets replaces all instances of the given secrets with [REDACTED] in the error message. func redactSecrets(err error, secrets ...string) error { lastErr := unwrapToLast(err) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 335855a72..cd4d2b4bd 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -1186,6 +1186,8 @@ func (e *Engine) detectChunk(ctx context.Context, data detectableChunk) { results = e.filterResults(ctx, data.detector, results) } + AssignDuplicateLineOffsets(&data.chunk, results) + for _, res := range results { e.processResult(ctx, res, data.chunk, data.decoder, data.detector.Description(), isFalsePositive) } @@ -1339,21 +1341,39 @@ func SupportsLineNumbers(sourceType sourcespb.SourceType) bool { } } +// effectiveSecret returns the canonical secret string for a result, preferring +// the primary secret value and falling back to Raw. Callers that compute or +// consume chunk offsets must go through this helper so the two stay in sync. +func effectiveSecret(r *detectors.Result) string { + secret := r.GetPrimarySecretValue() + if secret == "" { + secret = string(r.Raw) + } + return secret +} + // FragmentLineOffset sets the line number for a provided source chunk with a given detector result. func FragmentLineOffset(chunk *sources.Chunk, result *detectors.Result) (int64, bool) { - // get the primary secret value from the result if set - secret := result.GetPrimarySecretValue() - if secret == "" { - secret = string(result.Raw) + secretBytes := []byte(effectiveSecret(result)) + + // Locate the byte offset of the secret in chunk.Data. If a chunk offset was + // pre-assigned (for duplicate secrets), use it directly to find the correct + // occurrence instead of always matching the first one. + var offset int + if result.HasChunkOffset() { + offset = int(result.ChunkOffset()) + } else { + offset = bytes.Index(chunk.Data, secretBytes) + if offset == -1 { + return 0, false + } } - before, after, found := bytes.Cut(chunk.Data, []byte(secret)) - if !found { - return 0, false - } - lineNumber := int64(bytes.Count(before, []byte("\n"))) + lineNumber := int64(bytes.Count(chunk.Data[:offset], []byte("\n"))) result.SetPrimarySecretLine(lineNumber) - // If the line contains the ignore tag, we should ignore the result. + + // If the line containing the secret has the ignore tag, we should ignore the result. + after := chunk.Data[offset+len(secretBytes):] endLine := bytes.Index(after, []byte("\n")) if endLine == -1 { endLine = len(after) @@ -1364,6 +1384,46 @@ func FragmentLineOffset(chunk *sources.Chunk, result *detectors.Result) (int64, return lineNumber, false } +// AssignDuplicateLineOffsets pre-computes byte offsets for results that share the same +// secret value within a chunk. This allows FragmentLineOffset to locate the correct +// occurrence instead of always finding the first one. +func AssignDuplicateLineOffsets(chunk *sources.Chunk, results []detectors.Result) { + // Group result indices by their secret value. + type group struct { + secret string + indices []int + } + seen := make(map[string]int) // secret -> index into groups slice + var groups []group + + for i := range results { + secret := effectiveSecret(&results[i]) + if idx, ok := seen[secret]; ok { + groups[idx].indices = append(groups[idx].indices, i) + } else { + seen[secret] = len(groups) + groups = append(groups, group{secret: secret, indices: []int{i}}) + } + } + + for _, g := range groups { + if len(g.indices) <= 1 { + continue + } + secretBytes := []byte(g.secret) + searchStart := 0 + for _, ri := range g.indices { + pos := bytes.Index(chunk.Data[searchStart:], secretBytes) + if pos == -1 { + break + } + absPos := searchStart + pos + results[ri].SetChunkOffset(int64(absPos)) + searchStart = absPos + len(secretBytes) + } + } +} + // FragmentFirstLineAndLink extracts the first line number and the link from the chunk metadata. // It returns: // - The first line number of the fragment. diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index d68960096..7f1ba8d4a 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -231,6 +231,93 @@ func TestFragmentLineOffsetWithPrimarySecretMultiline(t *testing.T) { assert.Equal(t, int64(2), lineOffset) } +// TestFragmentLineOffset_DuplicateSecrets verifies that when the same secret +// appears on multiple lines within a chunk, each result receives the correct +// line number rather than always reporting the first occurrence's line. +// Regression test for https://github.com/trufflesecurity/trufflehog/issues/2502 +func TestFragmentLineOffset_DuplicateSecrets(t *testing.T) { + secret := []byte("AKIA1234567890ABCDEF") + chunk := &sources.Chunk{ + Data: []byte("line1\n" + // line 0 + "line2\n" + // line 1 + "AKIA1234567890ABCDEF\n" + // line 2 (first occurrence) + "line4\n" + // line 3 + "AKIA1234567890ABCDEF\n" + // line 4 (second occurrence) + "line6\n" + // line 5 + "AKIA1234567890ABCDEF\n"), // line 6 (third occurrence) + } + + results := []detectors.Result{ + {Raw: secret}, + {Raw: secret}, + {Raw: secret}, + } + expectedLines := []int64{2, 4, 6} + + AssignDuplicateLineOffsets(chunk, results) + + seen := make(map[int64]bool) + for i, res := range results { + lineOffset, _ := FragmentLineOffset(chunk, &res) + assert.Equal(t, expectedLines[i], lineOffset, + "result[%d]: expected line %d but got %d", i, expectedLines[i], lineOffset) + assert.False(t, seen[lineOffset], + "result[%d]: line %d was already reported by a previous result (duplicate line number)", i, lineOffset) + seen[lineOffset] = true + } +} + +func TestAssignDuplicateLineOffsets(t *testing.T) { + chunk := &sources.Chunk{ + Data: []byte("aaa\nbbb\naaa\nccc\naaa\n"), + } + results := []detectors.Result{ + {Raw: []byte("aaa")}, + {Raw: []byte("aaa")}, + {Raw: []byte("aaa")}, + {Raw: []byte("bbb")}, + } + AssignDuplicateLineOffsets(chunk, results) + + // Duplicates get offsets assigned. + assert.True(t, results[0].HasChunkOffset()) + assert.Equal(t, int64(0), results[0].ChunkOffset()) + + assert.True(t, results[1].HasChunkOffset()) + assert.Equal(t, int64(8), results[1].ChunkOffset()) // "aaa\nbbb\n" = 8 bytes + + assert.True(t, results[2].HasChunkOffset()) + assert.Equal(t, int64(16), results[2].ChunkOffset()) // "aaa\nbbb\naaa\nccc\n" = 16 bytes + + // Unique secret does not get an offset. + assert.False(t, results[3].HasChunkOffset()) +} + +func TestFragmentLineOffset_DuplicateSecretsWithIgnoreTag(t *testing.T) { + secret := []byte("mysecret") + chunk := &sources.Chunk{ + Data: []byte("mysecret\nfoo\nmysecret trufflehog:ignore\nbar\nmysecret\n"), + } + results := []detectors.Result{ + {Raw: secret}, + {Raw: secret}, + {Raw: secret}, + } + AssignDuplicateLineOffsets(chunk, results) + + line0, ignored0 := FragmentLineOffset(chunk, &results[0]) + assert.Equal(t, int64(0), line0) + assert.False(t, ignored0) + + line1, ignored1 := FragmentLineOffset(chunk, &results[1]) + assert.Equal(t, int64(2), line1) + assert.True(t, ignored1) + + line2, ignored2 := FragmentLineOffset(chunk, &results[2]) + assert.Equal(t, int64(4), line2) + assert.False(t, ignored2) +} + func setupFragmentLineOffsetBench(totalLines, needleLine int) (*sources.Chunk, *detectors.Result) { data := make([]byte, 0, 4096) needle := []byte("needle")