fix: parsing oss output [IDE-517] (#611)

Co-authored-by: Bastian Doetsch <[email protected]>
Co-authored-by: Shawky <[email protected]>

infrastructure/oss/cli_scanner.go

@@ -263,16 +263,7 @@ func (cliScanner *CLIScanner) unmarshallAndRetrieveAnalysis(ctx context.Context,
 	}
 
 	for _, scanResult := range scanResults {
-		targetFile := cliScanner.determineTargetFile(scanResult.DisplayTargetFile)
-		targetFilePath := scanResult.Path
-		if targetFilePath == "" {
-			targetFilePath = workDir
-		}
-		if targetFilePath == "" {
-			targetFilePath = targetFile
-		} else {
-			targetFilePath = filepath.Join(targetFilePath, targetFile)
-		}
+		targetFilePath := getAbsTargetFilePath(scanResult, workDir)
 		fileContent, err := os.ReadFile(targetFilePath)
 		if err != nil {
 			// don't fail the scan if we can't read the file. No annotations with ranges, though.
@@ -284,6 +275,31 @@ func (cliScanner *CLIScanner) unmarshallAndRetrieveAnalysis(ctx context.Context,
 	return issues
 }
 
+func getAbsTargetFilePath(scanResult scanResult, workDir string) string {
+	displayTargetFile := determineTargetFile(scanResult.DisplayTargetFile)
+
+	// if displayTargetFile is an absolute path, no need to do antyhing more
+	isAbs := filepath.IsAbs(displayTargetFile)
+	if isAbs {
+		return displayTargetFile
+	}
+
+	pathJoinedToBaseName := filepath.Join(scanResult.Path, displayTargetFile)
+
+	// if displayTargetFile is a subpath to the workdir add rel path to workdir
+	if uri.FolderContains(workDir, displayTargetFile) {
+		relative, err := filepath.Rel(workDir, displayTargetFile)
+		if err != nil {
+			// if displayTargetFile is not relative, we need to join path with basename
+			return pathJoinedToBaseName
+		}
+		return filepath.Join(workDir, relative)
+	}
+
+	// if displayTargetFile is just a basename, we need to join path with basename
+	return pathJoinedToBaseName
+}
+
 func (cliScanner *CLIScanner) unmarshallOssJson(res []byte) (scanResults []scanResult, err error) {
 	output := string(res)
 	if strings.HasPrefix(output, "[") {
@@ -348,14 +364,13 @@ func (cliScanner *CLIScanner) handleError(path string, err error, res []byte, cm
 	return true, cliError
 }
 
-func (cliScanner *CLIScanner) determineTargetFile(displayTargetFile string) string {
+func determineTargetFile(displayTargetFile string) string {
 	fileName := filepath.Base(displayTargetFile)
-	targetFileName := lockFilesToManifestMap[fileName]
-	if targetFileName == "" {
+	manifestFileName := lockFilesToManifestMap[fileName]
+	if manifestFileName == "" {
 		return displayTargetFile
 	}
-	targetFileName = strings.Replace(displayTargetFile, fileName, targetFileName, 1)
-	return targetFileName
+	return strings.Replace(displayTargetFile, fileName, manifestFileName, 1)
 }
 
 func (cliScanner *CLIScanner) retrieveIssues(

infrastructure/oss/cli_scanner_test.go

@@ -0,0 +1,141 @@
+/*
+ * © 2024 Snyk Limited
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package oss
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/snyk/snyk-ls/internal/testutil"
+)
+
+//nolint:dupl // not a real duplicate
+func TestCLIScanner_getAbsTargetFilePathForPackageManagers(t *testing.T) {
+	testutil.NotOnWindows(t, "filepaths are os dependent")
+	testCases := []struct {
+		name              string
+		displayTargetFile string
+		workDir           string
+		path              string
+		expected          string
+	}{
+		{
+			name:              "NPM root directory",
+			displayTargetFile: "package-lock.json",
+			workDir:           "/Users/cata/git/playground/juice-shop", // if we mock the workDir
+			path:              "/Users/cata/git/playground/juice-shop",
+			expected:          "/Users/cata/git/playground/juice-shop/package.json",
+		},
+		{
+			name:              "Poetry Sub Project (below the working directory)",
+			displayTargetFile: "poetry-sample/pyproject.toml",
+			workDir:           "/Users/cata/git/playground/python-goof",
+			path:              "/Users/cata/git/playground/python-goof",
+			expected:          "/Users/cata/git/playground/python-goof/poetry-sample/pyproject.toml",
+		},
+		{
+			name:              "Gradle multi-module",
+			displayTargetFile: "build.gradle",
+			workDir:           "/Users/bdoetsch/workspace/gradle-multi-module",
+			path:              "/Users/bdoetsch/workspace/gradle-multi-module/sample-api",
+			expected:          "/Users/bdoetsch/workspace/gradle-multi-module/sample-api/build.gradle",
+		},
+		{
+			name:              "Go Modules deeply nested",
+			displayTargetFile: "build/resources/test/test-fixtures/oss/annotator/go.mod",
+			workDir:           "/Users/cata/git/snyk/hammerhead/snyk-intellij-plugin",
+			path:              "/Users/cata/git/snyk/hammerhead/snyk-intellij-plugin",
+			expected:          "/Users/cata/git/snyk/hammerhead/snyk-intellij-plugin/build/resources/test/test-fixtures/oss/annotator/go.mod",
+		},
+		{
+			name:              "Maven test fixtures",
+			displayTargetFile: "src/test/resources/test-fixtures/oss/annotator/pom.xml",
+			workDir:           "/Users/cata/git/snyk/hammerhead/snyk-intellij-plugin",
+			path:              "/Users/cata/git/snyk/hammerhead/snyk-intellij-plugin",
+			expected:          "/Users/cata/git/snyk/hammerhead/snyk-intellij-plugin/src/test/resources/test-fixtures/oss/annotator/pom.xml",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := getAbsTargetFilePath(
+				scanResult{DisplayTargetFile: tc.displayTargetFile, Path: tc.path},
+				tc.workDir,
+			)
+			assert.Equal(t, tc.expected, actual)
+		})
+	}
+}
+
+//nolint:dupl // not a real duplicate
+func TestCLIScanner_getAbsTargetFilePathForPackageManagers_Windows(t *testing.T) {
+	testutil.OnlyOnWindows(t, "filepaths are os dependent")
+	testCases := []struct {
+		name              string
+		displayTargetFile string
+		workDir           string
+		path              string
+		expected          string
+	}{
+		{
+			name:              "NPM root directory",
+			displayTargetFile: "package-lock.json",
+			workDir:           "C:\\a\\cata\\git\\playground\\juice-shop",
+			path:              "C:\\a\\cata\\git\\playground\\juice-shop",
+			expected:          "C:\\a\\cata\\git\\playground\\juice-shop\\package.json",
+		},
+		{
+			name:              "Poetry Sub Project (below the working directory)",
+			displayTargetFile: "poetry-sample\\pyproject.toml",
+			workDir:           "C:\\a\\cata\\git\\playground\\python-goof",
+			path:              "C:\\a\\cata\\git\\playground\\python-goof",
+			expected:          "C:\\a\\cata\\git\\playground\\python-goof\\poetry-sample\\pyproject.toml",
+		},
+		{
+			name:              "Gradle multi-module",
+			displayTargetFile: "build.gradle",
+			workDir:           "C:\\a\\bdoetsch\\workspace\\gradle-multi-module",
+			path:              "C:\\a\\bdoetsch\\workspace\\gradle-multi-module\\sample-api",
+			expected:          "C:\\a\\bdoetsch\\workspace\\gradle-multi-module\\sample-api\\build.gradle",
+		},
+		{
+			name:              "Go Modules deeply nested",
+			displayTargetFile: "build\\resources\\test\\test-fixtures\\oss\\annotator\\go.mod",
+			workDir:           "C:\\a\\cata\\git\\snyk\\hammerhead\\snyk-intellij-plugin",
+			path:              "C:\\a\\cata\\git\\snyk\\hammerhead\\snyk-intellij-plugin",
+			expected:          "C:\\a\\cata\\git\\snyk\\hammerhead\\snyk-intellij-plugin\\build\\resources\\test\\test-fixtures\\oss\\annotator\\go.mod",
+		},
+		{
+			name:              "Maven test fixtures",
+			displayTargetFile: "src\\test\\resources\\test-fixtures\\oss\\annotator\\pom.xml",
+			workDir:           "C:\\a\\cata\\git\\snyk\\hammerhead\\snyk-intellij-plugin",
+			path:              "C:\\a\\cata\\git\\snyk\\hammerhead\\snyk-intellij-plugin",
+			expected:          "C:\\a\\cata\\git\\snyk\\hammerhead\\snyk-intellij-plugin\\src\\test\\resources\\test-fixtures\\oss\\annotator\\pom.xml",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := getAbsTargetFilePath(
+				scanResult{DisplayTargetFile: tc.displayTargetFile, Path: tc.path},
+				tc.workDir,
+			)
+			assert.Equal(t, tc.expected, actual)
+		})
+	}
+}

infrastructure/oss/oss_test.go

@@ -58,12 +58,10 @@ func Test_toIssueSeverity(t *testing.T) {
 }
 
 func Test_determineTargetFile(t *testing.T) {
-	c := testutil.UnitTest(t)
-	scanner := NewCLIScanner(c, performance.NewInstrumentor(), error_reporting.NewTestErrorReporter(), cli.NewTestExecutor(), getLearnMock(t), notification.NewNotifier()).(*CLIScanner)
-	assert.Equal(t, "package.json", scanner.determineTargetFile("package-lock.json"))
-	assert.Equal(t, "pom.xml", scanner.determineTargetFile("pom.xml"))
-	assert.Equal(t, "asdf", scanner.determineTargetFile("asdf"))
-	assert.Equal(t, "js/package.json", scanner.determineTargetFile("js/package-lock.json"))
+	assert.Equal(t, "package.json", determineTargetFile("package-lock.json"))
+	assert.Equal(t, "pom.xml", determineTargetFile("pom.xml"))
+	assert.Equal(t, "asdf", determineTargetFile("asdf"))
+	assert.Equal(t, "js/package.json", determineTargetFile("js/package-lock.json"))
 }
 
 func Test_FindRange(t *testing.T) {