diff --git a/scanpullrequest/scanpullrequest_test.go b/scanpullrequest/scanpullrequest_test.go index 39efbc10e..e7beeb9fe 100644 --- a/scanpullrequest/scanpullrequest_test.go +++ b/scanpullrequest/scanpullrequest_test.go @@ -327,7 +327,7 @@ func TestGetNewVulnerabilities(t *testing.T) { SeverityDetails: formats.SeverityDetails{Severity: "low"}, ImpactedDependencyName: "component-C", }, - Cves: []formats.CveRow{{Id: "CVE-2023-4321", Applicability: &formats.Applicability{Status: "Applicable", Evidence: []formats.Evidence{{Location: formats.Location{File: "file1", StartLine: 1, StartColumn: 10}}}}}}, + Cves: []formats.CveRow{{Id: "CVE-2023-4321", Applicability: &formats.Applicability{Status: "Applicable", Evidence: []formats.Evidence{{Location: formats.Location{File: "file1", StartLine: 1, StartColumn: 10, EndLine: 2, EndColumn: 11, Snippet: "snippet"}}}}}}, Technology: coreutils.Yarn, }, { @@ -338,7 +338,7 @@ func TestGetNewVulnerabilities(t *testing.T) { SeverityDetails: formats.SeverityDetails{Severity: "low"}, ImpactedDependencyName: "component-D", }, - Cves: []formats.CveRow{{Id: "CVE-2023-4321", Applicability: &formats.Applicability{Status: "Applicable", Evidence: []formats.Evidence{{Location: formats.Location{File: "file1", StartLine: 1, StartColumn: 10}}}}}}, + Cves: []formats.CveRow{{Id: "CVE-2023-4321", Applicability: &formats.Applicability{Status: "Applicable", Evidence: []formats.Evidence{{Location: formats.Location{File: "file1", StartLine: 1, StartColumn: 10, EndLine: 2, EndColumn: 11, Snippet: "snippet"}}}}}}, Technology: coreutils.Yarn, }, } @@ -347,39 +347,16 @@ func TestGetNewVulnerabilities(t *testing.T) { vulnerabilities, licenses, err := createNewVulnerabilitiesRows( &audit.Results{ ExtendedScanResults: &xrayutils.ExtendedScanResults{ - XrayResults: []services.ScanResponse{previousScan}, - EntitledForJas: true, - ApplicabilityScanResults: []*sarif.Run{sarif.NewRunWithInformationURI("", ""). - WithResults([]*sarif.Result{ - sarif.NewRuleResult("applic_CVE-2023-4321"). - WithLocations([]*sarif.Location{ - sarif.NewLocationWithPhysicalLocation(sarif.NewPhysicalLocation(). - WithArtifactLocation(sarif.NewArtifactLocation(). - WithUri("file1")). - WithRegion(sarif.NewRegion(). - WithStartLine(1). - WithStartColumn(10))), - }), - }), - }, + XrayResults: []services.ScanResponse{previousScan}, + EntitledForJas: true, + ApplicabilityScanResults: []*sarif.Run{xrayutils.CreateRunWithDummyResults(xrayutils.CreateResultWithOneLocation("file1", 1, 10, 2, 11, "snippet", "applic_CVE-2023-4321", ""))}, }, }, &audit.Results{ ExtendedScanResults: &xrayutils.ExtendedScanResults{ - XrayResults: []services.ScanResponse{currentScan}, - EntitledForJas: true, - ApplicabilityScanResults: []*sarif.Run{sarif.NewRunWithInformationURI("", ""). - WithResults([]*sarif.Result{ - sarif.NewRuleResult("applic_CVE-2023-4321"). - WithLocations([]*sarif.Location{sarif.NewLocationWithPhysicalLocation(sarif.NewPhysicalLocation(). - WithArtifactLocation(sarif.NewArtifactLocation(). - WithUri("file1")). - WithRegion(sarif.NewRegion(). - WithStartLine(1). - WithStartColumn(10))), - }), - }), - }, + XrayResults: []services.ScanResponse{currentScan}, + EntitledForJas: true, + ApplicabilityScanResults: []*sarif.Run{xrayutils.CreateRunWithDummyResults(xrayutils.CreateResultWithOneLocation("file1", 1, 10, 2, 11, "snippet", "applic_CVE-2023-4321", ""))}, }, }, nil, @@ -515,38 +492,127 @@ func TestGetAllIssues(t *testing.T) { Licenses: []services.License{{Key: "Apache-2.0", Components: map[string]services.Component{"Dep-1": {FixedVersions: []string{"1.2.3"}}}}}, }}, ApplicabilityScanResults: []*sarif.Run{ - utils.GetRunWithDummyResults( - utils.GetDummyResultWithOneLocation("file", 0, 0, "", "applic_CVE-2022-2122", ""), - utils.GetDummyPassingResult("applic_CVE-2023-3122")), + xrayutils.CreateRunWithDummyResults( + xrayutils.CreateDummyPassingResult("applic_CVE-2023-3122"), + xrayutils.CreateResultWithOneLocation("file1", 1, 10, 2, 11, "snippet", "applic_CVE-2022-2122", ""), + ), + }, + IacScanResults: []*sarif.Run{ + xrayutils.CreateRunWithDummyResults( + xrayutils.CreateResultWithLocations("Missing auto upgrade was detected", "rule", xrayutils.ConvertToSarifLevel("high"), + xrayutils.CreateLocation("file1", 1, 10, 2, 11, "aws-violation"), + ), + ), }, SecretsScanResults: []*sarif.Run{ - utils.GetRunWithDummyResults( - utils.GetDummyResultWithOneLocation("index.js", 2, 13, "access token exposed", "", ""), + xrayutils.CreateRunWithDummyResults( + xrayutils.CreateResultWithLocations("Secret", "rule", xrayutils.ConvertToSarifLevel("high"), + xrayutils.CreateLocation("index.js", 5, 6, 7, 8, "access token exposed"), + ), + ), + }, + SastScanResults: []*sarif.Run{ + xrayutils.CreateRunWithDummyResults( + xrayutils.CreateResultWithLocations("XSS Vulnerability", "rule", xrayutils.ConvertToSarifLevel("high"), + xrayutils.CreateLocation("file1", 1, 10, 2, 11, "snippet"), + ), ), }, EntitledForJas: true, }, } + + expectedOutput := &utils.IssuesCollection{ + Vulnerabilities: []formats.VulnerabilityOrViolationRow{ + { + Applicable: "Applicable", + FixedVersions: []string{"1.2.3"}, + ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ + SeverityDetails: formats.SeverityDetails{Severity: "High", SeverityNumValue: 13}, + ImpactedDependencyName: "Dep-1", + }, + Cves: []formats.CveRow{{Id: "CVE-2022-2122", Applicability: &formats.Applicability{Status: "Applicable", Evidence: []formats.Evidence{{Location: formats.Location{File: "file1", StartLine: 1, StartColumn: 10, EndLine: 2, EndColumn: 11, Snippet: "snippet"}}}}}}, + }, + { + Applicable: "Not Applicable", + FixedVersions: []string{"1.2.2"}, + ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ + SeverityDetails: formats.SeverityDetails{Severity: "Low", SeverityNumValue: 2}, + ImpactedDependencyName: "Dep-2", + }, + Cves: []formats.CveRow{{Id: "CVE-2023-3122", Applicability: &formats.Applicability{Status: "Not Applicable"}}}, + }, + }, + Iacs: []formats.SourceCodeRow{ + { + SeverityDetails: formats.SeverityDetails{ + Severity: "High", + SeverityNumValue: 13, + }, + Finding: "Missing auto upgrade was detected", + Location: formats.Location{ + File: "file1", + StartLine: 1, + StartColumn: 10, + EndLine: 2, + EndColumn: 11, + Snippet: "aws-violation", + }, + }, + }, + Secrets: []formats.SourceCodeRow{ + { + SeverityDetails: formats.SeverityDetails{ + Severity: "High", + SeverityNumValue: 13, + }, + Finding: "Secret", + Location: formats.Location{ + File: "index.js", + StartLine: 5, + StartColumn: 6, + EndLine: 7, + EndColumn: 8, + Snippet: "access token exposed", + }, + }, + }, + Sast: []formats.SourceCodeRow{ + { + SeverityDetails: formats.SeverityDetails{ + Severity: "High", + SeverityNumValue: 13, + }, + Finding: "XSS Vulnerability", + Location: formats.Location{ + File: "file1", + StartLine: 1, + StartColumn: 10, + EndLine: 2, + EndColumn: 11, + Snippet: "snippet", + }, + }, + }, + Licenses: []formats.LicenseRow{ + { + LicenseKey: "Apache-2.0", + ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ + ImpactedDependencyName: "Dep-1", + }, + }, + }, + } + issuesRows, err := getAllIssues(auditResults, allowedLicenses) - assert.NoError(t, err) - assert.Len(t, issuesRows.Licenses, 1) - assert.Len(t, issuesRows.Vulnerabilities, 2) - assert.Len(t, issuesRows.Secrets, 1) - assert.Equal(t, auditResults.ExtendedScanResults.XrayResults[0].Licenses[0].Key, "Apache-2.0") - assert.Equal(t, "Dep-1", issuesRows.Licenses[0].ImpactedDependencyName) - vuln1 := auditResults.ExtendedScanResults.XrayResults[0].Vulnerabilities[0] - assert.Equal(t, vuln1.Cves[0].Id, issuesRows.Vulnerabilities[0].Cves[0].Id) - assert.Equal(t, vuln1.Severity, issuesRows.Vulnerabilities[0].Severity) - assert.Equal(t, vuln1.Components["Dep-1"].FixedVersions[0], issuesRows.Vulnerabilities[0].FixedVersions[0]) - vuln2 := auditResults.ExtendedScanResults.XrayResults[0].Vulnerabilities[1] - assert.Equal(t, vuln2.Cves[0].Id, issuesRows.Vulnerabilities[1].Cves[0].Id) - assert.Equal(t, vuln2.Severity, issuesRows.Vulnerabilities[1].Severity) - assert.Equal(t, vuln2.Components["Dep-2"].FixedVersions[0], issuesRows.Vulnerabilities[1].FixedVersions[0]) - assert.Equal(t, auditResults.ExtendedScanResults.XrayResults[0].Licenses[0].Key, issuesRows.Licenses[0].LicenseKey) - assert.Equal(t, "Dep-1", issuesRows.Licenses[0].ImpactedDependencyName) - assert.Equal(t, xrayutils.GetResultSeverity(auditResults.ExtendedScanResults.SecretsScanResults[0].Results[0]), issuesRows.Secrets[0].Severity) - assert.Equal(t, xrayutils.GetLocationFileName(auditResults.ExtendedScanResults.SecretsScanResults[0].Results[0].Locations[0]), issuesRows.Secrets[0].File) - assert.Equal(t, *auditResults.ExtendedScanResults.SecretsScanResults[0].Results[0].Locations[0].PhysicalLocation.Region.Snippet.Text, issuesRows.Secrets[0].Snippet) + + if assert.NoError(t, err) { + assert.ElementsMatch(t, expectedOutput.Vulnerabilities, issuesRows.Vulnerabilities) + assert.ElementsMatch(t, expectedOutput.Iacs, issuesRows.Iacs) + assert.ElementsMatch(t, expectedOutput.Secrets, issuesRows.Secrets) + assert.ElementsMatch(t, expectedOutput.Sast, issuesRows.Sast) + assert.ElementsMatch(t, expectedOutput.Licenses, issuesRows.Licenses) + } } func TestCreatePullRequestComment(t *testing.T) { @@ -893,12 +959,9 @@ func TestCreateNewIacRows(t *testing.T) { { name: "No vulnerabilities in source IaC results", targetIacResults: []*sarif.Result{ - sarif.NewRuleResult("").WithLevel(xrayutils.ConvertToSarifLevel("high")).WithLocations([]*sarif.Location{ - sarif.NewLocationWithPhysicalLocation(sarif.NewPhysicalLocation(). - WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file1")). - WithRegion(sarif.NewRegion().WithStartLine(1).WithStartColumn(10).WithSnippet(sarif.NewArtifactContent().WithText("aws violation"))), - ), - }), + xrayutils.CreateResultWithLocations("Missing auto upgrade was detected", "rule", xrayutils.ConvertToSarifLevel("high"), + xrayutils.CreateLocation("file1", 1, 10, 2, 11, "aws-violation"), + ), }, sourceIacResults: []*sarif.Result{}, expectedAddedIacVulnerabilities: []formats.SourceCodeRow{}, @@ -907,12 +970,9 @@ func TestCreateNewIacRows(t *testing.T) { name: "No vulnerabilities in target IaC results", targetIacResults: []*sarif.Result{}, sourceIacResults: []*sarif.Result{ - sarif.NewRuleResult("").WithLevel(xrayutils.ConvertToSarifLevel("high")).WithLocations([]*sarif.Location{ - sarif.NewLocationWithPhysicalLocation(sarif.NewPhysicalLocation(). - WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file1")). - WithRegion(sarif.NewRegion().WithStartLine(1).WithStartColumn(10).WithSnippet(sarif.NewArtifactContent().WithText("aws violation"))), - ), - }), + xrayutils.CreateResultWithLocations("Missing auto upgrade was detected", "rule", xrayutils.ConvertToSarifLevel("high"), + xrayutils.CreateLocation("file1", 1, 10, 2, 11, "aws-violation"), + ), }, expectedAddedIacVulnerabilities: []formats.SourceCodeRow{ { @@ -920,11 +980,14 @@ func TestCreateNewIacRows(t *testing.T) { Severity: "High", SeverityNumValue: 13, }, + Finding: "Missing auto upgrade was detected", Location: formats.Location{ File: "file1", StartLine: 1, StartColumn: 10, - Snippet: "aws violation", + EndLine: 2, + EndColumn: 11, + Snippet: "aws-violation", }, }, }, @@ -932,20 +995,14 @@ func TestCreateNewIacRows(t *testing.T) { { name: "Some new vulnerabilities in source IaC results", targetIacResults: []*sarif.Result{ - sarif.NewRuleResult("").WithLevel(xrayutils.ConvertToSarifLevel("high")).WithLocations([]*sarif.Location{ - sarif.NewLocationWithPhysicalLocation(sarif.NewPhysicalLocation(). - WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file1")). - WithRegion(sarif.NewRegion().WithStartLine(1).WithStartColumn(10).WithSnippet(sarif.NewArtifactContent().WithText("aws violation"))), - ), - }), + xrayutils.CreateResultWithLocations("Missing auto upgrade was detected", "rule", xrayutils.ConvertToSarifLevel("high"), + xrayutils.CreateLocation("file1", 1, 10, 2, 11, "aws-violation"), + ), }, sourceIacResults: []*sarif.Result{ - sarif.NewRuleResult("").WithLevel(xrayutils.ConvertToSarifLevel("medium")).WithLocations([]*sarif.Location{ - sarif.NewLocationWithPhysicalLocation(sarif.NewPhysicalLocation(). - WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file2")). - WithRegion(sarif.NewRegion().WithStartLine(2).WithStartColumn(5).WithSnippet(sarif.NewArtifactContent().WithText("gcp violation"))), - ), - }), + xrayutils.CreateResultWithLocations("enable_private_endpoint=false was detected", "rule", xrayutils.ConvertToSarifLevel("medium"), + xrayutils.CreateLocation("file2", 2, 5, 3, 6, "gcp-violation"), + ), }, expectedAddedIacVulnerabilities: []formats.SourceCodeRow{ { @@ -953,11 +1010,14 @@ func TestCreateNewIacRows(t *testing.T) { Severity: "Medium", SeverityNumValue: 11, }, + Finding: "enable_private_endpoint=false was detected", Location: formats.Location{ File: "file2", StartLine: 2, StartColumn: 5, - Snippet: "gcp violation", + EndLine: 3, + EndColumn: 6, + Snippet: "gcp-violation", }, }, }, @@ -966,8 +1026,8 @@ func TestCreateNewIacRows(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - targetIacRows := xrayutils.PrepareIacs([]*sarif.Run{sarif.NewRunWithInformationURI("", "").WithResults(tc.targetIacResults)}) - sourceIacRows := xrayutils.PrepareIacs([]*sarif.Run{sarif.NewRunWithInformationURI("", "").WithResults(tc.sourceIacResults)}) + targetIacRows := xrayutils.PrepareIacs([]*sarif.Run{xrayutils.CreateRunWithDummyResults(tc.targetIacResults...)}) + sourceIacRows := xrayutils.PrepareIacs([]*sarif.Run{xrayutils.CreateRunWithDummyResults(tc.sourceIacResults...)}) addedIacVulnerabilities := createNewSourceCodeRows(targetIacRows, sourceIacRows) assert.ElementsMatch(t, tc.expectedAddedIacVulnerabilities, addedIacVulnerabilities) }) @@ -984,12 +1044,9 @@ func TestCreateNewSecretRows(t *testing.T) { { name: "No vulnerabilities in source secrets results", targetSecretsResults: []*sarif.Result{ - sarif.NewRuleResult("").WithMessage(sarif.NewTextMessage("Secret")).WithLevel(xrayutils.ConvertToSarifLevel("high")).WithLocations([]*sarif.Location{ - sarif.NewLocationWithPhysicalLocation(sarif.NewPhysicalLocation(). - WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file1")). - WithRegion(sarif.NewRegion().WithStartLine(1).WithStartColumn(10).WithSnippet(sarif.NewArtifactContent().WithText("Sensitive information"))), - ), - }), + xrayutils.CreateResultWithLocations("Secret", "rule", xrayutils.ConvertToSarifLevel("high"), + xrayutils.CreateLocation("file1", 1, 10, 2, 11, "Sensitive information"), + ), }, sourceSecretsResults: []*sarif.Result{}, expectedAddedSecretsVulnerabilities: []formats.SourceCodeRow{}, @@ -998,12 +1055,9 @@ func TestCreateNewSecretRows(t *testing.T) { name: "No vulnerabilities in target secrets results", targetSecretsResults: []*sarif.Result{}, sourceSecretsResults: []*sarif.Result{ - sarif.NewRuleResult("").WithMessage(sarif.NewTextMessage("Secret")).WithLevel(xrayutils.ConvertToSarifLevel("high")).WithLocations([]*sarif.Location{ - sarif.NewLocationWithPhysicalLocation(sarif.NewPhysicalLocation(). - WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file1")). - WithRegion(sarif.NewRegion().WithStartLine(1).WithStartColumn(10).WithSnippet(sarif.NewArtifactContent().WithText("Sensitive information"))), - ), - }), + xrayutils.CreateResultWithLocations("Secret", "rule", xrayutils.ConvertToSarifLevel("high"), + xrayutils.CreateLocation("file1", 1, 10, 2, 11, "Sensitive information"), + ), }, expectedAddedSecretsVulnerabilities: []formats.SourceCodeRow{ { @@ -1016,6 +1070,8 @@ func TestCreateNewSecretRows(t *testing.T) { File: "file1", StartLine: 1, StartColumn: 10, + EndLine: 2, + EndColumn: 11, Snippet: "Sensitive information", }, }, @@ -1024,20 +1080,14 @@ func TestCreateNewSecretRows(t *testing.T) { { name: "Some new vulnerabilities in source secrets results", targetSecretsResults: []*sarif.Result{ - sarif.NewRuleResult("").WithMessage(sarif.NewTextMessage("Secret")).WithLevel(xrayutils.ConvertToSarifLevel("high")).WithLocations([]*sarif.Location{ - sarif.NewLocationWithPhysicalLocation(sarif.NewPhysicalLocation(). - WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file1")). - WithRegion(sarif.NewRegion().WithStartLine(1).WithStartColumn(10).WithSnippet(sarif.NewArtifactContent().WithText("Sensitive information"))), - ), - }), + xrayutils.CreateResultWithLocations("Secret", "rule", xrayutils.ConvertToSarifLevel("high"), + xrayutils.CreateLocation("file1", 1, 10, 2, 11, "Sensitive information"), + ), }, sourceSecretsResults: []*sarif.Result{ - sarif.NewRuleResult("").WithMessage(sarif.NewTextMessage("Secret")).WithLevel(xrayutils.ConvertToSarifLevel("medium")).WithLocations([]*sarif.Location{ - sarif.NewLocationWithPhysicalLocation(sarif.NewPhysicalLocation(). - WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file2")). - WithRegion(sarif.NewRegion().WithStartLine(2).WithStartColumn(5).WithSnippet(sarif.NewArtifactContent().WithText("Confidential data"))), - ), - }), + xrayutils.CreateResultWithLocations("Secret", "rule", xrayutils.ConvertToSarifLevel("medium"), + xrayutils.CreateLocation("file2", 2, 5, 3, 6, "Confidential data"), + ), }, expectedAddedSecretsVulnerabilities: []formats.SourceCodeRow{ { @@ -1050,6 +1100,8 @@ func TestCreateNewSecretRows(t *testing.T) { File: "file2", StartLine: 2, StartColumn: 5, + EndLine: 3, + EndColumn: 6, Snippet: "Confidential data", }, }, @@ -1059,14 +1111,99 @@ func TestCreateNewSecretRows(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - targetSecretsRows := xrayutils.PrepareSecrets([]*sarif.Run{sarif.NewRunWithInformationURI("", "").WithResults(tc.targetSecretsResults)}) - sourceSecretsRows := xrayutils.PrepareSecrets([]*sarif.Run{sarif.NewRunWithInformationURI("", "").WithResults(tc.sourceSecretsResults)}) + targetSecretsRows := xrayutils.PrepareSecrets([]*sarif.Run{xrayutils.CreateRunWithDummyResults(tc.targetSecretsResults...)}) + sourceSecretsRows := xrayutils.PrepareSecrets([]*sarif.Run{xrayutils.CreateRunWithDummyResults(tc.sourceSecretsResults...)}) addedSecretsVulnerabilities := createNewSourceCodeRows(targetSecretsRows, sourceSecretsRows) assert.ElementsMatch(t, tc.expectedAddedSecretsVulnerabilities, addedSecretsVulnerabilities) }) } } +func TestCreateNewSastRows(t *testing.T) { + testCases := []struct { + name string + targetSastResults []*sarif.Result + sourceSastResults []*sarif.Result + expectedAddedSastVulnerabilities []formats.SourceCodeRow + }{ + { + name: "No vulnerabilities in source Sast results", + targetSastResults: []*sarif.Result{ + xrayutils.CreateResultWithLocations("XSS Vulnerability", "rule", xrayutils.ConvertToSarifLevel("high"), + xrayutils.CreateLocation("file1", 1, 10, 2, 11, "snippet"), + ), + }, + sourceSastResults: []*sarif.Result{}, + expectedAddedSastVulnerabilities: []formats.SourceCodeRow{}, + }, + { + name: "No vulnerabilities in target Sast results", + targetSastResults: []*sarif.Result{}, + sourceSastResults: []*sarif.Result{ + xrayutils.CreateResultWithLocations("XSS Vulnerability", "rule", xrayutils.ConvertToSarifLevel("high"), + xrayutils.CreateLocation("file1", 1, 10, 2, 11, "snippet"), + ), + }, + expectedAddedSastVulnerabilities: []formats.SourceCodeRow{ + { + SeverityDetails: formats.SeverityDetails{ + Severity: "High", + SeverityNumValue: 13, + }, + Finding: "XSS Vulnerability", + Location: formats.Location{ + File: "file1", + StartLine: 1, + StartColumn: 10, + EndLine: 2, + EndColumn: 11, + Snippet: "snippet", + }, + }, + }, + }, + { + name: "Some new vulnerabilities in source Sast results", + targetSastResults: []*sarif.Result{ + xrayutils.CreateResultWithLocations("XSS Vulnerability", "rule", xrayutils.ConvertToSarifLevel("high"), + xrayutils.CreateLocation("file1", 1, 10, 2, 11, "snippet"), + ), + }, + sourceSastResults: []*sarif.Result{ + xrayutils.CreateResultWithLocations("Stack Trace Exposure", "rule", xrayutils.ConvertToSarifLevel("medium"), + xrayutils.CreateLocation("file2", 2, 5, 3, 6, "other-snippet"), + ), + }, + expectedAddedSastVulnerabilities: []formats.SourceCodeRow{ + { + SeverityDetails: formats.SeverityDetails{ + Severity: "Medium", + SeverityNumValue: 11, + }, + Finding: "Stack Trace Exposure", + Location: formats.Location{ + File: "file2", + StartLine: 2, + StartColumn: 5, + EndLine: 3, + EndColumn: 6, + Snippet: "other-snippet", + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + targetSastRows := xrayutils.PrepareSast([]*sarif.Run{xrayutils.CreateRunWithDummyResults(tc.targetSastResults...)}) + sourceSastRows := xrayutils.PrepareSast([]*sarif.Run{xrayutils.CreateRunWithDummyResults(tc.sourceSastResults...)}) + addedSastVulnerabilities := createNewSourceCodeRows(targetSastRows, sourceSastRows) + assert.ElementsMatch(t, tc.expectedAddedSastVulnerabilities, addedSastVulnerabilities) + }) + } +} + func TestDeletePreviousPullRequestMessages(t *testing.T) { repository := &utils.Repository{ Params: utils.Params{ diff --git a/utils/outputwriter/outputwriter.go b/utils/outputwriter/outputwriter.go index cb9eb1955..b6cc6f421 100644 --- a/utils/outputwriter/outputwriter.go +++ b/utils/outputwriter/outputwriter.go @@ -12,12 +12,14 @@ import ( const ( FrogbotTitlePrefix = "[šŸø Frogbot]" CommentGeneratedByFrogbot = "[šŸø JFrog Frogbot](https://github.com/jfrog/frogbot#readme)" + ReviewCommentId = "FrogbotReviewComment" vulnerabilitiesTableHeader = "\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS | CVES |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | :---------------------------------: |" vulnerabilitiesTableHeaderWithContextualAnalysis = "| SEVERITY | CONTEXTUAL ANALYSIS | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS | CVES |\n| :---------------------: | :----------------------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | :---------------------------------: |" iacTableHeader = "\n| SEVERITY | FILE | LINE:COLUMN | FINDING |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: |" vulnerableDependenciesTitle = "## šŸ“¦ Vulnerable Dependencies" summaryTitle = "### āœļø Summary" researchDetailsTitle = "## šŸ”¬ Research Details" + sastTitle = "## šŸŽÆ Static Application Security Testing (SAST) Vulnerability" iacTitle = "## šŸ› ļø Infrastructure as Code" licenseTitle = "## āš–ļø Violated Licenses" contextualAnalysisTitle = "## šŸ“¦šŸ” Contextual Analysis CVE Vulnerability\n" @@ -254,3 +256,11 @@ func getVulnerabilityDescriptionIdentifier(cveRows []formats.CveRow, xrayId stri } return fmt.Sprintf("[ %s ] ", identifier) } + +func GenerateReviewCommentContent(content string, writer OutputWriter) string { + return MarkdownComment(ReviewCommentId) + content + writer.Footer() +} + +func GetFallbackReviewCommentContent(content string, location formats.Location, writer OutputWriter) string { + return MarkdownComment(ReviewCommentId) + GetLocationDescription(location) + content + writer.Footer() +} diff --git a/utils/outputwriter/outputwriter_test.go b/utils/outputwriter/outputwriter_test.go index 2f3ccb8de..db4a06df5 100644 --- a/utils/outputwriter/outputwriter_test.go +++ b/utils/outputwriter/outputwriter_test.go @@ -62,3 +62,118 @@ func testGetLicensesTableContent(t *testing.T, writer OutputWriter) { expected = "\n| License1 | Comp1 1.0 | Dep1 2.0 |\n| License2 | Comp2 2.0 | Dep2 3.0 |" assert.Equal(t, expected, result) } + +func TestMarkAsQuote(t *testing.T) { + testCases := []struct { + input string + expectedOutput string + }{ + { + input: "", + expectedOutput: "``", + }, + { + input: "quote", + expectedOutput: "`quote`", + }, + } + for _, tc := range testCases { + assert.Equal(t, tc.expectedOutput, MarkAsQuote(tc.input)) + } +} + +func TestMarkAsCodeSnippet(t *testing.T) { + testCases := []struct { + input string + expectedOutput string + }{ + { + input: "", + expectedOutput: "```\n\n```", + }, + { + input: "snippet", + expectedOutput: "```\nsnippet\n```", + }, + } + for _, tc := range testCases { + assert.Equal(t, tc.expectedOutput, MarkAsCodeSnippet(tc.input)) + } +} + +func TestGetLocationDescription(t *testing.T) { + testCases := []struct { + input formats.Location + expectedOutput string + }{ + { + input: formats.Location{ + File: "file1", + StartLine: 1, + Snippet: "snippet", + }, + expectedOutput: "\n```\nsnippet\n```\nat `file1` (line 1)\n", + }, + { + input: formats.Location{ + File: "dir/other-dir/file1", + StartLine: 134, + Snippet: "clientTestUtils.ChangeDirAndAssert(t, prevWd)", + }, + expectedOutput: "\n```\nclientTestUtils.ChangeDirAndAssert(t, prevWd)\n```\nat `dir/other-dir/file1` (line 134)\n", + }, + } + for _, tc := range testCases { + assert.Equal(t, tc.expectedOutput, GetLocationDescription(tc.input)) + } +} + +func TestGetJasMarkdownDescription(t *testing.T) { + testCases := []struct { + severity string + finding string + expectedOutput string + }{ + { + severity: "High", + finding: "finding", + expectedOutput: "| Severity | Finding |\n| :--------------: | :---: |\n| High | finding |", + }, + { + severity: "Low", + finding: "finding (other)", + expectedOutput: "| Severity | Finding |\n| :--------------: | :---: |\n| Low | finding (other) |", + }, + } + for _, tc := range testCases { + assert.Equal(t, tc.expectedOutput, GetJasMarkdownDescription(tc.severity, tc.finding)) + } +} + +func TestGetApplicabilityMarkdownDescription(t *testing.T) { + testCases := []struct { + severity string + cve string + impactedDependency string + finding string + expectedOutput string + }{ + { + severity: "High", + cve: "CVE-100-234", + impactedDependency: "dependency:1.0.0", + finding: "applicable finding", + expectedOutput: "| Severity | Impacted Dependency | Finding | CVE |\n| :--------------: | :---: | :---: | :---: |\n| High | dependency:1.0.0 | applicable finding | CVE-100-234 |", + }, + { + severity: "Low", + cve: "CVE-222-233", + impactedDependency: "dependency:3.4.1", + finding: "applicable finding (diff)", + expectedOutput: "| Severity | Impacted Dependency | Finding | CVE |\n| :--------------: | :---: | :---: | :---: |\n| Low | dependency:3.4.1 | applicable finding (diff) | CVE-222-233 |", + }, + } + for _, tc := range testCases { + assert.Equal(t, tc.expectedOutput, GetApplicabilityMarkdownDescription(tc.severity, tc.cve, tc.impactedDependency, tc.finding)) + } +} diff --git a/utils/outputwriter/simplifiedoutput.go b/utils/outputwriter/simplifiedoutput.go index 3acf6145e..fbdeba32d 100644 --- a/utils/outputwriter/simplifiedoutput.go +++ b/utils/outputwriter/simplifiedoutput.go @@ -191,16 +191,16 @@ func (smo *SimplifiedOutput) SastReviewContent(severity, finding, fullDetails st ### Full description %s - ---- -### Code Flows - `, GetJasMarkdownDescription(smo.FormattedSeverity(severity, "Applicable"), finding), fullDetails, )) if len(codeFlows) > 0 { + contentBuilder.WriteString(` +--- +### Code Flows +`) for _, flow := range codeFlows { contentBuilder.WriteString(` diff --git a/utils/outputwriter/simplifiedoutput_test.go b/utils/outputwriter/simplifiedoutput_test.go index 1cfcb65e0..663a5c35d 100644 --- a/utils/outputwriter/simplifiedoutput_test.go +++ b/utils/outputwriter/simplifiedoutput_test.go @@ -375,3 +375,75 @@ func TestSimplifiedOutput_GetLicensesTableContent(t *testing.T) { writer := &SimplifiedOutput{} testGetLicensesTableContent(t, writer) } + +func TestSimplifiedOutput_SastReviewContent(t *testing.T) { + testCases := []struct { + name string + severity string + finding string + fullDetails string + expectedOutput string + codeFlows [][]formats.Location + }{ + { + name: "Sast review comment content", + severity: "Low", + finding: "Stack Trace Exposure", + fullDetails: "\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.", + codeFlows: [][]formats.Location{ + { + { + File: "file2", + StartLine: 1, + StartColumn: 2, + EndLine: 3, + EndColumn: 4, + Snippet: "other-snippet", + }, + { + File: "file", + StartLine: 0, + StartColumn: 0, + EndLine: 0, + EndColumn: 0, + Snippet: "snippet", + }, + }, + { + { + File: "file", + StartLine: 10, + StartColumn: 20, + EndLine: 10, + EndColumn: 30, + Snippet: "a-snippet", + }, + { + File: "file", + StartLine: 0, + StartColumn: 0, + EndLine: 0, + EndColumn: 0, + Snippet: "snippet", + }, + }, + }, + expectedOutput: "\n## šŸŽÆ Static Application Security Testing (SAST) Vulnerability\n\t\n| Severity | Finding |\n| :--------------: | :---: |\n| Low | Stack Trace Exposure |\n\n---\n### Full description\n\n\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.\n\n---\n### Code Flows\n\n\n---\nVulnerable data flow analysis result:\n\nā†˜ļø `other-snippet` (at file2 line 1)\n\nā†˜ļø `snippet` (at file line 0)\n\n\n---\n\n\n\n---\nVulnerable data flow analysis result:\n\nā†˜ļø `a-snippet` (at file line 10)\n\nā†˜ļø `snippet` (at file line 0)\n\n\n---\n\n", + }, + { + name: "No code flows", + severity: "Low", + finding: "Stack Trace Exposure", + fullDetails: "\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.", + expectedOutput: "\n## šŸŽÆ Static Application Security Testing (SAST) Vulnerability\n\t\n| Severity | Finding |\n| :--------------: | :---: |\n| Low | Stack Trace Exposure |\n\n---\n### Full description\n\n\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.\n", + }, + } + + so := &SimplifiedOutput{} + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + output := so.SastReviewContent(tc.severity, tc.finding, tc.fullDetails, tc.codeFlows) + assert.Equal(t, tc.expectedOutput, output) + }) + } +} diff --git a/utils/outputwriter/standardoutput_test.go b/utils/outputwriter/standardoutput_test.go index 291a675b0..5fdb45820 100644 --- a/utils/outputwriter/standardoutput_test.go +++ b/utils/outputwriter/standardoutput_test.go @@ -403,3 +403,137 @@ func TestStandardOutput_GetLicensesTableContent(t *testing.T) { writer := &StandardOutput{} testGetLicensesTableContent(t, writer) } + +func TestStandardOutput_ApplicableCveReviewContent(t *testing.T) { + testCases := []struct { + name string + severity, finding, fullDetails, cve, cveDetails, impactedDependency, remediation string + expectedOutput string + }{ + { + name: "Applicable CVE review comment content", + severity: "Critical", + finding: "The vulnerable function flask.Flask.run is called", + fullDetails: "The scanner checks whether the vulnerable `Development Server` of the `werkzeug` library is used by looking for calls to `werkzeug.serving.run_simple()`.", + cve: "CVE-2022-29361", + cveDetails: "cveDetails", + impactedDependency: "werkzeug:1.0.1", + remediation: "some remediation", + expectedOutput: "\n## šŸ“¦šŸ” Contextual Analysis CVE Vulnerability\n\n\n
\n\n| Severity | Impacted Dependency | Finding | CVE |\n| :--------------: | :---: | :---: | :---: |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableCriticalSeverity.png)
Critical | werkzeug:1.0.1 | The vulnerable function flask.Flask.run is called | CVE-2022-29361 |\n\n
\n\n
\n Description \n
\n\nThe scanner checks whether the vulnerable `Development Server` of the `werkzeug` library is used by looking for calls to `werkzeug.serving.run_simple()`.\n\n
\n\n
\n CVE details \n
\n\ncveDetails\n\n
\n\n\n
\n Remediation \n
\n\nsome remediation\n\n
\n\n", + }, + { + name: "No remediation", + severity: "Critical", + finding: "The vulnerable function flask.Flask.run is called", + fullDetails: "The scanner checks whether the vulnerable `Development Server` of the `werkzeug` library is used by looking for calls to `werkzeug.serving.run_simple()`.", + cve: "CVE-2022-29361", + cveDetails: "cveDetails", + impactedDependency: "werkzeug:1.0.1", + expectedOutput: "\n## šŸ“¦šŸ” Contextual Analysis CVE Vulnerability\n\n\n
\n\n| Severity | Impacted Dependency | Finding | CVE |\n| :--------------: | :---: | :---: | :---: |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableCriticalSeverity.png)
Critical | werkzeug:1.0.1 | The vulnerable function flask.Flask.run is called | CVE-2022-29361 |\n\n
\n\n
\n Description \n
\n\nThe scanner checks whether the vulnerable `Development Server` of the `werkzeug` library is used by looking for calls to `werkzeug.serving.run_simple()`.\n\n
\n\n
\n CVE details \n
\n\ncveDetails\n\n
\n\n", + }, + } + + so := &StandardOutput{} + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + output := so.ApplicableCveReviewContent(tc.severity, tc.finding, tc.fullDetails, tc.cve, tc.cveDetails, tc.impactedDependency, tc.remediation) + assert.Equal(t, tc.expectedOutput, output) + }) + } +} + +func TestStandardOutput_IacReviewContent(t *testing.T) { + testCases := []struct { + name string + severity, finding, fullDetails string + expectedOutput string + }{ + { + name: "Iac review comment content", + severity: "Medium", + finding: "Missing auto upgrade was detected", + fullDetails: "Resource `google_container_node_pool` should have `management.auto_upgrade=true`\n\nVulnerable example - \n```\nresource \"google_container_node_pool\" \"vulnerable_example\" {\n management {\n auto_upgrade = false\n }\n}\n```\n", + expectedOutput: "\n## šŸ› ļø Infrastructure as Code\n\n
\n\n| Severity | Finding |\n| :--------------: | :---: |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableMediumSeverity.png)
Medium | Missing auto upgrade was detected |\n\n
\n\n
\n Full description \n
\n\nResource `google_container_node_pool` should have `management.auto_upgrade=true`\n\nVulnerable example - \n```\nresource \"google_container_node_pool\" \"vulnerable_example\" {\n management {\n auto_upgrade = false\n }\n}\n```\n\n\n
\n\n", + }, + } + + so := &StandardOutput{} + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + output := so.IacReviewContent(tc.severity, tc.finding, tc.fullDetails) + assert.Equal(t, tc.expectedOutput, output) + }) + } +} + +func TestStandardOutput_SastReviewContent(t *testing.T) { + testCases := []struct { + name string + severity string + finding string + fullDetails string + expectedOutput string + codeFlows [][]formats.Location + }{ + { + name: "Sast review comment content", + severity: "Low", + finding: "Stack Trace Exposure", + fullDetails: "\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.", + codeFlows: [][]formats.Location{ + { + { + File: "file2", + StartLine: 1, + StartColumn: 2, + EndLine: 3, + EndColumn: 4, + Snippet: "other-snippet", + }, + { + File: "file", + StartLine: 0, + StartColumn: 0, + EndLine: 0, + EndColumn: 0, + Snippet: "snippet", + }, + }, + { + { + File: "file", + StartLine: 10, + StartColumn: 20, + EndLine: 10, + EndColumn: 30, + Snippet: "a-snippet", + }, + { + File: "file", + StartLine: 0, + StartColumn: 0, + EndLine: 0, + EndColumn: 0, + Snippet: "snippet", + }, + }, + }, + expectedOutput: "\n## šŸŽÆ Static Application Security Testing (SAST) Vulnerability \n\t\n
\n\n| Severity | Finding |\n| :--------------: | :---: |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableLowSeverity.png)
Low | Stack Trace Exposure |\n\n
\n\n
\n Full description \n
\n\n\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.\n\n
\n\n\n\n
\nCode Flows \n\n\n\n
\nVulnerable data flow analysis result \n
\n\nā†˜ļø `other-snippet` (at file2 line 1)\n\nā†˜ļø `snippet` (at file line 0)\n\n\n
\n\n\n\n
\nVulnerable data flow analysis result \n
\n\nā†˜ļø `a-snippet` (at file line 10)\n\nā†˜ļø `snippet` (at file line 0)\n\n\n
\n\n\n\n
\n\n", + }, + { + name: "No code flows", + severity: "Low", + finding: "Stack Trace Exposure", + fullDetails: "\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.", + expectedOutput: "\n## šŸŽÆ Static Application Security Testing (SAST) Vulnerability \n\t\n
\n\n| Severity | Finding |\n| :--------------: | :---: |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableLowSeverity.png)
Low | Stack Trace Exposure |\n\n
\n\n
\n Full description \n
\n\n\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.\n\n
\n\n", + }, + } + + so := &StandardOutput{} + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + output := so.SastReviewContent(tc.severity, tc.finding, tc.fullDetails, tc.codeFlows) + assert.Equal(t, tc.expectedOutput, output) + }) + } +} diff --git a/utils/reviewcomment.go b/utils/reviewcomment.go index 76c3df852..8eb8d22ea 100644 --- a/utils/reviewcomment.go +++ b/utils/reviewcomment.go @@ -24,8 +24,6 @@ const ( ApplicableComment ReviewCommentType = "Applicable" IacComment ReviewCommentType = "Iac" SastComment ReviewCommentType = "Sast" - - CommentId = "FrogbotReviewComment" ) func AddReviewComments(repo *Repository, pullRequestID int, client vcsclient.VcsClient, issues *IssuesCollection) (err error) { @@ -93,7 +91,7 @@ func deleteOldFallbackComments(repo *Repository, pullRequestID int, client vcscl func getFrogbotReviewComments(existingComments []vcsclient.CommentInfo) (reviewComments []vcsclient.CommentInfo) { for _, comment := range existingComments { - if strings.Contains(comment.Content, CommentId) { + if strings.Contains(comment.Content, outputwriter.ReviewCommentId) { log.Debug("Deleting comment id:", comment.ID) reviewComments = append(reviewComments, comment) } @@ -102,8 +100,7 @@ func getFrogbotReviewComments(existingComments []vcsclient.CommentInfo) (reviewC } func getRegularCommentContent(comment ReviewComment) string { - content := outputwriter.MarkdownComment(CommentId) - return content + outputwriter.GetLocationDescription(comment.Location) + comment.CommentInfo.Content + return outputwriter.MarkdownComment(outputwriter.ReviewCommentId) + outputwriter.GetLocationDescription(comment.Location) + comment.CommentInfo.Content } func getNewReviewComments(repo *Repository, issues *IssuesCollection) (commentsToAdd []ReviewComment) { @@ -119,11 +116,11 @@ func getNewReviewComments(repo *Repository, issues *IssuesCollection) (commentsT } } for _, iac := range issues.Iacs { - commentsToAdd = append(commentsToAdd, generateReviewComment(IacComment, iac.Location, generateReviewCommentContent(IacComment, iac, writer))) + commentsToAdd = append(commentsToAdd, generateReviewComment(IacComment, iac.Location, generateSourceCodeReviewContent(IacComment, iac, writer))) } for _, sast := range issues.Sast { - commentsToAdd = append(commentsToAdd, generateReviewComment(SastComment, sast.Location, generateReviewCommentContent(SastComment, sast, writer))) + commentsToAdd = append(commentsToAdd, generateReviewComment(SastComment, sast.Location, generateSourceCodeReviewContent(SastComment, sast, writer))) } return } @@ -142,13 +139,12 @@ func generateReviewComment(commentType ReviewCommentType, location formats.Locat } -func generateApplicabilityReviewContent(issue formats.Evidence, relatedCve formats.CveRow, relatedVulnerability formats.VulnerabilityOrViolationRow, writer outputwriter.OutputWriter) (content string) { - content = outputwriter.MarkdownComment(CommentId) +func generateApplicabilityReviewContent(issue formats.Evidence, relatedCve formats.CveRow, relatedVulnerability formats.VulnerabilityOrViolationRow, writer outputwriter.OutputWriter) string { remediation := "" if relatedVulnerability.JfrogResearchInformation != nil { remediation = relatedVulnerability.JfrogResearchInformation.Remediation } - content += writer.ApplicableCveReviewContent( + return outputwriter.GenerateReviewCommentContent(writer.ApplicableCveReviewContent( relatedVulnerability.Severity, issue.Reason, relatedCve.Applicability.ScannerDescription, @@ -156,30 +152,25 @@ func generateApplicabilityReviewContent(issue formats.Evidence, relatedCve forma relatedVulnerability.Summary, fmt.Sprintf("%s:%s", relatedVulnerability.ImpactedDependencyName, relatedVulnerability.ImpactedDependencyVersion), remediation, - ) - content += writer.Footer() - return + ), writer) } -func generateReviewCommentContent(commentType ReviewCommentType, issue formats.SourceCodeRow, writer outputwriter.OutputWriter) (content string) { - content = outputwriter.MarkdownComment(CommentId) +func generateSourceCodeReviewContent(commentType ReviewCommentType, issue formats.SourceCodeRow, writer outputwriter.OutputWriter) (content string) { switch commentType { case IacComment: - content += writer.IacReviewContent( + return outputwriter.GenerateReviewCommentContent(writer.IacReviewContent( issue.Severity, issue.Finding, issue.ScannerDescription, - ) + ), writer) case SastComment: - content += writer.SastReviewContent( + return outputwriter.GenerateReviewCommentContent(writer.SastReviewContent( issue.Severity, issue.Finding, issue.ScannerDescription, issue.CodeFlow, - ) + ), writer) } - - content += writer.Footer() return } diff --git a/utils/reviewcomment_test.go b/utils/reviewcomment_test.go new file mode 100644 index 000000000..b92548edd --- /dev/null +++ b/utils/reviewcomment_test.go @@ -0,0 +1,259 @@ +package utils + +import ( + "testing" + + "github.com/jfrog/frogbot/utils/outputwriter" + "github.com/jfrog/froggit-go/vcsclient" + "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" + "github.com/jfrog/jfrog-cli-core/v2/xray/formats" + "github.com/stretchr/testify/assert" +) + +func TestGetFrogbotReviewComments(t *testing.T) { + testCases := []struct { + name string + existingComments []vcsclient.CommentInfo + expectedOutput []vcsclient.CommentInfo + }{ + { + name: "No frogbot comments", + existingComments: []vcsclient.CommentInfo{ + { + Content: outputwriter.FrogbotTitlePrefix, + }, + { + Content: "some comment text" + outputwriter.MarkdownComment("with hidden comment"), + }, + { + Content: outputwriter.CommentGeneratedByFrogbot, + }, + }, + expectedOutput: []vcsclient.CommentInfo{}, + }, + { + name: "With frogbot comments", + existingComments: []vcsclient.CommentInfo{ + { + Content: outputwriter.FrogbotTitlePrefix, + }, + { + Content: outputwriter.MarkdownComment(outputwriter.ReviewCommentId) + "A Frogbot review comment", + }, + { + Content: "some comment text" + outputwriter.MarkdownComment("with hidden comment"), + }, + { + Content: outputwriter.ReviewCommentId, + }, + { + Content: outputwriter.CommentGeneratedByFrogbot, + }, + }, + expectedOutput: []vcsclient.CommentInfo{ + { + Content: outputwriter.MarkdownComment(outputwriter.ReviewCommentId) + "A Frogbot review comment", + }, + { + Content: outputwriter.ReviewCommentId, + }, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + output := getFrogbotReviewComments(tc.existingComments) + assert.ElementsMatch(t, tc.expectedOutput, output) + }) + } +} + +func TestGetNewReviewComments(t *testing.T) { + repo := &Repository{OutputWriter: &outputwriter.StandardOutput{}} + testCases := []struct { + name string + issues *IssuesCollection + expectedOutput []ReviewComment + }{ + { + name: "No issues for review comments", + issues: &IssuesCollection{ + Vulnerabilities: []formats.VulnerabilityOrViolationRow{ + { + Summary: "summary-2", + Applicable: "Applicable", + IssueId: "XRAY-2", + ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ + SeverityDetails: formats.SeverityDetails{Severity: "low"}, + ImpactedDependencyName: "component-C", + }, + Cves: []formats.CveRow{{Id: "CVE-2023-4321"}}, + Technology: coreutils.Npm, + }, + }, + Secrets: []formats.SourceCodeRow{ + { + SeverityDetails: formats.SeverityDetails{ + Severity: "High", + SeverityNumValue: 13, + }, + Finding: "Secret", + Location: formats.Location{ + File: "index.js", + StartLine: 5, + StartColumn: 6, + EndLine: 7, + EndColumn: 8, + Snippet: "access token exposed", + }, + }, + }, + }, + expectedOutput: []ReviewComment{}, + }, + { + name: "With issues for review comments", + issues: &IssuesCollection{ + Vulnerabilities: []formats.VulnerabilityOrViolationRow{ + { + Summary: "summary-2", + Applicable: "Applicable", + IssueId: "XRAY-2", + ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ + SeverityDetails: formats.SeverityDetails{Severity: "Low"}, + ImpactedDependencyName: "component-C", + }, + Cves: []formats.CveRow{{Id: "CVE-2023-4321", Applicability: &formats.Applicability{Status: "Applicable", Evidence: []formats.Evidence{{Location: formats.Location{File: "file1", StartLine: 1, StartColumn: 10, EndLine: 2, EndColumn: 11, Snippet: "snippet"}}}}}}, + Technology: coreutils.Npm, + }, + }, + Iacs: []formats.SourceCodeRow{ + { + SeverityDetails: formats.SeverityDetails{ + Severity: "High", + SeverityNumValue: 13, + }, + Finding: "Missing auto upgrade was detected", + Location: formats.Location{ + File: "file1", + StartLine: 1, + StartColumn: 10, + EndLine: 2, + EndColumn: 11, + Snippet: "aws-violation", + }, + }, + }, + Sast: []formats.SourceCodeRow{ + { + SeverityDetails: formats.SeverityDetails{ + Severity: "High", + SeverityNumValue: 13, + }, + Finding: "XSS Vulnerability", + Location: formats.Location{ + File: "file1", + StartLine: 1, + StartColumn: 10, + EndLine: 2, + EndColumn: 11, + Snippet: "snippet", + }, + }, + }, + }, + expectedOutput: []ReviewComment{ + { + Location: formats.Location{ + File: "file1", + StartLine: 1, + StartColumn: 10, + EndLine: 2, + EndColumn: 11, + Snippet: "snippet", + }, + Type: ApplicableComment, + CommentInfo: vcsclient.PullRequestComment{ + CommentInfo: vcsclient.CommentInfo{ + Content: outputwriter.GenerateReviewCommentContent(repo.ApplicableCveReviewContent("Low", "", "", "CVE-2023-4321", "summary-2", "component-C:", ""), repo.OutputWriter), + }, + PullRequestDiff: vcsclient.PullRequestDiff{ + OriginalFilePath: "file1", + OriginalStartLine: 1, + OriginalStartColumn: 10, + OriginalEndLine: 2, + OriginalEndColumn: 11, + NewFilePath: "file1", + NewStartLine: 1, + NewStartColumn: 10, + NewEndLine: 2, + NewEndColumn: 11, + }, + }, + }, + { + Location: formats.Location{ + File: "file1", + StartLine: 1, + StartColumn: 10, + EndLine: 2, + EndColumn: 11, + Snippet: "aws-violation", + }, + Type: IacComment, + CommentInfo: vcsclient.PullRequestComment{ + CommentInfo: vcsclient.CommentInfo{ + Content: outputwriter.GenerateReviewCommentContent(repo.IacReviewContent("High", "Missing auto upgrade was detected", ""), repo.OutputWriter), + }, + PullRequestDiff: vcsclient.PullRequestDiff{ + OriginalFilePath: "file1", + OriginalStartLine: 1, + OriginalStartColumn: 10, + OriginalEndLine: 2, + OriginalEndColumn: 11, + NewFilePath: "file1", + NewStartLine: 1, + NewStartColumn: 10, + NewEndLine: 2, + NewEndColumn: 11, + }, + }, + }, + { + Location: formats.Location{ + File: "file1", + StartLine: 1, + StartColumn: 10, + EndLine: 2, + EndColumn: 11, + Snippet: "snippet", + }, + Type: SastComment, + CommentInfo: vcsclient.PullRequestComment{ + CommentInfo: vcsclient.CommentInfo{ + Content: outputwriter.GenerateReviewCommentContent(repo.SastReviewContent("High", "XSS Vulnerability", "", [][]formats.Location{}), repo.OutputWriter), + }, + PullRequestDiff: vcsclient.PullRequestDiff{ + OriginalFilePath: "file1", + OriginalStartLine: 1, + OriginalStartColumn: 10, + OriginalEndLine: 2, + OriginalEndColumn: 11, + NewFilePath: "file1", + NewStartLine: 1, + NewStartColumn: 10, + NewEndLine: 2, + NewEndColumn: 11, + }, + }, + }, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + output := getNewReviewComments(repo, tc.issues) + assert.ElementsMatch(t, tc.expectedOutput, output) + }) + } +} diff --git a/utils/testsutils.go b/utils/testsutils.go index 6b3740caf..2fe2a8822 100644 --- a/utils/testsutils.go +++ b/utils/testsutils.go @@ -6,10 +6,8 @@ import ( goGitConfig "github.com/go-git/go-git/v5/config" "github.com/go-git/go-git/v5/plumbing/object" biutils "github.com/jfrog/build-info-go/utils" - "github.com/jfrog/gofrog/datastructures" "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-client-go/utils/io/fileutils" - "github.com/owenrumney/go-sarif/v2/sarif" "os" "path/filepath" "strings" @@ -107,40 +105,3 @@ func CreateDotGitWithCommit(t *testing.T, wd, port string, repositoriesPath ...s assert.NoError(t, err) } } - -func GetRunWithDummyResults(results ...*sarif.Result) *sarif.Run { - run := sarif.NewRunWithInformationURI("", "") - ids := datastructures.MakeSet[string]() - for _, result := range results { - if !ids.Exists(*result.RuleID) { - run.Tool.Driver.Rules = append(run.Tool.Driver.Rules, sarif.NewRule(*result.RuleID)) - ids.Add(*result.RuleID) - } - } - return run.WithResults(results) -} - -func GetDummyPassingResult(ruleId string) *sarif.Result { - kind := "pass" - return &sarif.Result{ - Kind: &kind, - RuleID: &ruleId, - } -} - -func GetDummyResultWithOneLocation(fileName string, startLine, startCol int, snippet, ruleId string, level string) *sarif.Result { - return &sarif.Result{ - Locations: []*sarif.Location{ - { - PhysicalLocation: &sarif.PhysicalLocation{ - ArtifactLocation: &sarif.ArtifactLocation{URI: &fileName}, - Region: &sarif.Region{ - StartLine: &startLine, - StartColumn: &startCol, - Snippet: &sarif.ArtifactContent{Text: &snippet}}}, - }, - }, - Level: &level, - RuleID: &ruleId, - } -} diff --git a/utils/utils_test.go b/utils/utils_test.go index 9146d6476..bb86b0c97 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -9,6 +9,8 @@ import ( "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-cli-core/v2/xray/formats" + "github.com/jfrog/jfrog-cli-core/v2/xray/utils" + "github.com/owenrumney/go-sarif/v2/sarif" "github.com/stretchr/testify/assert" ) @@ -374,3 +376,47 @@ func TestTechArrayToString(t *testing.T) { }) } } + +func TestPrepareRunsForGithubReport(t *testing.T) { + testCases := []struct { + run *sarif.Run + expectedOutput *sarif.Run + }{ + { + run: utils.CreateRunWithDummyResults(), + expectedOutput: sarif.NewRunWithInformationURI(sarifToolName, sarifToolUrl), + }, + { + run: sarif.NewRunWithInformationURI("other tool", "other url").WithResults([]*sarif.Result{ + utils.CreateResultWithOneLocation("file://root/dir/file", 0, 0, 0, 0, "snippet", "rule", "level"), + }).WithInvocations([]*sarif.Invocation{sarif.NewInvocation().WithWorkingDirectory(sarif.NewSimpleArtifactLocation("root/dir"))}), + expectedOutput: sarif.NewRunWithInformationURI(sarifToolName, sarifToolUrl).WithResults([]*sarif.Result{ + utils.CreateResultWithOneLocation("file", 0, 0, 0, 0, "snippet", "rule", "level"), + }).WithInvocations([]*sarif.Invocation{sarif.NewInvocation().WithWorkingDirectory(sarif.NewSimpleArtifactLocation("root/dir"))}), + }, + { + run: sarif.NewRunWithInformationURI("other tool", "other url").WithResults([]*sarif.Result{ + utils.CreateResultWithLocations("findings", "rule", "level", + utils.CreateLocation("file://root/dir/file", 0, 0, 0, 0, "snippet"), + utils.CreateLocation("file://root/dir/dir2/file2", 1, 1, 1, 1, "snippet2"), + ).WithCodeFlows([]*sarif.CodeFlow{utils.CreateCodeFlow(utils.CreateThreadFlow( + utils.CreateLocation("file://root/dir/other/file", 2, 2, 2, 2, "other"), + utils.CreateLocation("file://root/dir/file", 0, 0, 0, 0, "snippet"), + ))}), + }).WithInvocations([]*sarif.Invocation{sarif.NewInvocation().WithWorkingDirectory(sarif.NewSimpleArtifactLocation("root/dir"))}), + expectedOutput: sarif.NewRunWithInformationURI(sarifToolName, sarifToolUrl).WithResults([]*sarif.Result{ + utils.CreateResultWithLocations("findings", "rule", "level", + utils.CreateLocation("file", 0, 0, 0, 0, "snippet"), + utils.CreateLocation("dir2/file2", 1, 1, 1, 1, "snippet2"), + ).WithCodeFlows([]*sarif.CodeFlow{utils.CreateCodeFlow(utils.CreateThreadFlow( + utils.CreateLocation("other/file", 2, 2, 2, 2, "other"), + utils.CreateLocation("file", 0, 0, 0, 0, "snippet"), + ))}), + }).WithInvocations([]*sarif.Invocation{sarif.NewInvocation().WithWorkingDirectory(sarif.NewSimpleArtifactLocation("root/dir"))}), + }, + } + for _, tc := range testCases { + prepareRunsForGithubReport([]*sarif.Run{tc.run}) + assert.Equal(t, tc.expectedOutput, tc.run) + } +}