Always output relative paths in commands (jfrog#580)

Author: attiasas

jas/common.go

@@ -236,6 +236,7 @@ func processSarifRuns(sarifRuns []*sarif.Run, wd string, informationUrlSuffix st
 		sarifRun.Results = excludeSuppressResults(sarifRun.Results)
 		sarifRun.Results = excludeMinSeverityResults(sarifRun.Results, minSeverity)
 	}
+	sarifutils.ConvertRunsPathsToRelative(sarifRuns...)
 }
 
 func fillMissingRequiredDriverInformation(defaultJasInformationUri, defaultVersion string, run *sarif.Run) {

jas/common_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/owenrumney/go-sarif/v3/pkg/report/v210/sarif"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"golang.org/x/exp/slices"
 
 	jfrogAppsConfig "github.com/jfrog/jfrog-apps-config/go"
@@ -19,6 +20,7 @@ import (
 	"github.com/jfrog/jfrog-cli-security/utils/formats/sarifutils"
 	"github.com/jfrog/jfrog-cli-security/utils/jasutils"
 	"github.com/jfrog/jfrog-cli-security/utils/results"
+	"github.com/jfrog/jfrog-cli-security/utils/severityutils"
 	"github.com/jfrog/jfrog-cli-security/utils/techutils"
 )
 
@@ -593,3 +595,48 @@ func TestGetResultsToCompare(t *testing.T) {
 		})
 	}
 }
+
+func TestProcessSarifRuns(t *testing.T) {
+	wd, err := os.Getwd()
+	assert.NoError(t, err)
+
+	// Create dummy SARIF report.
+	dummyReport := sarif.NewReport()
+	dummyReport.AddRun(sarifutils.CreateRunWithDummyResults(
+		// Result below the minimum severity.
+		sarifutils.CreateResultWithOneLocation(fmt.Sprintf("file://%s", filepath.Join(wd, "file1")), 0, 1, 2, 3, "snippet", "rule1", "note"),
+		// Suppressed result.
+		sarifutils.CreateResultWithOneLocation(fmt.Sprintf("file://%s", filepath.Join(wd, "file3")), 0, 0, 0, 0, "snippet", "rule1", "warning").WithSuppressions([]*sarif.Suppression{sarif.NewSuppression()}),
+		// Valid result.
+		sarifutils.CreateResultWithOneLocation(fmt.Sprintf("file://%s", filepath.Join(wd, "dir", "file2")), 0, 0, 0, 0, "snippet", "rule1", "error"),
+	))
+
+	processSarifRuns(dummyReport.Runs, wd, "docs URL", severityutils.High)
+	run := dummyReport.Runs[0]
+
+	// Check Invocation added.
+	require.NotNil(t, run.Invocations)
+	require.Len(t, run.Invocations, 1)
+	require.NotNil(t, run.Invocations[0].WorkingDirectory)
+	require.NotNil(t, run.Invocations[0].WorkingDirectory.URI)
+	require.Equal(t, *run.Invocations[0].WorkingDirectory.URI, utils.ToURI(wd))
+
+	// Check driver info.
+	driver := run.Tool.Driver
+	require.NotNil(t, driver)
+	require.NotNil(t, driver.Version)
+	require.NotEmpty(t, *driver.Version)
+	require.NotNil(t, driver.InformationURI)
+	require.NotEmpty(t, *driver.InformationURI)
+
+	// Check severity level mapping.
+	require.Len(t, driver.Rules, 1)
+	rule := driver.Rules[0]
+	require.Equal(t, "8.9", sarifutils.GetRuleProperty(severityutils.SarifSeverityRuleProperty, rule))
+
+	// Check minimum severity and suppression filtering.
+	require.Len(t, run.Results, 1)
+	// Check file paths are relative and with / separators.
+	result := run.Results[0]
+	require.Equal(t, "dir/file2", sarifutils.GetLocationFileName(result.Locations[0]))
+}

sca/bom/buildinfo/technologies/gem/gem_test.go

@@ -12,7 +12,7 @@ import (
 	"github.com/jfrog/jfrog-cli-security/utils"
 )
 
