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

Gradle create fix pull request #478

Merged
merged 50 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from 49 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
664ef75
starting gradle package handler & adding to commonpackagehandler
eranturgeman Sep 6, 2023
0be931e
Merge branch 'dev' of https://github.com/jfrog/frogbot into gradle-cr…
eranturgeman Sep 6, 2023
e642da3
added up to row detection and started the fixers file
eranturgeman Sep 7, 2023
9f6d3f7
Merge branch 'dev' of https://github.com/jfrog/frogbot into gradle-cr…
eranturgeman Sep 7, 2023
c2993aa
basic framework completed
eranturgeman Sep 10, 2023
7a5357a
Merge branch 'dev' of https://github.com/jfrog/frogbot into gradle-cr…
eranturgeman Sep 11, 2023
5a04949
fixed some cases. start adding cases
eranturgeman Sep 11, 2023
c2ab98d
merging vulnerable rows detection and row's type detection
eranturgeman Sep 11, 2023
f34fccb
before deleting old fixBuildFile and related functions
eranturgeman Sep 12, 2023
379a046
stable version: fixing all Groovy direct dependencies
eranturgeman Sep 12, 2023
9458852
fixed some issues and improved code - groovy ready except notes
eranturgeman Sep 12, 2023
6ebe6d3
Merge branch 'dev' of https://github.com/jfrog/frogbot into gradle-cr…
eranturgeman Sep 13, 2023
9b652de
minor improvements
eranturgeman Sep 13, 2023
ea1b646
added tests for create-fix with gradle (groovy)
eranturgeman Sep 13, 2023
f6f440e
Merge branch 'dev' into gradle-create-fix-pull-request
eranturgeman Sep 13, 2023
36e81fb
supports kotlin
eranturgeman Sep 13, 2023
814e2b4
Merge remote-tracking branch 'eran-fork/gradle-create-fix-pull-reques…
eranturgeman Sep 13, 2023
a3a3aa6
minor fix
eranturgeman Sep 13, 2023
8fc2bb7
Merge branch 'dev' of https://github.com/jfrog/frogbot into gradle-cr…
eranturgeman Sep 14, 2023
8ebf0f8
added tests and improved logging
eranturgeman Sep 14, 2023
3e160df
simplified the flow. delete all redundant and fix tests
eranturgeman Sep 15, 2023
3a28d3f
fixed all flow and tests
eranturgeman Sep 18, 2023
5f3e453
.
eranturgeman Sep 18, 2023
16566a7
Merge branch 'dev' into gradle-create-fix-pull-request
eranturgeman Sep 18, 2023
47a5d0e
Merge branch 'dev' of https://github.com/jfrog/frogbot into gradle-cr…
eranturgeman Sep 18, 2023
bae7447
.
eranturgeman Sep 18, 2023
c5c8404
Merge remote-tracking branch 'eran-fork/gradle-create-fix-pull-reques…
eranturgeman Sep 18, 2023
9155fad
fixing issues
eranturgeman Sep 18, 2023
28d7424
Merge branch 'dev' of https://github.com/jfrog/frogbot into gradle-cr…
eranturgeman Sep 19, 2023
a50a6ed
Merge branch 'dev' into gradle-create-fix-pull-request
eranturgeman Sep 19, 2023
d4a3643
Merge branch 'dev' of https://github.com/jfrog/frogbot into gradle-cr…
eranturgeman Sep 19, 2023
5ae7443
Merge branch 'gradle-create-fix-pull-request' of https://github.com/e…
eranturgeman Sep 19, 2023
9e30d4b
fixing breaks after pull
eranturgeman Sep 19, 2023
31ab71c
adding patch for latest release
eranturgeman Sep 19, 2023
a4a58f2
.
eranturgeman Sep 19, 2023
12fab53
.
eranturgeman Sep 19, 2023
dd8fe0a
optimized flow and fixed tests
eranturgeman Sep 26, 2023
cab8168
Merge branch 'dev' of https://github.com/jfrog/frogbot into gradle-cr…
eranturgeman Sep 26, 2023
7639560
all old commented- before delete
eranturgeman Sep 26, 2023
3762450
old flow deleted
eranturgeman Sep 26, 2023
7d8595e
Merge branch 'dev' of https://github.com/jfrog/frogbot into gradle-cr…
eranturgeman Sep 26, 2023
36ec474
minor fix
eranturgeman Sep 26, 2023
9569ab4
minor fix
eranturgeman Sep 26, 2023
171e193
before deleting old fix flow with precompiled regexps
eranturgeman Sep 26, 2023
def29f7
fixed pr issues
eranturgeman Sep 27, 2023
dc92966
Merge branch 'dev' of https://github.com/jfrog/frogbot into gradle-cr…
eranturgeman Sep 27, 2023
c17eba6
minor fix
eranturgeman Sep 27, 2023
435895c
minor fix
eranturgeman Sep 27, 2023
7aadf8f
fixed pr issues, fixed tests and fixed readme
eranturgeman Sep 27, 2023
bb415b2
fixed pr issues
eranturgeman Sep 27, 2023
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
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ When Frogbot detects secrets that have been inadvertently exposed within the cod

