From 7d3e78998b45cccc013cadc44e24b21b54f826c4 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 22 Feb 2026 14:05:43 +0200 Subject: [PATCH 1/5] grouping same cves into the same record --- utils/outputwriter/outputcontent.go | 52 +++++++++++++++++++++++- utils/outputwriter/outputcontent_test.go | 38 +++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/utils/outputwriter/outputcontent.go b/utils/outputwriter/outputcontent.go index a28fe95be..439a517a2 100644 --- a/utils/outputwriter/outputcontent.go +++ b/utils/outputwriter/outputcontent.go @@ -421,12 +421,60 @@ func getScaLicenseViolationDetails(violation formats.LicenseViolationRow, writer // Sca Vulnerabilities +func vulnerabilitySummaryKey(v formats.VulnerabilityOrViolationRow) string { + ids := make([]string, 0, len(v.Cves)+1) + for _, cve := range v.Cves { + ids = append(ids, cve.Id) + } + sort.Strings(ids) + return v.IssueId + "|" + strings.Join(ids, ",") +} + +func aggregateVulnerabilitiesByCve(vulnerabilities []formats.VulnerabilityOrViolationRow) []formats.VulnerabilityOrViolationRow { + if len(vulnerabilities) == 0 { + return vulnerabilities + } + byKey := make(map[string]*formats.VulnerabilityOrViolationRow) + for i := range vulnerabilities { + v := &vulnerabilities[i] + key := vulnerabilitySummaryKey(*v) + if existing, ok := byKey[key]; ok { + existing.ImpactPaths = append(existing.ImpactPaths, v.ImpactPaths...) + if len(v.FixedVersions) > 0 { + existing.FixedVersions = append(existing.FixedVersions, v.FixedVersions...) + } + } else { + agg := *v + agg.ImpactPaths = append([][]formats.ComponentRow(nil), v.ImpactPaths...) + agg.FixedVersions = append([]string(nil), v.FixedVersions...) + byKey[key] = &agg + } + } + result := make([]formats.VulnerabilityOrViolationRow, 0, len(byKey)) + for _, v := range byKey { + result = append(result, *v) + } + // Preserve relative order by first occurrence + order := make(map[string]int) + for i, v := range vulnerabilities { + key := vulnerabilitySummaryKey(v) + if _, ok := order[key]; !ok { + order[key] = i + } + } + sort.Slice(result, func(i, j int) bool { + return order[vulnerabilitySummaryKey(result[i])] < order[vulnerabilitySummaryKey(result[j])] + }) + return result +} + func GetVulnerabilitiesContent(vulnerabilities []formats.VulnerabilityOrViolationRow, writer OutputWriter) (content []string) { if len(vulnerabilities) == 0 { return []string{} } - content = append(content, getVulnerabilitiesSummaryTable(vulnerabilities, writer)) - content = append(content, getScaSecurityIssueDetailsContent(vulnerabilities, false, writer)...) + aggregated := aggregateVulnerabilitiesByCve(vulnerabilities) + content = append(content, getVulnerabilitiesSummaryTable(aggregated, writer)) + content = append(content, getScaSecurityIssueDetailsContent(aggregated, false, writer)...) return ConvertContentToComments(content, writer, getDecoratorWithScaVulnerabilitiesTitle(writer)) } diff --git a/utils/outputwriter/outputcontent_test.go b/utils/outputwriter/outputcontent_test.go index 9f8ad6e36..739bfd5f5 100644 --- a/utils/outputwriter/outputcontent_test.go +++ b/utils/outputwriter/outputcontent_test.go @@ -2,6 +2,7 @@ package outputwriter import ( "path/filepath" + "strings" "testing" "github.com/jfrog/froggit-go/vcsutils" @@ -431,6 +432,43 @@ func TestVulnerabilitiesContent(t *testing.T) { } } +func TestVulnerabilitiesContent_aggregatesSameCveIntoOneRow(t *testing.T) { + sameCve := "CVE-2017-1000487" + vulnerabilities := []formats.VulnerabilityOrViolationRow{ + { + ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ + SeverityDetails: formats.SeverityDetails{Severity: "Critical"}, + ImpactedDependencyName: "org.codehaus.plexus:plexus-utils", + ImpactedDependencyVersion: "1.5.15", + }, + Applicable: "Not Applicable", + ImpactPaths: [][]formats.ComponentRow{ + {{Name: "root", Version: "1.0.0"}, {Name: "some-dep", Version: "1.0.0"}, {Name: "org.codehaus.plexus:plexus-utils", Version: "1.5.15"}}, + }, + Cves: []formats.CveRow{{Id: sameCve}}, + }, + { + ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ + SeverityDetails: formats.SeverityDetails{Severity: "Critical"}, + ImpactedDependencyName: "org.codehaus.plexus:plexus-utils", + ImpactedDependencyVersion: "1.5.1", + }, + Applicable: "Not Applicable", + ImpactPaths: [][]formats.ComponentRow{ + {{Name: "root", Version: "1.0.0"}, {Name: "other-dep", Version: "1.0.0"}, {Name: "org.codehaus.plexus:plexus-utils", Version: "1.5.1"}}, + }, + Cves: []formats.CveRow{{Id: sameCve}}, + }, + } + content := GetVulnerabilitiesContent(vulnerabilities, &SimplifiedOutput{}) + assert.NotEmpty(t, content) + tableSection := content[0] + assert.Contains(t, tableSection, sameCve, "output should mention the CVE") + assert.Contains(t, tableSection, "2 Transitive", "same CVE in two transitive paths should be aggregated and show 2 Transitive") + // CVE should appear only once in the table (one row), not twice + assert.Equal(t, 1, strings.Count(tableSection, sameCve), "CVE should appear once in the summary table (one aggregated row)") +} + func TestSecurityViolationsContent(t *testing.T) { testCases := []struct { name string From 97cefd43772bc42d4393dca88253dfbf14829a56 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 1 Mar 2026 12:13:03 +0200 Subject: [PATCH 2/5] after pr --- utils/outputwriter/outputcontent.go | 31 +++++++++++---- utils/outputwriter/outputcontent_test.go | 49 ++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/utils/outputwriter/outputcontent.go b/utils/outputwriter/outputcontent.go index 439a517a2..ba3c235a9 100644 --- a/utils/outputwriter/outputcontent.go +++ b/utils/outputwriter/outputcontent.go @@ -286,8 +286,9 @@ func getSecurityViolationsContent(issues issues.ScansIssuesCollection, writer Ou if len(issues.ScaViolations) == 0 { return []string{} } - content = append(content, getSecurityViolationsSummaryTable(issues.ScaViolations, writer)) - content = append(content, getScaSecurityIssueDetailsContent(issues.ScaViolations, true, writer)...) + aggregated := aggregateVulnerabilitiesOrViolationsByCve(issues.ScaViolations) + content = append(content, getSecurityViolationsSummaryTable(aggregated, writer)) + content = append(content, getScaSecurityIssueDetailsContent(aggregated, true, writer)...) return ConvertContentToComments(content, writer, getDecoratorWithSecurityViolationTitle(writer)) } @@ -422,15 +423,31 @@ func getScaLicenseViolationDetails(violation formats.LicenseViolationRow, writer // Sca Vulnerabilities func vulnerabilitySummaryKey(v formats.VulnerabilityOrViolationRow) string { - ids := make([]string, 0, len(v.Cves)+1) + ids := make([]string, 0, len(v.Cves)) for _, cve := range v.Cves { ids = append(ids, cve.Id) } sort.Strings(ids) - return v.IssueId + "|" + strings.Join(ids, ",") + if len(ids) == 0 { + return v.IssueId + } + return strings.Join(ids, ",") +} + +func uniqueStrings(s []string) []string { + seen := make(map[string]struct{}, len(s)) + out := make([]string, 0, len(s)) + for _, v := range s { + if _, ok := seen[v]; ok { + continue + } + seen[v] = struct{}{} + out = append(out, v) + } + return out } -func aggregateVulnerabilitiesByCve(vulnerabilities []formats.VulnerabilityOrViolationRow) []formats.VulnerabilityOrViolationRow { +func aggregateVulnerabilitiesOrViolationsByCve(vulnerabilities []formats.VulnerabilityOrViolationRow) []formats.VulnerabilityOrViolationRow { if len(vulnerabilities) == 0 { return vulnerabilities } @@ -441,7 +458,7 @@ func aggregateVulnerabilitiesByCve(vulnerabilities []formats.VulnerabilityOrViol if existing, ok := byKey[key]; ok { existing.ImpactPaths = append(existing.ImpactPaths, v.ImpactPaths...) if len(v.FixedVersions) > 0 { - existing.FixedVersions = append(existing.FixedVersions, v.FixedVersions...) + existing.FixedVersions = uniqueStrings(append(existing.FixedVersions, v.FixedVersions...)) } } else { agg := *v @@ -472,7 +489,7 @@ func GetVulnerabilitiesContent(vulnerabilities []formats.VulnerabilityOrViolatio if len(vulnerabilities) == 0 { return []string{} } - aggregated := aggregateVulnerabilitiesByCve(vulnerabilities) + aggregated := aggregateVulnerabilitiesOrViolationsByCve(vulnerabilities) content = append(content, getVulnerabilitiesSummaryTable(aggregated, writer)) content = append(content, getScaSecurityIssueDetailsContent(aggregated, false, writer)...) return ConvertContentToComments(content, writer, getDecoratorWithScaVulnerabilitiesTitle(writer)) diff --git a/utils/outputwriter/outputcontent_test.go b/utils/outputwriter/outputcontent_test.go index 739bfd5f5..26c74e6fd 100644 --- a/utils/outputwriter/outputcontent_test.go +++ b/utils/outputwriter/outputcontent_test.go @@ -432,6 +432,55 @@ func TestVulnerabilitiesContent(t *testing.T) { } } +func TestAggregateVulnerabilitiesByCve(t *testing.T) { + t.Run("empty returns empty", func(t *testing.T) { + got := aggregateVulnerabilitiesOrViolationsByCve(nil) + assert.Nil(t, got) + got = aggregateVulnerabilitiesOrViolationsByCve([]formats.VulnerabilityOrViolationRow{}) + assert.Empty(t, got) + }) + + t.Run("same CVE merged into one row with combined paths", func(t *testing.T) { + cve := "CVE-2017-1000487" + in := []formats.VulnerabilityOrViolationRow{ + { + ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ + ImpactedDependencyName: "pkg", ImpactedDependencyVersion: "1.0", + }, + ImpactPaths: [][]formats.ComponentRow{{{Name: "root", Version: "1.0"}, {Name: "pkg", Version: "1.0"}}}, + FixedVersions: []string{"2.0", "3.0"}, + Cves: []formats.CveRow{{Id: cve}}, + IssueId: "XRAY-111", + }, + { + ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ + ImpactedDependencyName: "pkg", ImpactedDependencyVersion: "2.0", + }, + ImpactPaths: [][]formats.ComponentRow{{{Name: "root", Version: "1.0"}, {Name: "other", Version: "1.0"}, {Name: "pkg", Version: "2.0"}}}, + FixedVersions: []string{"2.0", "4.0"}, // 2.0 duplicate + Cves: []formats.CveRow{{Id: cve}}, + IssueId: "XRAY-222", + }, + } + got := aggregateVulnerabilitiesOrViolationsByCve(in) + assert.Len(t, got, 1) + assert.Len(t, got[0].ImpactPaths, 2) + assert.Equal(t, []string{"2.0", "3.0", "4.0"}, got[0].FixedVersions) // deduplicated + assert.Equal(t, "pkg", got[0].ImpactedDependencyName) + assert.Equal(t, "1.0", got[0].ImpactedDependencyVersion) // first occurrence kept + assert.Equal(t, "XRAY-111", got[0].IssueId) + }) + + t.Run("different CVEs stay separate", func(t *testing.T) { + in := []formats.VulnerabilityOrViolationRow{ + {Cves: []formats.CveRow{{Id: "CVE-A"}}, ImpactPaths: [][]formats.ComponentRow{{{Name: "a", Version: "1"}}}}, + {Cves: []formats.CveRow{{Id: "CVE-B"}}, ImpactPaths: [][]formats.ComponentRow{{{Name: "b", Version: "1"}}}}, + } + got := aggregateVulnerabilitiesOrViolationsByCve(in) + assert.Len(t, got, 2) + }) +} + func TestVulnerabilitiesContent_aggregatesSameCveIntoOneRow(t *testing.T) { sameCve := "CVE-2017-1000487" vulnerabilities := []formats.VulnerabilityOrViolationRow{ From e901bb9155294a9d72df25bbb1c98602b967d862 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 1 Mar 2026 14:11:25 +0200 Subject: [PATCH 3/5] fix for go sec --- utils/outputwriter/outputcontent.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/utils/outputwriter/outputcontent.go b/utils/outputwriter/outputcontent.go index ba3c235a9..afd30e05d 100644 --- a/utils/outputwriter/outputcontent.go +++ b/utils/outputwriter/outputcontent.go @@ -464,7 +464,9 @@ func aggregateVulnerabilitiesOrViolationsByCve(vulnerabilities []formats.Vulnera agg := *v agg.ImpactPaths = append([][]formats.ComponentRow(nil), v.ImpactPaths...) agg.FixedVersions = append([]string(nil), v.FixedVersions...) - byKey[key] = &agg + heapCopy := new(formats.VulnerabilityOrViolationRow) + *heapCopy = agg + byKey[key] = heapCopy } } result := make([]formats.VulnerabilityOrViolationRow, 0, len(byKey)) From 539ffda6981c043a6b2dcae822203fae059af0d7 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 1 Mar 2026 14:20:09 +0200 Subject: [PATCH 4/5] getAbsolutePathUnderWd --- packageupdaters/commonpackageupdater.go | 21 +++++++++++++++++++++ packageupdaters/conanpackageupdater.go | 6 +++++- packageupdaters/gradlepackageupdater.go | 9 +++++++-- packageupdaters/npmpackageupdater.go | 12 ++++++++++-- 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/packageupdaters/commonpackageupdater.go b/packageupdaters/commonpackageupdater.go index 37cccf10a..61d7fd9cb 100644 --- a/packageupdaters/commonpackageupdater.go +++ b/packageupdaters/commonpackageupdater.go @@ -3,6 +3,7 @@ package packageupdaters import ( "fmt" "io/fs" + "os" "os/exec" "path/filepath" "regexp" @@ -138,6 +139,26 @@ func BuildPackageWithVersionRegex(impactedName, impactedVersion, dependencyLineF return regexp.MustCompile(regexpCompleteFormat) } +func getAbsolutePathUnderWd(path string) (string, error) { + absPath, err := filepath.Abs(filepath.Clean(path)) + if err != nil { + return "", fmt.Errorf("failed to resolve path %q: %w", path, err) + } + wd, err := os.Getwd() + if err != nil { + return "", fmt.Errorf("failed to get working directory: %w", err) + } + wdAbs, err := filepath.Abs(wd) + if err != nil { + return "", fmt.Errorf("failed to resolve working directory: %w", err) + } + rel, err := filepath.Rel(wdAbs, absPath) + if err != nil || strings.HasPrefix(rel, "..") || rel == ".." { + return "", fmt.Errorf("path %q is outside project directory", path) + } + return absPath, nil +} + func GetVulnerabilityLocations(vulnDetails *utils.VulnerabilityDetails, namesFilters []string, ignoreFilters []string) []string { pathsSet := datastructures.MakeSet[string]() for _, component := range vulnDetails.Components { diff --git a/packageupdaters/conanpackageupdater.go b/packageupdaters/conanpackageupdater.go index 4a084688e..5491f1148 100644 --- a/packageupdaters/conanpackageupdater.go +++ b/packageupdaters/conanpackageupdater.go @@ -68,7 +68,11 @@ func (conan *ConanPackageUpdater) updateConanFile(conanFilePath string, vulnDeta log.Debug(fmt.Sprintf("impacted dependency '%s' not found in descriptor '%s', moving to the next descriptor if exists...", impactedDependency, conanFilePath)) return false, nil } - if err = os.WriteFile(conanFilePath, []byte(fixedFile), 0600); err != nil { + safePath, err := getAbsolutePathUnderWd(conanFilePath) + if err != nil { + return false, err + } + if err = os.WriteFile(safePath, []byte(fixedFile), 0600); err != nil { err = fmt.Errorf("an error occured while writing the fixed version of %s to the requirements file '%s': %s", vulnDetails.ImpactedDependencyName, conanFilePath, err.Error()) } isFileChanged = true diff --git a/packageupdaters/gradlepackageupdater.go b/packageupdaters/gradlepackageupdater.go index 8715296fe..1c6b0cec7 100644 --- a/packageupdaters/gradlepackageupdater.go +++ b/packageupdaters/gradlepackageupdater.go @@ -2,10 +2,11 @@ package packageupdaters import ( "fmt" - "github.com/jfrog/frogbot/v2/utils" "os" "regexp" "strings" + + "github.com/jfrog/frogbot/v2/utils" ) const ( @@ -149,7 +150,11 @@ func writeUpdatedBuildFile(filePath string, fileContent string) (err error) { return } - err = os.WriteFile(filePath, []byte(fileContent), fileInfo.Mode()) + safePath, err := getAbsolutePathUnderWd(filePath) + if err != nil { + return err + } + err = os.WriteFile(safePath, []byte(fileContent), fileInfo.Mode()) if err != nil { err = fmt.Errorf("couldn't write fixes to file '%s': %q", filePath, err) } diff --git a/packageupdaters/npmpackageupdater.go b/packageupdaters/npmpackageupdater.go index a89cbd293..fcbe8b1d0 100644 --- a/packageupdaters/npmpackageupdater.go +++ b/packageupdaters/npmpackageupdater.go @@ -137,7 +137,11 @@ func (npm *NpmPackageUpdater) updateDescriptor(vulnDetails *utils.VulnerabilityD return nil, fmt.Errorf("failed to update version in descriptor: %w", err) } - if err = os.WriteFile(descriptorPath, updatedContent, 0644); err != nil { + safePath, err := getAbsolutePathUnderWd(descriptorPath) + if err != nil { + return nil, err + } + if err = os.WriteFile(safePath, updatedContent, 0644); err != nil { return nil, fmt.Errorf("failed to write updated descriptor '%s': %w", descriptorPath, err) } return backupContent, nil @@ -156,7 +160,11 @@ func (npm *NpmPackageUpdater) RegenerateLockfile(vulnDetails *utils.Vulnerabilit if err = npm.regenerateLockFileWithRetry(); err != nil { log.Warn(fmt.Sprintf("Failed to regenerate lock file after updating '%s' to version '%s': %s. Rolling back...", vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion, err.Error())) - if rollbackErr := os.WriteFile(descriptorPath, backupContent, 0644); rollbackErr != nil { + safePath, pathErr := getAbsolutePathUnderWd(descriptorPath) + if pathErr != nil { + return fmt.Errorf("failed to rollback descriptor: %w (original error: %v)", pathErr, err) + } + if rollbackErr := os.WriteFile(safePath, backupContent, 0644); rollbackErr != nil { return fmt.Errorf("failed to rollback descriptor after lock file regeneration failure: %w (original error: %v)", rollbackErr, err) } return err From 6dd17ac01bd7f066a86423173fea1e7ed245da6a Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 1 Mar 2026 14:26:07 +0200 Subject: [PATCH 5/5] getAbsolutePathUnderWd --- packageupdaters/conanpackageupdater.go | 1 + packageupdaters/gradlepackageupdater.go | 1 + packageupdaters/npmpackageupdater.go | 2 ++ 3 files changed, 4 insertions(+) diff --git a/packageupdaters/conanpackageupdater.go b/packageupdaters/conanpackageupdater.go index 5491f1148..8a886b90e 100644 --- a/packageupdaters/conanpackageupdater.go +++ b/packageupdaters/conanpackageupdater.go @@ -72,6 +72,7 @@ func (conan *ConanPackageUpdater) updateConanFile(conanFilePath string, vulnDeta if err != nil { return false, err } + // #nosec G703 -- path validated under working directory by getAbsolutePathUnderWd if err = os.WriteFile(safePath, []byte(fixedFile), 0600); err != nil { err = fmt.Errorf("an error occured while writing the fixed version of %s to the requirements file '%s': %s", vulnDetails.ImpactedDependencyName, conanFilePath, err.Error()) } diff --git a/packageupdaters/gradlepackageupdater.go b/packageupdaters/gradlepackageupdater.go index 1c6b0cec7..e1173f2ba 100644 --- a/packageupdaters/gradlepackageupdater.go +++ b/packageupdaters/gradlepackageupdater.go @@ -154,6 +154,7 @@ func writeUpdatedBuildFile(filePath string, fileContent string) (err error) { if err != nil { return err } + // #nosec G703 -- path validated under working directory by getAbsolutePathUnderWd err = os.WriteFile(safePath, []byte(fileContent), fileInfo.Mode()) if err != nil { err = fmt.Errorf("couldn't write fixes to file '%s': %q", filePath, err) diff --git a/packageupdaters/npmpackageupdater.go b/packageupdaters/npmpackageupdater.go index fcbe8b1d0..f94a9e2c2 100644 --- a/packageupdaters/npmpackageupdater.go +++ b/packageupdaters/npmpackageupdater.go @@ -141,6 +141,7 @@ func (npm *NpmPackageUpdater) updateDescriptor(vulnDetails *utils.VulnerabilityD if err != nil { return nil, err } + // #nosec G703 -- path validated under working directory by getAbsolutePathUnderWd if err = os.WriteFile(safePath, updatedContent, 0644); err != nil { return nil, fmt.Errorf("failed to write updated descriptor '%s': %w", descriptorPath, err) } @@ -164,6 +165,7 @@ func (npm *NpmPackageUpdater) RegenerateLockfile(vulnDetails *utils.Vulnerabilit if pathErr != nil { return fmt.Errorf("failed to rollback descriptor: %w (original error: %v)", pathErr, err) } + // #nosec G703 -- path validated under working directory by getAbsolutePathUnderWd if rollbackErr := os.WriteFile(safePath, backupContent, 0644); rollbackErr != nil { return fmt.Errorf("failed to rollback descriptor after lock file regeneration failure: %w (original error: %v)", rollbackErr, err) }