diff --git a/packageupdaters/commonpackageupdater.go b/packageupdaters/commonpackageupdater.go index 37cccf10a..53e8b02ee 100644 --- a/packageupdaters/commonpackageupdater.go +++ b/packageupdaters/commonpackageupdater.go @@ -10,7 +10,6 @@ import ( "github.com/jfrog/frogbot/v2/utils" "github.com/jfrog/gofrog/datastructures" - "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-security/utils/techutils" "github.com/jfrog/jfrog-client-go/utils/log" "golang.org/x/exp/slices" @@ -51,11 +50,8 @@ func GetCompatiblePackageUpdater(vulnDetails *utils.VulnerabilityDetails, detail return } -// TODO delete serverDetails and depsRepo after refactoring all package handlers if they are no longer needed -type CommonPackageUpdater struct { - serverDetails *config.ServerDetails - depsRepo string -} +// TODO can be deleted if not needed after refactoring all package updaters +type CommonPackageUpdater struct{} // UpdateDependency updates the impacted package to the fixed version func (cph *CommonPackageUpdater) UpdateDependency(vulnDetails *utils.VulnerabilityDetails, installationCommand string, extraArgs ...string) (err error) { diff --git a/packageupdaters/commonpackageupdater_test.go b/packageupdaters/commonpackageupdater_test.go index 5ff157052..1aa889b38 100644 --- a/packageupdaters/commonpackageupdater_test.go +++ b/packageupdaters/commonpackageupdater_test.go @@ -34,8 +34,7 @@ type dependencyFixTest struct { } const ( - requirementsFile = "oslo.config>=1.12.1,<1.13\noslo.utils<5.0,>=4.0.0\nparamiko==2.7.2\npasslib<=1.7.4\nprance>=0.9.0\nprompt-toolkit~=1.0.15\npyinotify>0.9.6\nPyJWT>1.7.1\nurllib3 > 1.1.9, < 1.5.*" - GoPackageDescriptor = "go.mod" + requirementsFile = "oslo.config>=1.12.1,<1.13\noslo.utils<5.0,>=4.0.0\nparamiko==2.7.2\npasslib<=1.7.4\nprance>=0.9.0\nprompt-toolkit~=1.0.15\npyinotify>0.9.6\nPyJWT>1.7.1\nurllib3 > 1.1.9, < 1.5.*" ) type pipPackageRegexTest struct { @@ -55,22 +54,25 @@ func TestUpdateDependency(t *testing.T) { // Go test cases { { - vulnDetails: createVulnerabilityDetails(techutils.Go, "golang.org/x/crypto", "", "0.0.0-20201216223049-8b5274cf687f", false, ""), - scanDetails: scanDetails, - fixSupported: true, - descriptorsToCheck: []string{GoPackageDescriptor}, + vulnDetails: createVulnerabilityDetails(techutils.Go, "golang.org/x/crypto", "", "0.0.0-20201216223049-8b5274cf687f", false, "go.mod"), + scanDetails: scanDetails, + fixSupported: true, + descriptorsToCheck: []string{"go.mod"}, + lockFileToVerifyItsChange: "go.sum", }, { - vulnDetails: createVulnerabilityDetails(techutils.Go, "github.com/gin-gonic/gin", "", "1.7.7", true, ""), - scanDetails: scanDetails, - fixSupported: true, - descriptorsToCheck: []string{GoPackageDescriptor}, + vulnDetails: createVulnerabilityDetails(techutils.Go, "github.com/google/uuid", "", "1.3.0", true, "go.mod"), + scanDetails: scanDetails, + fixSupported: true, + descriptorsToCheck: []string{"go.mod"}, + lockFileToVerifyItsChange: "go.sum", }, { - vulnDetails: createVulnerabilityDetails(techutils.Go, "github.com/google/uuid", "", "1.3.0", true, ""), - scanDetails: scanDetails, - fixSupported: true, - descriptorsToCheck: []string{GoPackageDescriptor}, + testcaseInfo: "no-location-evidence", + vulnDetails: createVulnerabilityDetails(techutils.Go, "github.com/google/uuid", "", "1.3.0", true), + scanDetails: scanDetails, + fixSupported: true, + errorExpected: true, }, }, diff --git a/packageupdaters/gopackageupdater.go b/packageupdaters/gopackageupdater.go index f30ed3baf..6b48c30ab 100644 --- a/packageupdaters/gopackageupdater.go +++ b/packageupdaters/gopackageupdater.go @@ -1,21 +1,225 @@ package packageupdaters import ( + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "github.com/jfrog/frogbot/v2/utils" - golangutils "github.com/jfrog/jfrog-cli-artifactory/artifactory/commands/golang" + "github.com/jfrog/jfrog-client-go/utils/log" +) + +const ( + goFlagModEditEnv = "GOFLAGS=-mod=mod" + goWorkOffEnv = "GOWORK=off" + goModFileName = "go.mod" + goSumFileName = "go.sum" + goVendorDirName = "vendor" + goTidyContinueOnError = "-e" ) -type GoPackageUpdater struct { - CommonPackageUpdater +type GoPackageUpdater struct{} + +type goModuleBackup struct { + goModPath string + goModContent []byte + goSumPath string + goSumContent []byte +} + +func (gpu *GoPackageUpdater) UpdateDependency(vulnDetails *utils.VulnerabilityDetails) error { + descriptorPaths := GetVulnerabilityLocations(vulnDetails, []string{goModFileName}, []string{goVendorDirName}) + if len(descriptorPaths) == 0 { + return fmt.Errorf("no descriptor evidence was found for package %s", vulnDetails.ImpactedDependencyName) + } + + originalWd, err := os.Getwd() + if err != nil { + return fmt.Errorf("failed to get current working directory: %w", err) + } + + env := gpu.buildGoCommandEnv() + + var failingDescriptors []string + for _, descriptorPath := range descriptorPaths { + if fixErr := gpu.fixVulnerabilityAndTidy(vulnDetails, descriptorPath, originalWd, env); fixErr != nil { + failedFixErrorMsg := fmt.Errorf("failed to fix '%s' in descriptor '%s': %w", vulnDetails.ImpactedDependencyName, descriptorPath, fixErr) + log.Warn(failedFixErrorMsg.Error()) + err = errors.Join(err, failedFixErrorMsg) + failingDescriptors = append(failingDescriptors, descriptorPath) + } + } + if err != nil { + return fmt.Errorf("encountered errors while fixing '%s' vulnerability in descriptors [%s]: %w", vulnDetails.ImpactedDependencyName, strings.Join(failingDescriptors, ", "), err) + } + + return nil +} + +func (gpu *GoPackageUpdater) fixVulnerabilityAndTidy(vulnDetails *utils.VulnerabilityDetails, descriptorPath, originalWd string, env []string) (err error) { + backup, backupErr := gpu.backupModuleFiles(descriptorPath) + if backupErr != nil { + return backupErr + } + + descriptorDir := filepath.Dir(descriptorPath) + if err = os.Chdir(descriptorDir); err != nil { + return fmt.Errorf("failed to change directory to '%s': %w", descriptorDir, err) + } + defer func() { + if chErr := os.Chdir(originalWd); chErr != nil { + err = errors.Join(err, fmt.Errorf("failed to return to original directory: %w", chErr)) + } + }() + + if err = gpu.updateDependency(vulnDetails, env); err != nil { + log.Warn(fmt.Sprintf("Failed to update '%s' to version '%s': %s. Rolling back...", vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion, err.Error())) + if rollbackErr := gpu.restoreModuleFiles(backup); rollbackErr != nil { + return fmt.Errorf("failed to rollback module files after go get failure: %w (original error: %v)", rollbackErr, err) + } + return err + } + + lockFileTracked, checkErr := utils.IsFileTrackedByGit(backup.goSumPath, originalWd) + if checkErr != nil { + log.Debug(fmt.Sprintf("Failed to check if lock file is tracked in git: %s. Proceeding with lock file regeneration.", checkErr.Error())) + lockFileTracked = true + } + + if !lockFileTracked { + log.Debug(fmt.Sprintf("Lock file '%s' is not tracked in git, skipping lock file regeneration", backup.goSumPath)) + return nil + } + + if err = gpu.tidyLockFiles(descriptorDir, env); err != nil { + log.Warn(fmt.Sprintf("Failed to tidy module files after updating '%s' to version '%s': %s. Rolling back...", vulnDetails.ImpactedDependencyName, vulnDetails.SuggestedFixedVersion, err.Error())) + if rollbackErr := gpu.restoreModuleFiles(backup); rollbackErr != nil { + return fmt.Errorf("failed to rollback module files after tidy failure: %w (original error: %v)", rollbackErr, err) + } + return err + } + + log.Debug(fmt.Sprintf("Successfully updated '%s' from version '%s' to '%s' in descriptor '%s'", vulnDetails.ImpactedDependencyName, vulnDetails.ImpactedDependencyVersion, vulnDetails.SuggestedFixedVersion, descriptorPath)) + return nil } -func (golang *GoPackageUpdater) UpdateDependency(vulnDetails *utils.VulnerabilityDetails) error { - // Configure resolution from an Artifactory server if needed - if golang.depsRepo != "" { - if err := golangutils.SetArtifactoryAsResolutionServer(golang.serverDetails, golang.depsRepo, golangutils.GoProxyUrlParams{}); err != nil { +func (gpu *GoPackageUpdater) buildGoCommandEnv() []string { + return append(os.Environ(), goFlagModEditEnv, goWorkOffEnv) +} + +func (gpu *GoPackageUpdater) backupModuleFiles(goModPath string) (*goModuleBackup, error) { + goModContent, err := os.ReadFile(goModPath) + if err != nil { + return nil, fmt.Errorf("failed to read '%s': %w", goModPath, err) + } + + // We assume go.sum resides under the same directory as go.mod + descriptorDir := filepath.Dir(goModPath) + goSumPath := filepath.Join(descriptorDir, goSumFileName) + goSumContent, err := os.ReadFile(goSumPath) + if err != nil { + return nil, fmt.Errorf("failed to read '%s': %w", goSumPath, err) + } + + backup := &goModuleBackup{ + goModPath: goModPath, + goModContent: make([]byte, len(goModContent)), + goSumPath: goSumPath, + goSumContent: make([]byte, len(goSumContent)), + } + copy(backup.goModContent, goModContent) + copy(backup.goSumContent, goSumContent) + + return backup, nil +} + +func (gpu *GoPackageUpdater) restoreModuleFiles(backup *goModuleBackup) error { + if err := os.WriteFile(backup.goModPath, backup.goModContent, 0644); err != nil { + return fmt.Errorf("failed to restore '%s': %w", backup.goModPath, err) + } + if err := os.WriteFile(backup.goSumPath, backup.goSumContent, 0644); err != nil { + return fmt.Errorf("failed to restore '%s': %w", backup.goSumPath, err) + } + log.Debug(fmt.Sprintf("Successfully rolled back '%s' and '%s' to original state", backup.goModPath, backup.goSumPath)) + return nil +} + +func (gpu *GoPackageUpdater) updateDependency(vulnDetails *utils.VulnerabilityDetails, env []string) error { + impactedPackage := strings.ToLower(vulnDetails.ImpactedDependencyName) + fixedVersion := strings.TrimSpace(vulnDetails.SuggestedFixedVersion) + + if !strings.HasPrefix(fixedVersion, "v") { + fixedVersion = "v" + fixedVersion + } + fixedPackage := strings.TrimSpace(impactedPackage) + "@" + fixedVersion + + cmd := exec.Command("go", "get", fixedPackage) + cmd.Env = env + log.Debug(fmt.Sprintf("Running 'go get %s'", fixedPackage)) + + //#nosec G204 -- False positive - the subprocess only runs after the user's approval. + output, err := cmd.CombinedOutput() + if len(output) > 0 { + log.Debug(fmt.Sprintf("go get output:\n%s", string(output))) + } + + if err != nil { + return fmt.Errorf("go get failed: %s\n%s", err.Error(), output) + } + return nil +} + +func (gpu *GoPackageUpdater) tidyLockFiles(descriptorDir string, env []string) error { + cmd := exec.Command("go", "mod", "tidy", goTidyContinueOnError) + cmd.Env = env + log.Debug("Running 'go mod tidy'") + + //#nosec G204 -- False positive - the subprocess only runs after the user's approval. + output, err := cmd.CombinedOutput() + if len(output) > 0 { + log.Debug(fmt.Sprintf("go mod tidy output:\n%s", string(output))) + } + + if err != nil { + return fmt.Errorf("go mod tidy failed: %s\n%s", err.Error(), output) + } + + if gpu.hasVendorDirectory(descriptorDir) { + if err := gpu.updateVendor(env); err != nil { return err } } - // In Golang, we can address every dependency as a direct dependency. - return golang.CommonPackageUpdater.UpdateDependency(vulnDetails, vulnDetails.Technology.GetPackageInstallationCommand()) + + return nil +} + +func (gpu *GoPackageUpdater) hasVendorDirectory(descriptorDir string) bool { + vendorModulesPath := filepath.Join(descriptorDir, goVendorDirName, "modules.txt") + if _, err := os.Stat(vendorModulesPath); err == nil { + log.Debug(fmt.Sprintf("Detected vendor directory at: %s", vendorModulesPath)) + return true + } + return false +} + +func (gpu *GoPackageUpdater) updateVendor(env []string) error { + vendorCmd := exec.Command("go", "mod", "vendor") + vendorCmd.Env = env + log.Debug("Running 'go mod vendor' to update vendored dependencies") + + //#nosec G204 -- False positive - the subprocess only runs after the user's approval. + vendorOutput, err := vendorCmd.CombinedOutput() + if len(vendorOutput) > 0 { + log.Debug(fmt.Sprintf("go mod vendor output:\n%s", string(vendorOutput))) + } + + if err != nil { + return fmt.Errorf("go mod vendor failed: %s\n%s", err.Error(), vendorOutput) + } + + log.Debug("Successfully updated vendor directory") + return nil } diff --git a/packageupdaters/gopackageupdater_test.go b/packageupdaters/gopackageupdater_test.go new file mode 100644 index 000000000..9a51cbeb7 --- /dev/null +++ b/packageupdaters/gopackageupdater_test.go @@ -0,0 +1,99 @@ +package packageupdaters + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBackupModuleFiles(t *testing.T) { + testcases := []struct { + name string + goModContent []byte + goSumContent []byte + modifiedGoMod []byte + modifiedGoSum []byte + }{ + { + name: "backup and restore after files were modified", + goModContent: []byte("module example.com/test\n\ngo 1.21\n\nrequire github.com/some/pkg v1.0.0\n"), + goSumContent: []byte("github.com/some/pkg v1.0.0 h1:abc123=\ngithub.com/some/pkg v1.0.0/go.mod h1:def456=\n"), + modifiedGoMod: []byte("module example.com/test\n\ngo 1.21\n\nrequire github.com/some/pkg v2.0.0\n"), + modifiedGoSum: []byte("github.com/some/pkg v2.0.0 h1:xyz789=\n"), + }, + { + name: "backup preserves empty go.sum", + goModContent: []byte("module example.com/test\n\ngo 1.21\n"), + goSumContent: []byte(""), + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + tmpDir := t.TempDir() + goModPath := filepath.Join(tmpDir, goModFileName) + goSumPath := filepath.Join(tmpDir, goSumFileName) + assert.NoError(t, os.WriteFile(goModPath, tc.goModContent, 0644)) + assert.NoError(t, os.WriteFile(goSumPath, tc.goSumContent, 0644)) + + gpu := &GoPackageUpdater{} + backup, err := gpu.backupModuleFiles(goModPath) + assert.NoError(t, err) + assert.Equal(t, tc.goModContent, backup.goModContent) + assert.Equal(t, tc.goSumContent, backup.goSumContent) + assert.Equal(t, goModPath, backup.goModPath) + assert.Equal(t, goSumPath, backup.goSumPath) + + if tc.modifiedGoMod != nil { + assert.NoError(t, os.WriteFile(goModPath, tc.modifiedGoMod, 0644)) + assert.NoError(t, os.WriteFile(goSumPath, tc.modifiedGoSum, 0644)) + + assert.NoError(t, gpu.restoreModuleFiles(backup)) + + restoredGoMod, err := os.ReadFile(goModPath) + assert.NoError(t, err) + assert.Equal(t, tc.goModContent, restoredGoMod) + + restoredGoSum, err := os.ReadFile(goSumPath) + assert.NoError(t, err) + assert.Equal(t, tc.goSumContent, restoredGoSum) + } + }) + } +} + +func TestHasVendorDirectory(t *testing.T) { + testcases := []struct { + name string + setupVendor bool + expectedResult bool + }{ + { + name: "vendor directory with modules.txt", + setupVendor: true, + expectedResult: true, + }, + { + name: "no vendor directory", + setupVendor: false, + expectedResult: false, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + tmpDir := t.TempDir() + + if tc.setupVendor { + vendorDir := filepath.Join(tmpDir, goVendorDirName) + assert.NoError(t, os.MkdirAll(vendorDir, 0755)) + assert.NoError(t, os.WriteFile(filepath.Join(vendorDir, "modules.txt"), []byte("# vendor modules\n"), 0644)) + } + + gpu := &GoPackageUpdater{} + assert.Equal(t, tc.expectedResult, gpu.hasVendorDirectory(tmpDir)) + }) + } +} diff --git a/packageupdaters/npmpackageupdater.go b/packageupdaters/npmpackageupdater.go index a89cbd293..d8d0a1f25 100644 --- a/packageupdaters/npmpackageupdater.go +++ b/packageupdaters/npmpackageupdater.go @@ -52,9 +52,7 @@ var npmInstallEnvVars = map[string]string{ ciEnv: "true", } -type NpmPackageUpdater struct { - CommonPackageUpdater -} +type NpmPackageUpdater struct{} func (npm *NpmPackageUpdater) UpdateDependency(vulnDetails *utils.VulnerabilityDetails) error { if vulnDetails.IsDirectDependency { @@ -95,7 +93,7 @@ func (npm *NpmPackageUpdater) updateDirectDependency(vulnDetails *utils.Vulnerab } func (npm *NpmPackageUpdater) fixVulnerabilityAndRegenerateLock(vulnDetails *utils.VulnerabilityDetails, descriptorPath string, originalWd string) error { - backupContent, err := npm.updateDescriptor(vulnDetails, descriptorPath) + backupContent, err := npm.updateDependency(vulnDetails, descriptorPath) if err != nil { return err } @@ -123,7 +121,7 @@ func (npm *NpmPackageUpdater) fixVulnerabilityAndRegenerateLock(vulnDetails *uti return nil } -func (npm *NpmPackageUpdater) updateDescriptor(vulnDetails *utils.VulnerabilityDetails, descriptorPath string) ([]byte, error) { +func (npm *NpmPackageUpdater) updateDependency(vulnDetails *utils.VulnerabilityDetails, descriptorPath string) ([]byte, error) { descriptorContent, err := os.ReadFile(descriptorPath) if err != nil { return nil, fmt.Errorf("failed to read file '%s': %w", descriptorPath, err) diff --git a/scanrepository/scanrepository.go b/scanrepository/scanrepository.go index b509e11a4..aca15f7fc 100644 --- a/scanrepository/scanrepository.go +++ b/scanrepository/scanrepository.go @@ -40,6 +40,7 @@ const ( // TODO add to this map every tech that we support fixing in V3 var supportedAutoFixTechnologies = []techutils.Technology{ techutils.Npm, + techutils.Go, } type ScanRepositoryCmd struct {