### Automatic pull requests creation
Frogbot scans your Git repositories periodically and automatically creates pull requests for upgrading vulnerable dependencies to a version with a fix.
> **_NOTE:_** Currently not supported in Gradle.

![](./images/fix-pr.png)

Expand Down
2 changes: 2 additions & 0 deletions packagehandlers/commonpackagehandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ func GetCompatiblePackageHandler(vulnDetails *utils.VulnerabilityDetails, detail
handler = &MavenPackageHandler{depsRepo: details.DepsRepo, ServerDetails: details.ServerDetails}
case coreutils.Nuget:
handler = &NugetPackageHandler{}
case coreutils.Gradle:
handler = &GradlePackageHandler{}
default:
handler = &UnsupportedPackageHandler{}
}
Expand Down
177 changes: 177 additions & 0 deletions packagehandlers/gradlepackagehandler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
package packagehandlers

import (
"fmt"
"github.com/jfrog/frogbot/utils"
"io/fs"
"os"
"path/filepath"
"regexp"
"strings"
)

const (
groovyDescriptorFileSuffix = "build.gradle"
kotlinDescriptorFileSuffix = "build.gradle.kts"
apostrophes = "[\\\"|\\']"
directMapRegexpEntry = "\\s*%s\\s*[:|=]\\s*"
directStringWithVersionFormat = "%s:%s:%s"
)

var directMapWithVersionRegexp string

func init() {
// Initializing a regexp pattern for map dependencies
// Example: group: "junit", name: "junit", version: "1.0.0" | group = "junit", name = "junit", version = "1.0.0"
directMapWithVersionRegexp = getMapRegexpEntry("group") + "," + getMapRegexpEntry("name") + "," + getMapRegexpEntry("version")
}

type GradlePackageHandler struct {
CommonPackageHandler
}

func (gph *GradlePackageHandler) UpdateDependency(vulnDetails *utils.VulnerabilityDetails) error {
if vulnDetails.IsDirectDependency {
return gph.updateDirectDependency(vulnDetails)
}

return &utils.ErrUnsupportedFix{
PackageName: vulnDetails.ImpactedDependencyName,
FixedVersion: vulnDetails.SuggestedFixedVersion,
ErrorType: utils.IndirectDependencyFixNotSupported,
}
}

func (gph *GradlePackageHandler) updateDirectDependency(vulnDetails *utils.VulnerabilityDetails) (err error) {
if !isVersionSupportedForFix(vulnDetails.ImpactedDependencyVersion) {
return &utils.ErrUnsupportedFix{
PackageName: vulnDetails.ImpactedDependencyName,
FixedVersion: vulnDetails.SuggestedFixedVersion,
ErrorType: utils.UnsupportedForFixVulnerableVersion,
}
}

eranturgeman marked this conversation as resolved.
Show resolved Hide resolved
// A gradle project may contain several descriptor files in several sub-modules. Each vulnerability may be found in each of the descriptor files.
// Therefore we iterate over every descriptor file for each vulnerability and try to find and fix it.
descriptorFilesPaths, err := getDescriptorFilesPaths()
if err != nil {
return
}

isAnyDescriptorFileChanged := false
for _, descriptorFilePath := range descriptorFilesPaths {
var isFileChanged bool
isFileChanged, err = fixVulnerabilityIfExists(descriptorFilePath, vulnDetails)
if err != nil {
return
}
// We use logical OR to save information over all descriptor files whether there is at least one file that has been changed
isAnyDescriptorFileChanged = isAnyDescriptorFileChanged || isFileChanged
}

if !isAnyDescriptorFileChanged {
err = fmt.Errorf("impacted package '%s' was not found or could not be fixed in all descriptor files", vulnDetails.ImpactedDependencyName)
}
return
}

