From de990d23e2907c10bd6b92a6c295d4263e24744c Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 19 Sep 2023 17:39:55 +0300 Subject: [PATCH 01/26] Fix Sarif output driver issues and Xray Sca locations --- xray/utils/resultwriter.go | 32 ++++++++++++++++++++++++++++++-- xray/utils/sarifutils.go | 8 +++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index 1e8abcca4..9a43598f6 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -210,8 +210,10 @@ func addXrayCveIssueToSarifRun(cves []formats.CveRow, issueId, severity, file st } cveId := GetIssueIdentifier(cves, issueId) msg := getVulnerabilityOrViolationSarifHeadline(impactedDependencyName, impactedDependencyVersion, cveId) - location := sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri(file))) - + location, err := getXrayIssueLocationIfValidExists(file, run, markdownOutput) + if err != nil { + return err + } if rule, isNewRule := addResultToSarifRun(cveId, msg, severity, location, run); isNewRule { cveRuleProperties := sarif.NewPropertyBag() if maxCveScore != MissingCveScore { @@ -232,6 +234,32 @@ func addXrayCveIssueToSarifRun(cves []formats.CveRow, issueId, severity, file st return nil } +// Xray GetPackageDescriptor can return multiple types of content. +// This could cause the sarif content not to be valid. if not override, we should handle all those situations: +// Full path - should be used as is +// Relative path - should be converted to full path +// Non path - should not be used as location +func getXrayIssueLocationIfValidExists(file string, run *sarif.Run, override bool) (location *sarif.Location, err error) { + location = sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri(file))) + if override { + // Use the content as is + return + } + // Check if full path + exists, err := fileutils.IsFileExists(file, false) + if err != nil || exists { + return + } + // Check if relative path + fullPath := GetFullLocationFileName(file, run.Invocations) + location = sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + fullPath))) + if exists, err = fileutils.IsFileExists(fullPath, false); err != nil || exists { + return + } + // Not usable content + return nil, 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 diff --git a/xray/utils/sarifutils.go b/xray/utils/sarifutils.go index 5b5d31ab2..32b35bff0 100644 --- a/xray/utils/sarifutils.go +++ b/xray/utils/sarifutils.go @@ -175,7 +175,6 @@ func GetRelativeLocationFileName(location *sarif.Location, invocations []*sarif. if len(invocations) > 0 { wd = GetInvocationWorkingDirectory(invocations[0]) } - GetLocationFileName(location) filePath := GetLocationFileName(location) if filePath != "" { return ExtractRelativePath(filePath, wd) @@ -183,6 +182,13 @@ func GetRelativeLocationFileName(location *sarif.Location, invocations []*sarif. return "" } +func GetFullLocationFileName(relative string, invocations []*sarif.Invocation) string { + if len(invocations) == 0 { + return relative + } + return filepath.Join(GetInvocationWorkingDirectory(invocations[0]), relative) +} + func SetLocationFileName(location *sarif.Location, fileName string) { if location != nil && location.PhysicalLocation != nil && location.PhysicalLocation.Region != nil && location.PhysicalLocation.Region.Snippet != nil { location.PhysicalLocation.ArtifactLocation.URI = &fileName From 53e747f1632550c11fc8533d038371a2a7aa4e7c Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 19 Sep 2023 18:53:03 +0300 Subject: [PATCH 02/26] Fix Sarif output driver issues --- xray/utils/resultwriter.go | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index 9a43598f6..77aebfbf4 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -4,9 +4,6 @@ import ( "bytes" "encoding/json" "fmt" - "strconv" - "strings" - "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-cli-core/v2/xray/formats" clientUtils "github.com/jfrog/jfrog-client-go/utils" @@ -15,6 +12,9 @@ import ( "github.com/jfrog/jfrog-client-go/utils/log" "github.com/jfrog/jfrog-client-go/xray/services" "github.com/owenrumney/go-sarif/v2/sarif" + "strconv" + "strings" + "unicode" ) type OutputFormat string @@ -128,10 +128,10 @@ func GenerateSarifContentFromResults(extendedResults *ExtendedScanResults, isMul } report.Runs = append(report.Runs, xrayRun) - report.Runs = append(report.Runs, extendedResults.ApplicabilityScanResults...) - report.Runs = append(report.Runs, extendedResults.IacScanResults...) - report.Runs = append(report.Runs, extendedResults.SecretsScanResults...) - report.Runs = append(report.Runs, extendedResults.SastScanResults...) + report.Runs = append(report.Runs, fillMissingRequiredInformationForJas(extendedResults.ApplicabilityScanResults)...) + report.Runs = append(report.Runs, fillMissingRequiredInformationForJas(extendedResults.IacScanResults)...) + report.Runs = append(report.Runs, fillMissingRequiredInformationForJas(extendedResults.SecretsScanResults)...) + report.Runs = append(report.Runs, fillMissingRequiredInformationForJas(extendedResults.SastScanResults)...) out, err := json.Marshal(report) if err != nil { @@ -141,6 +141,29 @@ func GenerateSarifContentFromResults(extendedResults *ExtendedScanResults, isMul return clientUtils.IndentJson(out), nil } +func fillMissingRequiredInformationForJas(runs []*sarif.Run) []*sarif.Run { + defaultJasInformationUri := "https://jfrog.com/devops-native-security/" + defaultVersion := GetAnalyzerManagerVersion() + for _, run := range runs { + driver := run.Tool.Driver + if driver.InformationURI == nil { + driver.InformationURI = &defaultJasInformationUri + } + if driver.Version == nil || !isValidVersion(*driver.Version) { + driver.InformationURI = &defaultVersion + } + } + return runs +} + +func isValidVersion(version string) bool { + if len(version) == 0 { + return false + } + firstChar := rune(version[0]) + return unicode.IsDigit(firstChar) +} + func convertXrayResponsesToSarifRun(extendedResults *ExtendedScanResults, isMultipleRoots, includeLicenses, markdownOutput bool) (run *sarif.Run, err error) { xrayJson, err := convertXrayScanToSimpleJson(extendedResults, isMultipleRoots, includeLicenses, true) if err != nil { From 2ecc294f924ad45405d661efefed8781ce9cf384 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 19 Sep 2023 19:00:59 +0300 Subject: [PATCH 03/26] Fix bug --- xray/utils/resultwriter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index 77aebfbf4..0425a4521 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -150,7 +150,7 @@ func fillMissingRequiredInformationForJas(runs []*sarif.Run) []*sarif.Run { driver.InformationURI = &defaultJasInformationUri } if driver.Version == nil || !isValidVersion(*driver.Version) { - driver.InformationURI = &defaultVersion + driver.Version = &defaultVersion } } return runs From d26ce782f7b374aa15099ad036ddfba149a69005 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 19 Sep 2023 19:04:04 +0300 Subject: [PATCH 04/26] rename SCA --- xray/utils/resultwriter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index 0425a4521..c64b3c4c4 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -169,7 +169,7 @@ func convertXrayResponsesToSarifRun(extendedResults *ExtendedScanResults, isMult if err != nil { return } - xrayRun := sarif.NewRunWithInformationURI("JFrog Xray Sca", "https://jfrog.com/xray/") + xrayRun := sarif.NewRunWithInformationURI("JFrog Xray SCA", "https://jfrog.com/xray/") xrayRun.Tool.Driver.Version = &extendedResults.XrayVersion if len(xrayJson.Vulnerabilities) > 0 || len(xrayJson.SecurityViolations) > 0 { if err = extractXrayIssuesToSarifRun(xrayRun, xrayJson, markdownOutput); err != nil { From 4cac21427952baead12a1b6f14b5cc241b1ddb7a Mon Sep 17 00:00:00 2001 From: attiasas Date: Wed, 20 Sep 2023 10:08:19 +0300 Subject: [PATCH 05/26] add specific uri --- xray/utils/resultwriter.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index c64b3c4c4..efc3a8c72 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -128,10 +128,10 @@ func GenerateSarifContentFromResults(extendedResults *ExtendedScanResults, isMul } report.Runs = append(report.Runs, xrayRun) - report.Runs = append(report.Runs, fillMissingRequiredInformationForJas(extendedResults.ApplicabilityScanResults)...) - report.Runs = append(report.Runs, fillMissingRequiredInformationForJas(extendedResults.IacScanResults)...) - report.Runs = append(report.Runs, fillMissingRequiredInformationForJas(extendedResults.SecretsScanResults)...) - report.Runs = append(report.Runs, fillMissingRequiredInformationForJas(extendedResults.SastScanResults)...) + report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://jfrog.com/xray/contextual-vulnerabilities/", extendedResults.ApplicabilityScanResults)...) + report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://jfrog.com/xray/iac-security-check/", extendedResults.IacScanResults)...) + report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://jfrog.com/xray/secrets-detection-source-binaries/", extendedResults.SecretsScanResults)...) + report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://jfrog.com/sast/", extendedResults.SastScanResults)...) out, err := json.Marshal(report) if err != nil { @@ -141,8 +141,7 @@ func GenerateSarifContentFromResults(extendedResults *ExtendedScanResults, isMul return clientUtils.IndentJson(out), nil } -func fillMissingRequiredInformationForJas(runs []*sarif.Run) []*sarif.Run { - defaultJasInformationUri := "https://jfrog.com/devops-native-security/" +func fillMissingRequiredInformationForJas(defaultJasInformationUri string, runs []*sarif.Run) []*sarif.Run { defaultVersion := GetAnalyzerManagerVersion() for _, run := range runs { driver := run.Tool.Driver From 79037efe66cc361c7b68fdcb16f06c1b633f40cf Mon Sep 17 00:00:00 2001 From: attiasas Date: Wed, 20 Sep 2023 10:17:32 +0300 Subject: [PATCH 06/26] reposition override --- xray/utils/resultwriter.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index efc3a8c72..d1ef8f6a8 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -263,10 +263,6 @@ func addXrayCveIssueToSarifRun(cves []formats.CveRow, issueId, severity, file st // Non path - should not be used as location func getXrayIssueLocationIfValidExists(file string, run *sarif.Run, override bool) (location *sarif.Location, err error) { location = sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri(file))) - if override { - // Use the content as is - return - } // Check if full path exists, err := fileutils.IsFileExists(file, false) if err != nil || exists { @@ -274,8 +270,12 @@ func getXrayIssueLocationIfValidExists(file string, run *sarif.Run, override boo } // Check if relative path fullPath := GetFullLocationFileName(file, run.Invocations) - location = sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + fullPath))) if exists, err = fileutils.IsFileExists(fullPath, false); err != nil || exists { + location = sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + fullPath))) + return + } + if override { + // Use the content as is return } // Not usable content From 46ad98fbe3fd353e1fbab60b41c50a6fed16ee0a Mon Sep 17 00:00:00 2001 From: attiasas Date: Wed, 20 Sep 2023 10:18:47 +0300 Subject: [PATCH 07/26] reposition override --- xray/utils/resultwriter.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index d1ef8f6a8..efc3a8c72 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -263,6 +263,10 @@ func addXrayCveIssueToSarifRun(cves []formats.CveRow, issueId, severity, file st // Non path - should not be used as location func getXrayIssueLocationIfValidExists(file string, run *sarif.Run, override bool) (location *sarif.Location, err error) { location = sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri(file))) + if override { + // Use the content as is + return + } // Check if full path exists, err := fileutils.IsFileExists(file, false) if err != nil || exists { @@ -270,12 +274,8 @@ func getXrayIssueLocationIfValidExists(file string, run *sarif.Run, override boo } // Check if relative path fullPath := GetFullLocationFileName(file, run.Invocations) + location = sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + fullPath))) if exists, err = fileutils.IsFileExists(fullPath, false); err != nil || exists { - location = sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + fullPath))) - return - } - if override { - // Use the content as is return } // Not usable content From 1d97508ea949c49a116b5964b05286fc646d78b1 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 26 Sep 2023 10:07:05 +0300 Subject: [PATCH 08/26] add test --- xray/utils/sarifutils_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/xray/utils/sarifutils_test.go b/xray/utils/sarifutils_test.go index 20a92de41..eefcd6289 100644 --- a/xray/utils/sarifutils_test.go +++ b/xray/utils/sarifutils_test.go @@ -294,6 +294,37 @@ func TestGetRelativeLocationFileName(t *testing.T) { } } +func TestGetFullLocationFileName(t *testing.T) { + tests := []struct { + file string + invocations []*sarif.Invocation + expectedOutput string + }{ + { + file: "root/someDir/another/file", + invocations: []*sarif.Invocation{}, + expectedOutput: "root/someDir/another/file", + }, + { + file: "/another/file", + invocations: []*sarif.Invocation{ + {WorkingDirectory: sarif.NewSimpleArtifactLocation("/root/someDir/")}, + {WorkingDirectory: sarif.NewSimpleArtifactLocation("/not/relevant")}, + }, + expectedOutput: "/root/someDir/another/file", + }, + { + file: "another/file", + invocations: []*sarif.Invocation{{WorkingDirectory: sarif.NewSimpleArtifactLocation("/root/someDir")}}, + expectedOutput: "/root/someDir/another/file", + }, + } + + for _, test := range tests { + assert.Equal(t, test.expectedOutput, GetFullLocationFileName(test.file, test.invocations)) + } +} + func TestSetLocationFileName(t *testing.T) { tests := []struct { location *sarif.Location From f50acfbd6df3d436fa859b1c564a4d4df555bd9a Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 26 Sep 2023 10:29:59 +0300 Subject: [PATCH 09/26] update information uri --- xray/utils/resultwriter.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index efc3a8c72..570c6e9b0 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -128,10 +128,10 @@ func GenerateSarifContentFromResults(extendedResults *ExtendedScanResults, isMul } report.Runs = append(report.Runs, xrayRun) - report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://jfrog.com/xray/contextual-vulnerabilities/", extendedResults.ApplicabilityScanResults)...) - report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://jfrog.com/xray/iac-security-check/", extendedResults.IacScanResults)...) - report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://jfrog.com/xray/secrets-detection-source-binaries/", extendedResults.SecretsScanResults)...) - report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://jfrog.com/sast/", extendedResults.SastScanResults)...) + report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://docs.jfrog-applications.jfrog.io/jfrog-security-features/contextual-analysis", extendedResults.ApplicabilityScanResults)...) + report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://docs.jfrog-applications.jfrog.io/jfrog-security-features/infrastructure-as-code-iac", extendedResults.IacScanResults)...) + report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://docs.jfrog-applications.jfrog.io/jfrog-security-features/secrets", extendedResults.SecretsScanResults)...) + report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://docs.jfrog-applications.jfrog.io/jfrog-security-features/sast", extendedResults.SastScanResults)...) out, err := json.Marshal(report) if err != nil { @@ -168,7 +168,7 @@ func convertXrayResponsesToSarifRun(extendedResults *ExtendedScanResults, isMult if err != nil { return } - xrayRun := sarif.NewRunWithInformationURI("JFrog Xray SCA", "https://jfrog.com/xray/") + xrayRun := sarif.NewRunWithInformationURI("JFrog Xray SCA", "https://docs.jfrog-applications.jfrog.io/jfrog-security-features/sca") xrayRun.Tool.Driver.Version = &extendedResults.XrayVersion if len(xrayJson.Vulnerabilities) > 0 || len(xrayJson.SecurityViolations) > 0 { if err = extractXrayIssuesToSarifRun(xrayRun, xrayJson, markdownOutput); err != nil { From c6f29dd69ffe9cf711781e43a76f843f8dfa55bd Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 26 Sep 2023 11:43:38 +0300 Subject: [PATCH 10/26] split sarifutils and move test funcs --- xray/utils/sarifutils.go | 61 --------------------------------- xray/utils/test_sarifutils.go | 64 +++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 61 deletions(-) create mode 100644 xray/utils/test_sarifutils.go diff --git a/xray/utils/sarifutils.go b/xray/utils/sarifutils.go index 664b179bd..5f9d86eb3 100644 --- a/xray/utils/sarifutils.go +++ b/xray/utils/sarifutils.go @@ -265,64 +265,3 @@ func GetInvocationWorkingDirectory(invocation *sarif.Invocation) string { } return "" } - -func CreateRunWithDummyResults(results ...*sarif.Result) *sarif.Run { - run := sarif.NewRunWithInformationURI("", "") - for _, result := range results { - if result.RuleID != nil { - run.AddRule(*result.RuleID) - } - run.AddResult(result) - } - return run -} - -func CreateResultWithLocations(msg, ruleId, level string, locations ...*sarif.Location) *sarif.Result { - return &sarif.Result{ - Message: *sarif.NewTextMessage(msg), - Locations: locations, - Level: &level, - RuleID: &ruleId, - } -} - -func CreateLocation(fileName string, startLine, startCol, endLine, endCol int, snippet string) *sarif.Location { - return &sarif.Location{ - PhysicalLocation: &sarif.PhysicalLocation{ - ArtifactLocation: &sarif.ArtifactLocation{URI: &fileName}, - Region: &sarif.Region{ - StartLine: &startLine, - StartColumn: &startCol, - EndLine: &endLine, - EndColumn: &endCol, - Snippet: &sarif.ArtifactContent{Text: &snippet}}}, - } -} - -func CreateDummyPassingResult(ruleId string) *sarif.Result { - kind := "pass" - return &sarif.Result{ - Kind: &kind, - RuleID: &ruleId, - } -} - -func CreateResultWithOneLocation(fileName string, startLine, startCol, endLine, endCol int, snippet, ruleId, level string) *sarif.Result { - return CreateResultWithLocations("", ruleId, level, CreateLocation(fileName, startLine, startCol, endLine, endCol, snippet)) -} - -func CreateCodeFlow(threadFlows ...*sarif.ThreadFlow) *sarif.CodeFlow { - flow := sarif.NewCodeFlow() - for _, threadFlow := range threadFlows { - flow.AddThreadFlow(threadFlow) - } - return flow -} - -func CreateThreadFlow(locations ...*sarif.Location) *sarif.ThreadFlow { - stackStrace := sarif.NewThreadFlow() - for _, location := range locations { - stackStrace.AddLocation(sarif.NewThreadFlowLocation().WithLocation(location)) - } - return stackStrace -} diff --git a/xray/utils/test_sarifutils.go b/xray/utils/test_sarifutils.go new file mode 100644 index 000000000..5034b1eb6 --- /dev/null +++ b/xray/utils/test_sarifutils.go @@ -0,0 +1,64 @@ +package utils + +import "github.com/owenrumney/go-sarif/v2/sarif" + +func CreateRunWithDummyResults(results ...*sarif.Result) *sarif.Run { + run := sarif.NewRunWithInformationURI("", "") + for _, result := range results { + if result.RuleID != nil { + run.AddRule(*result.RuleID) + } + run.AddResult(result) + } + return run +} + +func CreateResultWithLocations(msg, ruleId, level string, locations ...*sarif.Location) *sarif.Result { + return &sarif.Result{ + Message: *sarif.NewTextMessage(msg), + Locations: locations, + Level: &level, + RuleID: &ruleId, + } +} + +func CreateLocation(fileName string, startLine, startCol, endLine, endCol int, snippet string) *sarif.Location { + return &sarif.Location{ + PhysicalLocation: &sarif.PhysicalLocation{ + ArtifactLocation: &sarif.ArtifactLocation{URI: &fileName}, + Region: &sarif.Region{ + StartLine: &startLine, + StartColumn: &startCol, + EndLine: &endLine, + EndColumn: &endCol, + Snippet: &sarif.ArtifactContent{Text: &snippet}}}, + } +} + +func CreateDummyPassingResult(ruleId string) *sarif.Result { + kind := "pass" + return &sarif.Result{ + Kind: &kind, + RuleID: &ruleId, + } +} + +func CreateResultWithOneLocation(fileName string, startLine, startCol, endLine, endCol int, snippet, ruleId, level string) *sarif.Result { + return CreateResultWithLocations("", ruleId, level, CreateLocation(fileName, startLine, startCol, endLine, endCol, snippet)) +} + +func CreateCodeFlow(threadFlows ...*sarif.ThreadFlow) *sarif.CodeFlow { + flow := sarif.NewCodeFlow() + for _, threadFlow := range threadFlows { + flow.AddThreadFlow(threadFlow) + } + return flow +} + +func CreateThreadFlow(locations ...*sarif.Location) *sarif.ThreadFlow { + stackStrace := sarif.NewThreadFlow() + for _, location := range locations { + stackStrace.AddLocation(sarif.NewThreadFlowLocation().WithLocation(location)) + } + return stackStrace +} From cb563be347a0aa8cfb384d8fd18a35b182be9c17 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 26 Sep 2023 14:08:18 +0300 Subject: [PATCH 11/26] fix for windows --- xray/utils/sarifutils_test.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/xray/utils/sarifutils_test.go b/xray/utils/sarifutils_test.go index eefcd6289..1ffca65dc 100644 --- a/xray/utils/sarifutils_test.go +++ b/xray/utils/sarifutils_test.go @@ -1,6 +1,7 @@ package utils import ( + "path" "testing" "github.com/owenrumney/go-sarif/v2/sarif" @@ -301,22 +302,22 @@ func TestGetFullLocationFileName(t *testing.T) { expectedOutput string }{ { - file: "root/someDir/another/file", + file: path.Join("root","someDir","another","file"), invocations: []*sarif.Invocation{}, - expectedOutput: "root/someDir/another/file", + expectedOutput: path.Join("root","someDir","another","file"), }, { - file: "/another/file", + file: path.Join("another","file"), invocations: []*sarif.Invocation{ - {WorkingDirectory: sarif.NewSimpleArtifactLocation("/root/someDir/")}, - {WorkingDirectory: sarif.NewSimpleArtifactLocation("/not/relevant")}, + {WorkingDirectory: sarif.NewSimpleArtifactLocation(path.Join("root","someDir"))}, + {WorkingDirectory: sarif.NewSimpleArtifactLocation(path.Join("not","relevant"))}, }, - expectedOutput: "/root/someDir/another/file", + expectedOutput: path.Join("root","someDir","another","file"), }, { file: "another/file", - invocations: []*sarif.Invocation{{WorkingDirectory: sarif.NewSimpleArtifactLocation("/root/someDir")}}, - expectedOutput: "/root/someDir/another/file", + invocations: []*sarif.Invocation{{WorkingDirectory: sarif.NewSimpleArtifactLocation(path.Join("root","someDir"))}}, + expectedOutput: path.Join("root","someDir","another","file"), }, } From 6065ccf074ad551f37e4619daacbf6861e571a8f Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 26 Sep 2023 14:17:54 +0300 Subject: [PATCH 12/26] Remove Sast support check --- xray/commands/audit/jasrunner.go | 3 --- xray/utils/analyzermanager.go | 6 ------ xray/utils/resultwriter.go | 3 --- 3 files changed, 12 deletions(-) diff --git a/xray/commands/audit/jasrunner.go b/xray/commands/audit/jasrunner.go index 76a59ff8e..c528fdadc 100644 --- a/xray/commands/audit/jasrunner.go +++ b/xray/commands/audit/jasrunner.go @@ -52,9 +52,6 @@ func runJasScannersAndSetResults(scanResults *utils.ExtendedScanResults, directD if err != nil { return } - if !utils.IsSastSupported() { - return - } if progress != nil { progress.SetHeadlineMsg("Running SAST scanning") } diff --git a/xray/utils/analyzermanager.go b/xray/utils/analyzermanager.go index 12784921c..936224df2 100644 --- a/xray/utils/analyzermanager.go +++ b/xray/utils/analyzermanager.go @@ -3,7 +3,6 @@ package utils import ( "errors" "fmt" - "github.com/jfrog/gofrog/version" "os" "os/exec" "path" @@ -24,7 +23,6 @@ const ( ApplicabilityFeatureId = "contextual_analysis" AnalyzerManagerZipName = "analyzerManager.zip" defaultAnalyzerManagerVersion = "1.3.2.2019257" - minAnalyzerManagerVersionForSast = "1.3" analyzerManagerDownloadPath = "xsc-gen-exe-analyzer-manager-local/v1" analyzerManagerDirName = "analyzerManager" analyzerManagerExecutableName = "analyzerManager" @@ -139,10 +137,6 @@ func GetAnalyzerManagerVersion() string { return defaultAnalyzerManagerVersion } -func IsSastSupported() bool { - return version.NewVersion(GetAnalyzerManagerVersion()).AtLeast(minAnalyzerManagerVersionForSast) -} - func GetAnalyzerManagerDirAbsolutePath() (string, error) { jfrogDir, err := config.GetJfrogDependenciesPath() if err != nil { diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index 570c6e9b0..0e6374353 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -98,9 +98,6 @@ func printScanResultsTables(results *ExtendedScanResults, isBinaryScan, includeV if err = PrintIacTable(results.IacScanResults, results.EntitledForJas); err != nil { return } - if !IsSastSupported() { - return - } return PrintSastTable(results.SastScanResults, results.EntitledForJas) } From 0782ab954ae1b9b2e243b91eac554417876fbd1f Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 26 Sep 2023 15:31:34 +0300 Subject: [PATCH 13/26] review changes --- .../jas/applicability/applicabilitymanager.go | 7 ++-- .../applicabilitymanager_test.go | 6 +-- xray/commands/audit/jas/common.go | 22 +++++++++- xray/commands/audit/jas/iac/iacscanner.go | 7 ++-- .../commands/audit/jas/iac/iacscanner_test.go | 4 +- xray/commands/audit/jas/sast/sastscanner.go | 5 ++- .../audit/jas/sast/sastscanner_test.go | 4 +- .../audit/jas/secrets/secretsscanner.go | 7 ++-- .../audit/jas/secrets/secretsscanner_test.go | 4 +- xray/utils/resultwriter.go | 40 +++++-------------- xray/utils/sarifutils_test.go | 16 ++++---- 11 files changed, 63 insertions(+), 59 deletions(-) diff --git a/xray/commands/audit/jas/applicability/applicabilitymanager.go b/xray/commands/audit/jas/applicability/applicabilitymanager.go index b038e4371..c3b83e462 100644 --- a/xray/commands/audit/jas/applicability/applicabilitymanager.go +++ b/xray/commands/audit/jas/applicability/applicabilitymanager.go @@ -16,8 +16,9 @@ import ( ) const ( - applicabilityScanType = "analyze-applicability" - applicabilityScanCommand = "ca" + applicabilityScanType = "analyze-applicability" + applicabilityScanCommand = "ca" + applicabilityDocsUrlSuffix = "contextual-analysis" ) type ApplicabilityScanManager struct { @@ -112,7 +113,7 @@ func (asm *ApplicabilityScanManager) Run(wd string) (err error) { if err = asm.runAnalyzerManager(); err != nil { return } - workingDirResults, err := jas.ReadJasScanRunsFromFile(asm.scanner.ResultsFileName, wd) + workingDirResults, err := jas.ReadJasScanRunsFromFile(asm.scanner.ResultsFileName, wd, applicabilityDocsUrlSuffix) if err != nil { return } diff --git a/xray/commands/audit/jas/applicability/applicabilitymanager_test.go b/xray/commands/audit/jas/applicability/applicabilitymanager_test.go index b05d69130..36a5fd22c 100644 --- a/xray/commands/audit/jas/applicability/applicabilitymanager_test.go +++ b/xray/commands/audit/jas/applicability/applicabilitymanager_test.go @@ -280,7 +280,7 @@ func TestParseResults_EmptyResults_AllCvesShouldGetUnknown(t *testing.T) { // Act var err error - applicabilityManager.applicabilityScanResults, err = jas.ReadJasScanRunsFromFile(applicabilityManager.scanner.ResultsFileName, scanner.WorkingDirs[0]) + applicabilityManager.applicabilityScanResults, err = jas.ReadJasScanRunsFromFile(applicabilityManager.scanner.ResultsFileName, scanner.WorkingDirs[0], applicabilityDocsUrlSuffix) if assert.NoError(t, err) { assert.Len(t, applicabilityManager.applicabilityScanResults, 1) @@ -297,7 +297,7 @@ func TestParseResults_ApplicableCveExist(t *testing.T) { // Act var err error - applicabilityManager.applicabilityScanResults, err = jas.ReadJasScanRunsFromFile(applicabilityManager.scanner.ResultsFileName, scanner.WorkingDirs[0]) + applicabilityManager.applicabilityScanResults, err = jas.ReadJasScanRunsFromFile(applicabilityManager.scanner.ResultsFileName, scanner.WorkingDirs[0], applicabilityDocsUrlSuffix) if assert.NoError(t, err) && assert.NotNil(t, applicabilityManager.applicabilityScanResults) { assert.Len(t, applicabilityManager.applicabilityScanResults, 1) @@ -314,7 +314,7 @@ func TestParseResults_AllCvesNotApplicable(t *testing.T) { // Act var err error - applicabilityManager.applicabilityScanResults, err = jas.ReadJasScanRunsFromFile(applicabilityManager.scanner.ResultsFileName, scanner.WorkingDirs[0]) + applicabilityManager.applicabilityScanResults, err = jas.ReadJasScanRunsFromFile(applicabilityManager.scanner.ResultsFileName, scanner.WorkingDirs[0], applicabilityDocsUrlSuffix) if assert.NoError(t, err) && assert.NotNil(t, applicabilityManager.applicabilityScanResults) { assert.Len(t, applicabilityManager.applicabilityScanResults, 1) diff --git a/xray/commands/audit/jas/common.go b/xray/commands/audit/jas/common.go index 2dfada8dc..e37c67863 100644 --- a/xray/commands/audit/jas/common.go +++ b/xray/commands/audit/jas/common.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strings" "testing" + "unicode" rtutils "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" "github.com/jfrog/jfrog-cli-core/v2/utils/config" @@ -103,7 +104,7 @@ func deleteJasProcessFiles(configFile string, resultFile string) error { return errorutils.CheckError(err) } -func ReadJasScanRunsFromFile(fileName, wd string) (sarifRuns []*sarif.Run, err error) { +func ReadJasScanRunsFromFile(fileName, wd, informationUrlSuffix string) (sarifRuns []*sarif.Run, err error) { if sarifRuns, err = utils.ReadScanRunsFromFile(fileName); err != nil { return } @@ -113,12 +114,31 @@ func ReadJasScanRunsFromFile(fileName, wd string) (sarifRuns []*sarif.Run, err e // Also used to calculate relative paths if needed with it sarifRun.Invocations[0].WorkingDirectory.WithUri(wd) // Process runs values + fillMissingRequiredDriverInformation(utils.BaseDocumentationURL+informationUrlSuffix, utils.GetAnalyzerManagerVersion(), sarifRun) sarifRun.Results = excludeSuppressResults(sarifRun.Results) addScoreToRunRules(sarifRun) } return } +func fillMissingRequiredDriverInformation(defaultJasInformationUri, defaultVersion string, run *sarif.Run) { + driver := run.Tool.Driver + if driver.InformationURI == nil { + driver.InformationURI = &defaultJasInformationUri + } + if driver.Version == nil || !isValidVersion(*driver.Version) { + driver.Version = &defaultVersion + } +} + +func isValidVersion(version string) bool { + if len(version) == 0 { + return false + } + firstChar := rune(version[0]) + return unicode.IsDigit(firstChar) +} + func excludeSuppressResults(sarifResults []*sarif.Result) []*sarif.Result { results := []*sarif.Result{} for _, sarifResult := range sarifResults { diff --git a/xray/commands/audit/jas/iac/iacscanner.go b/xray/commands/audit/jas/iac/iacscanner.go index c4ccfdd39..03ef52d88 100644 --- a/xray/commands/audit/jas/iac/iacscanner.go +++ b/xray/commands/audit/jas/iac/iacscanner.go @@ -10,8 +10,9 @@ import ( ) const ( - iacScannerType = "iac-scan-modules" - iacScanCommand = "iac" + iacScannerType = "iac-scan-modules" + iacScanCommand = "iac" + iacDocsUrlSuffix = "infrastructure-as-code-iac" ) type IacScanManager struct { @@ -56,7 +57,7 @@ func (iac *IacScanManager) Run(wd string) (err error) { if err = iac.runAnalyzerManager(); err != nil { return } - workingDirResults, err := jas.ReadJasScanRunsFromFile(scanner.ResultsFileName, wd) + workingDirResults, err := jas.ReadJasScanRunsFromFile(scanner.ResultsFileName, wd, iacDocsUrlSuffix) if err != nil { return } diff --git a/xray/commands/audit/jas/iac/iacscanner_test.go b/xray/commands/audit/jas/iac/iacscanner_test.go index a2332421d..484693ab6 100644 --- a/xray/commands/audit/jas/iac/iacscanner_test.go +++ b/xray/commands/audit/jas/iac/iacscanner_test.go @@ -57,7 +57,7 @@ func TestIacParseResults_EmptyResults(t *testing.T) { // Act var err error - iacScanManager.iacScannerResults, err = jas.ReadJasScanRunsFromFile(iacScanManager.scanner.ResultsFileName, scanner.WorkingDirs[0]) + iacScanManager.iacScannerResults, err = jas.ReadJasScanRunsFromFile(iacScanManager.scanner.ResultsFileName, scanner.WorkingDirs[0], iacDocsUrlSuffix) if assert.NoError(t, err) && assert.NotNil(t, iacScanManager.iacScannerResults) { assert.Len(t, iacScanManager.iacScannerResults, 1) assert.Empty(t, iacScanManager.iacScannerResults[0].Results) @@ -73,7 +73,7 @@ func TestIacParseResults_ResultsContainIacViolations(t *testing.T) { // Act var err error - iacScanManager.iacScannerResults, err = jas.ReadJasScanRunsFromFile(iacScanManager.scanner.ResultsFileName, scanner.WorkingDirs[0]) + iacScanManager.iacScannerResults, err = jas.ReadJasScanRunsFromFile(iacScanManager.scanner.ResultsFileName, scanner.WorkingDirs[0], iacDocsUrlSuffix) if assert.NoError(t, err) && assert.NotNil(t, iacScanManager.iacScannerResults) { assert.Len(t, iacScanManager.iacScannerResults, 1) assert.Len(t, iacScanManager.iacScannerResults[0].Results, 4) diff --git a/xray/commands/audit/jas/sast/sastscanner.go b/xray/commands/audit/jas/sast/sastscanner.go index d4402cd68..b7ed62c64 100644 --- a/xray/commands/audit/jas/sast/sastscanner.go +++ b/xray/commands/audit/jas/sast/sastscanner.go @@ -11,7 +11,8 @@ import ( ) const ( - sastScanCommand = "zd" + sastScanCommand = "zd" + sastDocsUrlSuffix = "sast" ) type SastScanManager struct { @@ -45,7 +46,7 @@ func (ssm *SastScanManager) Run(wd string) (err error) { if err = ssm.runAnalyzerManager(wd); err != nil { return } - workingDirRuns, err := jas.ReadJasScanRunsFromFile(scanner.ResultsFileName, wd) + workingDirRuns, err := jas.ReadJasScanRunsFromFile(scanner.ResultsFileName, wd, sastDocsUrlSuffix) if err != nil { return } diff --git a/xray/commands/audit/jas/sast/sastscanner_test.go b/xray/commands/audit/jas/sast/sastscanner_test.go index 6ed1980f3..c3dca7115 100644 --- a/xray/commands/audit/jas/sast/sastscanner_test.go +++ b/xray/commands/audit/jas/sast/sastscanner_test.go @@ -36,7 +36,7 @@ func TestSastParseResults_EmptyResults(t *testing.T) { // Act var err error - sastScanManager.sastScannerResults, err = jas.ReadJasScanRunsFromFile(sastScanManager.scanner.ResultsFileName, scanner.WorkingDirs[0]) + sastScanManager.sastScannerResults, err = jas.ReadJasScanRunsFromFile(sastScanManager.scanner.ResultsFileName, scanner.WorkingDirs[0], sastDocsUrlSuffix) // Assert if assert.NoError(t, err) && assert.NotNil(t, sastScanManager.sastScannerResults) { @@ -57,7 +57,7 @@ func TestSastParseResults_ResultsContainIacViolations(t *testing.T) { // Act var err error - sastScanManager.sastScannerResults, err = jas.ReadJasScanRunsFromFile(sastScanManager.scanner.ResultsFileName, scanner.WorkingDirs[0]) + sastScanManager.sastScannerResults, err = jas.ReadJasScanRunsFromFile(sastScanManager.scanner.ResultsFileName, scanner.WorkingDirs[0], sastDocsUrlSuffix) // Assert if assert.NoError(t, err) && assert.NotNil(t, sastScanManager.sastScannerResults) { diff --git a/xray/commands/audit/jas/secrets/secretsscanner.go b/xray/commands/audit/jas/secrets/secretsscanner.go index 6f0b985ff..e2c9fbd17 100644 --- a/xray/commands/audit/jas/secrets/secretsscanner.go +++ b/xray/commands/audit/jas/secrets/secretsscanner.go @@ -11,8 +11,9 @@ import ( ) const ( - secretsScanCommand = "sec" - secretsScannerType = "secrets-scan" + secretsScanCommand = "sec" + secretsScannerType = "secrets-scan" + secretsDocsUrlSuffix = "secrets" ) type SecretScanManager struct { @@ -56,7 +57,7 @@ func (s *SecretScanManager) Run(wd string) (err error) { if err = s.runAnalyzerManager(); err != nil { return } - workingDirRuns, err := jas.ReadJasScanRunsFromFile(scanner.ResultsFileName, wd) + workingDirRuns, err := jas.ReadJasScanRunsFromFile(scanner.ResultsFileName, wd, secretsDocsUrlSuffix) if err != nil { return } diff --git a/xray/commands/audit/jas/secrets/secretsscanner_test.go b/xray/commands/audit/jas/secrets/secretsscanner_test.go index 14e917e16..da972bf01 100644 --- a/xray/commands/audit/jas/secrets/secretsscanner_test.go +++ b/xray/commands/audit/jas/secrets/secretsscanner_test.go @@ -65,7 +65,7 @@ func TestParseResults_EmptyResults(t *testing.T) { // Act var err error - secretScanManager.secretsScannerResults, err = jas.ReadJasScanRunsFromFile(secretScanManager.scanner.ResultsFileName, scanner.WorkingDirs[0]) + secretScanManager.secretsScannerResults, err = jas.ReadJasScanRunsFromFile(secretScanManager.scanner.ResultsFileName, scanner.WorkingDirs[0], secretsDocsUrlSuffix) // Assert if assert.NoError(t, err) && assert.NotNil(t, secretScanManager.secretsScannerResults) { @@ -88,7 +88,7 @@ func TestParseResults_ResultsContainSecrets(t *testing.T) { // Act var err error - secretScanManager.secretsScannerResults, err = jas.ReadJasScanRunsFromFile(secretScanManager.scanner.ResultsFileName, scanner.WorkingDirs[0]) + secretScanManager.secretsScannerResults, err = jas.ReadJasScanRunsFromFile(secretScanManager.scanner.ResultsFileName, scanner.WorkingDirs[0], secretsDocsUrlSuffix) // Assert if assert.NoError(t, err) && assert.NotNil(t, secretScanManager.secretsScannerResults) { diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index 0e6374353..15d393501 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -4,6 +4,9 @@ import ( "bytes" "encoding/json" "fmt" + "strconv" + "strings" + "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-cli-core/v2/xray/formats" clientUtils "github.com/jfrog/jfrog-client-go/utils" @@ -12,9 +15,6 @@ import ( "github.com/jfrog/jfrog-client-go/utils/log" "github.com/jfrog/jfrog-client-go/xray/services" "github.com/owenrumney/go-sarif/v2/sarif" - "strconv" - "strings" - "unicode" ) type OutputFormat string @@ -25,6 +25,8 @@ const ( Json OutputFormat = "json" SimpleJson OutputFormat = "simple-json" Sarif OutputFormat = "sarif" + + BaseDocumentationURL = "https://docs.jfrog-applications.jfrog.io/jfrog-security-features/" ) const MissingCveScore = "0" @@ -125,10 +127,10 @@ func GenerateSarifContentFromResults(extendedResults *ExtendedScanResults, isMul } report.Runs = append(report.Runs, xrayRun) - report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://docs.jfrog-applications.jfrog.io/jfrog-security-features/contextual-analysis", extendedResults.ApplicabilityScanResults)...) - report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://docs.jfrog-applications.jfrog.io/jfrog-security-features/infrastructure-as-code-iac", extendedResults.IacScanResults)...) - report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://docs.jfrog-applications.jfrog.io/jfrog-security-features/secrets", extendedResults.SecretsScanResults)...) - report.Runs = append(report.Runs, fillMissingRequiredInformationForJas("https://docs.jfrog-applications.jfrog.io/jfrog-security-features/sast", extendedResults.SastScanResults)...) + report.Runs = append(report.Runs, extendedResults.ApplicabilityScanResults...) + report.Runs = append(report.Runs, extendedResults.IacScanResults...) + report.Runs = append(report.Runs, extendedResults.SecretsScanResults...) + report.Runs = append(report.Runs, extendedResults.SastScanResults...) out, err := json.Marshal(report) if err != nil { @@ -138,34 +140,12 @@ func GenerateSarifContentFromResults(extendedResults *ExtendedScanResults, isMul return clientUtils.IndentJson(out), nil } -func fillMissingRequiredInformationForJas(defaultJasInformationUri string, runs []*sarif.Run) []*sarif.Run { - defaultVersion := GetAnalyzerManagerVersion() - for _, run := range runs { - driver := run.Tool.Driver - if driver.InformationURI == nil { - driver.InformationURI = &defaultJasInformationUri - } - if driver.Version == nil || !isValidVersion(*driver.Version) { - driver.Version = &defaultVersion - } - } - return runs -} - -func isValidVersion(version string) bool { - if len(version) == 0 { - return false - } - firstChar := rune(version[0]) - return unicode.IsDigit(firstChar) -} - func convertXrayResponsesToSarifRun(extendedResults *ExtendedScanResults, isMultipleRoots, includeLicenses, markdownOutput bool) (run *sarif.Run, err error) { xrayJson, err := convertXrayScanToSimpleJson(extendedResults, isMultipleRoots, includeLicenses, true) if err != nil { return } - xrayRun := sarif.NewRunWithInformationURI("JFrog Xray SCA", "https://docs.jfrog-applications.jfrog.io/jfrog-security-features/sca") + xrayRun := sarif.NewRunWithInformationURI("JFrog Xray SCA", BaseDocumentationURL+"sca") xrayRun.Tool.Driver.Version = &extendedResults.XrayVersion if len(xrayJson.Vulnerabilities) > 0 || len(xrayJson.SecurityViolations) > 0 { if err = extractXrayIssuesToSarifRun(xrayRun, xrayJson, markdownOutput); err != nil { diff --git a/xray/utils/sarifutils_test.go b/xray/utils/sarifutils_test.go index 1ffca65dc..596b3061d 100644 --- a/xray/utils/sarifutils_test.go +++ b/xray/utils/sarifutils_test.go @@ -302,22 +302,22 @@ func TestGetFullLocationFileName(t *testing.T) { expectedOutput string }{ { - file: path.Join("root","someDir","another","file"), + file: path.Join("root", "someDir", "another", "file"), invocations: []*sarif.Invocation{}, - expectedOutput: path.Join("root","someDir","another","file"), + expectedOutput: path.Join("root", "someDir", "another", "file"), }, { - file: path.Join("another","file"), + file: path.Join("another", "file"), invocations: []*sarif.Invocation{ - {WorkingDirectory: sarif.NewSimpleArtifactLocation(path.Join("root","someDir"))}, - {WorkingDirectory: sarif.NewSimpleArtifactLocation(path.Join("not","relevant"))}, + {WorkingDirectory: sarif.NewSimpleArtifactLocation(path.Join("root", "someDir"))}, + {WorkingDirectory: sarif.NewSimpleArtifactLocation(path.Join("not", "relevant"))}, }, - expectedOutput: path.Join("root","someDir","another","file"), + expectedOutput: path.Join("root", "someDir", "another", "file"), }, { file: "another/file", - invocations: []*sarif.Invocation{{WorkingDirectory: sarif.NewSimpleArtifactLocation(path.Join("root","someDir"))}}, - expectedOutput: path.Join("root","someDir","another","file"), + invocations: []*sarif.Invocation{{WorkingDirectory: sarif.NewSimpleArtifactLocation(path.Join("root", "someDir"))}}, + expectedOutput: path.Join("root", "someDir", "another", "file"), }, } From 11049cb0f90cf3b9ab5874833343765ba69663f8 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 26 Sep 2023 15:55:16 +0300 Subject: [PATCH 14/26] fix tests --- xray/utils/sarifutils_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/xray/utils/sarifutils_test.go b/xray/utils/sarifutils_test.go index 596b3061d..62651e60f 100644 --- a/xray/utils/sarifutils_test.go +++ b/xray/utils/sarifutils_test.go @@ -314,11 +314,6 @@ func TestGetFullLocationFileName(t *testing.T) { }, expectedOutput: path.Join("root", "someDir", "another", "file"), }, - { - file: "another/file", - invocations: []*sarif.Invocation{{WorkingDirectory: sarif.NewSimpleArtifactLocation(path.Join("root", "someDir"))}}, - expectedOutput: path.Join("root", "someDir", "another", "file"), - }, } for _, test := range tests { From 57e71bdfcd24bbfed7cf5e2665ada00f31b767d6 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 26 Sep 2023 16:27:34 +0300 Subject: [PATCH 15/26] fix tests --- xray/utils/sarifutils_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/xray/utils/sarifutils_test.go b/xray/utils/sarifutils_test.go index 62651e60f..33358fe7a 100644 --- a/xray/utils/sarifutils_test.go +++ b/xray/utils/sarifutils_test.go @@ -1,7 +1,7 @@ package utils import ( - "path" + "path/filepath" "testing" "github.com/owenrumney/go-sarif/v2/sarif" @@ -302,17 +302,17 @@ func TestGetFullLocationFileName(t *testing.T) { expectedOutput string }{ { - file: path.Join("root", "someDir", "another", "file"), + file: filepath.Join("root", "someDir", "another", "file"), invocations: []*sarif.Invocation{}, - expectedOutput: path.Join("root", "someDir", "another", "file"), + expectedOutput: filepath.Join("root", "someDir", "another", "file"), }, { - file: path.Join("another", "file"), + file: filepath.Join("another", "file"), invocations: []*sarif.Invocation{ - {WorkingDirectory: sarif.NewSimpleArtifactLocation(path.Join("root", "someDir"))}, - {WorkingDirectory: sarif.NewSimpleArtifactLocation(path.Join("not", "relevant"))}, + {WorkingDirectory: sarif.NewSimpleArtifactLocation(filepath.Join("root", "someDir"))}, + {WorkingDirectory: sarif.NewSimpleArtifactLocation(filepath.Join("not", "relevant"))}, }, - expectedOutput: path.Join("root", "someDir", "another", "file"), + expectedOutput: filepath.Join("root", "someDir", "another", "file"), }, } From df8398f80438f577119fb90da54171f1d26f2b97 Mon Sep 17 00:00:00 2001 From: attiasas Date: Mon, 2 Oct 2023 11:48:50 +0300 Subject: [PATCH 16/26] fix review --- utils/coreutils/techutils.go | 20 +++++----- xray/utils/resultwriter.go | 70 ++++++++++++++++++++++----------- xray/utils/resultwriter_test.go | 70 +++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 35 deletions(-) diff --git a/utils/coreutils/techutils.go b/utils/coreutils/techutils.go index 58ea37a44..60b94dc5a 100644 --- a/utils/coreutils/techutils.go +++ b/utils/coreutils/techutils.go @@ -45,7 +45,7 @@ type TechData struct { // Whether Contextual Analysis supported in this technology. applicabilityScannable bool // The file that handles the project's dependencies. - packageDescriptor string + packageDescriptor []string // Formal name of the technology formal string // The executable name of the technology @@ -60,21 +60,21 @@ var technologiesData = map[Technology]TechData{ Maven: { indicators: []string{"pom.xml"}, ciSetupSupport: true, - packageDescriptor: "pom.xml", + packageDescriptor: []string{"pom.xml"}, execCommand: "mvn", applicabilityScannable: true, }, Gradle: { indicators: []string{".gradle", ".gradle.kts"}, ciSetupSupport: true, - packageDescriptor: "build.gradle, build.gradle.kts", + packageDescriptor: []string{"build.gradle", "build.gradle.kts"}, applicabilityScannable: true, }, Npm: { indicators: []string{"package.json", "package-lock.json", "npm-shrinkwrap.json"}, exclude: []string{".yarnrc.yml", "yarn.lock", ".yarn"}, ciSetupSupport: true, - packageDescriptor: "package.json", + packageDescriptor: []string{"package.json"}, formal: string(Npm), packageVersionOperator: "@", packageInstallationCommand: "install", @@ -82,26 +82,27 @@ var technologiesData = map[Technology]TechData{ }, Yarn: { indicators: []string{".yarnrc.yml", "yarn.lock", ".yarn"}, - packageDescriptor: "package.json", + packageDescriptor: []string{"package.json"}, packageVersionOperator: "@", applicabilityScannable: true, }, Go: { indicators: []string{"go.mod"}, - packageDescriptor: "go.mod", + packageDescriptor: []string{"go.mod"}, packageVersionOperator: "@v", packageInstallationCommand: "get", }, Pip: { packageType: Pypi, indicators: []string{"setup.py", "requirements.txt"}, + packageDescriptor: []string{"setup.py", "requirements.txt"}, exclude: []string{"Pipfile", "Pipfile.lock", "pyproject.toml", "poetry.lock"}, applicabilityScannable: true, }, Pipenv: { packageType: Pypi, indicators: []string{"Pipfile", "Pipfile.lock"}, - packageDescriptor: "Pipfile", + packageDescriptor: []string{"Pipfile"}, packageVersionOperator: "==", packageInstallationCommand: "install", applicabilityScannable: true, @@ -153,10 +154,7 @@ func (tech Technology) GetPackageType() string { return technologiesData[tech].packageType } -func (tech Technology) GetPackageDescriptor() string { - if technologiesData[tech].packageDescriptor == "" { - return tech.ToFormal() + " Package Descriptor" - } +func (tech Technology) GetPackageDescriptor() []string { return technologiesData[tech].packageDescriptor } diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index 900ad1922..2c09fcfd6 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -217,7 +217,7 @@ func extractXrayIssuesToSarifRun(run *sarif.Run, xrayJson formats.SimpleJsonResu vulnerability.Cves, vulnerability.IssueId, vulnerability.Severity, - vulnerability.Technology.GetPackageDescriptor(), + vulnerability.Technology, vulnerability.Components, vulnerability.Applicable, vulnerability.ImpactedDependencyName, @@ -235,7 +235,7 @@ func extractXrayIssuesToSarifRun(run *sarif.Run, xrayJson formats.SimpleJsonResu violation.Cves, violation.IssueId, violation.Severity, - violation.Technology.GetPackageDescriptor(), + violation.Technology, violation.Components, violation.Applicable, violation.ImpactedDependencyName, @@ -257,14 +257,14 @@ func extractXrayIssuesToSarifRun(run *sarif.Run, xrayJson formats.SimpleJsonResu return nil } -func addXrayCveIssueToSarifRun(cves []formats.CveRow, issueId, severity, file string, components []formats.ComponentRow, applicable, impactedDependencyName, impactedDependencyVersion, summary string, fixedVersions []string, markdownOutput bool, run *sarif.Run) error { +func addXrayCveIssueToSarifRun(cves []formats.CveRow, issueId, severity string, tech coreutils.Technology, components []formats.ComponentRow, applicable, impactedDependencyName, impactedDependencyVersion, summary string, fixedVersions []string, markdownOutput bool, run *sarif.Run) error { maxCveScore, err := findMaxCVEScore(cves) if err != nil { return err } cveId := GetIssueIdentifier(cves, issueId) msg := getVulnerabilityOrViolationSarifHeadline(impactedDependencyName, impactedDependencyVersion, cveId) - location, err := getXrayIssueLocationIfValidExists(file, run, markdownOutput) + location, err := getXrayIssueLocationIfValidExists(tech, run, markdownOutput) if err != nil { return err } @@ -288,30 +288,52 @@ func addXrayCveIssueToSarifRun(cves []formats.CveRow, issueId, severity, file st return nil } -// Xray GetPackageDescriptor can return multiple types of content. -// This could cause the sarif content not to be valid. if not override, we should handle all those situations: -// Full path - should be used as is -// Relative path - should be converted to full path -// Non path - should not be used as location -func getXrayIssueLocationIfValidExists(file string, run *sarif.Run, override bool) (location *sarif.Location, err error) { - location = sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri(file))) - if override { - // Use the content as is - return +func getDescriptorFullPath(tech coreutils.Technology, run *sarif.Run) (string, error) { + descriptors := tech.GetPackageDescriptor() + if len(descriptors) == 1 { + // Generate the full path + return GetFullLocationFileName(strings.TrimSpace(descriptors[0]), run.Invocations), nil } - // Check if full path - exists, err := fileutils.IsFileExists(file, false) - if err != nil || exists { - return + for _, optional := range descriptors { + // If multiple options return first to match + full := GetFullLocationFileName(strings.TrimSpace(optional), run.Invocations) + if exists, err := fileutils.IsFileExists(full, false); err != nil { + return "", err + } else if exists { + return full, nil + } + } + return "", nil +} + +func getDescriptorPath(tech coreutils.Technology) string { + descriptors := tech.GetPackageDescriptor() + for _, descriptor := range descriptors { + if strings.TrimSpace(descriptor) != "" { + return strings.TrimSpace(descriptor) + } } - // Check if relative path - fullPath := GetFullLocationFileName(file, run.Invocations) - location = sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + fullPath))) - if exists, err = fileutils.IsFileExists(fullPath, false); err != nil || exists { + return "" +} + +// Get the descriptor location with the Xray issues if exists. +// If location not exists and markdown provided, will return a string with: " Package Descriptor" else nil +func getXrayIssueLocationIfValidExists(tech coreutils.Technology, run *sarif.Run, markdown bool) (location *sarif.Location, err error) { + descriptorPath := getDescriptorPath(tech) + if !markdown { + descriptorPath, err = getDescriptorFullPath(tech, run) + if err != nil { + return + } + } + if strings.TrimSpace(descriptorPath) == "" { + // Can't calculate actual file location + if markdown { + return sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri(tech.ToFormal() + " Package Descriptor"))), nil + } return } - // Not usable content - return nil, nil + 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) { diff --git a/xray/utils/resultwriter_test.go b/xray/utils/resultwriter_test.go index 57458ea34..6b322a3a4 100644 --- a/xray/utils/resultwriter_test.go +++ b/xray/utils/resultwriter_test.go @@ -1,9 +1,14 @@ package utils import ( + "os" + "path/filepath" "testing" + "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" + "github.com/jfrog/jfrog-cli-core/v2/utils/tests" "github.com/jfrog/jfrog-cli-core/v2/xray/formats" + "github.com/owenrumney/go-sarif/v2/sarif" "github.com/stretchr/testify/assert" ) @@ -134,3 +139,68 @@ func TestFindMaxCVEScore(t *testing.T) { }) } } + +func TestGetXrayIssueLocationIfValidExists(t *testing.T) { + testDir, done := tests.CreateTempDirWithCallbackAndAssert(t) + invocation := sarif.NewInvocation().WithWorkingDirectory(sarif.NewSimpleArtifactLocation(testDir)) + file, err := os.Create(filepath.Join(testDir, "go.mod")) + assert.NoError(t, err) + assert.NotNil(t, file) + file, err = os.Create(filepath.Join(testDir, "build.gradle.kts")) + assert.NoError(t, err) + assert.NotNil(t, file) + + testCases := []struct { + name string + tech coreutils.Technology + run *sarif.Run + markdown bool + expectedOutput *sarif.Location + }{ + { + name: "No descriptor information", + tech: coreutils.Pip, + run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), + markdown: false, + expectedOutput: nil, + }, + { + name: "No descriptor information - markdown", + tech: coreutils.Poetry, + run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), + markdown: true, + expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri(coreutils.Poetry.ToFormal() + " Package Descriptor"))), + }, + { + name: "One descriptor information", + tech: coreutils.Go, + run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), + markdown: false, + expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + filepath.Join(testDir, "go.mod")))), + }, + { + name: "One descriptor information - markdown", + tech: coreutils.Maven, + run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), + markdown: true, + expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://pom.xml"))), + }, + { + name: "Multiple descriptor information", + tech: coreutils.Gradle, + run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), + markdown: false, + expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + filepath.Join(testDir, "build.gradle.kts")))), + }, + + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + output, err := getXrayIssueLocationIfValidExists(tc.tech, tc.run, tc.markdown) + if assert.NoError(t, err) { + assert.Equal(t, tc.expectedOutput, output) + } + }) + } + done() +} From a2deb814304e39379ee467f38d3921bc97de9032 Mon Sep 17 00:00:00 2001 From: attiasas Date: Mon, 2 Oct 2023 11:50:52 +0300 Subject: [PATCH 17/26] format --- xray/commands/audit/jas/commons_test.go | 2 +- xray/commands/audit/jas/sast/sastscanner.go | 2 +- xray/utils/resultwriter.go | 2 +- xray/utils/resultwriter_test.go | 47 ++++++++++----------- 4 files changed, 26 insertions(+), 27 deletions(-) diff --git a/xray/commands/audit/jas/commons_test.go b/xray/commands/audit/jas/commons_test.go index d600d7a73..c43cf94ab 100644 --- a/xray/commands/audit/jas/commons_test.go +++ b/xray/commands/audit/jas/commons_test.go @@ -126,4 +126,4 @@ func TestGetExcludePatterns(t *testing.T) { assert.ElementsMatch(t, actualExcludePatterns, expectedExcludePatterns) }) } -} \ No newline at end of file +} diff --git a/xray/commands/audit/jas/sast/sastscanner.go b/xray/commands/audit/jas/sast/sastscanner.go index f4133cf41..578bc6775 100644 --- a/xray/commands/audit/jas/sast/sastscanner.go +++ b/xray/commands/audit/jas/sast/sastscanner.go @@ -14,7 +14,7 @@ import ( ) const ( - sastScannerType = "sast" + sastScannerType = "sast" sastScanCommand = "zd" sastDocsUrlSuffix = "sast" ) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index 2c09fcfd6..130aa0c9b 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -333,7 +333,7 @@ func getXrayIssueLocationIfValidExists(tech coreutils.Technology, run *sarif.Run } return } - return sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + descriptorPath))), nil + 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) { diff --git a/xray/utils/resultwriter_test.go b/xray/utils/resultwriter_test.go index 6b322a3a4..99719dbe2 100644 --- a/xray/utils/resultwriter_test.go +++ b/xray/utils/resultwriter_test.go @@ -151,48 +151,47 @@ func TestGetXrayIssueLocationIfValidExists(t *testing.T) { assert.NotNil(t, file) testCases := []struct { - name string + name string tech coreutils.Technology - run *sarif.Run - markdown bool + run *sarif.Run + markdown bool expectedOutput *sarif.Location }{ { - name: "No descriptor information", - tech: coreutils.Pip, - run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), - markdown: false, + name: "No descriptor information", + tech: coreutils.Pip, + run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), + markdown: false, expectedOutput: nil, }, { - name: "No descriptor information - markdown", - tech: coreutils.Poetry, - run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), - markdown: true, + name: "No descriptor information - markdown", + tech: coreutils.Poetry, + run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), + markdown: true, expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri(coreutils.Poetry.ToFormal() + " Package Descriptor"))), }, { - name: "One descriptor information", - tech: coreutils.Go, - run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), - markdown: false, + name: "One descriptor information", + tech: coreutils.Go, + run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), + markdown: false, expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + filepath.Join(testDir, "go.mod")))), }, { - name: "One descriptor information - markdown", - tech: coreutils.Maven, - run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), - markdown: true, + name: "One descriptor information - markdown", + tech: coreutils.Maven, + run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), + markdown: true, expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://pom.xml"))), }, { - name: "Multiple descriptor information", - tech: coreutils.Gradle, - run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), - markdown: false, + name: "Multiple descriptor information", + tech: coreutils.Gradle, + run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), + markdown: false, expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + filepath.Join(testDir, "build.gradle.kts")))), }, - } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { From 998e7ac404b5f20a18f18762e759f1ba0a5464d4 Mon Sep 17 00:00:00 2001 From: attiasas Date: Mon, 2 Oct 2023 12:11:05 +0300 Subject: [PATCH 18/26] fix test --- xray/utils/resultwriter_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xray/utils/resultwriter_test.go b/xray/utils/resultwriter_test.go index 99719dbe2..28624baa9 100644 --- a/xray/utils/resultwriter_test.go +++ b/xray/utils/resultwriter_test.go @@ -142,6 +142,7 @@ func TestFindMaxCVEScore(t *testing.T) { func TestGetXrayIssueLocationIfValidExists(t *testing.T) { testDir, done := tests.CreateTempDirWithCallbackAndAssert(t) + defer done() invocation := sarif.NewInvocation().WithWorkingDirectory(sarif.NewSimpleArtifactLocation(testDir)) file, err := os.Create(filepath.Join(testDir, "go.mod")) assert.NoError(t, err) @@ -201,5 +202,4 @@ func TestGetXrayIssueLocationIfValidExists(t *testing.T) { } }) } - done() } From 0b5f0c4307d8ffe5e0189c46e1545c466e522312 Mon Sep 17 00:00:00 2001 From: attiasas Date: Mon, 2 Oct 2023 12:41:07 +0300 Subject: [PATCH 19/26] fix tests --- xray/utils/resultwriter_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/xray/utils/resultwriter_test.go b/xray/utils/resultwriter_test.go index 28624baa9..a00ac27c9 100644 --- a/xray/utils/resultwriter_test.go +++ b/xray/utils/resultwriter_test.go @@ -141,15 +141,17 @@ func TestFindMaxCVEScore(t *testing.T) { } func TestGetXrayIssueLocationIfValidExists(t *testing.T) { - testDir, done := tests.CreateTempDirWithCallbackAndAssert(t) - defer done() + testDir, cleanup := tests.CreateTempDirWithCallbackAndAssert(t) + defer cleanup() invocation := sarif.NewInvocation().WithWorkingDirectory(sarif.NewSimpleArtifactLocation(testDir)) file, err := os.Create(filepath.Join(testDir, "go.mod")) assert.NoError(t, err) assert.NotNil(t, file) - file, err = os.Create(filepath.Join(testDir, "build.gradle.kts")) + defer func() { assert.NoError(t, file.Close()) }() + file2, err := os.Create(filepath.Join(testDir, "build.gradle.kts")) assert.NoError(t, err) - assert.NotNil(t, file) + assert.NotNil(t, file2) + defer func() { assert.NoError(t, file2.Close()) }() testCases := []struct { name string From 7d4a59ba2e573a6c9fff88b27135ce5cd935af02 Mon Sep 17 00:00:00 2001 From: attiasas Date: Mon, 2 Oct 2023 15:51:04 +0300 Subject: [PATCH 20/26] remove trim --- xray/utils/resultwriter.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index 130aa0c9b..4d104bf6f 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -309,9 +309,7 @@ func getDescriptorFullPath(tech coreutils.Technology, run *sarif.Run) (string, e func getDescriptorPath(tech coreutils.Technology) string { descriptors := tech.GetPackageDescriptor() for _, descriptor := range descriptors { - if strings.TrimSpace(descriptor) != "" { - return strings.TrimSpace(descriptor) - } + return descriptor } return "" } From 2ebdb0c12719f06cfeebf3dd7ed1e0ed7550baef Mon Sep 17 00:00:00 2001 From: attiasas Date: Mon, 2 Oct 2023 16:05:26 +0300 Subject: [PATCH 21/26] review changes --- utils/coreutils/techutils.go | 20 ++++++++++---------- xray/utils/resultwriter.go | 8 ++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/utils/coreutils/techutils.go b/utils/coreutils/techutils.go index 60b94dc5a..3880da6ad 100644 --- a/utils/coreutils/techutils.go +++ b/utils/coreutils/techutils.go @@ -44,8 +44,8 @@ type TechData struct { ciSetupSupport bool // Whether Contextual Analysis supported in this technology. applicabilityScannable bool - // The file that handles the project's dependencies. - packageDescriptor []string + // The files that handle the project's dependencies. + packageDescriptors []string // Formal name of the technology formal string // The executable name of the technology @@ -60,21 +60,21 @@ var technologiesData = map[Technology]TechData{ Maven: { indicators: []string{"pom.xml"}, ciSetupSupport: true, - packageDescriptor: []string{"pom.xml"}, + packageDescriptors: []string{"pom.xml"}, execCommand: "mvn", applicabilityScannable: true, }, Gradle: { indicators: []string{".gradle", ".gradle.kts"}, ciSetupSupport: true, - packageDescriptor: []string{"build.gradle", "build.gradle.kts"}, + packageDescriptors: []string{"build.gradle", "build.gradle.kts"}, applicabilityScannable: true, }, Npm: { indicators: []string{"package.json", "package-lock.json", "npm-shrinkwrap.json"}, exclude: []string{".yarnrc.yml", "yarn.lock", ".yarn"}, ciSetupSupport: true, - packageDescriptor: []string{"package.json"}, + packageDescriptors: []string{"package.json"}, formal: string(Npm), packageVersionOperator: "@", packageInstallationCommand: "install", @@ -82,27 +82,27 @@ var technologiesData = map[Technology]TechData{ }, Yarn: { indicators: []string{".yarnrc.yml", "yarn.lock", ".yarn"}, - packageDescriptor: []string{"package.json"}, + packageDescriptors: []string{"package.json"}, packageVersionOperator: "@", applicabilityScannable: true, }, Go: { indicators: []string{"go.mod"}, - packageDescriptor: []string{"go.mod"}, + packageDescriptors: []string{"go.mod"}, packageVersionOperator: "@v", packageInstallationCommand: "get", }, Pip: { packageType: Pypi, indicators: []string{"setup.py", "requirements.txt"}, - packageDescriptor: []string{"setup.py", "requirements.txt"}, + packageDescriptors: []string{"setup.py", "requirements.txt"}, exclude: []string{"Pipfile", "Pipfile.lock", "pyproject.toml", "poetry.lock"}, applicabilityScannable: true, }, Pipenv: { packageType: Pypi, indicators: []string{"Pipfile", "Pipfile.lock"}, - packageDescriptor: []string{"Pipfile"}, + packageDescriptors: []string{"Pipfile"}, packageVersionOperator: "==", packageInstallationCommand: "install", applicabilityScannable: true, @@ -155,7 +155,7 @@ func (tech Technology) GetPackageType() string { } func (tech Technology) GetPackageDescriptor() []string { - return technologiesData[tech].packageDescriptor + return technologiesData[tech].packageDescriptors } func (tech Technology) IsCiSetup() bool { diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index 4d104bf6f..be252e737 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -294,13 +294,13 @@ func getDescriptorFullPath(tech coreutils.Technology, run *sarif.Run) (string, e // Generate the full path return GetFullLocationFileName(strings.TrimSpace(descriptors[0]), run.Invocations), nil } - for _, optional := range descriptors { + for _, descriptor := range descriptors { // If multiple options return first to match - full := GetFullLocationFileName(strings.TrimSpace(optional), run.Invocations) - if exists, err := fileutils.IsFileExists(full, false); err != nil { + absolutePath := GetFullLocationFileName(strings.TrimSpace(descriptor), run.Invocations) + if exists, err := fileutils.IsFileExists(absolutePath, false); err != nil { return "", err } else if exists { - return full, nil + return absolutePath, nil } } return "", nil From 15ad8d2d9b645e4ebd407a7e8244dcf60244e4b3 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 3 Oct 2023 10:44:42 +0300 Subject: [PATCH 22/26] done changes --- xray/utils/resultwriter.go | 69 +++++++++++++++------------------ xray/utils/resultwriter_test.go | 2 +- xray/utils/sarifutils.go | 2 +- 3 files changed, 33 insertions(+), 40 deletions(-) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index be252e737..a44f5bd97 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -117,7 +117,11 @@ func (rw *ResultsWriter) PrintScanResults() error { case Json: return PrintJson(rw.results.getXrayScanResults()) case Sarif: - sarifFile, err := GenerateSarifContentFromResults(rw.results, rw.isMultipleRoots, rw.includeLicenses, false) + sarifReport, err := GenereateSarifReportFromResults(rw.results, rw.isMultipleRoots, rw.includeLicenses) + if err != nil { + return err + } + sarifFile, err := ConvertSarifReportToString(sarifReport) if err != nil { return err } @@ -171,12 +175,12 @@ func printMessage(message string) { log.Output("💬" + message) } -func GenerateSarifContentFromResults(extendedResults *ExtendedScanResults, isMultipleRoots, includeLicenses, markdownOutput bool) (sarifStr string, err error) { - report, err := NewReport() +func GenereateSarifReportFromResults(extendedResults *ExtendedScanResults, isMultipleRoots, includeLicenses bool) (report *sarif.Report, err error) { + report, err = NewReport() if err != nil { return } - xrayRun, err := convertXrayResponsesToSarifRun(extendedResults, isMultipleRoots, includeLicenses, markdownOutput) + xrayRun, err := convertXrayResponsesToSarifRun(extendedResults, isMultipleRoots, includeLicenses) if err != nil { return } @@ -187,15 +191,18 @@ func GenerateSarifContentFromResults(extendedResults *ExtendedScanResults, isMul report.Runs = append(report.Runs, extendedResults.SecretsScanResults...) report.Runs = append(report.Runs, extendedResults.SastScanResults...) + return +} + +func ConvertSarifReportToString(report *sarif.Report) (sarifStr string, err error) { out, err := json.Marshal(report) if err != nil { return "", errorutils.CheckError(err) } - return clientUtils.IndentJson(out), nil } -func convertXrayResponsesToSarifRun(extendedResults *ExtendedScanResults, isMultipleRoots, includeLicenses, markdownOutput bool) (run *sarif.Run, err error) { +func convertXrayResponsesToSarifRun(extendedResults *ExtendedScanResults, isMultipleRoots, includeLicenses bool) (run *sarif.Run, err error) { xrayJson, err := convertXrayScanToSimpleJson(extendedResults, isMultipleRoots, includeLicenses, true) if err != nil { return @@ -203,7 +210,7 @@ func convertXrayResponsesToSarifRun(extendedResults *ExtendedScanResults, isMult xrayRun := sarif.NewRunWithInformationURI("JFrog Xray SCA", BaseDocumentationURL+"sca") xrayRun.Tool.Driver.Version = &extendedResults.XrayVersion if len(xrayJson.Vulnerabilities) > 0 || len(xrayJson.SecurityViolations) > 0 { - if err = extractXrayIssuesToSarifRun(xrayRun, xrayJson, markdownOutput); err != nil { + if err = extractXrayIssuesToSarifRun(xrayRun, xrayJson); err != nil { return } } @@ -211,7 +218,7 @@ func convertXrayResponsesToSarifRun(extendedResults *ExtendedScanResults, isMult return } -func extractXrayIssuesToSarifRun(run *sarif.Run, xrayJson formats.SimpleJsonResults, markdownOutput bool) error { +func extractXrayIssuesToSarifRun(run *sarif.Run, xrayJson formats.SimpleJsonResults) error { for _, vulnerability := range xrayJson.Vulnerabilities { if err := addXrayCveIssueToSarifRun( vulnerability.Cves, @@ -224,7 +231,6 @@ func extractXrayIssuesToSarifRun(run *sarif.Run, xrayJson formats.SimpleJsonResu vulnerability.ImpactedDependencyVersion, vulnerability.Summary, vulnerability.FixedVersions, - markdownOutput, run, ); err != nil { return err @@ -242,7 +248,6 @@ func extractXrayIssuesToSarifRun(run *sarif.Run, xrayJson formats.SimpleJsonResu violation.ImpactedDependencyVersion, violation.Summary, violation.FixedVersions, - markdownOutput, run, ); err != nil { return err @@ -257,14 +262,14 @@ func extractXrayIssuesToSarifRun(run *sarif.Run, xrayJson formats.SimpleJsonResu return nil } -func addXrayCveIssueToSarifRun(cves []formats.CveRow, issueId, severity string, tech coreutils.Technology, components []formats.ComponentRow, applicable, impactedDependencyName, impactedDependencyVersion, summary string, fixedVersions []string, markdownOutput bool, run *sarif.Run) error { +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) if err != nil { return err } cveId := GetIssueIdentifier(cves, issueId) msg := getVulnerabilityOrViolationSarifHeadline(impactedDependencyName, impactedDependencyVersion, cveId) - location, err := getXrayIssueLocationIfValidExists(tech, run, markdownOutput) + location, err := getXrayIssueLocationIfValidExists(tech, run) if err != nil { return err } @@ -274,16 +279,15 @@ func addXrayCveIssueToSarifRun(cves []formats.CveRow, issueId, severity string, cveRuleProperties.Add("security-severity", maxCveScore) } rule.WithProperties(cveRuleProperties.Properties) - if markdownOutput { - formattedDirectDependencies, err := getDirectDependenciesFormatted(components) - if err != nil { + formattedDirectDependencies, err := getDirectDependenciesFormatted(components) + if err != nil { return err - } - markdownDescription := getSarifTableDescription(formattedDirectDependencies, maxCveScore, applicable, fixedVersions) + "\n" - rule.WithMarkdownHelp(markdownDescription) - } else { - rule.WithDescription(summary) } + markdownDescription := getSarifTableDescription(formattedDirectDependencies, maxCveScore, applicable, fixedVersions) + "\n" + rule.WithHelp(&sarif.MultiformatMessageString{ + Text: &summary, + Markdown: &markdownDescription, + }) } return nil } @@ -306,29 +310,18 @@ func getDescriptorFullPath(tech coreutils.Technology, run *sarif.Run) (string, e return "", nil } -func getDescriptorPath(tech coreutils.Technology) string { - descriptors := tech.GetPackageDescriptor() - for _, descriptor := range descriptors { - return descriptor - } - return "" -} - // Get the descriptor location with the Xray issues if exists. // If location not exists and markdown provided, will return a string with: " Package Descriptor" else nil -func getXrayIssueLocationIfValidExists(tech coreutils.Technology, run *sarif.Run, markdown bool) (location *sarif.Location, err error) { - descriptorPath := getDescriptorPath(tech) - if !markdown { - descriptorPath, err = getDescriptorFullPath(tech, run) - if err != nil { - return - } +func getXrayIssueLocationIfValidExists(tech coreutils.Technology, run *sarif.Run) (location *sarif.Location, err error) { + descriptorPath, err := getDescriptorFullPath(tech, run) + if err != nil { + return } if strings.TrimSpace(descriptorPath) == "" { // Can't calculate actual file location - if markdown { - return sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri(tech.ToFormal() + " Package Descriptor"))), nil - } + // if markdown { + // return sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri(tech.ToFormal() + " Package Descriptor"))), nil + // } return } return sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + descriptorPath))), nil diff --git a/xray/utils/resultwriter_test.go b/xray/utils/resultwriter_test.go index a00ac27c9..b3e989dd1 100644 --- a/xray/utils/resultwriter_test.go +++ b/xray/utils/resultwriter_test.go @@ -198,7 +198,7 @@ func TestGetXrayIssueLocationIfValidExists(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - output, err := getXrayIssueLocationIfValidExists(tc.tech, tc.run, tc.markdown) + output, err := getXrayIssueLocationIfValidExists(tc.tech, tc.run) if assert.NoError(t, err) { assert.Equal(t, tc.expectedOutput, output) } diff --git a/xray/utils/sarifutils.go b/xray/utils/sarifutils.go index 5f9d86eb3..0da577140 100644 --- a/xray/utils/sarifutils.go +++ b/xray/utils/sarifutils.go @@ -162,7 +162,7 @@ func GetFullLocationFileName(relative string, invocations []*sarif.Invocation) s } func SetLocationFileName(location *sarif.Location, fileName string) { - if location != nil && location.PhysicalLocation != nil && location.PhysicalLocation.Region != nil && location.PhysicalLocation.Region.Snippet != nil { + if location != nil && location.PhysicalLocation != nil && location.PhysicalLocation.ArtifactLocation != nil { location.PhysicalLocation.ArtifactLocation.URI = &fileName } } From 5c7d21a18f99d04729d39e10a6b765c4f12f8795 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 3 Oct 2023 10:44:59 +0300 Subject: [PATCH 23/26] format --- xray/utils/resultwriter.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index a44f5bd97..cdc978e8f 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -281,11 +281,11 @@ func addXrayCveIssueToSarifRun(cves []formats.CveRow, issueId, severity string, rule.WithProperties(cveRuleProperties.Properties) formattedDirectDependencies, err := getDirectDependenciesFormatted(components) if err != nil { - return err + return err } markdownDescription := getSarifTableDescription(formattedDirectDependencies, maxCveScore, applicable, fixedVersions) + "\n" rule.WithHelp(&sarif.MultiformatMessageString{ - Text: &summary, + Text: &summary, Markdown: &markdownDescription, }) } From d2e55bccd6f2e6e9598333ec10ff778af5e146ec Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 3 Oct 2023 10:59:20 +0300 Subject: [PATCH 24/26] fix tests --- xray/utils/resultwriter_test.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/xray/utils/resultwriter_test.go b/xray/utils/resultwriter_test.go index b3e989dd1..9acc6e28b 100644 --- a/xray/utils/resultwriter_test.go +++ b/xray/utils/resultwriter_test.go @@ -157,42 +157,24 @@ func TestGetXrayIssueLocationIfValidExists(t *testing.T) { name string tech coreutils.Technology run *sarif.Run - markdown bool expectedOutput *sarif.Location }{ { name: "No descriptor information", tech: coreutils.Pip, run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), - markdown: false, expectedOutput: nil, }, - { - name: "No descriptor information - markdown", - tech: coreutils.Poetry, - run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), - markdown: true, - expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri(coreutils.Poetry.ToFormal() + " Package Descriptor"))), - }, { name: "One descriptor information", tech: coreutils.Go, run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), - markdown: false, expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + filepath.Join(testDir, "go.mod")))), }, - { - name: "One descriptor information - markdown", - tech: coreutils.Maven, - run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), - markdown: true, - expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://pom.xml"))), - }, { name: "Multiple descriptor information", tech: coreutils.Gradle, run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), - markdown: false, expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + filepath.Join(testDir, "build.gradle.kts")))), }, } From 306e9f3810f2fad8599f78b65ae7f0bc64e1ca43 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 3 Oct 2023 11:15:04 +0300 Subject: [PATCH 25/26] cleanup --- xray/utils/resultwriter.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index cdc978e8f..553f37388 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -311,17 +311,12 @@ func getDescriptorFullPath(tech coreutils.Technology, run *sarif.Run) (string, e } // Get the descriptor location with the Xray issues if exists. -// If location not exists and markdown provided, will return a string with: " Package Descriptor" else nil func getXrayIssueLocationIfValidExists(tech coreutils.Technology, run *sarif.Run) (location *sarif.Location, err error) { descriptorPath, err := getDescriptorFullPath(tech, run) if err != nil { return } if strings.TrimSpace(descriptorPath) == "" { - // Can't calculate actual file location - // if markdown { - // return sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri(tech.ToFormal() + " Package Descriptor"))), nil - // } return } return sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + descriptorPath))), nil From 719a5e899a6397ef5bbb8a6cc3c65a0c27366af8 Mon Sep 17 00:00:00 2001 From: attiasas Date: Tue, 3 Oct 2023 11:17:04 +0300 Subject: [PATCH 26/26] add test --- xray/utils/resultwriter_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/xray/utils/resultwriter_test.go b/xray/utils/resultwriter_test.go index 9acc6e28b..a67dfe86c 100644 --- a/xray/utils/resultwriter_test.go +++ b/xray/utils/resultwriter_test.go @@ -171,6 +171,12 @@ func TestGetXrayIssueLocationIfValidExists(t *testing.T) { run: CreateRunWithDummyResults().WithInvocations([]*sarif.Invocation{invocation}), expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + filepath.Join(testDir, "go.mod")))), }, + { + name: "One descriptor information - no invocation", + tech: coreutils.Go, + run: CreateRunWithDummyResults(), + expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://go.mod"))), + }, { name: "Multiple descriptor information", tech: coreutils.Gradle,