From bf89c89e09021007f55c75febbe8f6716b914b51 Mon Sep 17 00:00:00 2001
From: ahrav <ahravdutta02@gmail.com>
Date: Tue, 5 Nov 2024 08:53:18 -0800
Subject: [PATCH] [bug] - Correct Line Number Calculation (#3550)

* correclty report line number

* add addiitonal test cases
---
 pkg/engine/engine.go      |   8 ++-
 pkg/engine/engine_test.go | 116 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go
index 1c79fe50f9a3..d4af8d5a2ffe 100644
--- a/pkg/engine/engine.go
+++ b/pkg/engine/engine.go
@@ -1285,8 +1285,14 @@ func FragmentFirstLineAndLink(chunk *sources.Chunk) (int64, *int64, string) {
 		fragmentStart = &metadata.AzureRepos.Line
 		link = metadata.AzureRepos.Link
 	default:
-		return 0, nil, ""
+		return 1, nil, ""
+	}
+
+	// Ensure we maintain 1-based line indexing if fragmentStart is not set or is 0.
+	if *fragmentStart == 0 {
+		*fragmentStart = 1
 	}
+
 	return *fragmentStart, fragmentStart, link
 }
 
diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go
index 80e317aa9eef..9be786d93742 100644
--- a/pkg/engine/engine_test.go
+++ b/pkg/engine/engine_test.go
@@ -281,6 +281,108 @@ func TestEngine_DuplicateSecrets(t *testing.T) {
 	assert.Equal(t, want, e.GetMetrics().UnverifiedSecretsFound)
 }
 
+// lineCaptureDispatcher is a test dispatcher that captures the line number
+// of detected secrets. It implements the Dispatcher interface and is used
+// to verify that the Engine correctly identifies and reports the line numbers
+// where secrets are found in the source code.
+type lineCaptureDispatcher struct{ line int64 }
+
+func (d *lineCaptureDispatcher) Dispatch(_ context.Context, result detectors.ResultWithMetadata) error {
+	d.line = result.SourceMetadata.GetFilesystem().GetLine()
+	return nil
+}
+
+func TestEngineLineVariations(t *testing.T) {
+	tests := []struct {
+		name         string
+		content      string
+		expectedLine int64
+	}{
+		{
+			name: "secret on first line",
+			content: `AKIA2OGYBAH6STMMNXNN
+aws_secret_access_key = 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`,
+			expectedLine: 1,
+		},
+		{
+			name: "secret after multiple newlines",
+			content: `
+
+
+AKIA2OGYBAH6STMMNXNN
+aws_secret_access_key = 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`,
+			expectedLine: 4,
+		},
+		{
+			name: "secret with mixed whitespace before",
+			content: `first line
+   
+		
+AKIA2OGYBAH6STMMNXNN
+aws_secret_access_key = 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`,
+			expectedLine: 4,
+		},
+		{
+			name: "secret with content after",
+			content: `[default]
+region = us-east-1
+AKIA2OGYBAH6STMMNXNN
+aws_secret_access_key = 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
+more content
+even more`,
+			expectedLine: 3,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+			defer cancel()
+
+			tmpFile, err := os.CreateTemp("", "test_aws_credentials")
+			assert.NoError(t, err)
+			defer os.Remove(tmpFile.Name())
+
+			err = os.WriteFile(tmpFile.Name(), []byte(tt.content), os.ModeAppend)
+			assert.NoError(t, err)
+
+			const defaultOutputBufferSize = 64
+			opts := []func(*sources.SourceManager){
+				sources.WithSourceUnits(),
+				sources.WithBufferedOutput(defaultOutputBufferSize),
+			}
+
+			sourceManager := sources.NewManager(opts...)
+			lineCapturer := new(lineCaptureDispatcher)
+
+			conf := Config{
+				Concurrency:   1,
+				Decoders:      decoders.DefaultDecoders(),
+				Detectors:     DefaultDetectors(),
+				Verify:        false,
+				SourceManager: sourceManager,
+				Dispatcher:    lineCapturer,
+			}
+
+			eng, err := NewEngine(ctx, &conf)
+			assert.NoError(t, err)
+
+			eng.Start(ctx)
+
+			cfg := sources.FilesystemConfig{Paths: []string{tmpFile.Name()}}
+			err = eng.ScanFileSystem(ctx, cfg)
+			assert.NoError(t, err)
+
+			assert.NoError(t, eng.Finish(ctx))
+			want := uint64(1)
+			assert.Equal(t, want, eng.GetMetrics().UnverifiedSecretsFound)
+			assert.Equal(t, tt.expectedLine, lineCapturer.line)
+		})
+	}
+}
+
 // TestEngine_VersionedDetectorsVerifiedSecrets is a test that detects ALL verified secrets across
 // versioned detectors.
 func TestEngine_VersionedDetectorsVerifiedSecrets(t *testing.T) {
@@ -637,6 +739,20 @@ func TestFragmentFirstLineAndLink(t *testing.T) {
 			expectedLine: 5,
 			expectedLink: "https://example.azure.com",
 		},
+		{
+			name: "Line number not set",
+			chunk: &sources.Chunk{
+				SourceMetadata: &source_metadatapb.MetaData{
+					Data: &source_metadatapb.MetaData_Github{
+						Github: &source_metadatapb.Github{
+							Link: "https://example.github.com",
+						},
+					},
+				},
+			},
+			expectedLine: 1,
+			expectedLink: "https://example.github.com",
+		},
 		{
 			name:         "Unsupported Type",
 			chunk:        &sources.Chunk{},