// Checks if the impacted version is currently supported for fix
func isVersionSupportedForFix(impactedVersion string) bool {
if strings.Contains(impactedVersion, "+") ||
(strings.Contains(impactedVersion, "[") || strings.Contains(impactedVersion, "(")) ||
strings.Contains(impactedVersion, "latest.release") {
return false
}
return true
}

// Collects all descriptor files absolute paths
func getDescriptorFilesPaths() (descriptorFilesPaths []string, err error) {
err = filepath.WalkDir(".", func(path string, d fs.DirEntry, innerErr error) error {
if innerErr != nil {
return fmt.Errorf("error has occured when trying to access or traverse the files system: %s", err.Error())
}

if strings.HasSuffix(path, groovyDescriptorFileSuffix) || strings.HasSuffix(path, kotlinDescriptorFileSuffix) {
var absFilePath string
absFilePath, innerErr = filepath.Abs(path)
if innerErr != nil {
return fmt.Errorf("couldn't retrieve file's absolute path for './%s':%s", path, innerErr.Error())
}
descriptorFilesPaths = append(descriptorFilesPaths, absFilePath)
}
return nil
})
return
}

// Fixes all direct occurrences of the given vulnerability in the given descriptor file, if vulnerability occurs
func fixVulnerabilityIfExists(descriptorFilePath string, vulnDetails *utils.VulnerabilityDetails) (isFileChanged bool, err error) {
isFileChanged = false
eranturgeman marked this conversation as resolved.
Show resolved Hide resolved

byteFileContent, err := os.ReadFile(descriptorFilePath)
if err != nil {
err = fmt.Errorf("couldn't read file '%s': %s", descriptorFilePath, err.Error())
return
}
fileContent := string(byteFileContent)
originalFile := fileContent

depGroup, depName, err := getVulnerabilityGroupAndName(vulnDetails.ImpactedDependencyName)
if err != nil {
return
}

// Fixing all vulnerable rows given in a string format. For Example: implementation "junit:junit:4.7"
directStringVulnerableRow := fmt.Sprintf(directStringWithVersionFormat, depGroup, depName, vulnDetails.ImpactedDependencyVersion)
directStringFixedRow := fmt.Sprintf(directStringWithVersionFormat, depGroup, depName, vulnDetails.SuggestedFixedVersion)
fileContent = strings.ReplaceAll(fileContent, directStringVulnerableRow, directStringFixedRow)

// Fixing all vulnerable rows given in a map format. For Example: implementation group: "junit", name: "junit", version: "4.7"
mapRegexpForVulnerability := fmt.Sprintf(directMapWithVersionRegexp, depGroup, depName, vulnDetails.ImpactedDependencyVersion)
regexpCompiler := regexp.MustCompile(mapRegexpForVulnerability)
if rowsMatches := regexpCompiler.FindAllString(fileContent, -1); rowsMatches != nil {
for _, entry := range rowsMatches {
fixedRow := strings.Replace(entry, vulnDetails.ImpactedDependencyVersion, vulnDetails.SuggestedFixedVersion, 1)
fileContent = strings.ReplaceAll(fileContent, entry, fixedRow)
}
}

// If there is no changes in the file we finish dealing with the current descriptor file
if fileContent == originalFile {
return
}
isFileChanged = true

err = writeUpdatedBuildFile(descriptorFilePath, fileContent)
eranturgeman marked this conversation as resolved.
Show resolved Hide resolved
return
}

