Skip to content

Commit e6cc8fc

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 causes fixes to be applied to wrong directories. This fix: - Processes each target separately using ConvertTargetToSimpleJson - Maintains working directory association for accurate fixing - Handles both single and multiple auto-detected targets uniformly Depends on: jfrog/jfrog-cli-security#<PR_NUMBER> (ConvertTargetToSimpleJson function)
1 parent 3f1e261 commit e6cc8fc

File tree

4 files changed

+114
-15
lines changed

4 files changed

+114
-15
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ 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
130130

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

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=

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,25 @@ 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+
currPathVulnerabilities, err := cfp.getVulnerabilitiesMapForTarget(target, scanResults)
210+
if err != nil {
211+
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 {
212+
return totalFindings, err
213+
}
214+
continue
211215
}
212-
continue
213-
}
214-
if len(currPathVulnerabilities) > 0 {
215-
fixNeeded = true
216+
if len(currPathVulnerabilities) > 0 {
217+
isFixNeeded = true
218+
}
219+
vulnerabilitiesByPathMap[targetPath] = currPathVulnerabilities
216220
}
217-
vulnerabilitiesByPathMap[fullPathWd] = currPathVulnerabilities
218221
}
219222
if repository.DetectionOnly {
220223
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 {
224+
} else if isFixNeeded {
222225
return totalFindings, cfp.fixVulnerablePackages(repository, vulnerabilitiesByPathMap)
223226
}
224227
return totalFindings, nil
@@ -250,6 +253,18 @@ func (cfp *ScanRepositoryCmd) getVulnerabilitiesMap(scanResults *results.Securit
250253
return vulnerabilitiesMap, nil
251254
}
252255

256+
func (cfp *ScanRepositoryCmd) getVulnerabilitiesMapForTarget(target *results.TargetResults, scanResults *results.SecurityCommandResults) (map[string]*utils.VulnerabilityDetails, error) {
257+
convertor := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{
258+
IncludeVulnerabilities: scanResults.IncludesVulnerabilities(),
259+
HasViolationContext: scanResults.HasViolationContext(),
260+
})
261+
simpleJsonResult, err := convertor.ConvertTargetToSimpleJson(target, scanResults)
262+
if err != nil {
263+
return nil, err
264+
}
265+
return cfp.createVulnerabilitiesMapFromSimpleJson(simpleJsonResult)
266+
}
267+
253268
func (cfp *ScanRepositoryCmd) fixVulnerablePackages(repository *utils.Repository, vulnerabilitiesByWdMap map[string]map[string]*utils.VulnerabilityDetails) (err error) {
254269
if cfp.aggregateFixes {
255270
err = cfp.fixIssuesSinglePR(repository, vulnerabilitiesByWdMap)
@@ -598,6 +613,29 @@ func (cfp *ScanRepositoryCmd) createVulnerabilitiesMap(scanResults *results.Secu
598613
return vulnerabilitiesMap, nil
599614
}
600615

616+
func (cfp *ScanRepositoryCmd) createVulnerabilitiesMapFromSimpleJson(simpleJsonResult formats.SimpleJsonResults) (map[string]*utils.VulnerabilityDetails, error) {
617+
vulnerabilitiesMap := map[string]*utils.VulnerabilityDetails{}
618+
var err error
619+
620+
if len(simpleJsonResult.Vulnerabilities) > 0 {
621+
for i := range simpleJsonResult.Vulnerabilities {
622+
if err = cfp.addVulnerabilityToFixVersionsMap(&simpleJsonResult.Vulnerabilities[i], vulnerabilitiesMap); err != nil {
623+
return nil, err
624+
}
625+
}
626+
} else if len(simpleJsonResult.SecurityViolations) > 0 {
627+
for i := range simpleJsonResult.SecurityViolations {
628+
if err = cfp.addVulnerabilityToFixVersionsMap(&simpleJsonResult.SecurityViolations[i], vulnerabilitiesMap); err != nil {
629+
return nil, err
630+
}
631+
}
632+
}
633+
if len(vulnerabilitiesMap) > 0 {
634+
log.Debug("Frogbot will attempt to resolve the following vulnerable dependencies:\n", strings.Join(maps.Keys(vulnerabilitiesMap), ",\n"))
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: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -696,6 +696,69 @@ func TestCreateVulnerabilitiesMap(t *testing.T) {
696696
}
697697
}
698698

699+
func TestGetVulnerabilitiesMapForTarget(t *testing.T) {
700+
cfp := &ScanRepositoryCmd{}
701+
702+
// Create scan results with multiple targets
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+
{
712+
Cves: []services.Cve{{Id: "CVE-2023-1111"}},
713+
Severity: "High",
714+
Components: map[string]services.Component{
715+
"pkg1": {
716+
FixedVersions: []string{"1.0.0"},
717+
ImpactPaths: [][]services.ImpactPathNode{{{ComponentId: "root"}, {ComponentId: "pkg1"}}},
718+
},
719+
},
720+
},
721+
},
722+
}},
723+
},
724+
},
725+
{
726+
ScanTarget: results.ScanTarget{Target: "project2"},
727+
ScaResults: &results.ScaScanResults{
728+
DeprecatedXrayResults: []services.ScanResponse{{
729+
Vulnerabilities: []services.Vulnerability{
730+
{
731+
Cves: []services.Cve{{Id: "CVE-2023-2222"}},
732+
Severity: "Critical",
733+
Components: map[string]services.Component{
734+
"pkg2": {
735+
FixedVersions: []string{"2.0.0"},
736+
ImpactPaths: [][]services.ImpactPathNode{{{ComponentId: "root"}, {ComponentId: "pkg2"}}},
737+
},
738+
},
739+
},
740+
},
741+
}},
742+
},
743+
},
744+
},
745+
}
746+
747+
// Test converting first target
748+
vulnsMap1, err := cfp.getVulnerabilitiesMapForTarget(scanResults.Targets[0], scanResults)
749+
assert.NoError(t, err)
750+
assert.Len(t, vulnsMap1, 1)
751+
assert.Contains(t, vulnsMap1, "pkg1")
752+
assert.NotContains(t, vulnsMap1, "pkg2") // Should NOT contain pkg2
753+
754+
// Test converting second target
755+
vulnsMap2, err := cfp.getVulnerabilitiesMapForTarget(scanResults.Targets[1], scanResults)
756+
assert.NoError(t, err)
757+
assert.Len(t, vulnsMap2, 1)
758+
assert.Contains(t, vulnsMap2, "pkg2")
759+
assert.NotContains(t, vulnsMap2, "pkg1") // Should NOT contain pkg1
760+
}
761+
699762
// Verifies unsupported packages return specific error
700763
// Other logic is implemented inside each package-handler.
701764
func TestUpdatePackageToFixedVersion(t *testing.T) {

0 commit comments

Comments
 (0)