-var expectedUniqueDeps = []string{"rubygems://puma:5.6.9", "rubygems://nio4r:2.7.4"}
+var expectedUniqueDeps = []string{"rubygems://puma:5.6.9", "rubygems://nio4r:2.7.5"}
 
 var expectedResult = &xrayUtils.GraphNode{
 	Id: "root",
@@ -21,7 +21,7 @@ var expectedResult = &xrayUtils.GraphNode{
 			Id: "rubygems://puma:5.6.9",
 			Nodes: []*xrayUtils.GraphNode{
 				{
-					Id:    "rubygems://nio4r:2.7.4",
+					Id:    "rubygems://nio4r:2.7.5",
 					Nodes: []*xrayUtils.GraphNode{},
 				},
 			},
@@ -52,15 +52,15 @@ func TestBuildDependencyTree(t *testing.T) {
 }
 
 // expectedUniqueDeps should be defined
-// expectedUniqueDeps := []string{"rubygems://puma:5.6.9", "rubygems://nio4r:2.7.4"}
+// expectedUniqueDeps := []string{"rubygems://puma:5.6.9", "rubygems://nio4r:2.7.5"}
 func TestCalculateUniqueDeps(t *testing.T) {
 	var input = &xrayUtils.GraphNode{
 		Nodes: []*xrayUtils.GraphNode{
 			{
 				Id: "rubygems://puma:5.6.9",
 				Nodes: []*xrayUtils.GraphNode{
 					{
-						Id:    "rubygems://nio4r:2.7.4",
+						Id:    "rubygems://nio4r:2.7.5",
 						Nodes: []*xrayUtils.GraphNode{},
 					},
 				},

sca/bom/buildinfo/technologies/pnpm/pnpm_test.go

@@ -43,7 +43,7 @@ func TestBuildDependencyTreeLimitedDepth(t *testing.T) {
 			name:      "With transitive dependencies",
 			treeDepth: "1",
 			expectedUniqueDeps: []string{
-				"npm://axios:1.12.2",
+				"npm://axios:1.13.1",
 				"npm://balaganjs:1.0.0",
 				"npm://yargs:13.3.0",
 				"npm://zen-website:1.0.0",
@@ -53,7 +53,7 @@ func TestBuildDependencyTreeLimitedDepth(t *testing.T) {
 				Nodes: []*xrayUtils.GraphNode{
 					{
 						Id:    "npm://balaganjs:1.0.0",
-						Nodes: []*xrayUtils.GraphNode{{Id: "npm://axios:1.12.2"}, {Id: "npm://yargs:13.3.0"}},
+						Nodes: []*xrayUtils.GraphNode{{Id: "npm://axios:1.13.1"}, {Id: "npm://yargs:13.3.0"}},
 					},
 				},
 			},

tests/testdata/output/audit/audit_simple_json.json

@@ -10,7 +10,7 @@
           "name": "lodash",
           "version": "4.17.0",
           "location": {
-            "file": "Users/user/project-with-issues/package.json"
+            "file": "package.json"
           }
         }
       ],
@@ -58,7 +58,7 @@
           "name": "lodash",
           "version": "4.17.0",
           "location": {
-            "file": "Users/user/project-with-issues/package.json"
+            "file": "package.json"
           }
         }
       ],
@@ -110,7 +110,7 @@
           "name": "lodash",
           "version": "4.17.0",
           "location": {
-            "file": "Users/user/project-with-issues/package.json"
+            "file": "package.json"
           }
         }
       ],
@@ -170,7 +170,7 @@
           "name": "lodash",
           "version": "4.17.0",
           "location": {
-            "file": "Users/user/project-with-issues/package.json"
+            "file": "package.json"
           }
         }
       ],
@@ -216,7 +216,7 @@
           "name": "jake",
           "version": "10.8.7",
           "location": {
-            "file": "Users/user/project-with-issues/package.json"
+            "file": "package.json"
           }
         }
       ],
@@ -299,7 +299,7 @@
           "name": "lodash",
           "version": "4.17.0",
           "location": {
-            "file": "Users/user/project-with-issues/package.json"
+            "file": "package.json"
           }
         }
       ],
@@ -349,7 +349,7 @@
           "name": "jake",
           "version": "10.8.7",
           "location": {
-            "file": "Users/user/project-with-issues/package.json"
+            "file": "package.json"
           }
         }
       ],
@@ -425,7 +425,7 @@
           "name": "lodash",
           "version": "4.17.0",
           "location": {
-            "file": "Users/user/project-with-issues/package.json"
+            "file": "package.json"
           }
         }
       ],
@@ -449,7 +449,7 @@
           "name": "lodash",
           "version": "4.17.0",
           "location": {
-            "file": "Users/user/project-with-issues/package.json"
+            "file": "package.json"
           }
         }
       ],

utils/results/common.go

@@ -231,6 +231,10 @@ func getBestMatch(target ScanTarget, descriptors []string) string {
 			bestMatch = descriptor
 		}
 	}