// Returns separated 'group' and 'name' for a given vulnerability name
func getVulnerabilityGroupAndName(impactedDependencyName string) (depGroup string, depName string, err error) {
seperatedImpactedDepName := strings.Split(impactedDependencyName, ":")
if len(seperatedImpactedDepName) != 2 {
err = fmt.Errorf("unable to parse impacted dependency name '%s'", impactedDependencyName)
return
}
return seperatedImpactedDepName[0], seperatedImpactedDepName[1], err
}

func getMapRegexpEntry(mapEntry string) string {
return fmt.Sprintf(directMapRegexpEntry, mapEntry) + apostrophes + "%s" + apostrophes
}

// Writes the updated content of the descriptor's file into the file
func writeUpdatedBuildFile(filePath string, fileContent string) (err error) {
fileInfo, err := os.Stat(filePath)
if err != nil {
err = fmt.Errorf("couldn't get file info for file '%s': %s", filePath, err.Error())
return
}

err = os.WriteFile(filePath, []byte(fileContent), fileInfo.Mode())
if err != nil {
err = fmt.Errorf("couldn't write fixes to file '%s': %q", filePath, err)
}
return
}
167 changes: 160 additions & 7 deletions packagehandlers/packagehandlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,50 @@ func TestUpdateDependency(t *testing.T) {
fixSupported: true,
},
},

// Gradle test cases
{
{
vulnDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "4.13.1",
IsDirectDependency: false,
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Gradle, ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ImpactedDependencyName: "commons-collections:commons-collections", ImpactedDependencyVersion: "3.2"}},
},
fixSupported: false,
},
{ // Unsupported fix: dynamic version
vulnDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "3.2.2",
IsDirectDependency: true,
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Gradle, ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ImpactedDependencyName: "commons-collections:commons-collections", ImpactedDependencyVersion: "3.+"}},
},
fixSupported: false,
},
{ // Unsupported fix: latest version
vulnDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "3.2.2",
IsDirectDependency: true,
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Gradle, ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ImpactedDependencyName: "commons-collections:commons-collections", ImpactedDependencyVersion: "latest.release"}},
},
fixSupported: false,
},
{ // Unsupported fix: range version
vulnDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "3.2.2",
IsDirectDependency: true,
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Gradle, ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ImpactedDependencyName: "commons-collections:commons-collections", ImpactedDependencyVersion: "[3.0, 3.5.6)"}},
},
fixSupported: false,
},
{
vulnDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "4.13.1",
IsDirectDependency: true,
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Gradle, ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ImpactedDependencyName: "junit:junit", ImpactedDependencyVersion: "4.7"}},
},
fixSupported: true,
},
},
}

for _, testBatch := range testCases {
Expand Down Expand Up @@ -551,13 +595,11 @@ func createTempDirAndChdir(t *testing.T, testdataDir string, tech string) func()
func assertFixVersionInPackageDescriptor(t *testing.T, test dependencyFixTest, packageDescriptor string) {
file, err := os.ReadFile(packageDescriptor)
assert.NoError(t, err)
if test.fixSupported {
assert.Contains(t, string(file), test.vulnDetails.SuggestedFixedVersion)
// Verify that case-sensitive packages in python are lowered
assert.Contains(t, string(file), strings.ToLower(test.vulnDetails.ImpactedDependencyName))
} else {
assert.NotContains(t, string(file), test.vulnDetails)
}

assert.Contains(t, string(file), test.vulnDetails.SuggestedFixedVersion)
// Verify that case-sensitive packages in python are lowered
assert.Contains(t, string(file), strings.ToLower(test.vulnDetails.ImpactedDependencyName))

}

// This function is intended to add unique checks for specific package managers
Expand All @@ -568,6 +610,13 @@ func uniquePackageManagerChecks(t *testing.T, test dependencyFixTest) {
case coreutils.Go:
packageDescriptor := extraArgs[0]
assertFixVersionInPackageDescriptor(t, test, packageDescriptor)
case coreutils.Gradle:
descriptorFilesPaths, err := getDescriptorFilesPaths()
assert.NoError(t, err)
assert.Equal(t, len(descriptorFilesPaths), 2, "incorrect number of descriptor files found")
for _, packageDescriptor := range descriptorFilesPaths {
assertFixVersionInPackageDescriptor(t, test, packageDescriptor)
}
default:
}
}
Expand Down Expand Up @@ -598,3 +647,107 @@ func TestGetFixedPackage(t *testing.T) {
assert.Equal(t, test.expectedOutput, fixedPackageArgs)
}
}

