Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor after Results changes in security-cli #724

Open
wants to merge 19 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ require (
gopkg.in/warnings.v0 v0.1.2 // indirect
)

// replace github.com/jfrog/jfrog-cli-security => github.com/attiasas/jfrog-cli-security dev
// attiasas:refactor_output
replace github.com/jfrog/jfrog-cli-security => github.com/attiasas/jfrog-cli-security v0.0.0-20240729092929-ce719c9700fe

// replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 dev

Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,8 @@ github.com/apache/thrift v0.16.0/go.mod h1:PHK3hniurgQaNMZYaCLEqXKsYK8upmhPbmdP2
github.com/apparentlymart/go-textseg/v13 v13.0.0/go.mod h1:ZK2fH7c4NqDTLtiYLvIkEghdlcqw7yxLeM89kiTRPUo=
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
github.com/attiasas/jfrog-cli-security v0.0.0-20240729092929-ce719c9700fe h1:eFMN2aC+1wwuY1JsoQJ5II3kHFvpYewvZfEE+pBnAOk=
github.com/attiasas/jfrog-cli-security v0.0.0-20240729092929-ce719c9700fe/go.mod h1:0m+jdJgsLF2QHl4f/t9JHuJ9E0Oqf9kK24UWsjAtvsE=
github.com/boombuler/barcode v1.0.0/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8=
github.com/boombuler/barcode v1.0.1/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8=
github.com/bradleyjkemp/cupaloy/v2 v2.8.0 h1:any4BmKE+jGIaMpnU8YgH/I2LPiLBufr6oMMlVBbn9M=
Expand Down Expand Up @@ -901,8 +903,6 @@ github.com/jfrog/jfrog-apps-config v1.0.1 h1:mtv6k7g8A8BVhlHGlSveapqf4mJfonwvXYL
github.com/jfrog/jfrog-apps-config v1.0.1/go.mod h1:8AIIr1oY9JuH5dylz2S6f8Ym2MaadPLR6noCBO4C22w=
github.com/jfrog/jfrog-cli-core/v2 v2.54.1 h1:oNIsqUVJ/P17qEcHgj9/c1nfO23stqqj1sHB7ldFNmQ=
github.com/jfrog/jfrog-cli-core/v2 v2.54.1/go.mod h1:o8Ux0XiXWayxBXbtkMd5Vbs2YJZZDNiS9jtN6yQ4Ur8=
github.com/jfrog/jfrog-cli-security v1.6.4 h1:GzKgICdgxgaMQNHHACrURcThjP48lh2p0tWiYzWQGuY=
github.com/jfrog/jfrog-cli-security v1.6.4/go.mod h1:ViFPXeznp/e73yCYu3aogHJbIYt6E32SujbppRoeem8=
github.com/jfrog/jfrog-client-go v1.43.2 h1:NLSTTSFUkrNiSYs8rpRW7/sd6gDTPOi/eMVkGEarXq0=
github.com/jfrog/jfrog-client-go v1.43.2/go.mod h1:JUevXnjHbGL0MIIPs48L/axJMW/q4ioWMR1e1NuVn8w=
github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible h1:jdpOPRN1zP63Td1hDQbZW73xKmzDvZHzVdNYxhnTMDA=
Expand Down
2 changes: 1 addition & 1 deletion packagehandlers/packagehandlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
biutils "github.com/jfrog/build-info-go/utils"
"github.com/jfrog/frogbot/v2/utils"
"github.com/jfrog/jfrog-cli-security/commands/audit/sca/java"
"github.com/jfrog/jfrog-cli-security/formats"
"github.com/jfrog/jfrog-cli-security/utils/formats"
"github.com/jfrog/jfrog-cli-security/utils/techutils"
"github.com/jfrog/jfrog-client-go/utils/io/fileutils"
"github.com/stretchr/testify/assert"
Expand Down
144 changes: 46 additions & 98 deletions scanpullrequest/scanpullrequest.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import (
"github.com/jfrog/froggit-go/vcsclient"
"github.com/jfrog/froggit-go/vcsutils"
"github.com/jfrog/gofrog/datastructures"
"github.com/jfrog/jfrog-cli-security/formats"
securityutils "github.com/jfrog/jfrog-cli-security/utils"
"github.com/jfrog/jfrog-cli-security/utils/formats"
"github.com/jfrog/jfrog-cli-security/utils/jasutils"
"github.com/jfrog/jfrog-cli-security/utils/results"
"github.com/jfrog/jfrog-cli-security/utils/results/conversion"
"github.com/jfrog/jfrog-cli-security/utils/xsc"
"github.com/jfrog/jfrog-client-go/utils/log"
"github.com/jfrog/jfrog-client-go/xray/services"
Expand Down Expand Up @@ -170,7 +172,7 @@ func auditPullRequestInProject(repoConfig *utils.Repository, scanDetails *utils.
}()

// Audit source branch
var sourceResults *securityutils.Results
var sourceResults *results.SecurityCommandResults
workingDirs := utils.GetFullPathWorkingDirs(scanDetails.Project.WorkingDirs, sourceBranchWd)
log.Info("Scanning source branch...")
sourceResults, err = scanDetails.RunInstallAndAudit(workingDirs...)
Expand All @@ -179,8 +181,7 @@ func auditPullRequestInProject(repoConfig *utils.Repository, scanDetails *utils.
}

// Set JAS output flags
sourceScanResults := sourceResults.ExtendedScanResults
repoConfig.OutputWriter.SetJasOutputFlags(sourceScanResults.EntitledForJas, len(sourceScanResults.ApplicabilityScanResults) > 0)
repoConfig.OutputWriter.SetJasOutputFlags(sourceResults.EntitledForJas, len(sourceResults.GetJasScansResults(jasutils.Applicability)) > 0)

// Get all issues that exist in the source branch
if repoConfig.IncludeAllVulnerabilities {
Expand All @@ -199,7 +200,7 @@ func auditPullRequestInProject(repoConfig *utils.Repository, scanDetails *utils.
return
}

func auditTargetBranch(repoConfig *utils.Repository, scanDetails *utils.ScanDetails, sourceScanResults *securityutils.Results) (newIssues *utils.IssuesCollection, targetBranchWd string, err error) {
func auditTargetBranch(repoConfig *utils.Repository, scanDetails *utils.ScanDetails, sourceScanResults *results.SecurityCommandResults) (newIssues *utils.IssuesCollection, targetBranchWd string, err error) {
// Download target branch (if needed)
cleanupTarget := func() error { return nil }
if !repoConfig.IncludeAllVulnerabilities {
Expand All @@ -213,7 +214,7 @@ func auditTargetBranch(repoConfig *utils.Repository, scanDetails *utils.ScanDeta
}()

// Set target branch scan details
var targetResults *securityutils.Results
var targetResults *results.SecurityCommandResults
workingDirs := utils.GetFullPathWorkingDirs(scanDetails.Project.WorkingDirs, targetBranchWd)
log.Info("Scanning target branch...")
targetResults, err = scanDetails.RunInstallAndAudit(workingDirs...)
Expand All @@ -226,52 +227,62 @@ func auditTargetBranch(repoConfig *utils.Repository, scanDetails *utils.ScanDeta
return
}

func getAllIssues(results *securityutils.Results, allowedLicenses []string) (*utils.IssuesCollection, error) {
func getAllIssues(cmdResults *results.SecurityCommandResults, allowedLicenses []string) (*utils.IssuesCollection, error) {
log.Info("Frogbot is configured to show all vulnerabilities")
scanResults := results.ExtendedScanResults
xraySimpleJson, err := securityutils.ConvertXrayScanToSimpleJson(results, results.IsMultipleProject(), false, true, allowedLicenses)
simpleJsonResults, err := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{
AllowedLicenses: allowedLicenses,
IncludeLicenses: true,
SimplifiedOutput: true,
}).ConvertToSimpleJson(cmdResults)
if err != nil {
return nil, err
}
return &utils.IssuesCollection{
Vulnerabilities: append(xraySimpleJson.Vulnerabilities, xraySimpleJson.SecurityViolations...),
Iacs: securityutils.PrepareIacs(scanResults.IacScanResults),
Secrets: securityutils.PrepareSecrets(scanResults.SecretsScanResults),
Sast: securityutils.PrepareSast(scanResults.SastScanResults),
Licenses: xraySimpleJson.LicensesViolations,
Vulnerabilities: append(simpleJsonResults.Vulnerabilities, simpleJsonResults.SecurityViolations...),
Iacs: simpleJsonResults.Iacs,
Secrets: simpleJsonResults.Secrets,
Sast: simpleJsonResults.Sast,
Licenses: simpleJsonResults.LicensesViolations,
}, nil
}

// Returns all the issues found in the source branch that didn't exist in the target branch.
func getNewlyAddedIssues(targetResults, sourceResults *securityutils.Results, allowedLicenses []string) (*utils.IssuesCollection, error) {
func getNewlyAddedIssues(targetResults, sourceResults *results.SecurityCommandResults, allowedLicenses []string) (*utils.IssuesCollection, error) {
var err error
convertor := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{IncludeLicenses: len(allowedLicenses) > 0, AllowedLicenses: allowedLicenses, SimplifiedOutput: true})
simpleJsonSource, err := convertor.ConvertToSimpleJson(sourceResults)
if err != nil {
return nil, err
}
simpleJsonTarget, err := convertor.ConvertToSimpleJson(targetResults)
if err != nil {
return nil, err
}

var newVulnerabilitiesOrViolations []formats.VulnerabilityOrViolationRow
if len(simpleJsonSource.Vulnerabilities) > 0 || len(simpleJsonSource.SecurityViolations) > 0 {
newVulnerabilitiesOrViolations = append(
getUniqueVulnerabilityOrViolationRows(simpleJsonTarget.Vulnerabilities, simpleJsonSource.Vulnerabilities),
getUniqueVulnerabilityOrViolationRows(simpleJsonTarget.SecurityViolations, simpleJsonSource.SecurityViolations)...,
)
}

var newLicenses []formats.LicenseRow
var err error
if len(sourceResults.GetScaScansXrayResults()) > 0 {
if newVulnerabilitiesOrViolations, newLicenses, err = createNewVulnerabilitiesRows(targetResults, sourceResults, allowedLicenses); err != nil {
return nil, err
}
if len(simpleJsonSource.LicensesViolations) > 0 {
newLicenses = getUniqueLicenseRows(simpleJsonTarget.LicensesViolations, simpleJsonSource.LicensesViolations)
}

var newIacs []formats.SourceCodeRow
if len(sourceResults.ExtendedScanResults.IacScanResults) > 0 {
targetIacRows := securityutils.PrepareIacs(targetResults.ExtendedScanResults.IacScanResults)
sourceIacRows := securityutils.PrepareIacs(sourceResults.ExtendedScanResults.IacScanResults)
newIacs = createNewSourceCodeRows(targetIacRows, sourceIacRows)
if len(simpleJsonSource.Iacs) > 0 {
newIacs = createNewSourceCodeRows(simpleJsonTarget.Iacs, simpleJsonSource.Iacs)
}

var newSecrets []formats.SourceCodeRow
if len(sourceResults.ExtendedScanResults.SecretsScanResults) > 0 {
targetSecretsRows := securityutils.PrepareIacs(targetResults.ExtendedScanResults.SecretsScanResults)
sourceSecretsRows := securityutils.PrepareIacs(sourceResults.ExtendedScanResults.SecretsScanResults)
newSecrets = createNewSourceCodeRows(targetSecretsRows, sourceSecretsRows)
if len(simpleJsonSource.Secrets) > 0 {
newSecrets = createNewSourceCodeRows(simpleJsonTarget.Secrets, simpleJsonSource.Secrets)
}

var newSast []formats.SourceCodeRow
if len(targetResults.ExtendedScanResults.SastScanResults) > 0 {
targetSastRows := securityutils.PrepareSast(targetResults.ExtendedScanResults.SastScanResults)
sourceSastRows := securityutils.PrepareSast(sourceResults.ExtendedScanResults.SastScanResults)
newSast = createNewSourceCodeRows(targetSastRows, sourceSastRows)
if len(simpleJsonSource.Sast) > 0 {
newSast = createNewSourceCodeRows(simpleJsonTarget.Sast, simpleJsonSource.Sast)
}

return &utils.IssuesCollection{
Expand All @@ -297,40 +308,6 @@ func createNewSourceCodeRows(targetResults, sourceResults []formats.SourceCodeRo
return addedSourceCodeVulnerabilities
}

// Create vulnerabilities rows. The rows should contain only the new issues added by this PR
func createNewVulnerabilitiesRows(targetResults, sourceResults *securityutils.Results, allowedLicenses []string) (vulnerabilityOrViolationRows []formats.VulnerabilityOrViolationRow, licenseRows []formats.LicenseRow, err error) {
targetScanAggregatedResults := aggregateScanResults(targetResults.GetScaScansXrayResults())
sourceScanAggregatedResults := aggregateScanResults(sourceResults.GetScaScansXrayResults())

if len(sourceScanAggregatedResults.Violations) > 0 {
return getNewViolations(&targetScanAggregatedResults, &sourceScanAggregatedResults, sourceResults)
}
if len(sourceScanAggregatedResults.Vulnerabilities) > 0 {
if vulnerabilityOrViolationRows, err = getNewSecurityVulnerabilities(&targetScanAggregatedResults, &sourceScanAggregatedResults, sourceResults); err != nil {
return
}
}
var newLicenses []formats.LicenseRow
if newLicenses, err = getNewLicenseRows(&targetScanAggregatedResults, &sourceScanAggregatedResults); err != nil {
return
}
licenseRows = securityutils.GetViolatedLicenses(allowedLicenses, newLicenses)
return
}

func getNewSecurityVulnerabilities(targetScan, sourceScan *services.ScanResponse, auditResults *securityutils.Results) (newVulnerabilitiesRows []formats.VulnerabilityOrViolationRow, err error) {
targetVulnerabilitiesRows, err := securityutils.PrepareVulnerabilities(targetScan.Vulnerabilities, auditResults, auditResults.IsMultipleProject(), true)
if err != nil {
return newVulnerabilitiesRows, err
}
sourceVulnerabilitiesRows, err := securityutils.PrepareVulnerabilities(sourceScan.Vulnerabilities, auditResults, auditResults.IsMultipleProject(), true)
if err != nil {
return newVulnerabilitiesRows, err
}
newVulnerabilitiesRows = getUniqueVulnerabilityOrViolationRows(targetVulnerabilitiesRows, sourceVulnerabilitiesRows)
return
}

func getUniqueVulnerabilityOrViolationRows(targetRows, sourceRows []formats.VulnerabilityOrViolationRow) []formats.VulnerabilityOrViolationRow {
existingRows := make(map[string]formats.VulnerabilityOrViolationRow)
var newRows []formats.VulnerabilityOrViolationRow
Expand All @@ -345,35 +322,6 @@ func getUniqueVulnerabilityOrViolationRows(targetRows, sourceRows []formats.Vuln
return newRows
}

func getNewViolations(targetScan, sourceScan *services.ScanResponse, auditResults *securityutils.Results) (newSecurityViolationsRows []formats.VulnerabilityOrViolationRow, newLicenseViolationsRows []formats.LicenseRow, err error) {
targetSecurityViolationsRows, targetLicenseViolationsRows, _, err := securityutils.PrepareViolations(targetScan.Violations, auditResults, auditResults.IsMultipleProject(), true)
if err != nil {
return
}
sourceSecurityViolationsRows, sourceLicenseViolationsRows, _, err := securityutils.PrepareViolations(sourceScan.Violations, auditResults, auditResults.IsMultipleProject(), true)
if err != nil {
return
}
newSecurityViolationsRows = getUniqueVulnerabilityOrViolationRows(targetSecurityViolationsRows, sourceSecurityViolationsRows)
if len(sourceLicenseViolationsRows) > 0 {
newLicenseViolationsRows = getUniqueLicenseRows(targetLicenseViolationsRows, sourceLicenseViolationsRows)
}
return
}

func getNewLicenseRows(targetScan, sourceScan *services.ScanResponse) (newLicenses []formats.LicenseRow, err error) {
targetLicenses, err := securityutils.PrepareLicenses(targetScan.Licenses)
if err != nil {
return
}
sourceLicenses, err := securityutils.PrepareLicenses(sourceScan.Licenses)
if err != nil {
return
}
newLicenses = getUniqueLicenseRows(targetLicenses, sourceLicenses)
return
}

func getUniqueLicenseRows(targetRows, sourceRows []formats.LicenseRow) []formats.LicenseRow {
existingLicenses := make(map[string]formats.LicenseRow)
var newLicenses []formats.LicenseRow
Expand Down
Loading
Loading