Fix bugs when converting results to SARIF (#234)

tests/testdata/output/audit/audit_sarif.json

@@ -1106,7 +1106,7 @@
           "ruleIndex": 6,
           "level": "warning",
           "message": {
-            "text": "[CVE-2020-28500] lodash 4.17.0"
+            "text": "Security violation [CVE-2020-28500] lodash 4.17.0"
           },
           "locations": [
             {

tests/testdata/output/dockerscan/docker_results.json

@@ -616,7 +616,7 @@
                 ],
                 "executionSuccessful": true,
                 "workingDirectory": {
-                  "uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017/image.tar"
+                  "uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017"
                 }
               }
             ],
@@ -815,7 +815,7 @@
                 ],
                 "executionSuccessful": true,
                 "workingDirectory": {
-                  "uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017/image.tar"
+                  "uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017"
                 }
               }
             ],

tests/testdata/output/dockerscan/docker_sarif.json

@@ -119,7 +119,7 @@
           ],
           "executionSuccessful": true,
           "workingDirectory": {
-            "uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017/image.tar"
+            "uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017"
           }
         }
       ],
@@ -393,7 +393,7 @@
         {
           "executionSuccessful": true,
           "workingDirectory": {
-            "uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017/image.tar"
+            "uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017"
           }
         }
       ],

utils/formats/sarifutils/sarifutils.go

@@ -37,6 +37,17 @@ func GetToolVersion(run *sarif.Run) string {
 	return ""
 }
 