func TestGradleGetDescriptorFilesPaths(t *testing.T) {
currDir, err := os.Getwd()
assert.NoError(t, err)
tmpDir, err := os.MkdirTemp("", "")
assert.NoError(t, err)
assert.NoError(t, biutils.CopyDir(filepath.Join("..", "testdata", "projects", "gradle"), tmpDir, true, nil))
assert.NoError(t, os.Chdir(tmpDir))
defer func() {
assert.NoError(t, os.Chdir(currDir))
}()
finalPath, err := os.Getwd()
assert.NoError(t, err)

expectedResults := []string{filepath.Join(finalPath, groovyDescriptorFileSuffix), filepath.Join(finalPath, "innerProjectForTest", kotlinDescriptorFileSuffix)}

buildFilesPaths, err := getDescriptorFilesPaths()
assert.NoError(t, err)
assert.ElementsMatch(t, expectedResults, buildFilesPaths)
}

func TestGradleFixVulnerabilityIfExists(t *testing.T) {
currDir, err := os.Getwd()
assert.NoError(t, err)

tmpDir, err := os.MkdirTemp("", "")
assert.NoError(t, err)
assert.NoError(t, biutils.CopyDir(filepath.Join("..", "testdata", "projects", "gradle"), tmpDir, true, nil))
assert.NoError(t, os.Chdir(tmpDir))
defer func() {
assert.NoError(t, os.Chdir(currDir))
}()

descriptorFiles, err := getDescriptorFilesPaths()
assert.NoError(t, err)

vulnerabilityDetails := &utils.VulnerabilityDetails{
SuggestedFixedVersion: "4.13.1",
IsDirectDependency: true,
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Gradle, ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ImpactedDependencyName: "junit:junit", ImpactedDependencyVersion: "4.7"}}}

for _, descriptorFile := range descriptorFiles {
var isFileChanged bool
isFileChanged, err = fixVulnerabilityIfExists(descriptorFile, vulnerabilityDetails)
assert.NoError(t, err)
assert.True(t, isFileChanged)
compareFixedFileToComparisonFile(t, descriptorFile)
}
}

func compareFixedFileToComparisonFile(t *testing.T, descriptorFileAbsPath string) {
var compareFilePath string
if strings.HasSuffix(descriptorFileAbsPath, groovyDescriptorFileSuffix) {
curDirPath := strings.TrimSuffix(descriptorFileAbsPath, groovyDescriptorFileSuffix)
compareFilePath = filepath.Join(curDirPath, "fixedBuildGradleForCompare.txt")
} else {
curDirPath := strings.TrimSuffix(descriptorFileAbsPath, kotlinDescriptorFileSuffix)
compareFilePath = filepath.Join(curDirPath, "fixedBuildGradleKtsForCompare.txt")
}

expectedFileContent, err := os.ReadFile(descriptorFileAbsPath)
assert.NoError(t, err)

fixedFileContent, err := os.ReadFile(compareFilePath)
assert.NoError(t, err)

assert.ElementsMatch(t, expectedFileContent, fixedFileContent)
}

func TestGradleIsVersionSupportedForFix(t *testing.T) {
var testcases = []struct {
impactedVersion string
expectedResult bool
}{
{
impactedVersion: "10.+",
expectedResult: false,
},
{
impactedVersion: "[10.3, 11.0)",
expectedResult: false,
},
{
impactedVersion: "(10.4.2, 11.7.8)",
expectedResult: false,
},
{
impactedVersion: "latest.release",
expectedResult: false,
},
{
impactedVersion: "5.5",
expectedResult: true,
},
{
impactedVersion: "9.0.13-beta",
expectedResult: true,
},
}

for _, testcase := range testcases {
assert.Equal(t, testcase.expectedResult, isVersionSupportedForFix(testcase.impactedVersion))
}
}
Loading
Loading