Skip to content

Commit d0561b0

Browse files
committed
Fix multi-target flattening in scan-repository
When JFrog CLI auto-detects multiple working directories, ConvertToSimpleJson flattens all results together, losing the association between vulnerabilities and their specific working directories. This fix: 1. Uses IncludeTargets parameter to filter each target separately 2. Limits package handler file walks to current directory only - Prevents fixing vulnerabilities in auto-detected subdirectory targets - Each target processes its own descriptor files independently Depends on: IncludeTargets feature in jfrog-cli-security (currently in attiasas/convert_include_targets branch)
1 parent 3f1e261 commit d0561b0

File tree

6 files changed

+129
-18
lines changed

6 files changed

+129
-18
lines changed

go.mod

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ require (
126126
gopkg.in/warnings.v0 v0.1.2 // indirect
127127
)
128128

129-
// replace github.com/jfrog/jfrog-cli-security => github.com/jfrog/jfrog-cli-security dev
129+
replace github.com/jfrog/jfrog-cli-security => ../jfrog-cli-security
130+
// Using attiasas/convert_include_targets branch for IncludeTargets feature
130131

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

go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,6 @@ 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=
153151
github.com/jfrog/jfrog-client-go v1.55.1-0.20251217080430-c92b763b7465 h1:Ff3BlNPndrAfa1xFI/ORFzfWTxQxF0buWG61PEJwd3U=
154152
github.com/jfrog/jfrog-client-go v1.55.1-0.20251217080430-c92b763b7465/go.mod h1:WQ5Y+oKYyHFAlCbHN925bWhnShTd2ruxZ6YTpb76fpU=
155153
github.com/jhump/protoreflect v1.15.1 h1:HUMERORf3I3ZdX05WaQ6MIpd/NJ434hTp5YiKgfCL6c=

packagehandlers/commonpackagehandler.go

Lines changed: 8 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,11 @@ 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+
// Skip subdirectories - they may be separate auto-detected targets that will be processed independently
116+
if d.IsDir() && path != "." {
117+
return filepath.SkipDir
118+
}
119+
114120
for _, regexpCompiler := range regexpPatternsCompilers {
115121
if match := regexpCompiler.FindString(path); match != "" {
116122
return filepath.SkipDir

packagehandlers/packagehandlers_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -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: 50 additions & 12 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,40 @@ 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+
target.ScaResults.Descriptors
209+
targetPath := target.Target
210+
convertor := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{
211+
IncludeVulnerabilities: scanResults.IncludesVulnerabilities(),
212+
HasViolationContext: scanResults.HasViolationContext(),
213+
IncludeTargets: []string{targetPath},
214+
})
215+
simpleJsonResult, err := convertor.ConvertToSimpleJson(scanResults)
216+
if err != nil {
217+
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 {
218+
return totalFindings, err
219+
}
220+
continue
211221
}
212-
continue
213-
}
214-
if len(currPathVulnerabilities) > 0 {
215-
fixNeeded = true
222+
223+
currPathVulnerabilities, err := cfp.createVulnerabilitiesMapFromSimpleJson(simpleJsonResult)
224+
if err != nil {
225+
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 {
226+
return totalFindings, err
227+
}
228+
continue
229+
}
230+
if len(currPathVulnerabilities) > 0 {
231+
isFixNeeded = true
232+
log.Debug(fmt.Sprintf("Found %d fixable vulnerabilities in '%s': %s", len(currPathVulnerabilities), targetPath, strings.Join(maps.Keys(currPathVulnerabilities), ", ")))
233+
}
234+
vulnerabilitiesByPathMap[targetPath] = currPathVulnerabilities
216235
}
217-
vulnerabilitiesByPathMap[fullPathWd] = currPathVulnerabilities
218236
}
219237
if repository.DetectionOnly {
220238
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 {
239+
} else if isFixNeeded {
222240
return totalFindings, cfp.fixVulnerablePackages(repository, vulnerabilitiesByPathMap)
223241
}
224242
return totalFindings, nil
@@ -598,6 +616,26 @@ func (cfp *ScanRepositoryCmd) createVulnerabilitiesMap(scanResults *results.Secu
598616
return vulnerabilitiesMap, nil
599617
}
600618

619+
func (cfp *ScanRepositoryCmd) createVulnerabilitiesMapFromSimpleJson(simpleJsonResult formats.SimpleJsonResults) (map[string]*utils.VulnerabilityDetails, error) {
620+
vulnerabilitiesMap := map[string]*utils.VulnerabilityDetails{}
621+
var err error
622+
623+
if len(simpleJsonResult.Vulnerabilities) > 0 {
624+
for i := range simpleJsonResult.Vulnerabilities {
625+
if err = cfp.addVulnerabilityToFixVersionsMap(&simpleJsonResult.Vulnerabilities[i], vulnerabilitiesMap); err != nil {
626+
return nil, err
627+
}
628+
}
629+
} else if len(simpleJsonResult.SecurityViolations) > 0 {
630+
for i := range simpleJsonResult.SecurityViolations {
631+
if err = cfp.addVulnerabilityToFixVersionsMap(&simpleJsonResult.SecurityViolations[i], vulnerabilitiesMap); err != nil {
632+
return nil, err
633+
}
634+
}
635+
}
636+
return vulnerabilitiesMap, nil
637+
}
638+
601639
func (cfp *ScanRepositoryCmd) addVulnerabilityToFixVersionsMap(vulnerability *formats.VulnerabilityOrViolationRow, vulnerabilitiesMap map[string]*utils.VulnerabilityDetails) error {
602640
if len(vulnerability.FixedVersions) == 0 {
603641
return nil

scanrepository/scanrepository_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -696,6 +696,74 @@ func TestCreateVulnerabilitiesMap(t *testing.T) {
696696
}
697697
}
698698

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

0 commit comments

Comments
 (0)