+func CopyRun(run *sarif.Run) *sarif.Run {
+	copy := CopyRunMetadata(run)
+	if copy.Tool.Driver != nil {
+		copy.Tool.Driver.Rules = CopyRules(run.Tool.Driver.Rules...)
+	}
+	for _, result := range run.Results {
+		copy.Results = append(copy.Results, CopyResult(result))
+	}
+	return copy
+}
+
 func CopyRunMetadata(run *sarif.Run) (copied *sarif.Run) {
 	if run == nil {
 		return
@@ -64,6 +75,21 @@ func CopyRunMetadata(run *sarif.Run) (copied *sarif.Run) {
 	return
 }
 
+func CopyRules(rules ...*sarif.ReportingDescriptor) (copied []*sarif.ReportingDescriptor) {
+	for _, rule := range rules {
+		cloned := sarif.NewRule(rule.ID)
+		cloned.HelpURI = copyStrAttribute(rule.HelpURI)
+		cloned.Name = copyStrAttribute(rule.Name)
+		cloned.ShortDescription = copyMultiMsgAttribute(rule.ShortDescription)
+		cloned.FullDescription = copyMultiMsgAttribute(rule.FullDescription)
+		cloned.Help = copyMultiMsgAttribute(rule.Help)
+		cloned.Properties = rule.Properties
+		cloned.MessageStrings = rule.MessageStrings
+		copied = append(copied, cloned)
+	}
+	return
+}
+
 func GetRunToolFullName(run *sarif.Run) string {
 	if run.Tool.Driver != nil && run.Tool.Driver.FullName != nil {
 		return *run.Tool.Driver.FullName
@@ -134,9 +160,9 @@ func CopyResult(result *sarif.Result) *sarif.Result {
 		RuleIndex:    result.RuleIndex,
 		Kind:         result.Kind,
 		Fingerprints: result.Fingerprints,
-		CodeFlows:    result.CodeFlows,
+		CodeFlows:    copyCodeFlows(result.CodeFlows...),
 		Level:        result.Level,
-		Message:      result.Message,
+		Message:      copyMsgAttribute(result.Message),
 		PropertyBag:  result.PropertyBag,
 	}
 	for _, location := range result.Locations {
@@ -145,6 +171,47 @@ func CopyResult(result *sarif.Result) *sarif.Result {
 	return copied
 }
 
+func copyCodeFlows(flows ...*sarif.CodeFlow) []*sarif.CodeFlow {
+	var copied []*sarif.CodeFlow
+	for _, flow := range flows {
+		copied = append(copied, copyCodeFlow(flow))
+	}
+	return copied
+}
+
+func copyCodeFlow(flow *sarif.CodeFlow) *sarif.CodeFlow {
+	copied := &sarif.CodeFlow{}
+	for _, threadFlow := range flow.ThreadFlows {
+		copied.ThreadFlows = append(copied.ThreadFlows, copyThreadFlow(threadFlow))
+	}
+	return copied
+}
+
+func copyThreadFlow(threadFlow *sarif.ThreadFlow) *sarif.ThreadFlow {
+	copied := &sarif.ThreadFlow{}
+	for _, location := range threadFlow.Locations {
+		copied.Locations = append(copied.Locations, sarif.NewThreadFlowLocation().WithLocation(CopyLocation(location.Location)))
+	}
+	return copied
+}
+
+func copyMsgAttribute(attr sarif.Message) sarif.Message {
+	return sarif.Message{
+		Text:     copyStrAttribute(attr.Text),
+		Markdown: copyStrAttribute(attr.Markdown),
+	}
+}
+
+func copyMultiMsgAttribute(attr *sarif.MultiformatMessageString) *sarif.MultiformatMessageString {
+	if attr == nil {
+		return nil
+	}
+	return &sarif.MultiformatMessageString{
+		Text:     copyStrAttribute(attr.Text),
+		Markdown: copyStrAttribute(attr.Markdown),
+	}
+}
+
 func copyStrAttribute(attr *string) *string {
 	if attr == nil {
 		return nil
@@ -190,9 +257,9 @@ func CopyLocation(location *sarif.Location) *sarif.Location {
 	copied.Properties = location.Properties
 	for _, logicalLocation := range location.LogicalLocations {
 		copied.LogicalLocations = append(copied.LogicalLocations, &sarif.LogicalLocation{
-			Name:               logicalLocation.Name,
-			FullyQualifiedName: logicalLocation.FullyQualifiedName,
-			DecoratedName:      logicalLocation.DecoratedName,
+			Name:               copyStrAttribute(logicalLocation.Name),
+			FullyQualifiedName: copyStrAttribute(logicalLocation.FullyQualifiedName),
+			DecoratedName:      copyStrAttribute(logicalLocation.DecoratedName),
 			Kind:               logicalLocation.Kind,
 			PropertyBag:        logicalLocation.PropertyBag,
 		})

utils/results/conversion/sarifparser/sarifparser.go

@@ -110,8 +110,13 @@ func (sc *CmdResultsSarifConverter) ParseNewTargetResults(target results.ScanTar
 func (sc *CmdResultsSarifConverter) createScaRun(target results.ScanTarget, errorCount int) *sarif.Run {
 	run := sarif.NewRunWithInformationURI(ScaScannerToolName, utils.BaseDocumentationURL+"sca")
 	run.Tool.Driver.Version = &sc.xrayVersion
+	wd := target.Target
+	if sc.currentCmdType.IsTargetBinary() {
+		// For binary, the target is a file and not a directory
+		wd = filepath.Dir(wd)
+	}
 	run.Invocations = append(run.Invocations, sarif.NewInvocation().
-		WithWorkingDirectory(sarif.NewSimpleArtifactLocation(target.Target)).
+		WithWorkingDirectory(sarif.NewSimpleArtifactLocation(wd)).
 		WithExecutionSuccess(errorCount == 0),
 	)
 	return run
@@ -240,7 +245,7 @@ func addSarifScaVulnerability(cmdType utils.CommandType, sarifResults *[]*sarif.
 		if err != nil {
 			return err
 		}
-		currentResults, currentRule := parseScaToSarifFormat(cmdType, vulnerability.IssueId, vulnerability.Summary, markdownDescription, maxCveScore, getScaIssueSarifHeadline, cves, severity, applicabilityStatus, impactedPackagesName, impactedPackagesVersion, fixedVersions, directComponents)
+		currentResults, currentRule := parseScaToSarifFormat(cmdType, vulnerability.IssueId, vulnerability.Summary, markdownDescription, maxCveScore, getScaVulnerabilitySarifHeadline, cves, severity, applicabilityStatus, impactedPackagesName, impactedPackagesVersion, fixedVersions, directComponents)
 		cveImpactedComponentRuleId := results.GetScaIssueId(impactedPackagesName, impactedPackagesVersion, results.GetIssueIdentifier(cves, vulnerability.IssueId, "_"))
 		if _, ok := (*rules)[cveImpactedComponentRuleId]; !ok {
 			// New Rule
@@ -261,7 +266,7 @@ func addSarifScaSecurityViolation(cmdType utils.CommandType, sarifResults *[]*sa
 		if err != nil {
 			return err
 		}
-		currentResults, currentRule := parseScaToSarifFormat(cmdType, violation.IssueId, violation.Summary, markdownDescription, maxCveScore, getScaIssueSarifHeadline, cves, severity, applicabilityStatus, impactedPackagesName, impactedPackagesVersion, fixedVersions, directComponents, violation.WatchName)
+		currentResults, currentRule := parseScaToSarifFormat(cmdType, violation.IssueId, violation.Summary, markdownDescription, maxCveScore, getScaSecurityViolationSarifHeadline, cves, severity, applicabilityStatus, impactedPackagesName, impactedPackagesVersion, fixedVersions, directComponents, violation.WatchName)
 		cveImpactedComponentRuleId := results.GetScaIssueId(impactedPackagesName, impactedPackagesVersion, results.GetIssueIdentifier(cves, violation.IssueId, "_"))
 		if _, ok := (*rules)[cveImpactedComponentRuleId]; !ok {
 			// New Rule
@@ -396,10 +401,14 @@ func getDirectDependenciesFormatted(directDependencies []formats.ComponentRow) (
 	return strings.TrimSuffix(formattedDirectDependencies.String(), "<br/>"), nil
 }
 
-func getScaIssueSarifHeadline(depName, version, issueId string) string {
+func getScaVulnerabilitySarifHeadline(depName, version, issueId string) string {
 	return fmt.Sprintf("[%s] %s %s", issueId, depName, version)
 }
 
+func getScaSecurityViolationSarifHeadline(depName, version, key string) string {
+	return fmt.Sprintf("Security violation %s", getScaVulnerabilitySarifHeadline(depName, version, key))
+}
+
 func getXrayLicenseSarifHeadline(depName, version, key string) string {
 	return fmt.Sprintf("License violation [%s] in %s %s", key, depName, version)
 }
@@ -417,21 +426,21 @@ func getScaLicenseViolationMarkdown(depName, version, key string, directDependen
 }
 
 func patchRunsToPassIngestionRules(cmdType utils.CommandType, subScanType utils.SubScanType, patchBinaryPaths bool, target results.ScanTarget, runs ...*sarif.Run) []*sarif.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, runs...)
 	patchedRuns := []*sarif.Run{}
 	// Patch changes may alter the original run, so we will create a new run for each
 	for _, run := range runs {
-		patched := sarifutils.CopyRunMetadata(run)
+		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)
 		}
 		if patched.Tool.Driver != nil {
-			patched.Tool.Driver.Rules = patchRules(cmdType, subScanType, run.Tool.Driver.Rules...)
+			patched.Tool.Driver.Rules = patchRules(cmdType, subScanType, patched.Tool.Driver.Rules...)
 		}
-		patched.Results = patchResults(cmdType, subScanType, patchBinaryPaths, target, run, run.Results...)
+		patched.Results = patchResults(cmdType, subScanType, patchBinaryPaths, target, patched, patched.Results...)
 		patchedRuns = append(patchedRuns, patched)
 	}
 	return patchedRuns
@@ -470,28 +479,20 @@ func patchDockerSecretLocations(result *sarif.Result) {
 func patchRules(commandType utils.CommandType, subScanType utils.SubScanType, rules ...*sarif.ReportingDescriptor) (patched []*sarif.ReportingDescriptor) {
 	patched = []*sarif.ReportingDescriptor{}
 	for _, rule := range rules {
-		cloned := sarif.NewRule(rule.ID)
 		if rule.Name != nil && rule.ID == *rule.Name {
 			// SARIF1001 - if both 'id' and 'name' are present, they must be different. If they are identical, the tool must omit the 'name' property.
-			cloned.Name = rule.Name
+			rule.Name = nil
 		}
-		cloned.ShortDescription = rule.ShortDescription
 		if commandType.IsTargetBinary() && subScanType == utils.SecretsScan {
 			// Patch the rule name in case of binary scan
-			sarifutils.SetRuleShortDescriptionText(fmt.Sprintf("[Secret in Binary found] %s", sarifutils.GetRuleShortDescriptionText(rule)), cloned)
+			sarifutils.SetRuleShortDescriptionText(fmt.Sprintf("[Secret in Binary found] %s", sarifutils.GetRuleShortDescriptionText(rule)), rule)
 		}
-		cloned.FullDescription = rule.FullDescription
-		cloned.Help = rule.Help
-		if cloned.Help == nil {
+		if rule.Help == nil {
 			// Github code scanning ingestion rules rejects rules without help content.
 			// Patch by transferring the full description to the help field.
-			cloned.Help = rule.FullDescription
+			rule.Help = rule.FullDescription
 		}
-		cloned.HelpURI = rule.HelpURI
-		cloned.Properties = rule.Properties
-		cloned.MessageStrings = rule.MessageStrings
-
-		patched = append(patched, cloned)
+		patched = append(patched, rule)
 	}
 	return
 }
@@ -734,7 +735,7 @@ func calculateResultFingerprints(resultType utils.CommandType, run *sarif.Run, r
 	if !resultType.IsTargetBinary() {
 		return nil
 	}
-	ids := []string{sarifutils.GetRunToolName(run), sarifutils.GetResultRuleId(result)}
+	ids := []string{sarifutils.GetRunToolName(run), sarifutils.GetResultRuleId(result), getResultWatches(result)}
 	for _, location := range sarifutils.GetResultFileLocations(result) {
 		ids = append(ids, strings.ReplaceAll(location, string(filepath.Separator), "/"))
 	}
@@ -747,3 +748,12 @@ func calculateResultFingerprints(resultType utils.CommandType, run *sarif.Run, r
 	sarifutils.SetResultFingerprint(jfrogFingerprintAlgorithmName, hashValue, result)
 	return nil
 }
+
+func getResultWatches(result *sarif.Result) (watches string) {
+	if watchesProperty, ok := result.Properties[WatchSarifPropertyKey]; ok {
+		if watchesValue, ok := watchesProperty.(string); ok {
+			return watchesValue
+		}
+	}
+	return
+}

utils/results/conversion/sarifparser/sarifparser_test.go

@@ -65,8 +65,13 @@ func TestGetComponentSarifLocation(t *testing.T) {
 }
 
 func TestGetVulnerabilityOrViolationSarifHeadline(t *testing.T) {
-	assert.Equal(t, "[CVE-2022-1234] loadsh 1.4.1", getScaIssueSarifHeadline("loadsh", "1.4.1", "CVE-2022-1234"))
-	assert.NotEqual(t, "[CVE-2022-1234] comp 1.4.1", getScaIssueSarifHeadline("comp", "1.2.1", "CVE-2022-1234"))
+	assert.Equal(t, "[CVE-2022-1234] loadsh 1.4.1", getScaVulnerabilitySarifHeadline("loadsh", "1.4.1", "CVE-2022-1234"))
+	assert.NotEqual(t, "[CVE-2022-1234] comp 1.4.1", getScaVulnerabilitySarifHeadline("comp", "1.2.1", "CVE-2022-1234"))
+}
+
+func TestGetScaSecurityViolationSarifHeadline(t *testing.T) {
+	assert.Equal(t, "Security violation [CVE-2022-1234] loadsh 1.4.1", getScaSecurityViolationSarifHeadline("loadsh", "1.4.1", "CVE-2022-1234"))
+	assert.NotEqual(t, "[CVE-2022-1234] comp 1.2.1", getScaSecurityViolationSarifHeadline("comp", "1.2.1", "CVE-2022-1234"))
 }
 
 func TestGetXrayLicenseSarifHeadline(t *testing.T) {
@@ -411,7 +416,7 @@ func TestPatchRunsToPassIngestionRules(t *testing.T) {
 					},
 					Invocations: []*sarif.Invocation{sarif.NewInvocation().WithWorkingDirectory(sarif.NewSimpleArtifactLocation(wd))},
 					Results: []*sarif.Result{
-						sarifutils.CreateDummyResultWithFingerprint(fmt.Sprintf("🔒 Found Secrets in Binary docker scanning:\nImage: dockerImage:imageVersion\nLayer (sha1): 9e88ea9de1b44baba5e96a79e33e4af64334b2bf129e838e12f6dae71b5c86f0\nFilepath: %s\nEvidence: snippet", filepath.Join("usr", "src", "app", "server", "index.js")), "", jfrogFingerprintAlgorithmName, "93d660ebfd39b1220c42c0beb6e4e863",
+						sarifutils.CreateDummyResultWithFingerprint(fmt.Sprintf("🔒 Found Secrets in Binary docker scanning:\nImage: dockerImage:imageVersion\nLayer (sha1): 9e88ea9de1b44baba5e96a79e33e4af64334b2bf129e838e12f6dae71b5c86f0\nFilepath: %s\nEvidence: snippet", filepath.Join("usr", "src", "app", "server", "index.js")), "", jfrogFingerprintAlgorithmName, "dee156c9fd75a4237102dc8fb29277a2",
 							sarifutils.CreateDummyLocationWithPathAndLogicalLocation(filepath.Join("usr", "src", "app", "server", "index.js"), "9e88ea9de1b44baba5e96a79e33e4af64334b2bf129e838e12f6dae71b5c86f0", "layer", "algorithm", "sha1"),
 						),
 					},

utils/validations/test_validate_sarif.go

@@ -191,7 +191,6 @@ func getResultByResultId(expected *sarif.Result, actual []*sarif.Result) *sarif.
 	log.Output("====================================")
 	log.Output(fmt.Sprintf(":: Actual results with expected results: %s", getResultId(expected)))
 	for _, result := range actual {
-
 		log.Output(fmt.Sprintf("Compare actual result (isPotential=%t, hasSameLocations=%t) with expected result: %s", isPotentialSimilarResults(expected, result), hasSameLocations(expected, result), getResultId(result)))
 		if isPotentialSimilarResults(expected, result) && hasSameLocations(expected, result) {
 			return result
@@ -202,7 +201,7 @@ func getResultByResultId(expected *sarif.Result, actual []*sarif.Result) *sarif.
 }
 
 func isPotentialSimilarResults(expected, actual *sarif.Result) bool {
-	return sarifutils.GetResultRuleId(actual) == sarifutils.GetResultRuleId(expected) && sarifutils.GetResultMsgText(actual) == sarifutils.GetResultMsgText(expected) && sarifutils.GetResultProperty(sarifparser.WatchSarifPropertyKey, actual) == sarifutils.GetResultProperty(sarifparser.WatchSarifPropertyKey, expected)
+	return sarifutils.GetResultRuleId(actual) == sarifutils.GetResultRuleId(expected) && sarifutils.GetResultProperty(sarifparser.WatchSarifPropertyKey, actual) == sarifutils.GetResultProperty(sarifparser.WatchSarifPropertyKey, expected)
 }
 
 func getResultId(result *sarif.Result) string {