Skip to content

Commit fd9d100

Browse files
authored
Fix multi-target flattening in scan-repository (#1003)
1 parent a14eddd commit fd9d100

File tree

6 files changed

+130
-32
lines changed

6 files changed

+130
-32
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ require (
1212
github.com/jfrog/gofrog v1.7.6
1313
github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20251211075913-35ebcd308e93
1414
github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20251210085744-f8481d179ac5
15-
github.com/jfrog/jfrog-cli-security v1.24.2
15+
github.com/jfrog/jfrog-cli-security v1.25.0
1616
github.com/jfrog/jfrog-client-go v1.55.1-0.20251217080430-c92b763b7465
1717
github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible
1818
github.com/owenrumney/go-sarif/v3 v3.2.3

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20251211075913-35ebcd308e93 h1:r
148148
github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20251211075913-35ebcd308e93/go.mod h1:7cCaRhXorlbyXZgiW5bplCExFxlnROaG21K12d8inpQ=
149149
github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20251210085744-f8481d179ac5 h1:GYE67ubwl+ZRw3CcXFUi49EwwQp6k+qS8sX0QuHDHO8=
150150
github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20251210085744-f8481d179ac5/go.mod h1:BMoGi2rG0udCCeaghqlNgiW3fTmT+TNnfTnBoWFYgcg=
151-
github.com/jfrog/jfrog-cli-security v1.24.2 h1:nyI0lNYR8i6yZYeBDsBJnURYsMnFKEmt7QH4vaNxtGM=
152-
github.com/jfrog/jfrog-cli-security v1.24.2/go.mod h1:3FXD5IkKtdQOm9CZk6cR7q0iC6PaGMnjqzZqRcQp2r0=
151+
github.com/jfrog/jfrog-cli-security v1.25.0 h1:DM29QsMkFLRD6adKCWISe3uHFVaOodIN56NH7ThpKKU=
152+
github.com/jfrog/jfrog-cli-security v1.25.0/go.mod h1:IV/+JhaLmyeMb8IAoJZYUq4gONW5BdrxgdT4w7SXgz0=
153153
github.com/jfrog/jfrog-client-go v1.55.1-0.20251217080430-c92b763b7465 h1:Ff3BlNPndrAfa1xFI/ORFzfWTxQxF0buWG61PEJwd3U=
154154
github.com/jfrog/jfrog-client-go v1.55.1-0.20251217080430-c92b763b7465/go.mod h1:WQ5Y+oKYyHFAlCbHN925bWhnShTd2ruxZ6YTpb76fpU=
155155
github.com/jhump/protoreflect v1.15.1 h1:HUMERORf3I3ZdX05WaQ6MIpd/NJ434hTp5YiKgfCL6c=

packagehandlers/commonpackagehandler.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,10 @@ func getFixedPackage(impactedPackage string, versionOperator string, suggestedFi
9393
return
9494
}
9595

96-
// Recursively scans the current directory for descriptor files based on the provided list of suffixes, while excluding paths that match the specified exclusion patterns.
96+
// Scans the current directory for descriptor files based on the provided list of suffixes, while excluding paths that match the specified exclusion patterns.
97+
// Only scans the current directory level (non-recursive) to avoid conflicts with auto-detected subdirectory targets that will be processed separately.
9798
// The patternsToExclude must be provided as regexp patterns. For instance, if the pattern ".*node_modules.*" is provided, any paths containing "node_modules" will be excluded from the result.
98-
// Returns a slice of all discovered descriptor files, represented as absolute paths.
99+
// Returns a slice of all discovered descriptor files in the current directory, represented as absolute paths.
99100
func (cph *CommonPackageHandler) GetAllDescriptorFilesFullPaths(descriptorFilesSuffixes []string, patternsToExclude ...string) (descriptorFilesFullPaths []string, err error) {
100101
if len(descriptorFilesSuffixes) == 0 {
101102
return
@@ -111,6 +112,10 @@ func (cph *CommonPackageHandler) GetAllDescriptorFilesFullPaths(descriptorFilesS
111112
return fmt.Errorf("an error has occurred when attempting to access or traverse the file system: %w", innerErr)
112113
}
113114

115+
if d.IsDir() && path != "." {
116+
return filepath.SkipDir
117+
}
118+
114119
for _, regexpCompiler := range regexpPatternsCompilers {
115120
if match := regexpCompiler.FindString(path); match != "" {
116121
return filepath.SkipDir

packagehandlers/packagehandlers_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ func TestUpdateDependency(t *testing.T) {
316316
},
317317
scanDetails: scanDetails,
318318
fixSupported: true,
319-
descriptorsToCheck: []string{"build.gradle", filepath.Join("innerProjectForTest", "build.gradle.kts")},
319+
descriptorsToCheck: []string{"build.gradle"},
320320
},
321321
},
322322

@@ -966,7 +966,7 @@ func TestGetAllDescriptorFilesFullPaths(t *testing.T) {
966966
{
967967
testProjectRepo: "gradle",
968968
suffixesToSearch: []string{groovyDescriptorFileSuffix, kotlinDescriptorFileSuffix},
969-
expectedResultSuffixes: []string{filepath.Join("innerProjectForTest", "build.gradle.kts"), "build.gradle"},
969+
expectedResultSuffixes: []string{"build.gradle"},
970970
},
971971
// This test case verifies that paths containing excluded patterns are omitted from the output
972972
{

scanrepository/scanrepository.go

Lines changed: 49 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ func (cfp *ScanRepositoryCmd) setCommandPrerequisites(repository *utils.Reposito
162162
}
163163

164164
func (cfp *ScanRepositoryCmd) scanAndFixProject(repository *utils.Repository) (int, error) {
165-
var fixNeeded bool
165+
var isFixNeeded bool
166166
totalFindings := 0
167167
// A map that contains the full project paths as a keys
168168
// The value is a map of vulnerable package names -> the scanDetails of the vulnerable packages.
@@ -203,22 +203,39 @@ func (cfp *ScanRepositoryCmd) scanAndFixProject(repository *utils.Repository) (i
203203
if repository.DetectionOnly {
204204
continue
205205
}
206-
// Prepare the vulnerabilities map for each working dir path
207-
currPathVulnerabilities, err := cfp.getVulnerabilitiesMap(scanResults)
208-
if err != nil {
209-
if err = utils.CreateErrorIfPartialResultsDisabled(cfp.scanDetails.AllowPartialResults(), fmt.Sprintf("An error occurred while preparing the vulnerabilities map for '%s' working directory. Fixes will be skipped for this working directory", fullPathWd), err); err != nil {
210-
return totalFindings, err
206+
207+
for _, target := range scanResults.Targets {
208+
targetPath := target.Target
209+
convertor := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{
210+
IncludeVulnerabilities: scanResults.IncludesVulnerabilities(),
211+
HasViolationContext: scanResults.HasViolationContext(),
212+
IncludeTargets: []string{targetPath},
213+
})
214+
simpleJsonResult, err := convertor.ConvertToSimpleJson(scanResults)
215+
if err != nil {
216+
if err = utils.CreateErrorIfPartialResultsDisabled(cfp.scanDetails.AllowPartialResults(), fmt.Sprintf("An error occurred while preparing the vulnerabilities map for '%s' working directory. Fixes will be skipped for this working directory", targetPath), err); err != nil {
217+
return totalFindings, err
218+
}
219+
continue
211220
}
212-
continue
213-
}
214-
if len(currPathVulnerabilities) > 0 {
215-
fixNeeded = true
221+
222+
currPathVulnerabilities, err := cfp.createVulnerabilitiesMapFromSimpleJson(simpleJsonResult)
223+
if err != nil {
224+
if err = utils.CreateErrorIfPartialResultsDisabled(cfp.scanDetails.AllowPartialResults(), fmt.Sprintf("An error occurred while preparing the vulnerabilities map for '%s' working directory. Fixes will be skipped for this working directory", targetPath), err); err != nil {
225+
return totalFindings, err
226+
}
227+
continue
228+
}
229+
if len(currPathVulnerabilities) > 0 {
230+
isFixNeeded = true
231+
log.Debug(fmt.Sprintf("Found %d fixable vulnerabilities in '%s': %s", len(currPathVulnerabilities), targetPath, strings.Join(maps.Keys(currPathVulnerabilities), ", ")))
232+
}
233+
vulnerabilitiesByPathMap[targetPath] = currPathVulnerabilities
216234
}
217-
vulnerabilitiesByPathMap[fullPathWd] = currPathVulnerabilities
218235
}
219236
if repository.DetectionOnly {
220237
log.Info(fmt.Sprintf("This command is running in detection mode only. To enable automatic fixing of issues, set the '%s' environment variable to 'false'.", utils.DetectionOnlyEnv))
221-
} else if fixNeeded {
238+
} else if isFixNeeded {
222239
return totalFindings, cfp.fixVulnerablePackages(repository, vulnerabilitiesByPathMap)
223240
}
224241
return totalFindings, nil
@@ -237,19 +254,6 @@ func (cfp *ScanRepositoryCmd) scan(currentWorkingDir string) (*results.SecurityC
237254
return auditResults, nil
238255
}
239256

240-
func (cfp *ScanRepositoryCmd) getVulnerabilitiesMap(scanResults *results.SecurityCommandResults) (map[string]*utils.VulnerabilityDetails, error) {
241-
vulnerabilitiesMap, err := cfp.createVulnerabilitiesMap(scanResults)
242-
if err != nil {
243-
return nil, err
244-
}
245-
246-
// Nothing to fix, return
247-
if len(vulnerabilitiesMap) == 0 {
248-
log.Info("Didn't find vulnerable dependencies with existing fix versions for", cfp.scanDetails.RepoName)
249-
}
250-
return vulnerabilitiesMap, nil
251-
}
252-
253257
func (cfp *ScanRepositoryCmd) fixVulnerablePackages(repository *utils.Repository, vulnerabilitiesByWdMap map[string]map[string]*utils.VulnerabilityDetails) (err error) {
254258
if cfp.aggregateFixes {
255259
err = cfp.fixIssuesSinglePR(repository, vulnerabilitiesByWdMap)
@@ -598,6 +602,26 @@ func (cfp *ScanRepositoryCmd) createVulnerabilitiesMap(scanResults *results.Secu
598602
return vulnerabilitiesMap, nil
599603
}
600604

605+
func (cfp *ScanRepositoryCmd) createVulnerabilitiesMapFromSimpleJson(simpleJsonResult formats.SimpleJsonResults) (map[string]*utils.VulnerabilityDetails, error) {
606+
vulnerabilitiesMap := map[string]*utils.VulnerabilityDetails{}
607+
var err error
608+
609+
if len(simpleJsonResult.Vulnerabilities) > 0 {
610+
for i := range simpleJsonResult.Vulnerabilities {
611+
if err = cfp.addVulnerabilityToFixVersionsMap(&simpleJsonResult.Vulnerabilities[i], vulnerabilitiesMap); err != nil {
612+
return nil, err
613+
}
614+
}
615+
} else if len(simpleJsonResult.SecurityViolations) > 0 {
616+
for i := range simpleJsonResult.SecurityViolations {
617+
if err = cfp.addVulnerabilityToFixVersionsMap(&simpleJsonResult.SecurityViolations[i], vulnerabilitiesMap); err != nil {
618+
return nil, err
619+
}
620+
}
621+
}
622+
return vulnerabilitiesMap, nil
623+
}
624+
601625
func (cfp *ScanRepositoryCmd) addVulnerabilityToFixVersionsMap(vulnerability *formats.VulnerabilityOrViolationRow, vulnerabilitiesMap map[string]*utils.VulnerabilityDetails) error {
602626
if len(vulnerability.FixedVersions) == 0 {
603627
return nil

scanrepository/scanrepository_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/jfrog/jfrog-cli-security/utils/formats"
2424
"github.com/jfrog/jfrog-cli-security/utils/formats/violationutils"
2525
"github.com/jfrog/jfrog-cli-security/utils/results"
26+
"github.com/jfrog/jfrog-cli-security/utils/results/conversion"
2627
"github.com/jfrog/jfrog-cli-security/utils/severityutils"
2728
"github.com/jfrog/jfrog-cli-security/utils/techutils"
2829
"github.com/jfrog/jfrog-client-go/utils/io/fileutils"
@@ -696,6 +697,74 @@ func TestCreateVulnerabilitiesMap(t *testing.T) {
696697
}
697698
}
698699

700+
func TestMultiTargetVulnerabilitiesMap(t *testing.T) {
701+
cfp := &ScanRepositoryCmd{}
702+
703+
scanResults := &results.SecurityCommandResults{
704+
ResultsMetaData: results.ResultsMetaData{ResultContext: results.ResultContext{IncludeVulnerabilities: true}},
705+
Targets: []*results.TargetResults{
706+
{
707+
ScanTarget: results.ScanTarget{Target: "project1"},
708+
ScaResults: &results.ScaScanResults{
709+
DeprecatedXrayResults: []services.ScanResponse{{
710+
Vulnerabilities: []services.Vulnerability{{
711+
Cves: []services.Cve{{Id: "CVE-1"}},
712+
Severity: "High",
713+
Components: map[string]services.Component{
714+
"pkg1": {
715+
FixedVersions: []string{"1.0.0"},
716+
ImpactPaths: [][]services.ImpactPathNode{{{ComponentId: "root"}, {ComponentId: "pkg1"}}},
717+
},
718+
},
719+
}},
720+
}},
721+
},
722+
},
723+
{
724+
ScanTarget: results.ScanTarget{Target: "project2"},
725+
ScaResults: &results.ScaScanResults{
726+
DeprecatedXrayResults: []services.ScanResponse{{
727+
Vulnerabilities: []services.Vulnerability{{
728+
Cves: []services.Cve{{Id: "CVE-2"}},
729+
Severity: "Critical",
730+
Components: map[string]services.Component{
731+
"pkg2": {
732+
FixedVersions: []string{"2.0.0"},
733+
ImpactPaths: [][]services.ImpactPathNode{{{ComponentId: "root"}, {ComponentId: "pkg2"}}},
734+
},
735+
},
736+
}},
737+
}},
738+
},
739+
},
740+
},
741+
}
742+
743+
convertor1 := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{
744+
IncludeVulnerabilities: true,
745+
IncludeTargets: []string{"project1"},
746+
})
747+
simpleJson1, err := convertor1.ConvertToSimpleJson(scanResults)
748+
assert.NoError(t, err)
749+
vulnsMap1, err := cfp.createVulnerabilitiesMapFromSimpleJson(simpleJson1)
750+
assert.NoError(t, err)
751+
assert.Len(t, vulnsMap1, 1)
752+
assert.Contains(t, vulnsMap1, "pkg1")
753+
assert.NotContains(t, vulnsMap1, "pkg2")
754+
755+
convertor2 := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{
756+
IncludeVulnerabilities: true,
757+
IncludeTargets: []string{"project2"},
758+
})
759+
simpleJson2, err := convertor2.ConvertToSimpleJson(scanResults)
760+
assert.NoError(t, err)
761+
vulnsMap2, err := cfp.createVulnerabilitiesMapFromSimpleJson(simpleJson2)
762+
assert.NoError(t, err)
763+
assert.Len(t, vulnsMap2, 1)
764+
assert.Contains(t, vulnsMap2, "pkg2")
765+
assert.NotContains(t, vulnsMap2, "pkg1")
766+
}
767+
699768
// Verifies unsupported packages return specific error
700769
// Other logic is implemented inside each package-handler.
701770
func TestUpdatePackageToFixedVersion(t *testing.T) {

0 commit comments

Comments
 (0)