From 36ab1fbb710f4afd9395d43bbb0796a948342bf6 Mon Sep 17 00:00:00 2001 From: Michael Sverdlov Date: Tue, 12 Sep 2023 12:11:52 +0300 Subject: [PATCH] Unite relative path conversion (#947) --- xray/formats/conversion.go | 24 +++------- xray/formats/simplejsonapi.go | 1 - xray/formats/table.go | 13 ++---- xray/utils/analyzermanager.go | 2 +- xray/utils/resultstable.go | 52 ++++++++++----------- xray/utils/resultwriter.go | 32 ------------- xray/utils/sarifutils.go | 86 ++++++++--------------------------- 7 files changed, 53 insertions(+), 157 deletions(-) diff --git a/xray/formats/conversion.go b/xray/formats/conversion.go index 624791dfe..83bc09d91 100644 --- a/xray/formats/conversion.go +++ b/xray/formats/conversion.go @@ -146,32 +146,20 @@ func ConvertToSecretsTableRow(rows []SourceCodeRow) (tableRows []secretsTableRow tableRows = append(tableRows, secretsTableRow{ severity: rows[i].Severity, file: rows[i].File, - lineColumn: (strconv.Itoa(rows[i].StartLine) + ":" + strconv.Itoa(rows[i].StartColumn)), - text: rows[i].Snippet, + lineColumn: strconv.Itoa(rows[i].StartLine) + ":" + strconv.Itoa(rows[i].StartColumn), + secret: rows[i].Snippet, }) } return } -func ConvertToIacTableRow(rows []SourceCodeRow) (tableRows []iacTableRow) { +func ConvertToIacOrSastTableRow(rows []SourceCodeRow) (tableRows []iacOrSastTableRow) { for i := range rows { - tableRows = append(tableRows, iacTableRow{ + tableRows = append(tableRows, iacOrSastTableRow{ severity: rows[i].Severity, file: rows[i].File, - lineColumn: (strconv.Itoa(rows[i].StartLine) + ":" + strconv.Itoa(rows[i].StartColumn)), - text: rows[i].Snippet, - }) - } - return -} - -func ConvertToSastTableRow(rows []SourceCodeRow) (tableRows []sastTableRow) { - for i := range rows { - tableRows = append(tableRows, sastTableRow{ - severity: rows[i].Severity, - file: rows[i].File, - lineColumn: (strconv.Itoa(rows[i].StartLine) + ":" + strconv.Itoa(rows[i].StartColumn)), - text: rows[i].Snippet, + lineColumn: strconv.Itoa(rows[i].StartLine) + ":" + strconv.Itoa(rows[i].StartColumn), + finding: rows[i].Finding, }) } return diff --git a/xray/formats/simplejsonapi.go b/xray/formats/simplejsonapi.go index be4b7cc1a..9bbbdea60 100644 --- a/xray/formats/simplejsonapi.go +++ b/xray/formats/simplejsonapi.go @@ -78,7 +78,6 @@ type SourceCodeRow struct { Severity string `json:"severity"` SeverityNumValue int `json:"-"` // For sorting Location - Type string `json:"type"` Finding string `json:"finding,omitempty"` ScannerDescription string `json:"scannerDescription,omitempty"` CodeFlow [][]Location `json:"codeFlow,omitempty"` diff --git a/xray/formats/table.go b/xray/formats/table.go index c099b058d..ea21c845a 100644 --- a/xray/formats/table.go +++ b/xray/formats/table.go @@ -127,19 +127,12 @@ type secretsTableRow struct { severity string `col-name:"Severity"` file string `col-name:"File"` lineColumn string `col-name:"Line:Column"` - text string `col-name:"Secret"` + secret string `col-name:"Secret"` } -type iacTableRow struct { +type iacOrSastTableRow struct { severity string `col-name:"Severity"` file string `col-name:"File"` lineColumn string `col-name:"Line:Column"` - text string `col-name:"Finding"` -} - -type sastTableRow struct { - severity string `col-name:"Severity"` - file string `col-name:"File"` - lineColumn string `col-name:"Line:Column"` - text string `col-name:"Finding"` + finding string `col-name:"Finding"` } diff --git a/xray/utils/analyzermanager.go b/xray/utils/analyzermanager.go index 26a24bc13..70400d80e 100644 --- a/xray/utils/analyzermanager.go +++ b/xray/utils/analyzermanager.go @@ -23,7 +23,7 @@ const ( EntitlementsMinVersion = "3.66.5" ApplicabilityFeatureId = "contextual_analysis" AnalyzerManagerZipName = "analyzerManager.zip" - defaultAnalyzerManagerVersion = "1.2.4.1953469" + defaultAnalyzerManagerVersion = "1.2.4.2000151" minAnalyzerManagerVersionForSast = "1.3" analyzerManagerDownloadPath = "xsc-gen-exe-analyzer-manager-local/v1" analyzerManagerDirName = "analyzerManager" diff --git a/xray/utils/resultstable.go b/xray/utils/resultstable.go index 44909fd11..6a61fb1ad 100644 --- a/xray/utils/resultstable.go +++ b/xray/utils/resultstable.go @@ -302,23 +302,22 @@ func PrepareSecrets(secrets []*sarif.Run) []formats.SourceCodeRow { func prepareSecrets(secrets []*sarif.Run, isTable bool) []formats.SourceCodeRow { var secretsRows []formats.SourceCodeRow for _, secretRun := range secrets { - for _, secret := range secretRun.Results { - currSeverity := GetSeverity(GetResultSeverity(secret), Applicable) - for _, location := range secret.Locations { + for _, secretResult := range secretRun.Results { + currSeverity := GetSeverity(GetResultSeverity(secretResult), Applicable) + for _, location := range secretResult.Locations { secretsRows = append(secretsRows, formats.SourceCodeRow{ Severity: currSeverity.printableTitle(isTable), - Finding: GetResultMsgText(secret), + Finding: GetResultMsgText(secretResult), SeverityNumValue: currSeverity.numValue, Location: formats.Location{ - File: GetLocationFileName(location), + File: GetRelativeLocationFileName(location, secretRun.Invocations), StartLine: GetLocationStartLine(location), StartColumn: GetLocationStartColumn(location), EndLine: GetLocationEndLine(location), EndColumn: GetLocationEndColumn(location), Snippet: GetLocationSnippet(location), }, - Type: *secret.RuleID, }, ) } @@ -350,28 +349,27 @@ func PrepareIacs(iacs []*sarif.Run) []formats.SourceCodeRow { func prepareIacs(iacs []*sarif.Run, isTable bool) []formats.SourceCodeRow { var iacRows []formats.SourceCodeRow for _, iacRun := range iacs { - for _, iac := range iacRun.Results { + for _, iacResult := range iacRun.Results { scannerDescription := "" - if rule, err := iacRun.GetRuleById(*iac.RuleID); err == nil { + if rule, err := iacRun.GetRuleById(*iacResult.RuleID); err == nil { scannerDescription = GetRuleFullDescription(rule) } - currSeverity := GetSeverity(GetResultSeverity(iac), Applicable) - for _, location := range iac.Locations { + currSeverity := GetSeverity(GetResultSeverity(iacResult), Applicable) + for _, location := range iacResult.Locations { iacRows = append(iacRows, formats.SourceCodeRow{ Severity: currSeverity.printableTitle(isTable), - Finding: GetResultMsgText(iac), + Finding: GetResultMsgText(iacResult), ScannerDescription: scannerDescription, SeverityNumValue: currSeverity.numValue, Location: formats.Location{ - File: GetLocationFileName(location), + File: GetRelativeLocationFileName(location, iacRun.Invocations), StartLine: GetLocationStartLine(location), StartColumn: GetLocationStartColumn(location), EndLine: GetLocationEndLine(location), EndColumn: GetLocationEndColumn(location), Snippet: GetLocationSnippet(location), }, - Type: *iac.RuleID, }, ) } @@ -389,7 +387,7 @@ func PrintIacTable(iacs []*sarif.Run, entitledForIacScan bool) error { if entitledForIacScan { iacRows := prepareIacs(iacs, true) log.Output() - return coreutils.PrintTable(formats.ConvertToIacTableRow(iacRows), "Infrastructure as Code Vulnerabilities", + return coreutils.PrintTable(formats.ConvertToIacOrSastTableRow(iacRows), "Infrastructure as Code Vulnerabilities", "✨ No Infrastructure as Code vulnerabilities were found ✨", false) } return nil @@ -402,30 +400,30 @@ func PrepareSast(sasts []*sarif.Run) []formats.SourceCodeRow { func prepareSast(sasts []*sarif.Run, isTable bool) []formats.SourceCodeRow { var sastRows []formats.SourceCodeRow for _, sastRun := range sasts { - for _, sast := range sastRun.Results { + for _, sastResult := range sastRun.Results { scannerDescription := "" - if rule, err := sastRun.GetRuleById(*sast.RuleID); err == nil { + if rule, err := sastRun.GetRuleById(*sastResult.RuleID); err == nil { scannerDescription = GetRuleFullDescription(rule) } - currSeverity := GetSeverity(GetResultSeverity(sast), Applicable) - flows := toSourceCodeCodeFlowRow(sast.CodeFlows, isTable) - for _, location := range sast.Locations { + currSeverity := GetSeverity(GetResultSeverity(sastResult), Applicable) + + for _, location := range sastResult.Locations { + codeFlows := GetLocationRelatedCodeFlowsFromResult(location, sastResult) sastRows = append(sastRows, formats.SourceCodeRow{ Severity: currSeverity.printableTitle(isTable), - Finding: GetResultMsgText(sast), + Finding: GetResultMsgText(sastResult), ScannerDescription: scannerDescription, SeverityNumValue: currSeverity.numValue, Location: formats.Location{ - File: GetLocationFileName(location), + File: GetRelativeLocationFileName(location, sastRun.Invocations), StartLine: GetLocationStartLine(location), StartColumn: GetLocationStartColumn(location), EndLine: GetLocationEndLine(location), EndColumn: GetLocationEndColumn(location), Snippet: GetLocationSnippet(location), }, - Type: *sast.RuleID, - CodeFlow: flows, + CodeFlow: codeFlowToLocationFlow(codeFlows, sastRun.Invocations, isTable), }, ) } @@ -439,7 +437,7 @@ func prepareSast(sasts []*sarif.Run, isTable bool) []formats.SourceCodeRow { return sastRows } -func toSourceCodeCodeFlowRow(flows []*sarif.CodeFlow, isTable bool) (flowRows [][]formats.Location) { +func codeFlowToLocationFlow(flows []*sarif.CodeFlow, invocations []*sarif.Invocation, isTable bool) (flowRows [][]formats.Location) { if isTable { // Not displaying in table return @@ -449,7 +447,7 @@ func toSourceCodeCodeFlowRow(flows []*sarif.CodeFlow, isTable bool) (flowRows [] rowFlow := []formats.Location{} for _, stackTraceEntry := range stackTrace.Locations { rowFlow = append(rowFlow, formats.Location{ - File: GetLocationFileName(stackTraceEntry.Location), + File: GetRelativeLocationFileName(stackTraceEntry.Location, invocations), StartLine: GetLocationStartLine(stackTraceEntry.Location), StartColumn: GetLocationStartColumn(stackTraceEntry.Location), EndLine: GetLocationEndLine(stackTraceEntry.Location), @@ -467,7 +465,7 @@ func PrintSastTable(sast []*sarif.Run, entitledForSastScan bool) error { if entitledForSastScan { sastRows := prepareSast(sast, true) log.Output() - return coreutils.PrintTable(formats.ConvertToSastTableRow(sastRows), "Static Application Security Testing (SAST)", + return coreutils.PrintTable(formats.ConvertToIacOrSastTableRow(sastRows), "Static Application Security Testing (SAST)", "✨ No Static Application Security Testing vulnerabilities were found ✨", false) } return nil @@ -979,7 +977,7 @@ func getCveApplicability(cve formats.CveRow, applicabilityScanResults []*sarif.R for _, location := range foundResult.Locations { applicability.Evidence = append(applicability.Evidence, formats.Evidence{ Location: formats.Location{ - File: GetLocationFileName(location), + File: GetRelativeLocationFileName(location, applicabilityRun.Invocations), StartLine: GetLocationStartLine(location), StartColumn: GetLocationStartColumn(location), EndLine: GetLocationEndLine(location), diff --git a/xray/utils/resultwriter.go b/xray/utils/resultwriter.go index e2a82b2a8..dc611d057 100644 --- a/xray/utils/resultwriter.go +++ b/xray/utils/resultwriter.go @@ -92,50 +92,18 @@ func printScanResultsTables(results *ExtendedScanResults, isBinaryScan, includeV return } } - ConvertRunsPathsToRelative(results.SecretsScanResults) if err = PrintSecretsTable(results.SecretsScanResults, results.EntitledForJas); err != nil { return } - ConvertRunsPathsToRelative(results.IacScanResults) if err = PrintIacTable(results.IacScanResults, results.EntitledForJas); err != nil { return } if !IsSastSupported() { return } - ConvertRunsPathsToRelative(results.SastScanResults) return PrintSastTable(results.SastScanResults, results.EntitledForJas) } -// The paths at Sarif runs are absolute. -// Use this method if you need to translate the file paths to relative -func ConvertRunsPathsToRelative(runs []*sarif.Run) { - for _, sarifRun := range runs { - for _, invocation := range sarifRun.Invocations { - if wd := GetInvocationWorkingDirectory(invocation); len(wd) > 0 { - ConvertRunPathsToRelative(sarifRun, wd) - } - } - } -} - -func ConvertRunPathsToRelative(sarifRun *sarif.Run, wd string) { - for _, sarifResult := range sarifRun.Results { - // Convert paths in locations - for _, location := range sarifResult.Locations { - SetLocationFileName(location, ExtractRelativePath(GetLocationFileName(location), wd)) - } - // Convert paths in code flows - for _, codeFlows := range sarifResult.CodeFlows { - for _, threadFlows := range codeFlows.ThreadFlows { - for _, location := range threadFlows.Locations { - SetLocationFileName(location.Location, ExtractRelativePath(GetLocationFileName(location.Location), wd)) - } - } - } - } -} - func printMessages(messages []string) { if len(messages) > 0 { log.Output() diff --git a/xray/utils/sarifutils.go b/xray/utils/sarifutils.go index 623c727d6..8f7cdf3be 100644 --- a/xray/utils/sarifutils.go +++ b/xray/utils/sarifutils.go @@ -2,10 +2,10 @@ package utils import ( "fmt" + "path/filepath" "strconv" "strings" - "github.com/jfrog/gofrog/datastructures" "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/owenrumney/go-sarif/v2/sarif" ) @@ -128,36 +128,6 @@ func GetDiffFromRun(sources []*sarif.Run, targets []*sarif.Run) (runWithNewOnly return } -// Calculate new information that exists at the result and not at the source -func GetDiffFromResult(result *sarif.Result, source *sarif.Result) *sarif.Result { - newLocations := datastructures.MakeSet[*sarif.Location]() - newCodeFlows := []*sarif.CodeFlow{} - for _, targetLocation := range result.Locations { - if !IsLocationInResult(targetLocation, source) { - newLocations.Add(targetLocation) - newCodeFlows = append(newCodeFlows, GetLocationRelatedCodeFlowsFromResult(targetLocation, result)...) - continue - } - // Location in result, compare related code flows - for _, targetCodeFlow := range GetLocationRelatedCodeFlowsFromResult(targetLocation, result) { - for _, sourceCodeFlow := range GetLocationRelatedCodeFlowsFromResult(targetLocation, source) { - if !IsSameCodeFlow(targetCodeFlow, sourceCodeFlow) { - // Code flow does not exist at source, add it and it's related location - newLocations.Add(targetLocation) - newCodeFlows = append(newCodeFlows, targetCodeFlow) - } - } - } - } - // Create the result only with new information - return sarif.NewRuleResult(*result.RuleID). - WithKind(*result.Kind). - WithMessage(&result.Message). - WithLevel(*result.Level). - WithLocations(newLocations.ToSlice()). - WithCodeFlows(newCodeFlows) -} - func FilterResultsByRuleIdAndMsgText(source []*sarif.Result, ruleId, msgText string) (results []*sarif.Result) { for _, result := range source { if ruleId == *result.RuleID && msgText == GetResultMsgText(result) { @@ -172,7 +142,7 @@ func GetLocationRelatedCodeFlowsFromResult(location *sarif.Location, result *sar for _, stackTrace := range codeFlow.ThreadFlows { // The threadFlow is reverse stack trace. // The last location is the location that it relates to. - if IsSameLocation(location, stackTrace.Locations[len(stackTrace.Locations)-1].Location) { + if isSameLocation(location, stackTrace.Locations[len(stackTrace.Locations)-1].Location) { codeFlows = append(codeFlows, codeFlow) } } @@ -180,41 +150,7 @@ func GetLocationRelatedCodeFlowsFromResult(location *sarif.Location, result *sar return } -func IsSameCodeFlow(codeFlow *sarif.CodeFlow, other *sarif.CodeFlow) bool { - if len(codeFlow.ThreadFlows) != len(other.ThreadFlows) { - return false - } - // ThreadFlows is unordered list of stack trace - for _, stackTrace := range codeFlow.ThreadFlows { - foundMatch := false - for _, otherStackTrace := range other.ThreadFlows { - if len(stackTrace.Locations) != len(otherStackTrace.Locations) { - continue - } - for i, stackTraceLocation := range stackTrace.Locations { - if !IsSameLocation(stackTraceLocation.Location, otherStackTrace.Locations[i].Location) { - continue - } - } - foundMatch = true - } - if !foundMatch { - return false - } - } - return true -} - -func IsLocationInResult(location *sarif.Location, result *sarif.Result) bool { - for _, resultLocation := range result.Locations { - if IsSameLocation(location, resultLocation) { - return true - } - } - return false -} - -func IsSameLocation(location *sarif.Location, other *sarif.Location) bool { +func isSameLocation(location *sarif.Location, other *sarif.Location) bool { if location == other { return true } @@ -290,6 +226,19 @@ func GetLocationFileName(location *sarif.Location) string { return "" } +func GetRelativeLocationFileName(location *sarif.Location, invocations []*sarif.Invocation) string { + wd := "" + if len(invocations) > 0 { + wd = GetInvocationWorkingDirectory(invocations[0]) + } + GetLocationFileName(location) + filePath := GetLocationFileName(location) + if filePath != "" { + return ExtractRelativePath(filePath, wd) + } + return "" +} + 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 @@ -350,7 +299,8 @@ func ExtractRelativePath(resultPath string, projectRoot string) string { resultPath = strings.TrimPrefix(resultPath, "file://") // Get relative path - return strings.ReplaceAll(resultPath, projectRoot, "") + relativePath := strings.ReplaceAll(resultPath, projectRoot, "") + return strings.TrimPrefix(relativePath, string(filepath.Separator)) } func GetResultSeverity(result *sarif.Result) string {