Audit Xray scan Sarif output (#996)

jfrog · Oct 17, 2023 · b7891b0 · b7891b0
1 parent 720c324
commit b7891b0
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 66 deletions.
diff --git a/xray/commands/scan/buildscan.go b/xray/commands/scan/buildscan.go
@@ -101,7 +101,7 @@ func (bsc *BuildScanCommand) Run() (err error) {
 		Rescan:      bsc.rescan,
 	}
 
-	isFailBuildResponse, err := bsc.runBuildScanAndPrintResults(xrayManager, params)
+	isFailBuildResponse, err := bsc.runBuildScanAndPrintResults(xrayManager, xrayVersion, params)
 	if err != nil {
 		return err
 	}
@@ -112,7 +112,7 @@ func (bsc *BuildScanCommand) Run() (err error) {
 	return
 }
 
-func (bsc *BuildScanCommand) runBuildScanAndPrintResults(xrayManager *xray.XrayServicesManager, params services.XrayBuildParams) (isFailBuildResponse bool, err error) {
+func (bsc *BuildScanCommand) runBuildScanAndPrintResults(xrayManager *xray.XrayServicesManager, xrayVersion string, params services.XrayBuildParams) (isFailBuildResponse bool, err error) {
 	buildScanResults, noFailBuildPolicy, err := xrayManager.BuildScan(params, bsc.includeVulnerabilities)
 	if err != nil {
 		return false, err
@@ -126,7 +126,7 @@ func (bsc *BuildScanCommand) runBuildScanAndPrintResults(xrayManager *xray.XrayS
 		XrayDataUrl:     buildScanResults.MoreDetailsUrl,
 	}}
 
-	extendedScanResults := &xrutils.ExtendedScanResults{XrayResults: scanResponse}
+	extendedScanResults := &xrutils.ExtendedScanResults{XrayResults: scanResponse, XrayVersion: xrayVersion}
 
 	resultsPrinter := xrutils.NewResultsWriter(extendedScanResults).
 		SetOutputFormat(bsc.outputFormat).

diff --git a/xray/commands/scan/scan.go b/xray/commands/scan/scan.go
@@ -241,7 +241,7 @@ func (scanCmd *ScanCommand) Run() (err error) {
 	}
 	scanErrors = appendErrorSlice(scanErrors, fileProducerErrors)
 	scanErrors = appendErrorSlice(scanErrors, indexedFileProducerErrors)
-	extendedScanResults := &xrutils.ExtendedScanResults{XrayResults: flatResults}
+	extendedScanResults := &xrutils.ExtendedScanResults{XrayResults: flatResults, XrayVersion: xrayVersion}
 
 	if err = xrutils.NewResultsWriter(extendedScanResults).
 		SetOutputFormat(scanCmd.outputFormat).

diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go
@@ -220,76 +220,89 @@ func convertXrayResponsesToSarifRun(extendedResults *ExtendedScanResults, isMult
 
 func extractXrayIssuesToSarifRun(run *sarif.Run, xrayJson formats.SimpleJsonResults) error {
 	for _, vulnerability := range xrayJson.Vulnerabilities {
-		if err := addXrayCveIssueToSarifRun(
-			vulnerability.Cves,
-			vulnerability.IssueId,
-			vulnerability.Severity,
-			vulnerability.Technology,
-			vulnerability.Components,
-			vulnerability.Applicable,
-			vulnerability.ImpactedDependencyName,
-			vulnerability.ImpactedDependencyVersion,
-			vulnerability.Summary,
-			vulnerability.FixedVersions,
-			run,
-		); err != nil {
+		if err := addXrayCveIssueToSarifRun(vulnerability, run); err != nil {
 			return err
 		}
 	}
 	for _, violation := range xrayJson.SecurityViolations {
-		if err := addXrayCveIssueToSarifRun(
-			violation.Cves,
-			violation.IssueId,
-			violation.Severity,
-			violation.Technology,
-			violation.Components,
-			violation.Applicable,
-			violation.ImpactedDependencyName,
-			violation.ImpactedDependencyVersion,
-			violation.Summary,
-			violation.FixedVersions,
-			run,
-		); err != nil {
+		if err := addXrayCveIssueToSarifRun(violation, run); err != nil {
 			return err
 		}
 	}
 	for _, license := range xrayJson.LicensesViolations {
-		msg := getVulnerabilityOrViolationSarifHeadline(license.LicenseKey, license.ImpactedDependencyName, license.ImpactedDependencyVersion)
-		if rule, isNewRule := addResultToSarifRun(license.LicenseKey, msg, license.Severity, nil, run); isNewRule {
-			rule.WithDescription("License watch violations")
+		if err := addXrayLicenseViolationToSarifRun(license, run); err != nil {
+			return err
 		}
 	}
 	return nil
 }
 
-func addXrayCveIssueToSarifRun(cves []formats.CveRow, issueId, severity string, tech coreutils.Technology, components []formats.ComponentRow, applicable, impactedDependencyName, impactedDependencyVersion, summary string, fixedVersions []string, run *sarif.Run) error {
-	maxCveScore, err := findMaxCVEScore(cves)
+func addXrayCveIssueToSarifRun(issue formats.VulnerabilityOrViolationRow, run *sarif.Run) (err error) {
+	maxCveScore, err := findMaxCVEScore(issue.Cves)
 	if err != nil {
-		return err
+		return
 	}
-	cveId := GetIssueIdentifier(cves, issueId)
-	msg := getVulnerabilityOrViolationSarifHeadline(impactedDependencyName, impactedDependencyVersion, cveId)
-	location, err := getXrayIssueLocationIfValidExists(tech, run)
+	location, err := getXrayIssueLocationIfValidExists(issue.Technology, run)
 	if err != nil {
-		return err
+		return
 	}
-	if rule, isNewRule := addResultToSarifRun(cveId, msg, severity, location, run); isNewRule {
-		cveRuleProperties := sarif.NewPropertyBag()
-		if maxCveScore != MissingCveScore {
-			cveRuleProperties.Add("security-severity", maxCveScore)
-		}
-		rule.WithProperties(cveRuleProperties.Properties)
-		formattedDirectDependencies, err := getDirectDependenciesFormatted(components)
-		if err != nil {
-			return err
+	formattedDirectDependencies, err := getDirectDependenciesFormatted(issue.Components)
+	if err != nil {
+		return
+	}
+	cveId := GetIssueIdentifier(issue.Cves, issue.IssueId)
+	markdownDescription := getSarifTableDescription(formattedDirectDependencies, maxCveScore, issue.Applicable, issue.FixedVersions)
+	addXrayIssueToSarifRun(
+		cveId,
+		issue.ImpactedDependencyName,
+		issue.ImpactedDependencyVersion,
+		issue.Severity,
+		maxCveScore,
+		issue.Summary,
+		getXrayIssueSarifHeadline(issue.ImpactedDependencyName, issue.ImpactedDependencyVersion, cveId),
+		markdownDescription,
+		issue.Components,
+		location,
+		run,
+	)
+	return
+}
+
+func addXrayLicenseViolationToSarifRun(license formats.LicenseRow, run *sarif.Run) (err error) {
+	formattedDirectDependencies, err := getDirectDependenciesFormatted(license.Components)
+	if err != nil {
+		return
+	}
+	addXrayIssueToSarifRun(
+		license.LicenseKey,
+		license.ImpactedDependencyName,
+		license.ImpactedDependencyVersion,
+		license.Severity,
+		MissingCveScore,
+		getLicenseViolationSummary(license.ImpactedDependencyName, license.ImpactedDependencyVersion, license.LicenseKey),
+		getXrayLicenseSarifHeadline(license.ImpactedDependencyName, license.ImpactedDependencyVersion, license.LicenseKey),
+		getLicenseViolationMarkdown(license.ImpactedDependencyName, license.ImpactedDependencyVersion, license.LicenseKey, formattedDirectDependencies),
+		license.Components,
+		nil,
+		run,
+	)
+	return
+}
+
+func addXrayIssueToSarifRun(issueId, impactedDependencyName, impactedDependencyVersion, severity, severityScore, summary, title, markdownDescription string, components []formats.ComponentRow, location *sarif.Location, run *sarif.Run) {
+	// Add rule if not exists
+	ruleId := getXrayIssueSarifRuleId(impactedDependencyName, impactedDependencyVersion, issueId)
+	if rule, _ := run.GetRuleById(ruleId); rule == nil {
+		addXrayRule(ruleId, title, severityScore, summary, markdownDescription, run)
+	}
+	// Add result for each component
+	for _, directDependency := range components {
+		msg := getXrayIssueSarifHeadline(directDependency.Name, directDependency.Version, issueId)
+		if result := run.CreateResultForRule(ruleId).WithMessage(sarif.NewTextMessage(msg)).WithLevel(ConvertToSarifLevel(severity)); location != nil {
+			result.AddLocation(location)
 		}
-		markdownDescription := getSarifTableDescription(formattedDirectDependencies, maxCveScore, applicable, fixedVersions) + "\n"
-		rule.WithHelp(&sarif.MultiformatMessageString{
-			Text:     &summary,
-			Markdown: &markdownDescription,
-		})
 	}
-	return nil
+
 }
 
 func getDescriptorFullPath(tech coreutils.Technology, run *sarif.Run) (string, error) {
@@ -322,15 +335,20 @@ func getXrayIssueLocationIfValidExists(tech coreutils.Technology, run *sarif.Run
 	return sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + descriptorPath))), nil
 }
 
-func addResultToSarifRun(issueId, msg, severity string, location *sarif.Location, run *sarif.Run) (rule *sarif.ReportingDescriptor, isNewRule bool) {
-	if rule, _ = run.GetRuleById(issueId); rule == nil {
-		isNewRule = true
-		rule = run.AddRule(issueId)
-	}
-	if result := run.CreateResultForRule(issueId).WithMessage(sarif.NewTextMessage(msg)).WithLevel(ConvertToSarifLevel(severity)); location != nil {
-		result.AddLocation(location)
+func addXrayRule(ruleId, ruleDescription, maxCveScore, summary, markdownDescription string, run *sarif.Run) {
+	rule := run.AddRule(ruleId)
+
+	if maxCveScore != MissingCveScore {
+		cveRuleProperties := sarif.NewPropertyBag()
+		cveRuleProperties.Add("security-severity", maxCveScore)
+		rule.WithProperties(cveRuleProperties.Properties)
 	}
-	return
+
+	rule.WithDescription(ruleDescription)
+	rule.WithHelp(&sarif.MultiformatMessageString{
+		Text:     &summary,
+		Markdown: &markdownDescription,
+	})
 }
 
 func convertXrayScanToSimpleJson(extendedResults *ExtendedScanResults, isMultipleRoots, includeLicenses, simplifiedOutput bool) (formats.SimpleJsonResults, error) {
@@ -398,10 +416,26 @@ func GetIssueIdentifier(cvesRow []formats.CveRow, issueId string) string {
 	return identifier
 }
 
-func getVulnerabilityOrViolationSarifHeadline(depName, version, key string) string {
+func getXrayIssueSarifRuleId(depName, version, key string) string {
+	return fmt.Sprintf("%s_%s_%s", key, depName, version)
+}
+
+func getXrayIssueSarifHeadline(depName, version, key string) string {
 	return fmt.Sprintf("[%s] %s %s", key, depName, version)
 }
 
+func getXrayLicenseSarifHeadline(depName, version, key string) string {
+	return fmt.Sprintf("License violation [%s] %s %s", key, depName, version)
+}
+
+func getLicenseViolationSummary(depName, version, key string) string {
+	return fmt.Sprintf("Dependency %s version %s is using a license (%s) that is not allowed.", depName, version, key)
+}
+
+func getLicenseViolationMarkdown(depName, version, key, formattedDirectDependencies string) string {
+	return fmt.Sprintf("**The following direct dependencies are utilizing the `%s %s` dependency with `%s` license violation:**\n%s", depName, version, key, formattedDirectDependencies)
+}
+
 func getDirectDependenciesFormatted(directDependencies []formats.ComponentRow) (string, error) {
 	var formattedDirectDependencies strings.Builder
 	for _, dependency := range directDependencies {

diff --git a/xray/utils/resultwriter_test.go b/xray/utils/resultwriter_test.go
@@ -13,8 +13,8 @@ import (
 )
 
 func TestGetVulnerabilityOrViolationSarifHeadline(t *testing.T) {
-	assert.Equal(t, "[CVE-2022-1234] loadsh 1.4.1", getVulnerabilityOrViolationSarifHeadline("loadsh", "1.4.1", "CVE-2022-1234"))
-	assert.NotEqual(t, "[CVE-2022-1234] loadsh 1.4.1", getVulnerabilityOrViolationSarifHeadline("loadsh", "1.2.1", "CVE-2022-1234"))
+	assert.Equal(t, "[CVE-2022-1234] loadsh 1.4.1", getXrayIssueSarifHeadline("loadsh", "1.4.1", "CVE-2022-1234"))
+	assert.NotEqual(t, "[CVE-2022-1234] loadsh 1.4.1", getXrayIssueSarifHeadline("loadsh", "1.2.1", "CVE-2022-1234"))
 }
 
 func TestGetIssueIdentifier(t *testing.T) {