+	if bestMatch != "" {
+		// convert to relative path
+		bestMatch = utils.GetRelativePath(bestMatch, target.Target)
+	}
 	return bestMatch
 }
 

utils/results/conversion/sarifparser/sarifparser.go

@@ -615,7 +615,7 @@ func getComponentSarifLocation(cmtType utils.CommandType, component formats.Comp
 			logicalLocations = append(logicalLocations, logicalLocation)
 		}
 	}
-	location := sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithURI("file://" + filePath)))
+	location := sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithURI(filepath.ToSlash(filePath))))
 	if len(logicalLocations) > 0 {
 		location.WithLogicalLocations(logicalLocations)
 	}
@@ -696,9 +696,6 @@ func patchRunsToPassIngestionRules(baseJfrogUrl string, cmdType utils.CommandTyp
 	// Patch changes may alter the original run, so we will create a new run for each
 	for _, run := range runs {
 		patched := sarifutils.CopyRun(run)
-		// Since we run in temp directories files should be relative
-		// Patch by converting the file paths to relative paths according to the invocations
-		convertPaths(cmdType, subScanType, patched)
 		if cmdType.IsTargetBinary() && subScanType == utils.SecretsScan {
 			// Patch the tool name in case of binary scan
 			sarifutils.SetRunToolName(BinarySecretScannerToolName, patched)
@@ -712,20 +709,6 @@ func patchRunsToPassIngestionRules(baseJfrogUrl string, cmdType utils.CommandTyp
 	return patchedRuns
 }
 
-func convertPaths(commandType utils.CommandType, subScanType utils.SubScanType, runs ...*sarif.Run) {
-	// Convert base on invocation for source code
-	sarifutils.ConvertRunsPathsToRelative(runs...)
-	if !(commandType == utils.DockerImage && subScanType == utils.SecretsScan) {
-		return
-	}
-	for _, run := range runs {
-		for _, result := range run.Results {
-			// For Docker secret scan, patch the logical location if not exists
-			patchDockerSecretLocations(result)
-		}
-	}
-}
-
 // Patch the URI to be the file path from sha<number>/<hash>/
 // Extract the layer from the location URI, adds it as a logical location kind "layer"
 func patchDockerSecretLocations(result *sarif.Result) {
@@ -780,6 +763,10 @@ func patchResults(commandType utils.CommandType, subScanType utils.SubScanType,
 			log.Debug(fmt.Sprintf("[%s] Removing result [ruleId=%s] without locations: %s", subScanType.String(), sarifutils.GetResultRuleId(result), sarifutils.GetResultMsgText(result)))
 			continue
 		}
+		if commandType == utils.DockerImage && subScanType == utils.SecretsScan {
+			// For Docker secret scan, patch the logical location if not exists
+			patchDockerSecretLocations(result)
+		}
 		patchResultMsg(result, target, commandType, subScanType, isJasViolations)
 		if commandType.IsTargetBinary() {
 			if patchBinaryPaths {
@@ -850,7 +837,8 @@ func getDockerfileLocationIfExists(run *sarif.Run) string {
 			return location
 		}
 	}
-	if workspace := os.Getenv(utils.CurrentGithubWorkflowWorkspaceEnvVar); workspace != "" {
+	// Validate file path to prevent directory traversal
+	if workspace := os.Getenv(utils.CurrentGithubWorkflowWorkspaceEnvVar); workspace != "" && !strings.Contains(workspace, "..") {
 		if exists, err := fileutils.IsFileExists(filepath.Join(workspace, "Dockerfile"), false); err == nil && exists {
 			return filepath.Join(workspace, "Dockerfile")
 		}
@@ -862,7 +850,8 @@ func getGithubWorkflowsDirIfExists() string {
 	if exists, err := fileutils.IsDirExists(GithubBaseWorkflowDir, false); err == nil && exists {
 		return GithubBaseWorkflowDir
 	}
-	if workspace := os.Getenv(utils.CurrentGithubWorkflowWorkspaceEnvVar); workspace != "" {
+	// Validate file path to prevent directory traversal
+	if workspace := os.Getenv(utils.CurrentGithubWorkflowWorkspaceEnvVar); workspace != "" && !strings.Contains(workspace, "..") {
 		if exists, err := fileutils.IsDirExists(filepath.Join(workspace, GithubBaseWorkflowDir), false); err == nil && exists {
 			return filepath.Join(workspace, GithubBaseWorkflowDir)
 		}