From 30a8b689eaa07099ce45c009f068542035164fe6 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 14 Jun 2026 10:25:22 +0300 Subject: [PATCH 01/10] Move package updaters from Frogbot into jfrog-cli-security Relocates the package updater implementations (Go, Maven, Npm, Pnpm, Pip/Poetry/Pipenv) from the Frogbot repository into utils/remediation/packageupdaters/ so they can be shared by both Frogbot and the auto-fix action. Introduces a lean FixDetails struct and a GetCompatiblePackageUpdater factory. Removes unsupported technologies (Yarn, Gradle, Nuget, Conan). Moves test files and testdata alongside the implementation. Co-Authored-By: Claude Sonnet 4.6 --- go.mod | 4 + go.sum | 9 + .../packageupdaters/commonpackageupdater.go | 308 ++++++ .../commonpackageupdater_test.go | 979 ++++++++++++++++++ .../packageupdaters/gopackageupdater.go | 231 +++++ .../packageupdaters/gopackageupdater_test.go | 99 ++ .../packageupdaters/mavenpackageupdater.go | 199 ++++ .../mavenpackageupdater_test.go | 212 ++++ .../packageupdaters/npmpackageupdater.go | 190 ++++ .../packageupdaters/npmpackageupdater_test.go | 245 +++++ .../packageupdaters/pnpmpackageupdater.go | 156 +++ .../pnpmpackageupdater_test.go | 72 ++ .../packageupdaters/pythonpackageupdater.go | 116 +++ .../pythonpackageupdater_test.go | 30 + utils/remediation/packageupdaters/types.go | 48 + .../testdata/indirect-projects/go/go.mod.txt | 22 + .../testdata/indirect-projects/go/go.sum.txt | 33 + .../testdata/indirect-projects/go/main.go.txt | 10 + .../testdata/indirect-projects/maven/pom.xml | 67 ++ .../indirect-projects/pip/requirements.txt | 1 + .../testdata/indirect-projects/pip/setup.py | 9 + .../testdata/indirect-projects/pipenv/Pipfile | 12 + .../indirect-projects/pipenv/Pipfile.lock | 59 ++ .../indirect-projects/poetry/pyproject.toml | 13 + .../testdata/packageupdaters/pom.xml | 63 ++ .../testdata/projects/go/go.mod.txt | 13 + .../testdata/projects/go/go.sum.txt | 13 + .../testdata/projects/go/main.go.txt | 13 + .../testdata/projects/maven/multi1/pom.xml | 75 ++ .../testdata/projects/maven/pom.xml | 71 ++ .../noIssuesProject/package-lock.json | 13 + .../projects/noIssuesProject/package.json | 11 + .../projects/npm-rollback/package-lock.json | 24 + .../projects/npm-rollback/package.json | 16 + .../testdata/projects/npm/package-lock.json | 22 + .../testdata/projects/npm/package.json | 14 + .../testdata/projects/pip/requirements.txt | 2 + .../testdata/projects/pip/setup.py | 10 + .../testdata/projects/pipenv/Pipfile | 14 + .../testdata/projects/pipenv/Pipfile.lock | 52 + .../testdata/projects/poetry/pyproject.toml | 15 + 41 files changed, 3565 insertions(+) create mode 100644 utils/remediation/packageupdaters/commonpackageupdater.go create mode 100644 utils/remediation/packageupdaters/commonpackageupdater_test.go create mode 100644 utils/remediation/packageupdaters/gopackageupdater.go create mode 100644 utils/remediation/packageupdaters/gopackageupdater_test.go create mode 100644 utils/remediation/packageupdaters/mavenpackageupdater.go create mode 100644 utils/remediation/packageupdaters/mavenpackageupdater_test.go create mode 100644 utils/remediation/packageupdaters/npmpackageupdater.go create mode 100644 utils/remediation/packageupdaters/npmpackageupdater_test.go create mode 100644 utils/remediation/packageupdaters/pnpmpackageupdater.go create mode 100644 utils/remediation/packageupdaters/pnpmpackageupdater_test.go create mode 100644 utils/remediation/packageupdaters/pythonpackageupdater.go create mode 100644 utils/remediation/packageupdaters/pythonpackageupdater_test.go create mode 100644 utils/remediation/packageupdaters/types.go create mode 100755 utils/remediation/testdata/indirect-projects/go/go.mod.txt create mode 100755 utils/remediation/testdata/indirect-projects/go/go.sum.txt create mode 100755 utils/remediation/testdata/indirect-projects/go/main.go.txt create mode 100755 utils/remediation/testdata/indirect-projects/maven/pom.xml create mode 100755 utils/remediation/testdata/indirect-projects/pip/requirements.txt create mode 100755 utils/remediation/testdata/indirect-projects/pip/setup.py create mode 100755 utils/remediation/testdata/indirect-projects/pipenv/Pipfile create mode 100755 utils/remediation/testdata/indirect-projects/pipenv/Pipfile.lock create mode 100755 utils/remediation/testdata/indirect-projects/poetry/pyproject.toml create mode 100755 utils/remediation/testdata/packageupdaters/pom.xml create mode 100755 utils/remediation/testdata/projects/go/go.mod.txt create mode 100755 utils/remediation/testdata/projects/go/go.sum.txt create mode 100755 utils/remediation/testdata/projects/go/main.go.txt create mode 100755 utils/remediation/testdata/projects/maven/multi1/pom.xml create mode 100755 utils/remediation/testdata/projects/maven/pom.xml create mode 100644 utils/remediation/testdata/projects/noIssuesProject/package-lock.json create mode 100755 utils/remediation/testdata/projects/noIssuesProject/package.json create mode 100644 utils/remediation/testdata/projects/npm-rollback/package-lock.json create mode 100644 utils/remediation/testdata/projects/npm-rollback/package.json create mode 100644 utils/remediation/testdata/projects/npm/package-lock.json create mode 100755 utils/remediation/testdata/projects/npm/package.json create mode 100755 utils/remediation/testdata/projects/pip/requirements.txt create mode 100755 utils/remediation/testdata/projects/pip/setup.py create mode 100755 utils/remediation/testdata/projects/pipenv/Pipfile create mode 100755 utils/remediation/testdata/projects/pipenv/Pipfile.lock create mode 100755 utils/remediation/testdata/projects/poetry/pyproject.toml diff --git a/go.mod b/go.mod index 2363da76e..64a7f191a 100644 --- a/go.mod +++ b/go.mod @@ -124,6 +124,10 @@ require ( github.com/spf13/pflag v1.0.10 // indirect github.com/spf13/viper v1.21.0 // indirect github.com/subosito/gotenv v1.6.0 // indirect + github.com/tidwall/gjson v1.18.0 // indirect + github.com/tidwall/match v1.1.1 // indirect + github.com/tidwall/pretty v1.2.0 // indirect + github.com/tidwall/sjson v1.2.5 // indirect github.com/ulikunitz/xz v0.5.15 // indirect github.com/vbatts/tar-split v0.12.2 // indirect github.com/vbauerster/mpb/v8 v8.12.1 // indirect diff --git a/go.sum b/go.sum index 16d12a4c0..281b1a872 100644 --- a/go.sum +++ b/go.sum @@ -304,6 +304,15 @@ github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8 github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= github.com/terminalstatic/go-xsd-validate v0.1.6 h1:TenYeQ3eY631qNi1/cTmLH/s2slHPRKTTHT+XSHkepo= github.com/terminalstatic/go-xsd-validate v0.1.6/go.mod h1:18lsvYFofBflqCrvo1umpABZ99+GneNTw2kEEc8UPJw= +github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= +github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY= +github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= +github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= +github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= +github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs= +github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= +github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= +github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= github.com/ulikunitz/xz v0.5.8/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= github.com/ulikunitz/xz v0.5.15 h1:9DNdB5s+SgV3bQ2ApL10xRc35ck0DuIX/isZvIk+ubY= github.com/ulikunitz/xz v0.5.15/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= diff --git a/utils/remediation/packageupdaters/commonpackageupdater.go b/utils/remediation/packageupdaters/commonpackageupdater.go new file mode 100644 index 000000000..fa0347892 --- /dev/null +++ b/utils/remediation/packageupdaters/commonpackageupdater.go @@ -0,0 +1,308 @@ +package packageupdaters + +import ( + "errors" + "fmt" + "io/fs" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + "time" + + git "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing/object" + "github.com/jfrog/gofrog/datastructures" + "github.com/jfrog/jfrog-cli-security/utils/techutils" + "github.com/jfrog/jfrog-client-go/utils/log" + "github.com/tidwall/gjson" + "github.com/tidwall/sjson" + "golang.org/x/exp/slices" +) + +const ( + // NodePackageJSONFileName is the file name of the Node.js package manifest. + NodePackageJSONFileName = "package.json" + // NodeModulesDirName is the Node.js modules directory name. + NodeModulesDirName = "node_modules" + nodePackageJSONFileName = NodePackageJSONFileName + nodeModulesDirName = NodeModulesDirName + nodeDependenciesSection = "dependencies" + nodeDevDependenciesSection = "devDependencies" + nodeOptionalDependenciesSection = "optionalDependencies" + nodeOverridesSection = "overrides" + nodePackageManagerInstallTimeout = 15 * time.Minute +) + +var nodePackageManifestSections = []string{ + nodeDependenciesSection, + nodeDevDependenciesSection, + nodeOptionalDependenciesSection, + nodeOverridesSection, +} + +// SupportedFixTechnologies lists the technologies for which automatic dependency +// fixing is supported. +var SupportedFixTechnologies = []techutils.Technology{ + techutils.Npm, + techutils.Maven, + techutils.Pip, + techutils.Go, + techutils.Pnpm, +} + +func GetCompatiblePackageUpdater(fixDetails *FixDetails) (PackageUpdater, bool) { + switch fixDetails.Technology { + case techutils.Go: + return &GoPackageUpdater{}, true + case techutils.Pip: + return &PythonPackageUpdater{pipRequirementsFile: defaultRequirementFile}, true + case techutils.Npm: + return &NpmPackageUpdater{}, true + case techutils.Maven: + return &MavenPackageUpdater{}, true + case techutils.Pnpm: + return &PnpmPackageUpdater{}, true + default: + return nil, false + } +} + +type CommonPackageUpdater struct{} + +func EvidencePathLooksLikeNpmPackageCoordinate(evidenceFile string) bool { + dir := filepath.Dir(evidenceFile) + if dir == "." || dir == "" { + return false + } + for _, part := range strings.Split(filepath.ToSlash(dir), "/") { + if part == "" || part == "." { + continue + } + if strings.Contains(part, "@") && !strings.HasPrefix(part, "@") { + return true + } + } + return false +} + +func (cph *CommonPackageUpdater) CollectVulnerabilityDescriptorPaths(fixDetails *FixDetails, namesFilters []string, ignoreFilters []string) []string { + pathsSet := datastructures.MakeSet[string]() + for _, component := range fixDetails.Components { + for _, evidence := range component.Evidences { + if evidence.File == "" || techutils.IsTechnologyDescriptor(evidence.File) == techutils.NoTech || slices.ContainsFunc(ignoreFilters, func(pattern string) bool { return strings.Contains(evidence.File, pattern) }) { + continue + } + if len(namesFilters) == 0 || slices.Contains(namesFilters, filepath.Base(evidence.File)) { + pathsSet.Add(evidence.File) + } + } + } + return pathsSet.ToSlice() +} + +func (cph *CommonPackageUpdater) BuildPackageDependencyLineRegex(impactedName, impactedVersion, dependencyLineFormat string) *regexp.Regexp { + regexpFitImpactedName := strings.ToLower(regexp.QuoteMeta(impactedName)) + regexpFitImpactedVersion := strings.ToLower(regexp.QuoteMeta(impactedVersion)) + regexpCompleteFormat := fmt.Sprintf(strings.ToLower(dependencyLineFormat), regexpFitImpactedName, regexpFitImpactedVersion) + return regexp.MustCompile(regexpCompleteFormat) +} + +func EscapeJsonPathKey(key string) string { + r := strings.NewReplacer(".", "\\.", "*", "\\*", "?", "\\?") + return r.Replace(key) +} + +func (cph *CommonPackageUpdater) GetFixedPackageJSONManifest(content []byte, packageName, newVersion, descriptorPath string) ([]byte, error) { + updated := false + escapedName := EscapeJsonPathKey(packageName) + + for _, section := range nodePackageManifestSections { + path := section + "." + escapedName + if gjson.GetBytes(content, path).Exists() { + var err error + content, err = sjson.SetBytes(content, path, newVersion) + if err != nil { + return nil, fmt.Errorf("failed to set version for '%s' in section '%s': %w", packageName, section, err) + } + updated = true + } + } + + if !updated { + return nil, fmt.Errorf("package '%s' not found in allowed sections [%s] in '%s'", packageName, strings.Join(nodePackageManifestSections, ", "), descriptorPath) + } + return content, nil +} + +func (cph *CommonPackageUpdater) UpdatePackageJSONDescriptor(descriptorPath, packageName, newVersion string) ([]byte, error) { + descriptorContent, err := os.ReadFile(descriptorPath) + if err != nil { + return nil, fmt.Errorf("failed to read file '%s': %w", descriptorPath, err) + } + + backupContent := make([]byte, len(descriptorContent)) + copy(backupContent, descriptorContent) + + updatedContent, err := cph.GetFixedPackageJSONManifest(descriptorContent, packageName, newVersion, descriptorPath) + if err != nil { + return nil, fmt.Errorf("failed to update version in descriptor: %w", err) + } + + if err = os.WriteFile(descriptorPath, updatedContent, 0644); err != nil { + return nil, fmt.Errorf("failed to write updated descriptor '%s': %w", descriptorPath, err) + } + return backupContent, nil +} + +func (cph *CommonPackageUpdater) withDescriptorWorkingDir(descriptorPath, originalWd string, fn func() error) (err error) { + 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)) + } + }() + return fn() +} + +func (cph *CommonPackageUpdater) BuildEnvWithOverrides(overrides map[string]string) []string { + env := make([]string, 0, len(os.Environ())+len(overrides)) + for _, e := range os.Environ() { + key := strings.SplitN(e, "=", 2)[0] + if _, shouldOverride := overrides[key]; !shouldOverride { + env = append(env, e) + } + } + for key, value := range overrides { + env = append(env, fmt.Sprintf("%s=%s", key, value)) + } + return env +} + +func (cph *CommonPackageUpdater) UpdateDependency(fixDetails *FixDetails, installationCommand string, extraArgs ...string) (err error) { + impactedPackage := strings.ToLower(fixDetails.ImpactedDependencyName) + commandArgs := []string{installationCommand} + commandArgs = append(commandArgs, extraArgs...) + versionOperator := fixDetails.Technology.GetPackageVersionOperator() + fixedPackageArgs := GetFixedPackage(impactedPackage, versionOperator, fixDetails.SuggestedFixedVersion) + commandArgs = append(commandArgs, fixedPackageArgs...) + return runPackageMangerCommand(fixDetails.Technology.GetExecCommandName(), fixDetails.Technology.String(), commandArgs) +} + +func runPackageMangerCommand(commandName string, techName string, commandArgs []string) error { + fullCommand := commandName + " " + strings.Join(commandArgs, " ") + log.Debug(fmt.Sprintf("Running '%s'", fullCommand)) + cmd := exec.Command(commandName, commandArgs...) + if commandName == "pnpm" { + cmd.Env = EnvWithCorepackIntegrityWorkaround(os.Environ()) + } + output, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("failed to update %s dependency: '%s' command failed: %s\n%s", techName, fullCommand, err.Error(), output) + } + return nil +} + +func EnvWithCorepackIntegrityWorkaround(base []string) []string { + const key = "COREPACK_INTEGRITY_KEYS" + prefix := key + "=" + out := make([]string, 0, len(base)+1) + for _, e := range base { + if !strings.HasPrefix(e, prefix) { + out = append(out, e) + } + } + return append(out, prefix+"0") +} + +func GetFixedPackage(impactedPackage string, versionOperator string, suggestedFixedVersion string) (fixedPackageArgs []string) { + fixedPackageString := strings.TrimSpace(impactedPackage) + versionOperator + strings.TrimSpace(suggestedFixedVersion) + fixedPackageArgs = strings.Split(fixedPackageString, " ") + return +} + +func (cph *CommonPackageUpdater) GetAllDescriptorFilesFullPaths(descriptorFilesSuffixes []string, patternsToExclude ...string) (descriptorFilesFullPaths []string, err error) { + if len(descriptorFilesSuffixes) == 0 { + return + } + + var regexpPatternsCompilers []*regexp.Regexp + for _, patternToExclude := range patternsToExclude { + regexpPatternsCompilers = append(regexpPatternsCompilers, regexp.MustCompile(patternToExclude)) + } + + err = filepath.WalkDir(".", func(path string, d fs.DirEntry, innerErr error) error { + if innerErr != nil { + return fmt.Errorf("an error has occurred when attempting to access or traverse the file system: %w", innerErr) + } + + for _, regexpCompiler := range regexpPatternsCompilers { + if match := regexpCompiler.FindString(path); match != "" { + return filepath.SkipDir + } + } + + for _, assetFileSuffix := range descriptorFilesSuffixes { + if strings.HasSuffix(path, assetFileSuffix) { + var absFilePath string + absFilePath, innerErr = filepath.Abs(path) + if innerErr != nil { + return fmt.Errorf("couldn't retrieve file's absolute path for './%s': %w", path, innerErr) + } + descriptorFilesFullPaths = append(descriptorFilesFullPaths, absFilePath) + } + } + return nil + }) + if err != nil { + err = fmt.Errorf("failed to get descriptor files absolute paths: %w", err) + } + return +} + +func BuildPackageWithVersionRegex(impactedName, impactedVersion, dependencyLineFormat string) *regexp.Regexp { + var c CommonPackageUpdater + return c.BuildPackageDependencyLineRegex(impactedName, impactedVersion, dependencyLineFormat) +} + +func GetVulnerabilityLocations(fixDetails *FixDetails, namesFilters []string, ignoreFilters []string) []string { + var c CommonPackageUpdater + return c.CollectVulnerabilityDescriptorPaths(fixDetails, namesFilters, ignoreFilters) +} + +// IsFileTrackedByGit returns true if the given file is tracked by the git repository +// rooted at repoRootDir. +func IsFileTrackedByGit(filePath, repoRootDir string) (bool, error) { + repo, err := git.PlainOpen(repoRootDir) + if err != nil { + return false, fmt.Errorf("failed to open git repository at '%s': %w", repoRootDir, err) + } + + head, err := repo.Head() + if err != nil { + return false, fmt.Errorf("failed to get HEAD reference: %w", err) + } + + commit, err := repo.CommitObject(head.Hash()) + if err != nil { + return false, fmt.Errorf("failed to get HEAD commit: %w", err) + } + + tree, err := commit.Tree() + if err != nil { + return false, fmt.Errorf("failed to get commit tree: %w", err) + } + + _, err = tree.File(filePath) + if errors.Is(err, object.ErrFileNotFound) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("failed to check file in commit tree: %w", err) + } + return true, nil +} diff --git a/utils/remediation/packageupdaters/commonpackageupdater_test.go b/utils/remediation/packageupdaters/commonpackageupdater_test.go new file mode 100644 index 000000000..c11bec866 --- /dev/null +++ b/utils/remediation/packageupdaters/commonpackageupdater_test.go @@ -0,0 +1,979 @@ +package packageupdaters + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/jfrog/build-info-go/tests" + "github.com/jfrog/jfrog-cli-security/utils/formats" + "github.com/jfrog/jfrog-cli-security/utils/techutils" + "github.com/jfrog/jfrog-client-go/utils/io/fileutils" + "github.com/stretchr/testify/assert" +) + +type dependencyFixTest struct { + fixDetails *FixDetails + fixSupported bool + errorExpected bool + specificTechVersion string + testDirName string + descriptorsToCheck []string + testcaseInfo string + // For this param give the relative path from the test project root + lockFileToVerifyItsChange string + // Verifies descriptor content is unchanged after error (for rollback testing) + descriptorToVerifyNoChange string +} + +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.*" +) + +type pipPackageRegexTest struct { + packageName string + expectedRequirement string +} + +func TestUpdateDependency(t *testing.T) { + if strings.TrimSuffix(os.Getenv("JF_URL"), "/") == "" { + t.Skipf("skipping: JF_URL is not set (package updater integration tests run in CI with platform credentials)") + } + + testCases := [][]dependencyFixTest{ + // Go test cases + { + { + fixDetails: createFixDetails(techutils.Go, "golang.org/x/crypto", "", "0.0.0-20201216223049-8b5274cf687f", false, "go.mod"), + fixSupported: true, + descriptorsToCheck: []string{"go.mod"}, + lockFileToVerifyItsChange: "go.sum", + }, + { + fixDetails: createFixDetails(techutils.Go, "github.com/google/uuid", "", "1.3.0", true, "go.mod"), + fixSupported: true, + descriptorsToCheck: []string{"go.mod"}, + lockFileToVerifyItsChange: "go.sum", + }, + { + testcaseInfo: "no-location-evidence", + fixDetails: createFixDetails(techutils.Go, "github.com/google/uuid", "", "1.3.0", true), + fixSupported: true, + errorExpected: true, + }, + }, + + // Python test cases (includes pip, pipenv, poetry) + { + { + fixDetails: createFixDetails(techutils.Pip, "urllib3", "", "1.25.9", false, ""), + fixSupported: false, + }, + { + fixDetails: createFixDetails(techutils.Poetry, "urllib3", "", "1.25.9", false, ""), + fixSupported: false, + }, + { + fixDetails: createFixDetails(techutils.Pipenv, "urllib3", "", "1.25.9", false, ""), + fixSupported: false, + }, + { + fixDetails: createFixDetails(techutils.Pip, "pyjwt", "", "2.4.0", true, ""), + fixSupported: true, + descriptorsToCheck: []string{"requirements.txt"}, + }, + { + fixDetails: createFixDetails(techutils.Pip, "Pyjwt", "", "2.4.0", true, ""), + fixSupported: true, + descriptorsToCheck: []string{"requirements.txt"}, + }, + { + fixDetails: createFixDetails(techutils.Poetry, "pyjwt", "", "2.4.0", true, ""), + fixSupported: true, + descriptorsToCheck: []string{"pyproject.toml"}, + }, + { + fixDetails: createFixDetails(techutils.Pipenv, "pyjwt", "", "2.4.0", true, ""), + fixSupported: true, + descriptorsToCheck: []string{"Pipfile"}, + }, + }, + + // Npm test cases + { + { + // Test project doesn't exist for the testcase - we just check skipping indirect dependency fix + testcaseInfo: "test-skip-fixing-indirect", + fixDetails: createFixDetails(techutils.Npm, "mpath", "0.8.3", "0.8.4", false, "package-lock.json"), + fixSupported: false, + }, + { + fixDetails: createFixDetails(techutils.Npm, "minimist", "1.2.5", "1.2.6", true, "package.json", "package-lock.json"), + fixSupported: true, + descriptorsToCheck: []string{"package.json"}, + lockFileToVerifyItsChange: "package-lock.json", + }, + { + testcaseInfo: "no-location-evidence", + fixDetails: createFixDetails(techutils.Npm, "minimist", "1.2.5", "1.2.6", true), + fixSupported: true, + errorExpected: true, + }, + { + testcaseInfo: "rollback-on-npm-install-failure", + fixDetails: createFixDetails(techutils.Npm, "minimist", "1.2.5", "1.2.6", true, "package.json", "package-lock.json"), + testDirName: "npm-rollback", + fixSupported: true, + errorExpected: true, + descriptorToVerifyNoChange: "package.json", + }, + }, + + // Maven test cases + { + { + fixDetails: createFixDetails(techutils.Maven, "org.springframework:spring-core", "", "4.3.20", false, ""), + fixSupported: false, + }, + { + fixDetails: createFixDetails(techutils.Maven, "commons-io:commons-io", "", "2.7", true, filepath.Join("multi1", "pom.xml")), + fixSupported: true, + descriptorsToCheck: []string{filepath.Join("multi1", "pom.xml")}, + }, + }, + + // Pnpm test cases + { + { + fixDetails: createFixDetails(techutils.Pnpm, "mpath", "", "0.8.4", false, ""), + fixSupported: false, + testDirName: "npm", + }, + { + fixDetails: createFixDetails(techutils.Pnpm, "minimist", "1.2.5", "1.2.6", true, "package.json", "package-lock.json"), + fixSupported: true, + testDirName: "npm", + descriptorsToCheck: []string{"package.json"}, + }, + }, + } + + for _, testBatch := range testCases { + for _, test := range testBatch { + packageUpdater, _ := GetCompatiblePackageUpdater(test.fixDetails) + t.Run(getUpdateDependencyTestcaseName(test.fixDetails.Technology.String()+test.specificTechVersion, test.fixDetails.IsDirectDependency, test.testcaseInfo), + func(t *testing.T) { + testDataDir := getTestDataDir(t, test.fixDetails.IsDirectDependency) + testDirName := test.fixDetails.Technology.String() + if test.testDirName != "" { + testDirName = test.testDirName + } + cleanup := createTempDirAndChdir(t, testDataDir, testDirName+test.specificTechVersion) + defer cleanup() + + var lockFileContentBeforeUpdate []byte + if test.lockFileToVerifyItsChange != "" { + var readErr error + lockFileContentBeforeUpdate, readErr = os.ReadFile(test.lockFileToVerifyItsChange) + assert.NoError(t, readErr, "Failed to read lock file before update") + } + + var descriptorContentBeforeUpdate []byte + if test.descriptorToVerifyNoChange != "" { + var readErr error + descriptorContentBeforeUpdate, readErr = os.ReadFile(test.descriptorToVerifyNoChange) + assert.NoError(t, readErr, "Failed to read descriptor before update") + } + + err := packageUpdater.UpdateDependency(test.fixDetails) + if !test.fixSupported { + assert.Error(t, err) + assert.IsType(t, &ErrUnsupportedFix{}, err, "Expected unsupported fix error") + return + } + + if test.errorExpected { + assert.Error(t, err) + if test.descriptorToVerifyNoChange != "" { + descriptorContentAfter, readErr := os.ReadFile(test.descriptorToVerifyNoChange) + assert.NoError(t, readErr, "Failed to read descriptor after update") + assert.Equal(t, descriptorContentBeforeUpdate, descriptorContentAfter, "Descriptor should be unchanged after rollback") + } + return + } + + assert.NoError(t, err) + verifyDependencyUpdate(t, test) + + if test.lockFileToVerifyItsChange != "" { + lockFileContentAfter, readErr := os.ReadFile(test.lockFileToVerifyItsChange) + assert.NoError(t, readErr, "Failed to read lock file after update") + assert.NotEqual(t, lockFileContentBeforeUpdate, lockFileContentAfter, "Lock file should have been updated") + } + }) + } + } +} + +func getTestDataDir(t *testing.T, directDependency bool) string { + var projectDir string + if directDependency { + projectDir = "projects" + } else { + projectDir = "indirect-projects" + } + testdataDir, err := filepath.Abs(filepath.Join("..", "testdata", projectDir)) + assert.NoError(t, err) + return testdataDir +} + +func createTempDirAndChdir(t *testing.T, testdataDir string, tech string) func() { + // Create temp technology project + projectPath := filepath.Join(testdataDir, tech) + tmpProjectPath, cleanup := tests.CreateTestProject(t, projectPath) + currDir, err := os.Getwd() + assert.NoError(t, err) + assert.NoError(t, os.Chdir(tmpProjectPath)) + if tech == "go" { + err = removeTxtSuffix("go.mod.txt") + assert.NoError(t, err) + err = removeTxtSuffix("go.sum.txt") + assert.NoError(t, err) + err = removeTxtSuffix("main.go.txt") + assert.NoError(t, err) + } + return func() { + cleanup() + assert.NoError(t, os.Chdir(currDir)) + } +} + +func removeTxtSuffix(txtFileName string) error { + // go.sum.txt >> go.sum + return fileutils.MoveFile(txtFileName, strings.TrimSuffix(txtFileName, ".txt")) +} + +func assertFixVersionInPackageDescriptor(t *testing.T, test dependencyFixTest, packageDescriptors []string) { + for _, packageDescriptorToCheck := range packageDescriptors { + file, err := os.ReadFile(packageDescriptorToCheck) + assert.NoError(t, err) + + assert.Contains(t, string(file), test.fixDetails.SuggestedFixedVersion) + // Verify that case-sensitive packages in python are lowered + assert.Contains(t, string(file), strings.ToLower(test.fixDetails.ImpactedDependencyName)) + } +} + +// Verifies the expected dependency update happened and extra check that are unique to selected package managers +func verifyDependencyUpdate(t *testing.T, test dependencyFixTest) { + if len(test.descriptorsToCheck) == 0 { + assert.Fail(t, fmt.Sprintf("Please provide descriptor files to be inspected in the 'descriptorsToCheck' for %s test cases where a fix is supported.", test.fixDetails.Technology)) + } + + currDir, err := os.Getwd() + assert.NoError(t, err) + + var descriptorsFullPaths []string + for _, descriptorToCheck := range test.descriptorsToCheck { + descriptorsFullPaths = append(descriptorsFullPaths, filepath.Join(currDir, descriptorToCheck)) + } + + if test.fixDetails.Technology == techutils.Maven { + // In Maven descriptors the dependency's artifact name and group name are split into 2 different lines, therefore we change the ImpactedDependencyName to be the dependency's artifact name only + depArtifactAndGroup := strings.Split(test.fixDetails.ImpactedDependencyName, ":") + assert.Equal(t, len(depArtifactAndGroup), 2) + test.fixDetails.ImpactedDependencyName = depArtifactAndGroup[1] + } + assertFixVersionInPackageDescriptor(t, test, descriptorsFullPaths) +} + +func TestGetFixedPackage(t *testing.T) { + var testcases = []struct { + impactedPackage string + versionOperator string + suggestedFixedVersion string + expectedOutput []string + }{ + { + impactedPackage: "snappier", + versionOperator: " -v ", + suggestedFixedVersion: "1.1.1", + expectedOutput: []string{"snappier", "-v", "1.1.1"}, + }, + { + impactedPackage: "json", + versionOperator: "@", + suggestedFixedVersion: "10.0.0", + expectedOutput: []string{"json@10.0.0"}, + }, + } + + for _, test := range testcases { + fixedPackageArgs := GetFixedPackage(test.impactedPackage, test.versionOperator, test.suggestedFixedVersion) + assert.Equal(t, test.expectedOutput, fixedPackageArgs) + } +} + +func TestGetAllDescriptorFilesFullPaths(t *testing.T) { + var testcases = []struct { + testProjectRepo string + suffixesToSearch []string + expectedResultSuffixes []string + patternsToExclude []string + }{ + { + testProjectRepo: "maven", + suffixesToSearch: []string{"pom.xml"}, + expectedResultSuffixes: []string{"pom.xml"}, + }, + } + + currDir, outerErr := os.Getwd() + assert.NoError(t, outerErr) + + for _, testcase := range testcases { + tmpDir, err := os.MkdirTemp("", "") + assert.NoError(t, err) + assert.NoError(t, copyDir(filepath.Join("..", "testdata", "projects", testcase.testProjectRepo), tmpDir)) + assert.NoError(t, os.Chdir(tmpDir)) + + finalDirPath, err := os.Getwd() + assert.NoError(t, err) + + var expectedResults []string + for _, suffix := range testcase.expectedResultSuffixes { + expectedResults = append(expectedResults, filepath.Join(finalDirPath, suffix)) + } + + var cph CommonPackageUpdater + descriptorFilesFullPaths, err := cph.GetAllDescriptorFilesFullPaths(testcase.suffixesToSearch, testcase.patternsToExclude...) + assert.NoError(t, err) + assert.ElementsMatch(t, expectedResults, descriptorFilesFullPaths) + + assert.NoError(t, os.Chdir(currDir)) + assert.NoError(t, fileutils.RemoveTempDir(tmpDir)) + } +} + +func TestGetVulnerabilityLocations(t *testing.T) { + testcases := []struct { + name string + fixDetails *FixDetails + namesFilters []string + ignoreFilters []string + expectedPaths []string + }{ + { + name: "single component with descriptor evidence", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{{File: "/repo/package.json"}}}, + }, + }, + expectedPaths: []string{"/repo/package.json"}, + }, + { + name: "multiple components with same evidence - deduplicated", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{{File: "/repo/package.json"}}}, + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{{File: "/repo/package.json"}}}, + }, + }, + expectedPaths: []string{"/repo/package.json"}, + }, + { + name: "multiple components with different descriptor locations", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{{File: "/repo/app1/package.json"}}}, + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{{File: "/repo/app2/package.json"}}}, + }, + }, + expectedPaths: []string{"/repo/app1/package.json", "/repo/app2/package.json"}, + }, + { + name: "component with empty evidences", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{}}, + }, + }, + expectedPaths: []string{}, + }, + { + name: "component with empty file path in evidence", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{{File: ""}}}, + }, + }, + expectedPaths: []string{}, + }, + { + name: "no components", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{}, + }, + expectedPaths: []string{}, + }, + { + name: "non-descriptor evidences are filtered out", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{ + {File: "package-lock.json"}, + {File: "package.json"}, + }}, + }, + }, + expectedPaths: []string{"package.json"}, + }, + { + name: "filter by basename - match", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{{File: "package.json"}}}, + {Name: "lodash", Version: "4.17.0", Evidences: []formats.Location{{File: "go.mod"}}}, + }, + }, + namesFilters: []string{"package.json"}, + expectedPaths: []string{"package.json"}, + }, + { + name: "filter by basename - no match", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{{File: "package.json"}}}, + }, + }, + namesFilters: []string{"go.mod"}, + expectedPaths: []string{}, + }, + { + name: "filter by basename - full path matched by basename", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{{File: "/repo/apps/frontend/package.json"}}}, + {Name: "lodash", Version: "4.17.0", Evidences: []formats.Location{{File: "/repo/go.mod"}}}, + }, + }, + namesFilters: []string{"package.json"}, + expectedPaths: []string{"/repo/apps/frontend/package.json"}, + }, + { + name: "filter with multiple allowed names", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{{File: "package.json"}}}, + {Name: "lodash", Version: "4.17.0", Evidences: []formats.Location{{File: "go.mod"}}}, + {Name: "axios", Version: "0.21.0", Evidences: []formats.Location{{File: "pyproject.toml"}}}, + }, + }, + namesFilters: []string{"package.json", "go.mod"}, + expectedPaths: []string{"package.json", "go.mod"}, + }, + { + name: "empty filter returns all descriptor locations", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{ + {File: "package-lock.json"}, + {File: "package.json"}, + }}, + }, + }, + namesFilters: []string{}, + expectedPaths: []string{"package.json"}, + }, + { + name: "nil filter returns all descriptor locations", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{ + {File: "package-lock.json"}, + {File: "package.json"}, + }}, + }, + }, + namesFilters: nil, + expectedPaths: []string{"package.json"}, + }, + { + name: "multiple evidences per component - descriptors collected from all", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{ + {File: "/repo/app1/package.json"}, + {File: "/repo/app1/package-lock.json"}, + {File: "/repo/app2/package.json"}, + }}, + }, + }, + expectedPaths: []string{"/repo/app1/package.json", "/repo/app2/package.json"}, + }, + { + name: "ignoreFilters excludes paths containing pattern", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{ + {File: "package.json"}, + {File: "node_modules/minimist/package.json"}, + {File: "libs/node_modules/foo/package.json"}, + }}, + }, + }, + ignoreFilters: []string{"node_modules"}, + expectedPaths: []string{"package.json"}, + }, + { + name: "ignoreFilters with multiple patterns", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{ + {File: "package.json"}, + {File: "node_modules/minimist/package.json"}, + {File: "vendor/something/package.json"}, + }}, + }, + }, + ignoreFilters: []string{"node_modules", "vendor"}, + expectedPaths: []string{"package.json"}, + }, + { + name: "ignoreFilters nil does not filter", + fixDetails: &FixDetails{ + Components: []formats.ComponentRow{ + {Name: "minimist", Version: "1.2.5", Evidences: []formats.Location{ + {File: "package.json"}, + {File: "sub/package.json"}, + }}, + }, + }, + ignoreFilters: nil, + expectedPaths: []string{"package.json", "sub/package.json"}, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + result := GetVulnerabilityLocations(tc.fixDetails, tc.namesFilters, tc.ignoreFilters) + assert.ElementsMatch(t, tc.expectedPaths, result) + }) + } +} + +func TestEnvWithCorepackIntegrityWorkaround(t *testing.T) { + t.Parallel() + base := []string{"FOO=1", "COREPACK_INTEGRITY_KEYS=old-value", "BAR=2"} + out := EnvWithCorepackIntegrityWorkaround(base) + var foo, bar, corepack int + for _, e := range out { + switch { + case e == "FOO=1": + foo++ + case e == "BAR=2": + bar++ + case strings.HasPrefix(e, "COREPACK_INTEGRITY_KEYS="): + corepack++ + assert.Equal(t, "COREPACK_INTEGRITY_KEYS=0", e) + } + } + assert.Equal(t, 1, foo, "FOO should appear once") + assert.Equal(t, 1, bar, "BAR should appear once") + assert.Equal(t, 1, corepack, "COREPACK_INTEGRITY_KEYS should appear exactly once with value 0") +} + +func TestGetVulnerabilityRegexCompiler(t *testing.T) { + // Sample format patterns from different package managers + const ( + npmPattern = `\s*"%s"\s*:\s*"[~^]?%s"` + dotnetPattern = "include=[\\\"|\\']%s[\\\"|\\']\\s*version=[\\\"|\\']%s[\\\"|\\']" + simplePattern = `%s:%s` + ) + + testcases := []struct { + name string + packageName string + packageVer string + formatPattern string + testContent string + shouldMatch bool + }{ + // Basic matching + { + name: "basic npm match", + packageName: "lodash", + packageVer: "4.17.20", + formatPattern: npmPattern, + testContent: `"lodash": "4.17.20"`, + shouldMatch: true, + }, + { + name: "npm with caret prefix", + packageName: "lodash", + packageVer: "4.17.20", + formatPattern: npmPattern, + testContent: `"lodash": "^4.17.20"`, + shouldMatch: true, + }, + { + name: "npm with tilde prefix", + packageName: "lodash", + packageVer: "4.17.20", + formatPattern: npmPattern, + testContent: `"lodash": "~4.17.20"`, + shouldMatch: true, + }, + { + name: "npm version mismatch", + packageName: "lodash", + packageVer: "4.17.20", + formatPattern: npmPattern, + testContent: `"lodash": "4.17.21"`, + shouldMatch: false, + }, + { + name: "npm name mismatch", + packageName: "lodash", + packageVer: "4.17.20", + formatPattern: npmPattern, + testContent: `"underscore": "4.17.20"`, + shouldMatch: false, + }, + + // Case insensitivity + { + name: "case insensitive package name", + packageName: "PyJWT", + packageVer: "2.4.0", + formatPattern: simplePattern, + testContent: `pyjwt:2.4.0`, + shouldMatch: true, + }, + { + name: "case insensitive mixed case", + packageName: "LODASH", + packageVer: "4.17.20", + formatPattern: npmPattern, + testContent: `"lodash": "4.17.20"`, + shouldMatch: true, + }, + + // Scoped npm packages with @ + { + name: "scoped npm package", + packageName: "@types/node", + packageVer: "18.0.0", + formatPattern: npmPattern, + testContent: `"@types/node": "18.0.0"`, + shouldMatch: true, + }, + { + name: "scoped package with org", + packageName: "@angular/core", + packageVer: "15.0.0", + formatPattern: npmPattern, + testContent: `"@angular/core": "^15.0.0"`, + shouldMatch: true, + }, + + // Regex special characters in package name - should be escaped + { + name: "package name with dot", + packageName: "lodash.merge", + packageVer: "4.6.2", + formatPattern: npmPattern, + testContent: `"lodash.merge": "4.6.2"`, + shouldMatch: true, + }, + { + name: "dot should not match any character", + packageName: "lodash.merge", + packageVer: "4.6.2", + formatPattern: npmPattern, + testContent: `"lodashXmerge": "4.6.2"`, + shouldMatch: false, + }, + { + name: "package name with asterisk", + packageName: "test*package", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `test*package:1.0.0`, + shouldMatch: true, + }, + { + name: "asterisk should not match multiple chars", + packageName: "test*package", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `testABCpackage:1.0.0`, + shouldMatch: false, + }, + { + name: "package name with question mark", + packageName: "test?pkg", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `test?pkg:1.0.0`, + shouldMatch: true, + }, + { + name: "question mark should not match single char", + packageName: "test?pkg", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `testXpkg:1.0.0`, + shouldMatch: false, + }, + { + name: "package name with brackets", + packageName: "test[pkg]", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `test[pkg]:1.0.0`, + shouldMatch: true, + }, + { + name: "package name with parentheses", + packageName: "test(pkg)", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `test(pkg):1.0.0`, + shouldMatch: true, + }, + { + name: "package name with curly braces", + packageName: "test{pkg}", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `test{pkg}:1.0.0`, + shouldMatch: true, + }, + { + name: "package name with pipe", + packageName: "test|pkg", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `test|pkg:1.0.0`, + shouldMatch: true, + }, + { + name: "pipe should not match as OR", + packageName: "test|pkg", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `test:1.0.0`, + shouldMatch: false, + }, + { + name: "package name with caret and dollar", + packageName: "^test$", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `^test$:1.0.0`, + shouldMatch: true, + }, + { + name: "package name with backslash", + packageName: `test\pkg`, + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `test\pkg:1.0.0`, + shouldMatch: true, + }, + + // Version with special characters + { + name: "version with plus (build metadata)", + packageName: "mypackage", + packageVer: "1.0.0+build123", + formatPattern: simplePattern, + testContent: `mypackage:1.0.0+build123`, + shouldMatch: true, + }, + { + name: "plus in version should not match one-or-more", + packageName: "mypackage", + packageVer: "1.0.0+", + formatPattern: simplePattern, + testContent: `mypackage:1.0.00000`, + shouldMatch: false, + }, + { + name: "version with dots should match literally", + packageName: "pkg", + packageVer: "1.2.3", + formatPattern: simplePattern, + testContent: `pkg:1.2.3`, + shouldMatch: true, + }, + { + name: "dots should not match any char", + packageName: "pkg", + packageVer: "1.2.3", + formatPattern: simplePattern, + testContent: `pkg:1X2Y3`, + shouldMatch: false, + }, + + // Empty name and version edge cases + { + name: "empty package name matches empty", + packageName: "", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `:1.0.0`, + shouldMatch: true, + }, + { + name: "empty version matches empty", + packageName: "pkg", + packageVer: "", + formatPattern: simplePattern, + testContent: `pkg:`, + shouldMatch: true, + }, + { + name: "both empty", + packageName: "", + packageVer: "", + formatPattern: simplePattern, + testContent: `:`, + shouldMatch: true, + }, + + // Complex realistic scenarios + { + name: "dotnet pattern match", + packageName: "Newtonsoft.Json", + packageVer: "13.0.1", + formatPattern: dotnetPattern, + testContent: `Include="Newtonsoft.Json" Version="13.0.1"`, + shouldMatch: true, + }, + { + name: "dotnet single quotes", + packageName: "Newtonsoft.Json", + packageVer: "13.0.1", + formatPattern: dotnetPattern, + testContent: `Include='Newtonsoft.Json' Version='13.0.1'`, + shouldMatch: true, + }, + + // Whitespace handling + { + name: "npm with extra whitespace", + packageName: "lodash", + packageVer: "4.17.20", + formatPattern: npmPattern, + testContent: ` "lodash" : "4.17.20"`, + shouldMatch: true, + }, + { + name: "npm with tabs", + packageName: "lodash", + packageVer: "4.17.20", + formatPattern: npmPattern, + testContent: "\t\"lodash\"\t:\t\"4.17.20\"", + shouldMatch: true, + }, + + // Unicode characters (less common but possible) + { + name: "package name with unicode", + packageName: "пакет", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `пакет:1.0.0`, + shouldMatch: true, + }, + + // Long package names and versions + { + name: "very long package name", + packageName: "this-is-a-very-long-package-name-that-might-exist-in-real-world", + packageVer: "1.0.0", + formatPattern: simplePattern, + testContent: `this-is-a-very-long-package-name-that-might-exist-in-real-world:1.0.0`, + shouldMatch: true, + }, + { + name: "prerelease version", + packageName: "pkg", + packageVer: "1.0.0-alpha.1", + formatPattern: simplePattern, + testContent: `pkg:1.0.0-alpha.1`, + shouldMatch: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + regex := BuildPackageWithVersionRegex(tc.packageName, tc.packageVer, tc.formatPattern) + matches := regex.MatchString(strings.ToLower(tc.testContent)) + assert.Equal(t, tc.shouldMatch, matches, "Pattern: %s, Content: %s", regex.String(), tc.testContent) + }) + } +} + +func getUpdateDependencyTestcaseName(technology string, isDirect bool, extraTestInfo string) string { + testName := technology + if isDirect { + testName += "-direct-dep" + } else { + testName += "-indirect-dep" + } + if extraTestInfo != "" { + testName += "_(" + extraTestInfo + ")" + } + return testName +} + +func createFixDetails(technology techutils.Technology, packageName, packageVersion, fixedVersion string, isDirectDependency bool, evidencePaths ...string) *FixDetails { + var evidences []formats.Location + for _, path := range evidencePaths { + if path != "" { + evidences = append(evidences, formats.Location{File: path}) + } + } + return &FixDetails{ + SuggestedFixedVersion: fixedVersion, + IsDirectDependency: isDirectDependency, + Technology: technology, + ImpactedDependencyName: packageName, + ImpactedDependencyVersion: packageVersion, + Components: []formats.ComponentRow{ + { + Name: packageName, + Version: packageVersion, + Evidences: evidences, + }, + }, + } +} + +// copyDir is a simple helper to copy a directory tree for tests. +func copyDir(src, dst string) error { + return filepath.Walk(src, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + relPath, err := filepath.Rel(src, path) + if err != nil { + return err + } + dstPath := filepath.Join(dst, relPath) + if info.IsDir() { + return os.MkdirAll(dstPath, info.Mode()) + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + return os.WriteFile(dstPath, data, info.Mode()) + }) +} diff --git a/utils/remediation/packageupdaters/gopackageupdater.go b/utils/remediation/packageupdaters/gopackageupdater.go new file mode 100644 index 000000000..d5ac7663d --- /dev/null +++ b/utils/remediation/packageupdaters/gopackageupdater.go @@ -0,0 +1,231 @@ +package packageupdaters + +import ( + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/jfrog/jfrog-client-go/utils/log" +) + +const ( + goFlagModEditEnv = "GOFLAGS=-mod=mod" + goWorkOffEnv = "GOWORK=off" + // GoModFileName is the Go module descriptor file name. + GoModFileName = "go.mod" + // GoSumFileName is the Go checksum file name. + GoSumFileName = "go.sum" + // GoVendorDirName is the Go vendor directory name. + GoVendorDirName = "vendor" + goTidyContinueOnError = "-e" +) + +type GoPackageUpdater struct{} + +// GoModuleBackup holds the original content of go.mod and go.sum for rollback purposes. +type GoModuleBackup struct { + GoModPath string + GoModContent []byte + GoSumPath string + GoSumContent []byte +} + +func (gpu *GoPackageUpdater) UpdateDependency(fixDetails *FixDetails) error { + descriptorPaths := GetVulnerabilityLocations(fixDetails, []string{GoModFileName}, []string{GoVendorDirName}) + if len(descriptorPaths) == 0 { + return fmt.Errorf("no descriptor evidence was found for package %s", fixDetails.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(fixDetails, descriptorPath, originalWd, env); fixErr != nil { + failedFixErrorMsg := fmt.Errorf("failed to fix '%s' in descriptor '%s': %w", fixDetails.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", fixDetails.ImpactedDependencyName, strings.Join(failingDescriptors, ", "), err) + } + + return nil +} + +func (gpu *GoPackageUpdater) fixVulnerabilityAndTidy(fixDetails *FixDetails, 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(fixDetails, env); err != nil { + log.Warn(fmt.Sprintf("Failed to update '%s' to version '%s': %s. Rolling back...", fixDetails.ImpactedDependencyName, fixDetails.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 := 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...", fixDetails.ImpactedDependencyName, fixDetails.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'", fixDetails.ImpactedDependencyName, fixDetails.ImpactedDependencyVersion, fixDetails.SuggestedFixedVersion, descriptorPath)) + return nil +} + +func (gpu *GoPackageUpdater) buildGoCommandEnv() []string { + return append(os.Environ(), goFlagModEditEnv, goWorkOffEnv) +} + +func (gpu *GoPackageUpdater) BackupModuleFiles(GoModPath string) (*GoModuleBackup, error) { + //#nosec G304 -- go.mod path from scan workflow. + GoModContent, err := os.ReadFile(GoModPath) + if err != nil { + return nil, fmt.Errorf("failed to read '%s': %w", GoModPath, err) + } + + descriptorDir := filepath.Dir(GoModPath) + GoSumPath := filepath.Join(descriptorDir, GoSumFileName) + //#nosec G304 -- go.sum adjacent to go.mod from same scan workflow. + 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 { + //#nosec G306 -- 0644 for checked-out module files in workspace. + if err := os.WriteFile(backup.GoModPath, backup.GoModContent, 0644); err != nil { + return fmt.Errorf("failed to restore '%s': %w", backup.GoModPath, err) + } + //#nosec G306 -- 0644 for checked-out module files in workspace. + 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(fixDetails *FixDetails, env []string) error { + impactedPackage := strings.ToLower(fixDetails.ImpactedDependencyName) + fixedVersion := strings.TrimSpace(fixDetails.SuggestedFixedVersion) + + if !strings.HasPrefix(fixedVersion, "v") { + fixedVersion = "v" + fixedVersion + } + fixedPackage := strings.TrimSpace(impactedPackage) + "@" + fixedVersion + + //#nosec G204 -- runs only after user approval; arguments from vulnerability metadata. + cmd := exec.Command("go", "get", fixedPackage) + cmd.Env = env + log.Debug(fmt.Sprintf("Running 'go get %s'", fixedPackage)) + + 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 + } + } + + 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/utils/remediation/packageupdaters/gopackageupdater_test.go b/utils/remediation/packageupdaters/gopackageupdater_test.go new file mode 100644 index 000000000..67edb981b --- /dev/null +++ b/utils/remediation/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/utils/remediation/packageupdaters/mavenpackageupdater.go b/utils/remediation/packageupdaters/mavenpackageupdater.go new file mode 100644 index 000000000..603ff81d5 --- /dev/null +++ b/utils/remediation/packageupdaters/mavenpackageupdater.go @@ -0,0 +1,199 @@ +package packageupdaters + +import ( + "bytes" + "encoding/xml" + "errors" + "fmt" + "os" + "regexp" + "strings" + + "github.com/jfrog/jfrog-client-go/utils/log" +) + +const ( + mavenDependencySeparator = ":" + propertyPrefix = "${" + propertySuffix = "}" + pomFileName = "pom.xml" +) + +type MavenPackageUpdater struct{} + +type mavenProject struct { + XMLName xml.Name `xml:"project"` + Parent *mavenDep `xml:"parent"` + Properties *mavenProperties `xml:"properties"` + Dependencies []mavenDep `xml:"dependencies>dependency"` + DependencyManagement *mavenDepManagement `xml:"dependencyManagement"` +} + +type mavenProperties struct { + Props []mavenProperty `xml:",any"` +} + +type mavenProperty struct { + XMLName xml.Name + Value string `xml:",chardata"` +} + +type mavenDep struct { + GroupId string `xml:"groupId"` + ArtifactId string `xml:"artifactId"` + Version string `xml:"version"` +} + +type mavenDepManagement struct { + Dependencies []mavenDep `xml:"dependencies>dependency"` +} + +func (m *MavenPackageUpdater) UpdateDependency(fixDetails *FixDetails) error { + if !fixDetails.IsDirectDependency { + return &ErrUnsupportedFix{ + PackageName: fixDetails.ImpactedDependencyName, + FixedVersion: fixDetails.SuggestedFixedVersion, + ErrorType: IndirectDependencyFixNotSupported, + } + } + + groupId, artifactId, err := parseDependencyName(fixDetails.ImpactedDependencyName) + if err != nil { + return err + } + + pomPaths := GetVulnerabilityLocations(fixDetails, []string{pomFileName}, []string{}) + if len(pomPaths) == 0 { + return fmt.Errorf("no pom.xml locations found for %s - Components array is empty or missing Location data", fixDetails.ImpactedDependencyName) + } + log.Verbose(fmt.Sprintf("Found vulnerability %s occurrences for component %s in %s", fixDetails.IssueId, fixDetails.ImpactedDependencyVersion, strings.Join(pomPaths, ", "))) + + var failingDescriptors []string + for _, pomPath := range pomPaths { + if fixErr := m.updatePomFile(pomPath, groupId, artifactId, fixDetails.SuggestedFixedVersion); fixErr != nil { + log.Warn(fixErr.Error()) + err = errors.Join(err, fmt.Errorf("failed to fix '%s' in descriptor '%s': %w", fixDetails.ImpactedDependencyName, pomPath, fixErr)) + failingDescriptors = append(failingDescriptors, pomPath) + } else { + log.Debug("Updated successfully " + pomPath) + } + } + + if err != nil { + return fmt.Errorf("encountered errors while fixing '%s' vulnerability in descriptors [%s]: %w", fixDetails.ImpactedDependencyName, strings.Join(failingDescriptors, ", "), err) + } + return nil +} + +func (m *MavenPackageUpdater) updatePomFile(pomPath, groupId, artifactId, fixedVersion string) error { + //#nosec G304 -- pomPath from descriptor discovery in the scanned repository. + content, err := os.ReadFile(pomPath) + if err != nil { + return fmt.Errorf("failed to read %s: %w", pomPath, err) + } + + var project mavenProject + if err = xml.Unmarshal(content, &project); err != nil { + return fmt.Errorf("failed to parse %s: %w", pomPath, err) + } + + // Working buffer for the update chain. Each update returns a new slice (e.g. regexp.ReplaceAll), + // so we never mutate the original content. No backup or revert—we only write to the file on success. + currentContent := content + var updatedAny bool + + if updated, c := m.updateInParent(&project, groupId, artifactId, fixedVersion, currentContent); updated { + currentContent = c + updatedAny = true + } + if updated, c := m.updateInDependencies(&project, project.Dependencies, groupId, artifactId, fixedVersion, currentContent); updated { + currentContent = c + updatedAny = true + } + if project.DependencyManagement != nil { + if updated, c := m.updateInDependencies(&project, project.DependencyManagement.Dependencies, groupId, artifactId, fixedVersion, currentContent); updated { + currentContent = c + updatedAny = true + } + } + + if !updatedAny { + return fmt.Errorf("dependency %s not found in %s", toDependencyName(groupId, artifactId), pomPath) + } + + //#nosec G703 G306 -- path from scan workflow; 0644 for VCS-tracked sources. + if err = os.WriteFile(pomPath, currentContent, 0644); err != nil { + return fmt.Errorf("failed to write %s: %w", pomPath, err) + } + return nil +} + +func parseDependencyName(dependencyName string) (groupId, artifactId string, err error) { + parts := strings.Split(dependencyName, mavenDependencySeparator) + if len(parts) != 2 { + return "", "", fmt.Errorf("invalid Maven dependency name: %s. Expected format 'groupId:artifactId'", dependencyName) + } + return parts[0], parts[1], nil +} + +func toDependencyName(groupId, artifactId string) string { + return groupId + mavenDependencySeparator + artifactId +} + +func (m *MavenPackageUpdater) updateInParent(project *mavenProject, groupId, artifactId, fixedVersion string, content []byte) (bool, []byte) { + if project.Parent == nil { + return false, content + } + + if project.Parent.GroupId == groupId && project.Parent.ArtifactId == artifactId { + pattern := regexp.MustCompile(`(?s)(\s*` + regexp.QuoteMeta(groupId) + `\s*` + regexp.QuoteMeta(artifactId) + `\s*)[^<]+()`) + newContent := pattern.ReplaceAll(content, []byte("${1}"+fixedVersion+"${2}")) + if !bytes.Equal(content, newContent) { + log.Debug("Updated parent", toDependencyName(groupId, artifactId), "to", fixedVersion) + return true, newContent + } + } + return false, content +} + +func (m *MavenPackageUpdater) updateInDependencies(project *mavenProject, deps []mavenDep, groupId, artifactId, fixedVersion string, content []byte) (bool, []byte) { + for _, dep := range deps { + if dep.GroupId == groupId && dep.ArtifactId == artifactId { + if propertyName, isProperty := extractPropertyName(dep.Version); isProperty { + return m.updateProperty(project, propertyName, fixedVersion, content) + } + + pattern := regexp.MustCompile(`(?s)(` + regexp.QuoteMeta(groupId) + `\s*` + regexp.QuoteMeta(artifactId) + `\s*)[^<]+()`) + newContent := pattern.ReplaceAll(content, []byte("${1}"+fixedVersion+"${2}")) + if !bytes.Equal(content, newContent) { + log.Debug("Updated dependency", toDependencyName(groupId, artifactId), "to", fixedVersion) + return true, newContent + } + } + } + return false, content +} + +func extractPropertyName(version string) (string, bool) { + if strings.HasPrefix(version, propertyPrefix) && strings.HasSuffix(version, propertySuffix) { + return strings.TrimSuffix(strings.TrimPrefix(version, propertyPrefix), propertySuffix), true + } + return "", false +} + +func (m *MavenPackageUpdater) updateProperty(project *mavenProject, propertyName, newValue string, content []byte) (bool, []byte) { + if project.Properties == nil { + return false, content + } + + for _, prop := range project.Properties.Props { + if prop.XMLName.Local == propertyName { + pattern := regexp.MustCompile(`(<` + regexp.QuoteMeta(propertyName) + `>)[^<]+()`) + newContent := pattern.ReplaceAll(content, []byte("${1}"+newValue+"${2}")) + if !bytes.Equal(content, newContent) { + return true, newContent + } + } + } + return false, content +} diff --git a/utils/remediation/packageupdaters/mavenpackageupdater_test.go b/utils/remediation/packageupdaters/mavenpackageupdater_test.go new file mode 100644 index 000000000..b50fc9dfa --- /dev/null +++ b/utils/remediation/packageupdaters/mavenpackageupdater_test.go @@ -0,0 +1,212 @@ +package packageupdaters + +import ( + "errors" + "os" + "path/filepath" + "testing" + + biutils "github.com/jfrog/build-info-go/utils" + "github.com/jfrog/jfrog-cli-security/utils/formats" + "github.com/jfrog/jfrog-cli-security/utils/techutils" + "github.com/jfrog/jfrog-client-go/utils/io/fileutils" + "github.com/stretchr/testify/assert" +) + +func TestMavenUpdateDependency(t *testing.T) { + testProjectPath := filepath.Join("..", "testdata", "packageupdaters") + currDir, err := os.Getwd() + assert.NoError(t, err) + + propertyPOM := ` + + 4.0.0 + test + test + 1.0 + + + 2.9.8 + + + + + com.fasterxml.jackson.core + jackson-databind + ${jackson.version} + + +` + + parentPOM := ` + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 2.5.0 + + + test + test + 1.0 +` + + testCases := []struct { + name string + customPOM string // if non-empty, overwrites pom.xml after copying testdata + fixDetails *FixDetails + expectedContains []string + expectedNotContain []string + }{ + { + name: "RegularDependency", + fixDetails: &FixDetails{ + SuggestedFixedVersion: "1.1.5", + IsDirectDependency: true, + Technology: techutils.Maven, + ImpactedDependencyName: "org.jfrog.filespecs:file-specs-java", + Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, + }, + expectedContains: []string{"1.1.5"}, + expectedNotContain: []string{"1.1.1"}, + }, + { + name: "DependencyManagement", + fixDetails: &FixDetails{ + SuggestedFixedVersion: "2.15.0", + IsDirectDependency: true, + Technology: techutils.Maven, + ImpactedDependencyName: "com.fasterxml.jackson.core:jackson-core", + Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, + }, + expectedContains: []string{"2.15.0"}, + expectedNotContain: []string{"2.13.4"}, + }, + { + name: "PropertyVersion", + customPOM: propertyPOM, + fixDetails: &FixDetails{ + SuggestedFixedVersion: "2.13.0", + IsDirectDependency: true, + Technology: techutils.Maven, + ImpactedDependencyName: "com.fasterxml.jackson.core:jackson-databind", + Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, + }, + expectedContains: []string{"2.13.0"}, + expectedNotContain: []string{"2.9.8"}, + }, + { + name: "ParentPOM", + customPOM: parentPOM, + fixDetails: &FixDetails{ + SuggestedFixedVersion: "2.7.0", + IsDirectDependency: true, + Technology: techutils.Maven, + ImpactedDependencyName: "org.springframework.boot:spring-boot-starter-parent", + Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, + }, + expectedContains: []string{"2.7.0"}, + expectedNotContain: []string{"2.5.0"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "maven-test-*") + assert.NoError(t, err) + defer func() { + assert.NoError(t, fileutils.RemoveTempDir(tmpDir)) + }() + + assert.NoError(t, biutils.CopyDir(testProjectPath, tmpDir, true, nil)) + if tc.customPOM != "" { + assert.NoError(t, os.WriteFile(filepath.Join(tmpDir, "pom.xml"), []byte(tc.customPOM), 0644)) + } + assert.NoError(t, os.Chdir(tmpDir)) + defer func() { + assert.NoError(t, os.Chdir(currDir)) + }() + + updater := &MavenPackageUpdater{} + err = updater.UpdateDependency(tc.fixDetails) + assert.NoError(t, err) + + modifiedPom, err := os.ReadFile("pom.xml") + assert.NoError(t, err) + content := string(modifiedPom) + for _, s := range tc.expectedContains { + assert.Contains(t, content, s) + } + for _, s := range tc.expectedNotContain { + assert.NotContains(t, content, s) + } + }) + } +} + +func TestMavenUpdateDependencyErrors(t *testing.T) { + testProjectPath := filepath.Join("..", "testdata", "packageupdaters") + currDir, err := os.Getwd() + assert.NoError(t, err) + + testCases := []struct { + name string + fixDetails *FixDetails + useTestData bool + assertErr func(t *testing.T, err error) + }{ + { + name: "DependencyNotFound", + fixDetails: &FixDetails{ + SuggestedFixedVersion: "1.0.0", + IsDirectDependency: true, + Technology: techutils.Maven, + ImpactedDependencyName: "com.nonexistent:package", + Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, + }, + useTestData: true, + assertErr: func(t *testing.T, err error) { + assert.Error(t, err) + assert.Contains(t, err.Error(), "com.nonexistent:package") + }, + }, + { + name: "IndirectDependencyNotSupported", + fixDetails: &FixDetails{ + SuggestedFixedVersion: "1.0.0", + IsDirectDependency: false, + Technology: techutils.Maven, + ImpactedDependencyName: "org.springframework:spring-core", + Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, + }, + useTestData: false, + assertErr: func(t *testing.T, err error) { + assert.Error(t, err) + var unsupportedErr *ErrUnsupportedFix + assert.True(t, errors.As(err, &unsupportedErr)) + assert.Equal(t, IndirectDependencyFixNotSupported, unsupportedErr.ErrorType) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + if tc.useTestData { + tmpDir, err := os.MkdirTemp("", "maven-test-*") + assert.NoError(t, err) + defer func() { + assert.NoError(t, fileutils.RemoveTempDir(tmpDir)) + }() + assert.NoError(t, biutils.CopyDir(testProjectPath, tmpDir, true, nil)) + assert.NoError(t, os.Chdir(tmpDir)) + defer func() { + assert.NoError(t, os.Chdir(currDir)) + }() + } + updater := &MavenPackageUpdater{} + err := updater.UpdateDependency(tc.fixDetails) + tc.assertErr(t, err) + }) + } +} diff --git a/utils/remediation/packageupdaters/npmpackageupdater.go b/utils/remediation/packageupdaters/npmpackageupdater.go new file mode 100644 index 000000000..c7811b03c --- /dev/null +++ b/utils/remediation/packageupdaters/npmpackageupdater.go @@ -0,0 +1,190 @@ +package packageupdaters + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/jfrog/jfrog-client-go/utils/log" +) + +const ( + // CiEnv is the environment variable for CI mode. + CiEnv = "CI" + // ConfigIgnoreScriptsEnv disables npm lifecycle scripts. + ConfigIgnoreScriptsEnv = "NPM_CONFIG_IGNORE_SCRIPTS" + // ConfigAuditEnv disables npm audit output. + ConfigAuditEnv = "NPM_CONFIG_AUDIT" + // ConfigFundEnv disables npm fund output. + ConfigFundEnv = "NPM_CONFIG_FUND" + // ConfigLevelEnv sets the npm log level. + ConfigLevelEnv = "NPM_CONFIG_LOGLEVEL" + + npmPackageLockOnlyFlag = "--package-lock-only" + npmIgnoreScriptsFlag = "--ignore-scripts" + npmNoAuditFlag = "--no-audit" + npmLegacyPeerDepsFlag = "--legacy-peer-deps" + npmNoFundFlag = "--no-fund" + + npmLockFileName = "package-lock.json" + + npmEreresolveErrorPrefix = "ERESOLVE" + + // Unexported aliases referenced by pnpmpackageupdater.go. + ciEnv = CiEnv + configLevelEnv = ConfigLevelEnv +) + +// NpmInstallEnvVars are the environment variable overrides used during npm install. +var NpmInstallEnvVars = map[string]string{ + ConfigIgnoreScriptsEnv: "true", + ConfigAuditEnv: "false", + ConfigFundEnv: "false", + ConfigLevelEnv: "error", + CiEnv: "true", +} + +type NpmPackageUpdater struct { + CommonPackageUpdater +} + +func (npm *NpmPackageUpdater) UpdateDependency(fixDetails *FixDetails) error { + if fixDetails.IsDirectDependency { + return npm.updateDirectDependency(fixDetails) + } + return &ErrUnsupportedFix{ + PackageName: fixDetails.ImpactedDependencyName, + FixedVersion: fixDetails.SuggestedFixedVersion, + ErrorType: IndirectDependencyFixNotSupported, + } +} + +func (npm *NpmPackageUpdater) updateDirectDependency(fixDetails *FixDetails) error { + descriptorPaths := npm.CollectVulnerabilityDescriptorPaths(fixDetails, []string{nodePackageJSONFileName}, []string{nodeModulesDirName}) + if len(descriptorPaths) == 0 { + return fmt.Errorf("no descriptor evidence was found for package %s", fixDetails.ImpactedDependencyName) + } + + originalWd, err := os.Getwd() + if err != nil { + return fmt.Errorf("failed to get current working directory: %w", err) + } + + var failingDescriptors []string + for _, descriptorPath := range descriptorPaths { + if fixErr := npm.fixVulnerabilityAndRegenerateLock(fixDetails, descriptorPath, originalWd); fixErr != nil { + failedFixErrorMsg := fmt.Errorf("failed to fix '%s' in descriptor '%s': %w", fixDetails.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", fixDetails.ImpactedDependencyName, strings.Join(failingDescriptors, ", "), err) + } + + return nil +} + +func (npm *NpmPackageUpdater) fixVulnerabilityAndRegenerateLock(fixDetails *FixDetails, descriptorPath string, originalWd string) error { + backupContent, err := npm.UpdatePackageJSONDescriptor(descriptorPath, fixDetails.ImpactedDependencyName, fixDetails.SuggestedFixedVersion) + if err != nil { + return err + } + + descriptorDir := filepath.Dir(descriptorPath) + lockFilePath := filepath.Join(descriptorDir, npmLockFileName) + + lockFileTracked, checkErr := IsFileTrackedByGit(lockFilePath, 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", lockFilePath)) + return nil + } + + if err = npm.regenerateLockfile(fixDetails, descriptorPath, originalWd, backupContent); err != nil { + return err + } + + log.Debug(fmt.Sprintf("Successfully updated '%s' from version '%s' to '%s' in descriptor '%s'", fixDetails.ImpactedDependencyName, fixDetails.ImpactedDependencyVersion, fixDetails.SuggestedFixedVersion, descriptorPath)) + return nil +} + +func (npm *NpmPackageUpdater) regenerateLockfile(fixDetails *FixDetails, descriptorPath, originalWd string, backupContent []byte) error { + return npm.withDescriptorWorkingDir(descriptorPath, originalWd, func() error { + if err := npm.regenerateLockFileWithRetry(); err != nil { + log.Warn(fmt.Sprintf("Failed to regenerate lock file after updating '%s' to version '%s': %s. Rolling back...", fixDetails.ImpactedDependencyName, fixDetails.SuggestedFixedVersion, err.Error())) + //#nosec G306 -- 0644 is correct for a checked-out source file. + if rollbackErr := os.WriteFile(descriptorPath, backupContent, 0644); rollbackErr != nil { + return fmt.Errorf("failed to rollback descriptor after lock file regeneration failure: %w (original error: %v)", rollbackErr, err) + } + return err + } + return nil + }) +} + +func (npm *NpmPackageUpdater) GetFixedDescriptor(content []byte, packageName, newVersion, descriptorPath string) ([]byte, error) { + return npm.GetFixedPackageJSONManifest(content, packageName, newVersion, descriptorPath) +} + +func (npm *NpmPackageUpdater) regenerateLockFileWithRetry() error { + err := npm.runNpmInstall(false) + if err != nil { + if strings.Contains(err.Error(), npmEreresolveErrorPrefix) { + log.Debug(fmt.Sprintf("First npm install attempt failed due to peer dependency conflict. Retrying with %s...", npmLegacyPeerDepsFlag)) + if err = npm.runNpmInstall(true); err != nil { + return fmt.Errorf("npm install failed after retry with %s: %w", npmLegacyPeerDepsFlag, err) + } + return nil + } + log.Debug(fmt.Sprintf("First npm install attempt failed: %s. Retrying...", err.Error())) + if err = npm.runNpmInstall(false); err != nil { + return fmt.Errorf("npm install failed after retry: %w", err) + } + } + return nil +} + +func (npm *NpmPackageUpdater) runNpmInstall(useLegacyPeerDeps bool) error { + args := []string{ + "install", + npmPackageLockOnlyFlag, + npmIgnoreScriptsFlag, + npmNoAuditFlag, + npmNoFundFlag, + } + if useLegacyPeerDeps { + args = append(args, npmLegacyPeerDepsFlag) + } + + fullCommand := "npm " + strings.Join(args, " ") + log.Debug(fmt.Sprintf("Running '%s'", fullCommand)) + + ctx, cancel := context.WithTimeout(context.Background(), nodePackageManagerInstallTimeout) + defer cancel() + + //#nosec G204 -- False positive - the subprocess only runs after the user's approval + cmd := exec.CommandContext(ctx, "npm", args...) + + cmd.Env = npm.BuildEnvWithOverrides(NpmInstallEnvVars) + output, err := cmd.CombinedOutput() + + if errors.Is(ctx.Err(), context.DeadlineExceeded) || errors.Is(err, context.DeadlineExceeded) { + return fmt.Errorf("npm install timed out after %v", nodePackageManagerInstallTimeout) + } + + if err != nil { + return fmt.Errorf("npm install failed: %w\nOutput: %s", err, string(output)) + } + + return nil +} diff --git a/utils/remediation/packageupdaters/npmpackageupdater_test.go b/utils/remediation/packageupdaters/npmpackageupdater_test.go new file mode 100644 index 000000000..6e72e5f99 --- /dev/null +++ b/utils/remediation/packageupdaters/npmpackageupdater_test.go @@ -0,0 +1,245 @@ +package packageupdaters + +import ( + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetFixedDescriptor(t *testing.T) { + npm := &NpmPackageUpdater{} + + testcases := []struct { + name string + originalContent string + packageName string + newVersion string + expectedContent string + expectError bool + }{ + { + name: "update exact version", + originalContent: `{"dependencies": {"minimist": "1.2.5"}}`, + packageName: "minimist", + newVersion: "1.2.6", + expectedContent: `{"dependencies": {"minimist": "1.2.6"}}`, + expectError: false, + }, + { + name: "update version with caret prefix - removes prefix", + originalContent: `{"dependencies": {"lodash": "^4.17.0"}}`, + packageName: "lodash", + newVersion: "4.17.21", + expectedContent: `{"dependencies": {"lodash": "4.17.21"}}`, + expectError: false, + }, + { + name: "update version with tilde prefix - removes prefix", + originalContent: `{"dependencies": {"express": "~4.18.0"}}`, + packageName: "express", + newVersion: "4.18.2", + expectedContent: `{"dependencies": {"express": "4.18.2"}}`, + expectError: false, + }, + { + name: "update scoped package", + originalContent: `{"dependencies": {"@types/node": "18.0.0"}}`, + packageName: "@types/node", + newVersion: "18.11.0", + expectedContent: `{"dependencies": {"@types/node": "18.11.0"}}`, + expectError: false, + }, + { + name: "package not found", + originalContent: `{"dependencies": {"lodash": "4.17.0"}}`, + packageName: "minimist", + newVersion: "1.2.6", + expectedContent: "", + expectError: true, + }, + { + name: "update in devDependencies", + originalContent: `{"devDependencies": {"minimist": "1.2.5"}}`, + packageName: "minimist", + newVersion: "1.2.6", + expectedContent: `{"devDependencies": {"minimist": "1.2.6"}}`, + expectError: false, + }, + { + name: "update in optionalDependencies", + originalContent: `{"optionalDependencies": {"minimist": "1.2.5"}}`, + packageName: "minimist", + newVersion: "1.2.6", + expectedContent: `{"optionalDependencies": {"minimist": "1.2.6"}}`, + expectError: false, + }, + { + name: "update in overrides section", + originalContent: `{"dependencies": {"express": "4.18.0"}, "overrides": {"minimist": "1.2.5"}}`, + packageName: "minimist", + newVersion: "1.2.6", + expectedContent: `{"dependencies": {"express": "4.18.0"}, "overrides": {"minimist": "1.2.6"}}`, + expectError: false, + }, + { + name: "update in both dependencies and overrides", + originalContent: `{"dependencies": {"minimist": "1.2.5"}, "overrides": {"minimist": "1.2.5"}}`, + packageName: "minimist", + newVersion: "1.2.6", + expectedContent: `{"dependencies": {"minimist": "1.2.6"}, "overrides": {"minimist": "1.2.6"}}`, + expectError: false, + }, + { + name: "skip peerDependencies section - package only in peerDependencies", + originalContent: `{"dependencies": {"express": "4.18.0"}, "peerDependencies": {"minimist": "1.2.5"}}`, + packageName: "minimist", + newVersion: "1.2.6", + expectedContent: "", + expectError: true, + }, + { + name: "skip peerDependencies section - package in both dependencies and peerDependencies", + originalContent: `{"dependencies": {"minimist": "1.2.5"}, "peerDependencies": {"minimist": "1.2.5"}}`, + packageName: "minimist", + newVersion: "1.2.6", + expectedContent: `{"dependencies": {"minimist": "1.2.6"}, "peerDependencies": {"minimist": "1.2.5"}}`, + expectError: false, + }, + { + name: "update in multiple allowed sections", + originalContent: `{"dependencies": {"minimist": "1.2.5"}, "devDependencies": {"minimist": "1.2.5"}}`, + packageName: "minimist", + newVersion: "1.2.6", + expectedContent: `{"dependencies": {"minimist": "1.2.6"}, "devDependencies": {"minimist": "1.2.6"}}`, + expectError: false, + }, + { + name: "package name with dot", + originalContent: `{"dependencies": {"vue.config": "1.0.0"}}`, + packageName: "vue.config", + newVersion: "2.0.0", + expectedContent: `{"dependencies": {"vue.config": "2.0.0"}}`, + expectError: false, + }, + { + name: "no dependency sections", + originalContent: `{"name": "test", "version": "1.0.0"}`, + packageName: "minimist", + newVersion: "1.2.6", + expectedContent: "", + expectError: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + result, err := npm.GetFixedDescriptor([]byte(tc.originalContent), tc.packageName, tc.newVersion, "package.json") + + if tc.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tc.expectedContent, string(result)) + } + }) + } +} + +func TestEscapeJsonPathKey(t *testing.T) { + testcases := []struct { + name string + input string + expected string + }{ + { + name: "simple package name", + input: "lodash", + expected: "lodash", + }, + { + name: "scoped package", + input: "@types/node", + expected: "@types/node", + }, + { + name: "package with dot", + input: "vue.config", + expected: "vue\\.config", + }, + { + name: "package with multiple dots", + input: "some.package.name", + expected: "some\\.package\\.name", + }, + { + name: "package with asterisk", + input: "package*name", + expected: "package\\*name", + }, + { + name: "package with question mark", + input: "package?name", + expected: "package\\?name", + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + result := EscapeJsonPathKey(tc.input) + assert.Equal(t, tc.expected, result) + }) + } +} + +func TestBuildIsolatedEnv(t *testing.T) { + testcases := []struct { + name string + predefineEnv bool + }{ + { + name: "sets required env vars", + predefineEnv: false, + }, + { + name: "overrides conflicting user env vars", + predefineEnv: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + if tc.predefineEnv { + originalCI := os.Getenv(CiEnv) + defer func() { + assert.NoError(t, os.Setenv(CiEnv, originalCI)) + }() + assert.NoError(t, os.Setenv(CiEnv, "false")) + } + + npm := &NpmPackageUpdater{} + env := npm.BuildEnvWithOverrides(NpmInstallEnvVars) + + envMap := make(map[string]string) + envCount := make(map[string]int) + for _, e := range env { + parts := strings.SplitN(e, "=", 2) + if len(parts) == 2 { + envMap[parts[0]] = parts[1] + envCount[parts[0]]++ + } + } + + assert.Equal(t, "true", envMap[ConfigIgnoreScriptsEnv]) + assert.Equal(t, "false", envMap[ConfigAuditEnv]) + assert.Equal(t, "false", envMap[ConfigFundEnv]) + assert.Equal(t, "error", envMap[ConfigLevelEnv]) + assert.Equal(t, "true", envMap[CiEnv]) + + if tc.predefineEnv { + assert.Equal(t, 1, envCount[CiEnv], "CI should appear exactly once") + } + }) + } +} diff --git a/utils/remediation/packageupdaters/pnpmpackageupdater.go b/utils/remediation/packageupdaters/pnpmpackageupdater.go new file mode 100644 index 000000000..c44e9da61 --- /dev/null +++ b/utils/remediation/packageupdaters/pnpmpackageupdater.go @@ -0,0 +1,156 @@ +package packageupdaters + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/jfrog/jfrog-client-go/utils/log" +) + +const ( + pnpmLockFileName = "pnpm-lock.yaml" + pnpmLockfileOnlyFlag = "--lockfile-only" + pnpmIgnoreScriptsFlag = "--ignore-scripts" + pnpmNoFrozenLockfileFlag = "--no-frozen-lockfile" + PnpmFrozenLockfileEnv = "PNPM_FROZEN_LOCKFILE" +) + +// PnpmLockfileInstallEnvOverrides returns environment variable overrides for pnpm install +// when refreshing pnpm-lock.yaml after editing package.json. +func PnpmLockfileInstallEnvOverrides() map[string]string { + return map[string]string{ + PnpmFrozenLockfileEnv: "false", + configLevelEnv: "error", + ciEnv: "true", + } +} + +func PnpmFilterCoordinateStyleDescriptorPaths(paths []string) []string { + if len(paths) == 0 { + return paths + } + out := make([]string, 0, len(paths)) + for _, p := range paths { + if !EvidencePathLooksLikeNpmPackageCoordinate(p) { + out = append(out, p) + } + } + return out +} + +type PnpmPackageUpdater struct { + CommonPackageUpdater +} + +func (pnpm *PnpmPackageUpdater) UpdateDependency(fixDetails *FixDetails) error { + if fixDetails.IsDirectDependency { + return pnpm.updateDirectDependency(fixDetails) + } + return &ErrUnsupportedFix{ + PackageName: fixDetails.ImpactedDependencyName, + FixedVersion: fixDetails.SuggestedFixedVersion, + ErrorType: IndirectDependencyFixNotSupported, + } +} + +func (pnpm *PnpmPackageUpdater) updateDirectDependency(fixDetails *FixDetails) error { + descriptorPaths := pnpm.CollectVulnerabilityDescriptorPaths(fixDetails, []string{nodePackageJSONFileName}, []string{nodeModulesDirName}) + descriptorPaths = PnpmFilterCoordinateStyleDescriptorPaths(descriptorPaths) + if len(descriptorPaths) == 0 { + return fmt.Errorf("no descriptor evidence was found for package %s", fixDetails.ImpactedDependencyName) + } + + originalWd, err := os.Getwd() + if err != nil { + return fmt.Errorf("failed to get current working directory: %w", err) + } + + var failingDescriptors []string + for _, descriptorPath := range descriptorPaths { + if fixErr := pnpm.fixVulnerabilityAndRegenerateLock(fixDetails, descriptorPath, originalWd); fixErr != nil { + failedFixErrorMsg := fmt.Errorf("failed to fix '%s' in descriptor '%s': %w", fixDetails.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", fixDetails.ImpactedDependencyName, strings.Join(failingDescriptors, ", "), err) + } + + return nil +} + +func (pnpm *PnpmPackageUpdater) fixVulnerabilityAndRegenerateLock(fixDetails *FixDetails, descriptorPath string, originalWd string) error { + backupContent, err := pnpm.UpdatePackageJSONDescriptor(descriptorPath, fixDetails.ImpactedDependencyName, fixDetails.SuggestedFixedVersion) + if err != nil { + return err + } + + descriptorDir := filepath.Dir(descriptorPath) + lockFilePath := filepath.Join(descriptorDir, pnpmLockFileName) + + lockFileTracked, checkErr := IsFileTrackedByGit(lockFilePath, 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", lockFilePath)) + return nil + } + + if err = pnpm.regenerateLockfile(fixDetails, descriptorPath, originalWd, backupContent); err != nil { + return err + } + + log.Debug(fmt.Sprintf("Successfully updated '%s' from version '%s' to '%s' in descriptor '%s'", fixDetails.ImpactedDependencyName, fixDetails.ImpactedDependencyVersion, fixDetails.SuggestedFixedVersion, descriptorPath)) + return nil +} + +func (pnpm *PnpmPackageUpdater) regenerateLockfile(fixDetails *FixDetails, descriptorPath, originalWd string, backupContent []byte) error { + return pnpm.withDescriptorWorkingDir(descriptorPath, originalWd, func() error { + if err := pnpm.runPnpmInstallLockOnly(); err != nil { + log.Warn(fmt.Sprintf("Failed to regenerate lock file after updating '%s' to version '%s': %s. Rolling back...", fixDetails.ImpactedDependencyName, fixDetails.SuggestedFixedVersion, err.Error())) + //#nosec G306 -- 0644 is correct for a checked-out source file. + if rollbackErr := os.WriteFile(descriptorPath, backupContent, 0644); rollbackErr != nil { + return fmt.Errorf("failed to rollback descriptor after lock file regeneration failure: %w (original error: %v)", rollbackErr, err) + } + return err + } + return nil + }) +} + +func (pnpm *PnpmPackageUpdater) runPnpmInstallLockOnly() error { + args := []string{ + "install", + pnpmLockfileOnlyFlag, + pnpmIgnoreScriptsFlag, + pnpmNoFrozenLockfileFlag, + } + fullCommand := "pnpm " + strings.Join(args, " ") + log.Debug(fmt.Sprintf("Running '%s'", fullCommand)) + + ctx, cancel := context.WithTimeout(context.Background(), nodePackageManagerInstallTimeout) + defer cancel() + + //#nosec G204 -- False positive - the subprocess only runs after the user's approval + cmd := exec.CommandContext(ctx, "pnpm", args...) + cmd.Env = EnvWithCorepackIntegrityWorkaround(pnpm.BuildEnvWithOverrides(PnpmLockfileInstallEnvOverrides())) + + output, err := cmd.CombinedOutput() + if errors.Is(ctx.Err(), context.DeadlineExceeded) || errors.Is(err, context.DeadlineExceeded) { + return fmt.Errorf("pnpm install timed out after %v", nodePackageManagerInstallTimeout) + } + if err != nil { + return fmt.Errorf("pnpm install failed: %w\nOutput: %s", err, string(output)) + } + return nil +} diff --git a/utils/remediation/packageupdaters/pnpmpackageupdater_test.go b/utils/remediation/packageupdaters/pnpmpackageupdater_test.go new file mode 100644 index 000000000..550f50858 --- /dev/null +++ b/utils/remediation/packageupdaters/pnpmpackageupdater_test.go @@ -0,0 +1,72 @@ +package packageupdaters + +import ( + "strings" + "testing" + + "github.com/jfrog/jfrog-cli-security/utils/techutils" + "github.com/stretchr/testify/assert" +) + +func TestEvidencePathLooksLikeNpmPackageCoordinate(t *testing.T) { + t.Parallel() + tests := []struct { + path string + wantTrue bool + }{ + {"lodash@4.17.19/package.json", true}, + {"axios@0.21.1/package.json", true}, + {"nested/pkg@1.0.0-rc.1/sub/package.json", true}, + {"package.json", false}, + {"apps/web/package.json", false}, + {"node_modules/@types/node/package.json", false}, + {"node_modules/@scope/pkg/package.json", false}, + {"@types/node/package.json", false}, + } + for _, tt := range tests { + t.Run(tt.path, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.wantTrue, EvidencePathLooksLikeNpmPackageCoordinate(tt.path), tt.path) + }) + } +} + +func TestPnpmFilterCoordinateStyleDescriptorPaths(t *testing.T) { + t.Parallel() + in := []string{ + "lodash@4.17.19/package.json", + "axios@0.21.1/package.json", + "package.json", + "apps/web/package.json", + "node_modules/@types/node/package.json", + } + want := []string{"package.json", "apps/web/package.json", "node_modules/@types/node/package.json"} + assert.ElementsMatch(t, want, PnpmFilterCoordinateStyleDescriptorPaths(in)) +} + +func TestPnpmCollectLeavesNpmParityThenPnpmFilterDropsCoordinates(t *testing.T) { + t.Parallel() + pnpm := &PnpmPackageUpdater{} + vuln := createFixDetails(techutils.Pnpm, "lodash", "4.17.19", "4.17.21", true, + "lodash@4.17.19/package.json", "axios@0.21.1/package.json", "package.json") + raw := pnpm.CollectVulnerabilityDescriptorPaths(vuln, []string{NodePackageJSONFileName}, []string{NodeModulesDirName}) + assert.ElementsMatch(t, []string{"lodash@4.17.19/package.json", "axios@0.21.1/package.json", "package.json"}, raw) + assert.ElementsMatch(t, []string{"package.json"}, PnpmFilterCoordinateStyleDescriptorPaths(raw)) +} + +func TestPnpmLockRegenerationEnv(t *testing.T) { + t.Parallel() + pnpm := &PnpmPackageUpdater{} + env := EnvWithCorepackIntegrityWorkaround(pnpm.BuildEnvWithOverrides(PnpmLockfileInstallEnvOverrides())) + envMap := make(map[string]string) + for _, e := range env { + parts := strings.SplitN(e, "=", 2) + if len(parts) == 2 { + envMap[parts[0]] = parts[1] + } + } + assert.Equal(t, "false", envMap[PnpmFrozenLockfileEnv]) + assert.Equal(t, "error", envMap[ConfigLevelEnv]) + assert.Equal(t, "true", envMap[CiEnv]) + assert.Equal(t, "0", envMap["COREPACK_INTEGRITY_KEYS"]) +} diff --git a/utils/remediation/packageupdaters/pythonpackageupdater.go b/utils/remediation/packageupdaters/pythonpackageupdater.go new file mode 100644 index 000000000..1a3f4fcf0 --- /dev/null +++ b/utils/remediation/packageupdaters/pythonpackageupdater.go @@ -0,0 +1,116 @@ +package packageupdaters + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/jfrog/jfrog-cli-security/utils/techutils" +) + +const ( + defaultRequirementFile = "requirements.txt" + // Package names are case-insensitive with this prefix + PythonPackageRegexPrefix = "(?i)" + // Match all possible operators and versions syntax + PythonPackageRegexSuffix = "\\s*(([\\=\\<\\>\\~]=)|([\\>\\<]))\\s*(\\.|\\d)*(\\d|(\\.\\*))(\\,\\s*(([\\=\\<\\>\\~]=)|([\\>\\<])).*\\s*(\\.|\\d)*(\\d|(\\.\\*)))?" +) + +type PythonPackageUpdater struct { + pipRequirementsFile string + CommonPackageUpdater +} + +func (py *PythonPackageUpdater) UpdateDependency(fixDetails *FixDetails) error { + if fixDetails.IsDirectDependency { + return py.updateDirectDependency(fixDetails) + } + + return &ErrUnsupportedFix{ + PackageName: fixDetails.ImpactedDependencyName, + FixedVersion: fixDetails.SuggestedFixedVersion, + ErrorType: IndirectDependencyFixNotSupported, + } +} + +func (py *PythonPackageUpdater) updateDirectDependency(fixDetails *FixDetails) (err error) { + switch fixDetails.Technology { + case techutils.Poetry: + return py.handlePoetry(fixDetails) + case techutils.Pip: + return py.handlePip(fixDetails) + case techutils.Pipenv: + return py.CommonPackageUpdater.UpdateDependency(fixDetails, fixDetails.Technology.GetPackageInstallationCommand()) + default: + return errors.New("unknown python package manger: " + fixDetails.Technology.GetPackageType()) + } +} + +func (py *PythonPackageUpdater) handlePoetry(fixDetails *FixDetails) (err error) { + if err = py.CommonPackageUpdater.UpdateDependency(fixDetails, fixDetails.Technology.GetPackageInstallationCommand()); err != nil { + return + } + return runPackageMangerCommand(techutils.Poetry.GetExecCommandName(), techutils.Poetry.String(), []string{"update"}) +} + +func (py *PythonPackageUpdater) handlePip(fixDetails *FixDetails) (err error) { + var fixedFile string + fixedPackage := fixDetails.ImpactedDependencyName + "==" + fixDetails.SuggestedFixedVersion + currentFile, err := py.tryGetRequirementFile() + if err != nil { + return errors.New("failed to read pip requirements file: " + err.Error()) + } + re := regexp.MustCompile(PythonPackageRegexPrefix + "(" + fixDetails.ImpactedDependencyName + "|" + strings.ToLower(fixDetails.ImpactedDependencyName) + ")" + PythonPackageRegexSuffix) + if packageToReplace := re.FindString(currentFile); packageToReplace != "" { + fixedFile = strings.Replace(currentFile, packageToReplace, strings.ToLower(fixedPackage), 1) + } + if fixedFile == "" { + return fmt.Errorf("impacted package %s not found, fix failed", fixDetails.ImpactedDependencyName) + } + //#nosec G703 -- False positive - the path is determined by internal file scanning, not user input, and was already validated by the preceding Stat call. + if err = os.WriteFile(py.pipRequirementsFile, []byte(fixedFile), 0600); err != nil { + err = fmt.Errorf("an error occured while writing the fixed version of %s to the requirements file:\n%s", fixDetails.SuggestedFixedVersion, err.Error()) + } + return +} + +func (py *PythonPackageUpdater) tryGetRequirementFile() (string, error) { + if py.pipRequirementsFile != "" { + fileContent, err := py.tryReadRequirementFile(py.pipRequirementsFile) + if err != nil { + return "", err + } + return fileContent, nil + } else { + py.pipRequirementsFile = "setup.py" + fileContent, err := py.tryReadRequirementFile(py.pipRequirementsFile) + if err != nil { + py.pipRequirementsFile = "requirements.txt" + fileContent, err = py.tryReadRequirementFile(py.pipRequirementsFile) + if err != nil { + return "", err + } + return fileContent, nil + } + return fileContent, nil + } +} + +func (py *PythonPackageUpdater) tryReadRequirementFile(file string) (string, error) { + wd, err := os.Getwd() + if err != nil { + return "", err + } + fullPath := filepath.Join(wd, file) + if !strings.HasPrefix(filepath.Clean(fullPath), wd) { + return "", errors.New("wrong requirements file input: " + fullPath) + } + data, err := os.ReadFile(filepath.Clean(file)) + if err != nil { + return "", errors.New("an error occurred while attempting to read the requirements file:\n" + err.Error()) + } + return string(data), nil +} diff --git a/utils/remediation/packageupdaters/pythonpackageupdater_test.go b/utils/remediation/packageupdaters/pythonpackageupdater_test.go new file mode 100644 index 000000000..2fcfaa3b8 --- /dev/null +++ b/utils/remediation/packageupdaters/pythonpackageupdater_test.go @@ -0,0 +1,30 @@ +package packageupdaters + +import ( + "regexp" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestPipPackageRegex(t *testing.T) { + var pipPackagesRegexTests = []pipPackageRegexTest{ + {"oslo.config", "oslo.config>=1.12.1,<1.13"}, + {"oslo.utils", "oslo.utils<5.0,>=4.0.0"}, + {"paramiko", "paramiko==2.7.2"}, + {"passlib", "passlib<=1.7.4"}, + {"PassLib", "passlib<=1.7.4"}, + {"prance", "prance>=0.9.0"}, + {"prompt-toolkit", "prompt-toolkit~=1.0.15"}, + {"pyinotify", "pyinotify>0.9.6"}, + {"pyjwt", "pyjwt>1.7.1"}, + {"PyJWT", "pyjwt>1.7.1"}, + {"urllib3", "urllib3 > 1.1.9, < 1.5.*"}, + } + for _, pack := range pipPackagesRegexTests { + re := regexp.MustCompile(PythonPackageRegexPrefix + "(" + pack.packageName + "|" + strings.ToLower(pack.packageName) + ")" + PythonPackageRegexSuffix) + found := re.FindString(requirementsFile) + assert.Equal(t, pack.expectedRequirement, strings.ToLower(found)) + } +} diff --git a/utils/remediation/packageupdaters/types.go b/utils/remediation/packageupdaters/types.go new file mode 100644 index 000000000..b7a414463 --- /dev/null +++ b/utils/remediation/packageupdaters/types.go @@ -0,0 +1,48 @@ +package packageupdaters + +import ( + "fmt" + + "github.com/jfrog/jfrog-cli-security/utils/formats" + "github.com/jfrog/jfrog-cli-security/utils/techutils" +) + +// FixDetails holds the minimal information an updater needs to fix a vulnerability. +type FixDetails struct { + ImpactedDependencyName string + ImpactedDependencyVersion string + SuggestedFixedVersion string + IsDirectDependency bool + Technology techutils.Technology + // Components holds the evidence of where the dependency appears (used by updaters + // that discover descriptor paths from scan results). + Components []formats.ComponentRow + // IssueId is used for logging purposes only. + IssueId string +} + +// UnsupportedErrorType classifies the reason a fix is not supported. +type UnsupportedErrorType string + +const ( + IndirectDependencyFixNotSupported UnsupportedErrorType = "IndirectDependencyFixNotSupported" +) + +// ErrUnsupportedFix is returned when a fix cannot be applied for a known reason. +type ErrUnsupportedFix struct { + PackageName string + FixedVersion string + ErrorType UnsupportedErrorType +} + +func (err *ErrUnsupportedFix) Error() string { + if err.ErrorType == IndirectDependencyFixNotSupported { + return fmt.Sprintf("skipping fix of vulnerable package '%s' version '%s' - indirect dependency fix is not supported", err.PackageName, err.FixedVersion) + } + return fmt.Sprintf("skipping fix of vulnerable package '%s' - '%s', version '%s' - build tools dependency fix is not supported", err.PackageName, err.PackageName, err.FixedVersion) +} + +// PackageUpdater is the interface that all technology-specific updaters implement. +type PackageUpdater interface { + UpdateDependency(details *FixDetails) error +} diff --git a/utils/remediation/testdata/indirect-projects/go/go.mod.txt b/utils/remediation/testdata/indirect-projects/go/go.mod.txt new file mode 100755 index 000000000..eac5f66ae --- /dev/null +++ b/utils/remediation/testdata/indirect-projects/go/go.mod.txt @@ -0,0 +1,22 @@ +module goproject + +go 1.19 + +require github.com/gin-gonic/gin v1.7.3 + +require ( + github.com/gin-contrib/sse v0.1.0 // indirect + github.com/go-playground/locales v0.13.0 // indirect + github.com/go-playground/universal-translator v0.17.0 // indirect + github.com/go-playground/validator/v10 v10.4.1 // indirect + github.com/golang/protobuf v1.3.3 // indirect + github.com/json-iterator/go v1.1.9 // indirect + github.com/leodido/go-urn v1.2.0 // indirect + github.com/mattn/go-isatty v0.0.12 // indirect + github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421 // indirect + github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742 // indirect + github.com/ugorji/go/codec v1.1.7 // indirect + golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 // indirect + golang.org/x/sys v0.0.0-20200116001909-b77594299b42 // indirect + gopkg.in/yaml.v2 v2.2.8 // indirect +) diff --git a/utils/remediation/testdata/indirect-projects/go/go.sum.txt b/utils/remediation/testdata/indirect-projects/go/go.sum.txt new file mode 100755 index 000000000..032aefc80 --- /dev/null +++ b/utils/remediation/testdata/indirect-projects/go/go.sum.txt @@ -0,0 +1,33 @@ +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/gin-contrib/sse v0.1.0/go.mod h1:RHrZQHXnP2xjPF+u1gW/2HnVO7nvIa9PG3Gm+fLHvGI= +github.com/gin-gonic/gin v1.7.3/go.mod h1:jD2toBW3GZUr5UMcdrwQA10I7RuaFOl/SGeDjXkfUtY= +github.com/go-playground/assert/v2 v2.0.1/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4= +github.com/go-playground/locales v0.13.0/go.mod h1:taPMhCMXrRLJO55olJkUXHZBHCxTMfnGwq/HNwmWNS8= +github.com/go-playground/universal-translator v0.17.0/go.mod h1:UkSxE5sNxxRwHyU+Scu5vgOQjsIJAF8j9muTVoKLVtA= +github.com/go-playground/validator/v10 v10.4.1/go.mod h1:nlOn6nFhuKACm19sB/8EGNn9GlaMV7XkbRSipzJ0Ii4= +github.com/golang/protobuf v1.3.3/go.mod h1:vzj43D7+SQXF/4pzW/hwtAqwc6iTitCiVSaWz5lYuqw= +github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= +github.com/json-iterator/go v1.1.9/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4= +github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII= +github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= +github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= +github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +github.com/ugorji/go v1.1.7/go.mod h1:kZn38zHttfInRq0xu/PH0az30d+z6vm202qpg1oXVMw= +github.com/ugorji/go/codec v1.1.7/go.mod h1:Ax+UKWsSmolVDwsd+7N3ZtXu+yMGCf907BLYF3GoBXY= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/utils/remediation/testdata/indirect-projects/go/main.go.txt b/utils/remediation/testdata/indirect-projects/go/main.go.txt new file mode 100755 index 000000000..34a990c92 --- /dev/null +++ b/utils/remediation/testdata/indirect-projects/go/main.go.txt @@ -0,0 +1,10 @@ +package main + +import ( + "github.com/gin-gonic/gin" +) + +func main() { + print("test") + _ = gin.Default() +} diff --git a/utils/remediation/testdata/indirect-projects/maven/pom.xml b/utils/remediation/testdata/indirect-projects/maven/pom.xml new file mode 100755 index 000000000..ba270fc08 --- /dev/null +++ b/utils/remediation/testdata/indirect-projects/maven/pom.xml @@ -0,0 +1,67 @@ + + + 4.0.0 + org.jfrog.test + multi + 3.7-SNAPSHOT + pom + Simple Multi Modules Build + + + UTF-8 + 1.8 + 1.8 + + + + + junit + junit + 3.8.1 + test + + + + + + + + org.apache.maven.plugins + maven-jar-plugin + 2.4 + + + org.apache.maven.plugins + maven-war-plugin + 2.4 + + + org.apache.maven.plugins + maven-source-plugin + 2.1.2 + + + + + + org.apache.maven.plugins + maven-jar-plugin + + + false + + + + + org.apache.maven.plugins + maven-war-plugin + + + false + + + + + + diff --git a/utils/remediation/testdata/indirect-projects/pip/requirements.txt b/utils/remediation/testdata/indirect-projects/pip/requirements.txt new file mode 100755 index 000000000..802474953 --- /dev/null +++ b/utils/remediation/testdata/indirect-projects/pip/requirements.txt @@ -0,0 +1 @@ +requests==2.21.0 \ No newline at end of file diff --git a/utils/remediation/testdata/indirect-projects/pip/setup.py b/utils/remediation/testdata/indirect-projects/pip/setup.py new file mode 100755 index 000000000..3e10635fd --- /dev/null +++ b/utils/remediation/testdata/indirect-projects/pip/setup.py @@ -0,0 +1,9 @@ +from setuptools import setup, find_packages + +setup( + name="pip-example", + version="1.2.3", + install_requires=[ + "requests==2.21.0" + ], +) diff --git a/utils/remediation/testdata/indirect-projects/pipenv/Pipfile b/utils/remediation/testdata/indirect-projects/pipenv/Pipfile new file mode 100755 index 000000000..6d367636b --- /dev/null +++ b/utils/remediation/testdata/indirect-projects/pipenv/Pipfile @@ -0,0 +1,12 @@ +[[source]] +url = "https://pypi.python.org/simple" +verify_ssl = true +name = "pypi" + +[packages] +requests = "==2.21.0" + +[dev-packages] + +[requires] +python_version = "*" diff --git a/utils/remediation/testdata/indirect-projects/pipenv/Pipfile.lock b/utils/remediation/testdata/indirect-projects/pipenv/Pipfile.lock new file mode 100755 index 000000000..120702377 --- /dev/null +++ b/utils/remediation/testdata/indirect-projects/pipenv/Pipfile.lock @@ -0,0 +1,59 @@ +{ + "_meta": { + "hash": { + "sha256": "558fa3d8925a85d24792479c399941b74eda5d07bde65eace645285c07547361" + }, + "pipfile-spec": 6, + "requires": { + "python_version": "*" + }, + "sources": [ + { + "name": "pypi", + "url": "https://pypi.python.org/simple", + "verify_ssl": true + } + ] + }, + "default": { + "certifi": { + "hashes": [ + "sha256:35824b4c3a97115964b408844d64aa14db1cc518f6562e8d7261699d1350a9e3", + "sha256:4ad3232f5e926d6718ec31cfc1fcadfde020920e278684144551c91769c7bc18" + ], + "markers": "python_version >= '3.6'", + "version": "==2022.12.7" + }, + "chardet": { + "hashes": [ + "sha256:84ab92ed1c4d4f16916e05906b6b75a6c0fb5db821cc65e70cbd64a3e2a5eaae", + "sha256:fc323ffcaeaed0e0a02bf4d117757b98aed530d9ed4531e3e15460124c106691" + ], + "version": "==3.0.4" + }, + "idna": { + "hashes": [ + "sha256:c357b3f628cf53ae2c4c05627ecc484553142ca23264e593d327bcde5e9c3407", + "sha256:ea8b7f6188e6fa117537c3df7da9fc686d485087abf6ac197f9c46432f7e4a3c" + ], + "version": "==2.8" + }, + "requests": { + "hashes": [ + "sha256:502a824f31acdacb3a35b6690b5fbf0bc41d63a24a45c4004352b0242707598e", + "sha256:7bf2a778576d825600030a110f3c0e3e8edc51dfaafe1c146e39a2027784957b" + ], + "index": "pypi", + "version": "==2.21.0" + }, + "urllib3": { + "hashes": [ + "sha256:2393a695cd12afedd0dcb26fe5d50d0cf248e5a66f75dbd89a3d4eb333a61af4", + "sha256:a637e5fae88995b256e3409dc4d52c2e2e0ba32c42a6365fee8bbd2238de3cfb" + ], + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3' and python_version < '4'", + "version": "==1.24.3" + } + }, + "develop": {} +} diff --git a/utils/remediation/testdata/indirect-projects/poetry/pyproject.toml b/utils/remediation/testdata/indirect-projects/poetry/pyproject.toml new file mode 100755 index 000000000..4696f1574 --- /dev/null +++ b/utils/remediation/testdata/indirect-projects/poetry/pyproject.toml @@ -0,0 +1,13 @@ +[tool.poetry] +name = "poetry-project" +version = "0.1.0" +description = "" +authors = ["Your Name "] + +[tool.poetry.dependencies] +python = "^3.10" +requests = "==2.21.0" + +[build-system] +requires = ["poetry-core>=1.0.0"] +build-backend = "poetry.core.masonry.api" diff --git a/utils/remediation/testdata/packageupdaters/pom.xml b/utils/remediation/testdata/packageupdaters/pom.xml new file mode 100755 index 000000000..a8bf25093 --- /dev/null +++ b/utils/remediation/testdata/packageupdaters/pom.xml @@ -0,0 +1,63 @@ + + + 4.0.0 + + org.jenkins-ci.plugins + plugin + 4.53 + + + + 2.361 + true + 2.37.2 + + io.jenkins.plugins + jfrog + 1.1.x-SNAPSHOT + hpi + JFrog Plugin + + + + org.jfrog.filespecs + file-specs-java + 1.1.1 + + + + + + com.fasterxml.jackson.core + jackson-core + 2.13.4 + + + org.apache.httpcomponents + httpcore + 4.4.15 + + + + + + repo.jenkins-ci.org + https://repo.jenkins-ci.org/public/ + + + repo.jenkins-ci.org.releases + https://repo.jenkins-ci.org/releases/ + + + + + repo.jenkins-ci.org + https://repo.jenkins-ci.org/public/ + + + repo.jenkins-ci.org.releases + https://repo.jenkins-ci.org/releases/ + + + diff --git a/utils/remediation/testdata/projects/go/go.mod.txt b/utils/remediation/testdata/projects/go/go.mod.txt new file mode 100755 index 000000000..f33b477c2 --- /dev/null +++ b/utils/remediation/testdata/projects/go/go.mod.txt @@ -0,0 +1,13 @@ +module github.com/you/hello + +go 1.18 + +require ( + github.com/google/uuid v1.2.0 + github.com/sassoftware/go-rpmutils v0.1.0 +) + +require ( + github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 // indirect + golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9 // indirect +) diff --git a/utils/remediation/testdata/projects/go/go.sum.txt b/utils/remediation/testdata/projects/go/go.sum.txt new file mode 100755 index 000000000..4c1222bcd --- /dev/null +++ b/utils/remediation/testdata/projects/go/go.sum.txt @@ -0,0 +1,13 @@ +github.com/google/uuid v1.2.0 h1:qJYtXnJRWmpe7m/3XlyhrsLrEURqHRM2kxzoxXqyUDs= +github.com/google/uuid v1.2.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/sassoftware/go-rpmutils v0.1.0 h1:VLrna+tV+77Tclr956QkY/pTyyKomQlq2Xw6PuE8tsc= +github.com/sassoftware/go-rpmutils v0.1.0/go.mod h1:euhXULoBpvAxqrBHEyJS4Tsu3hHxUmQWNymxoJbzgUY= +github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 h1:nIPpBwaJSVYIxUFsDv3M8ofmx9yWTog9BfvIu0q41lo= +github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8/go.mod h1:HUYIGzjTL3rfEspMxjDjgmT5uz5wzYJKVo23qUhYTos= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9 h1:vEg9joUBmeBcK9iSJftGNf3coIG4HqZElCPehJsfAYM= +golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/utils/remediation/testdata/projects/go/main.go.txt b/utils/remediation/testdata/projects/go/main.go.txt new file mode 100755 index 000000000..bb2ec243b --- /dev/null +++ b/utils/remediation/testdata/projects/go/main.go.txt @@ -0,0 +1,13 @@ +package main + +import ( + "fmt" + "github.com/google/uuid" + "github.com/sassoftware/go-rpmutils" +) + +func main() { + fmt.Println("test") + uuid.New() + _, _ = rpmutils.ReadRpm(nil) +} diff --git a/utils/remediation/testdata/projects/maven/multi1/pom.xml b/utils/remediation/testdata/projects/maven/multi1/pom.xml new file mode 100755 index 000000000..bc31504bf --- /dev/null +++ b/utils/remediation/testdata/projects/maven/multi1/pom.xml @@ -0,0 +1,75 @@ + + + 4.0.0 + + org.jfrog.test + multi + 3.7-SNAPSHOT + + + multi1 + jar + Multi 1 + + + + apache + none + + + + + + + org.apache.maven.plugins + maven-source-plugin + + + attach-sources + + jar + + + + + + + org.apache.maven.plugins + maven-jar-plugin + + + + test-jar + + + + + + + + + + org.apache.commons + commons-email + 1.1 + compile + + + org.codehaus.plexus + plexus-utils + 1.5.1 + + + commons-io + commons-io + 1.4 + + + org.springframework + spring-aop + 2.5.6 + + + + diff --git a/utils/remediation/testdata/projects/maven/pom.xml b/utils/remediation/testdata/projects/maven/pom.xml new file mode 100755 index 000000000..0fb3e125c --- /dev/null +++ b/utils/remediation/testdata/projects/maven/pom.xml @@ -0,0 +1,71 @@ + + + 4.0.0 + org.jfrog.test + multi + 3.7-SNAPSHOT + pom + Simple Multi Modules Build + + + multi1 + + + + UTF-8 + 1.8 + 1.8 + + + + + junit + junit + 3.8.1 + test + + + + + + + + org.apache.maven.plugins + maven-jar-plugin + 2.4 + + + org.apache.maven.plugins + maven-war-plugin + 2.4 + + + org.apache.maven.plugins + maven-source-plugin + 2.1.2 + + + + + + org.apache.maven.plugins + maven-jar-plugin + + + false + + + + + org.apache.maven.plugins + maven-war-plugin + + + false + + + + + + diff --git a/utils/remediation/testdata/projects/noIssuesProject/package-lock.json b/utils/remediation/testdata/projects/noIssuesProject/package-lock.json new file mode 100644 index 000000000..61594161f --- /dev/null +++ b/utils/remediation/testdata/projects/noIssuesProject/package-lock.json @@ -0,0 +1,13 @@ +{ + "name": "npm", + "version": "1.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "npm", + "version": "1.0.0", + "license": "ISC" + } + } +} diff --git a/utils/remediation/testdata/projects/noIssuesProject/package.json b/utils/remediation/testdata/projects/noIssuesProject/package.json new file mode 100755 index 000000000..54cc8a6d4 --- /dev/null +++ b/utils/remediation/testdata/projects/noIssuesProject/package.json @@ -0,0 +1,11 @@ +{ + "name": "npm", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo Hello World" + }, + "author": "", + "license": "ISC" +} diff --git a/utils/remediation/testdata/projects/npm-rollback/package-lock.json b/utils/remediation/testdata/projects/npm-rollback/package-lock.json new file mode 100644 index 000000000..d605327db --- /dev/null +++ b/utils/remediation/testdata/projects/npm-rollback/package-lock.json @@ -0,0 +1,24 @@ +{ + "name": "npm-rollback-test", + "version": "1.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "npm-rollback-test", + "version": "1.0.0", + "license": "ISC", + "dependencies": { + "minimist": "1.2.5", + "this-package-does-not-exist-frogbot-test": "1.0.0" + } + }, + "node_modules/minimist": { + "version": "1.2.5", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.5.tgz", + "integrity": "sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw==", + "license": "MIT" + } + } +} + diff --git a/utils/remediation/testdata/projects/npm-rollback/package.json b/utils/remediation/testdata/projects/npm-rollback/package.json new file mode 100644 index 000000000..8cbb19efa --- /dev/null +++ b/utils/remediation/testdata/projects/npm-rollback/package.json @@ -0,0 +1,16 @@ +{ + "name": "npm-rollback-test", + "version": "1.0.0", + "description": "Test project for rollback functionality", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC", + "dependencies": { + "minimist": "1.2.5", + "this-package-does-not-exist-frogbot-test": "1.0.0" + } +} + diff --git a/utils/remediation/testdata/projects/npm/package-lock.json b/utils/remediation/testdata/projects/npm/package-lock.json new file mode 100644 index 000000000..dfb2a025a --- /dev/null +++ b/utils/remediation/testdata/projects/npm/package-lock.json @@ -0,0 +1,22 @@ +{ + "name": "npm", + "version": "1.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "npm", + "version": "1.0.0", + "license": "ISC", + "dependencies": { + "minimist": "1.2.5" + } + }, + "node_modules/minimist": { + "version": "1.2.5", + "resolved": "https://entplus.jfrog.io/artifactory/api/npm/npm-virtual/minimist/-/minimist-1.2.5.tgz", + "integrity": "sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw==", + "license": "MIT" + } + } +} diff --git a/utils/remediation/testdata/projects/npm/package.json b/utils/remediation/testdata/projects/npm/package.json new file mode 100755 index 000000000..dc07e6052 --- /dev/null +++ b/utils/remediation/testdata/projects/npm/package.json @@ -0,0 +1,14 @@ +{ + "name": "npm", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC", + "dependencies": { + "minimist": "1.2.5" + } +} diff --git a/utils/remediation/testdata/projects/pip/requirements.txt b/utils/remediation/testdata/projects/pip/requirements.txt new file mode 100755 index 000000000..65c96377f --- /dev/null +++ b/utils/remediation/testdata/projects/pip/requirements.txt @@ -0,0 +1,2 @@ +pexpect==4.8.0 +pyjwt==1.7.1 \ No newline at end of file diff --git a/utils/remediation/testdata/projects/pip/setup.py b/utils/remediation/testdata/projects/pip/setup.py new file mode 100755 index 000000000..5edae408c --- /dev/null +++ b/utils/remediation/testdata/projects/pip/setup.py @@ -0,0 +1,10 @@ +from setuptools import setup, find_packages + +setup( + name="pip-example", + version="1.2.3", + install_requires=[ + 'pexpect==4.8.0', + 'pyjwt==1.7.1' + ], +) diff --git a/utils/remediation/testdata/projects/pipenv/Pipfile b/utils/remediation/testdata/projects/pipenv/Pipfile new file mode 100755 index 000000000..6f02b6e70 --- /dev/null +++ b/utils/remediation/testdata/projects/pipenv/Pipfile @@ -0,0 +1,14 @@ +[[source]] +url = "https://pypi.python.org/simple" +verify_ssl = true +name = "pypi" + +[packages] +toml = "*" +pexpect = "4.8.0" +pyjwt = "==1.7.1" + +[dev-packages] + +[requires] +python_version = "*" diff --git a/utils/remediation/testdata/projects/pipenv/Pipfile.lock b/utils/remediation/testdata/projects/pipenv/Pipfile.lock new file mode 100755 index 000000000..207e689b1 --- /dev/null +++ b/utils/remediation/testdata/projects/pipenv/Pipfile.lock @@ -0,0 +1,52 @@ +{ + "_meta": { + "hash": { + "sha256": "21627225057b8f0fb829f001c8086c96f9e5033e6b9a025defc0c342b0e3a42b" + }, + "pipfile-spec": 6, + "requires": { + "python_version": "*" + }, + "sources": [ + { + "name": "pypi", + "url": "https://pypi.python.org/simple", + "verify_ssl": true + } + ] + }, + "default": { + "pexpect": { + "hashes": [ + "sha256:0b48a55dcb3c05f3329815901ea4fc1537514d6ba867a152b581d69ae3710937", + "sha256:fc65a43959d153d0114afe13997d439c22823a27cefceb5ff35c2178c6784c0c" + ], + "index": "pypi", + "version": "==4.8.0" + }, + "ptyprocess": { + "hashes": [ + "sha256:4b41f3967fce3af57cc7e94b888626c18bf37a083e3651ca8feeb66d492fef35", + "sha256:5c5d0a3b48ceee0b48485e0c26037c0acd7d29765ca3fbb5cb3831d347423220" + ], + "version": "==0.7.0" + }, + "pyjwt": { + "hashes": [ + "sha256:5c6eca3c2940464d106b99ba83b00c6add741c9becaec087fb7ccdefea71350e", + "sha256:8d59a976fb773f3e6a39c85636357c4f0e242707394cadadd9814f5cbaa20e96" + ], + "index": "pypi", + "version": "==1.7.1" + }, + "toml": { + "hashes": [ + "sha256:806143ae5bfb6a3c6e736a764057db0e6a0e05e338b5630894a5f779cabb4f9b", + "sha256:b3bda1d108d5dd99f4a20d24d9c348e91c4db7ab1b749200bded2f839ccbe68f" + ], + "index": "pypi", + "version": "==0.10.2" + } + }, + "develop": {} +} diff --git a/utils/remediation/testdata/projects/poetry/pyproject.toml b/utils/remediation/testdata/projects/poetry/pyproject.toml new file mode 100755 index 000000000..cc42d0070 --- /dev/null +++ b/utils/remediation/testdata/projects/poetry/pyproject.toml @@ -0,0 +1,15 @@ +[tool.poetry] +name = "poetry-project" +version = "0.1.0" +description = "" +authors = ["Your Name "] +package-mode = false + +[tool.poetry.dependencies] +python = "^3.10" +pexpect = "==4.8.0" +pyjwt = "==1.7.1" + +[build-system] +requires = ["poetry-core>=1.0.0"] +build-backend = "poetry.core.masonry.api" From 2bd84085e72502e6e18e77242fa6ec433dc1e8c9 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 14 Jun 2026 10:41:56 +0300 Subject: [PATCH 02/10] Fix Poetry/Pipenv factory routing, add factory test, tidy go.mod - Add techutils.Poetry and techutils.Pipenv to GetCompatiblePackageUpdater (both route to PythonPackageUpdater, which already handles them internally) - Add them to SupportedFixTechnologies for consistency - Add TestGetCompatiblePackageUpdater covering all supported and unsupported techs - Run go mod tidy to promote gjson/sjson from indirect to direct requires Co-Authored-By: Claude Sonnet 4.6 --- go.mod | 4 +-- .../packageupdaters/commonpackageupdater.go | 4 ++- .../commonpackageupdater_test.go | 33 +++++++++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 64a7f191a..78af5a7ee 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,8 @@ require ( github.com/owenrumney/go-sarif/v3 v3.2.3 github.com/package-url/packageurl-go v0.1.3 github.com/stretchr/testify v1.11.1 + github.com/tidwall/gjson v1.18.0 + github.com/tidwall/sjson v1.2.5 github.com/urfave/cli v1.22.17 github.com/virtuald/go-ordered-json v0.0.0-20170621173500-b18e6e673d74 golang.org/x/exp v0.0.0-20260527015227-08cc5374adb3 @@ -124,10 +126,8 @@ require ( github.com/spf13/pflag v1.0.10 // indirect github.com/spf13/viper v1.21.0 // indirect github.com/subosito/gotenv v1.6.0 // indirect - github.com/tidwall/gjson v1.18.0 // indirect github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.0 // indirect - github.com/tidwall/sjson v1.2.5 // indirect github.com/ulikunitz/xz v0.5.15 // indirect github.com/vbatts/tar-split v0.12.2 // indirect github.com/vbauerster/mpb/v8 v8.12.1 // indirect diff --git a/utils/remediation/packageupdaters/commonpackageupdater.go b/utils/remediation/packageupdaters/commonpackageupdater.go index fa0347892..3263893df 100644 --- a/utils/remediation/packageupdaters/commonpackageupdater.go +++ b/utils/remediation/packageupdaters/commonpackageupdater.go @@ -48,6 +48,8 @@ var SupportedFixTechnologies = []techutils.Technology{ techutils.Npm, techutils.Maven, techutils.Pip, + techutils.Poetry, + techutils.Pipenv, techutils.Go, techutils.Pnpm, } @@ -56,7 +58,7 @@ func GetCompatiblePackageUpdater(fixDetails *FixDetails) (PackageUpdater, bool) switch fixDetails.Technology { case techutils.Go: return &GoPackageUpdater{}, true - case techutils.Pip: + case techutils.Pip, techutils.Poetry, techutils.Pipenv: return &PythonPackageUpdater{pipRequirementsFile: defaultRequirementFile}, true case techutils.Npm: return &NpmPackageUpdater{}, true diff --git a/utils/remediation/packageupdaters/commonpackageupdater_test.go b/utils/remediation/packageupdaters/commonpackageupdater_test.go index c11bec866..031785c27 100644 --- a/utils/remediation/packageupdaters/commonpackageupdater_test.go +++ b/utils/remediation/packageupdaters/commonpackageupdater_test.go @@ -977,3 +977,36 @@ func copyDir(src, dst string) error { return os.WriteFile(dstPath, data, info.Mode()) }) } + +// TestGetCompatiblePackageUpdater verifies the factory routes every supported technology +// to the correct updater type and returns (nil, false) for unsupported ones. +func TestGetCompatiblePackageUpdater(t *testing.T) { + tests := []struct { + tech techutils.Technology + supported bool + updater PackageUpdater + }{ + {techutils.Npm, true, &NpmPackageUpdater{}}, + {techutils.Pnpm, true, &PnpmPackageUpdater{}}, + {techutils.Maven, true, &MavenPackageUpdater{}}, + {techutils.Go, true, &GoPackageUpdater{}}, + {techutils.Pip, true, &PythonPackageUpdater{}}, + {techutils.Poetry, true, &PythonPackageUpdater{}}, + {techutils.Pipenv, true, &PythonPackageUpdater{}}, + {techutils.Yarn, false, nil}, + {techutils.Gradle, false, nil}, + {techutils.Nuget, false, nil}, + {techutils.Conan, false, nil}, + } + for _, tt := range tests { + t.Run(tt.tech.String(), func(t *testing.T) { + updater, supported := GetCompatiblePackageUpdater(&FixDetails{Technology: tt.tech}) + assert.Equal(t, tt.supported, supported) + if tt.supported { + assert.IsType(t, tt.updater, updater) + } else { + assert.Nil(t, updater) + } + }) + } +} From afe5bb2cf5139c0c4a59caf3fabd1a23aa7d96cd Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 14 Jun 2026 10:58:45 +0300 Subject: [PATCH 03/10] after cr --- .../packageupdaters/commonpackageupdater.go | 17 ++++++----- .../packageupdaters/gopackageupdater.go | 12 +++----- .../packageupdaters/mavenpackageupdater.go | 2 -- .../packageupdaters/npmpackageupdater.go | 28 +++++-------------- .../packageupdaters/pnpmpackageupdater.go | 16 ++++------- .../pnpmpackageupdater_test.go | 2 +- utils/remediation/packageupdaters/types.go | 6 +--- 7 files changed, 27 insertions(+), 56 deletions(-) diff --git a/utils/remediation/packageupdaters/commonpackageupdater.go b/utils/remediation/packageupdaters/commonpackageupdater.go index 3263893df..5550ff775 100644 --- a/utils/remediation/packageupdaters/commonpackageupdater.go +++ b/utils/remediation/packageupdaters/commonpackageupdater.go @@ -22,12 +22,8 @@ import ( ) const ( - // NodePackageJSONFileName is the file name of the Node.js package manifest. - NodePackageJSONFileName = "package.json" - // NodeModulesDirName is the Node.js modules directory name. + NodePackageJSONFileName = "package.json" NodeModulesDirName = "node_modules" - nodePackageJSONFileName = NodePackageJSONFileName - nodeModulesDirName = NodeModulesDirName nodeDependenciesSection = "dependencies" nodeDevDependenciesSection = "devDependencies" nodeOptionalDependenciesSection = "optionalDependencies" @@ -42,8 +38,6 @@ var nodePackageManifestSections = []string{ nodeOverridesSection, } -// SupportedFixTechnologies lists the technologies for which automatic dependency -// fixing is supported. var SupportedFixTechnologies = []techutils.Technology{ techutils.Npm, techutils.Maven, @@ -277,7 +271,7 @@ func GetVulnerabilityLocations(fixDetails *FixDetails, namesFilters []string, ig } // IsFileTrackedByGit returns true if the given file is tracked by the git repository -// rooted at repoRootDir. +// rooted at repoRootDir. filePath may be absolute or relative. func IsFileTrackedByGit(filePath, repoRootDir string) (bool, error) { repo, err := git.PlainOpen(repoRootDir) if err != nil { @@ -299,7 +293,12 @@ func IsFileTrackedByGit(filePath, repoRootDir string) (bool, error) { return false, fmt.Errorf("failed to get commit tree: %w", err) } - _, err = tree.File(filePath) + // tree.File expects a slash-separated path relative to the repo root. + relPath, err := filepath.Rel(repoRootDir, filePath) + if err != nil { + return false, fmt.Errorf("failed to make path relative: %w", err) + } + _, err = tree.File(filepath.ToSlash(relPath)) if errors.Is(err, object.ErrFileNotFound) { return false, nil } diff --git a/utils/remediation/packageupdaters/gopackageupdater.go b/utils/remediation/packageupdaters/gopackageupdater.go index d5ac7663d..bc9ea0715 100644 --- a/utils/remediation/packageupdaters/gopackageupdater.go +++ b/utils/remediation/packageupdaters/gopackageupdater.go @@ -12,20 +12,16 @@ import ( ) const ( - goFlagModEditEnv = "GOFLAGS=-mod=mod" - goWorkOffEnv = "GOWORK=off" - // GoModFileName is the Go module descriptor file name. - GoModFileName = "go.mod" - // GoSumFileName is the Go checksum file name. - GoSumFileName = "go.sum" - // GoVendorDirName is the Go vendor directory name. + goFlagModEditEnv = "GOFLAGS=-mod=mod" + goWorkOffEnv = "GOWORK=off" + GoModFileName = "go.mod" + GoSumFileName = "go.sum" GoVendorDirName = "vendor" goTidyContinueOnError = "-e" ) type GoPackageUpdater struct{} -// GoModuleBackup holds the original content of go.mod and go.sum for rollback purposes. type GoModuleBackup struct { GoModPath string GoModContent []byte diff --git a/utils/remediation/packageupdaters/mavenpackageupdater.go b/utils/remediation/packageupdaters/mavenpackageupdater.go index 603ff81d5..2225d517a 100644 --- a/utils/remediation/packageupdaters/mavenpackageupdater.go +++ b/utils/remediation/packageupdaters/mavenpackageupdater.go @@ -97,8 +97,6 @@ func (m *MavenPackageUpdater) updatePomFile(pomPath, groupId, artifactId, fixedV return fmt.Errorf("failed to parse %s: %w", pomPath, err) } - // Working buffer for the update chain. Each update returns a new slice (e.g. regexp.ReplaceAll), - // so we never mutate the original content. No backup or revert—we only write to the file on success. currentContent := content var updatedAny bool diff --git a/utils/remediation/packageupdaters/npmpackageupdater.go b/utils/remediation/packageupdaters/npmpackageupdater.go index c7811b03c..65fd106e4 100644 --- a/utils/remediation/packageupdaters/npmpackageupdater.go +++ b/utils/remediation/packageupdaters/npmpackageupdater.go @@ -13,16 +13,11 @@ import ( ) const ( - // CiEnv is the environment variable for CI mode. - CiEnv = "CI" - // ConfigIgnoreScriptsEnv disables npm lifecycle scripts. + CiEnv = "CI" ConfigIgnoreScriptsEnv = "NPM_CONFIG_IGNORE_SCRIPTS" - // ConfigAuditEnv disables npm audit output. - ConfigAuditEnv = "NPM_CONFIG_AUDIT" - // ConfigFundEnv disables npm fund output. - ConfigFundEnv = "NPM_CONFIG_FUND" - // ConfigLevelEnv sets the npm log level. - ConfigLevelEnv = "NPM_CONFIG_LOGLEVEL" + ConfigAuditEnv = "NPM_CONFIG_AUDIT" + ConfigFundEnv = "NPM_CONFIG_FUND" + ConfigLevelEnv = "NPM_CONFIG_LOGLEVEL" npmPackageLockOnlyFlag = "--package-lock-only" npmIgnoreScriptsFlag = "--ignore-scripts" @@ -30,16 +25,10 @@ const ( npmLegacyPeerDepsFlag = "--legacy-peer-deps" npmNoFundFlag = "--no-fund" - npmLockFileName = "package-lock.json" - + npmLockFileName = "package-lock.json" npmEreresolveErrorPrefix = "ERESOLVE" - - // Unexported aliases referenced by pnpmpackageupdater.go. - ciEnv = CiEnv - configLevelEnv = ConfigLevelEnv ) -// NpmInstallEnvVars are the environment variable overrides used during npm install. var NpmInstallEnvVars = map[string]string{ ConfigIgnoreScriptsEnv: "true", ConfigAuditEnv: "false", @@ -64,7 +53,7 @@ func (npm *NpmPackageUpdater) UpdateDependency(fixDetails *FixDetails) error { } func (npm *NpmPackageUpdater) updateDirectDependency(fixDetails *FixDetails) error { - descriptorPaths := npm.CollectVulnerabilityDescriptorPaths(fixDetails, []string{nodePackageJSONFileName}, []string{nodeModulesDirName}) + descriptorPaths := npm.CollectVulnerabilityDescriptorPaths(fixDetails, []string{NodePackageJSONFileName}, []string{NodeModulesDirName}) if len(descriptorPaths) == 0 { return fmt.Errorf("no descriptor evidence was found for package %s", fixDetails.ImpactedDependencyName) } @@ -146,10 +135,7 @@ func (npm *NpmPackageUpdater) regenerateLockFileWithRetry() error { } return nil } - log.Debug(fmt.Sprintf("First npm install attempt failed: %s. Retrying...", err.Error())) - if err = npm.runNpmInstall(false); err != nil { - return fmt.Errorf("npm install failed after retry: %w", err) - } + return err } return nil } diff --git a/utils/remediation/packageupdaters/pnpmpackageupdater.go b/utils/remediation/packageupdaters/pnpmpackageupdater.go index c44e9da61..2fb06fe72 100644 --- a/utils/remediation/packageupdaters/pnpmpackageupdater.go +++ b/utils/remediation/packageupdaters/pnpmpackageupdater.go @@ -20,14 +20,10 @@ const ( PnpmFrozenLockfileEnv = "PNPM_FROZEN_LOCKFILE" ) -// PnpmLockfileInstallEnvOverrides returns environment variable overrides for pnpm install -// when refreshing pnpm-lock.yaml after editing package.json. -func PnpmLockfileInstallEnvOverrides() map[string]string { - return map[string]string{ - PnpmFrozenLockfileEnv: "false", - configLevelEnv: "error", - ciEnv: "true", - } +var PnpmLockfileInstallEnvOverrides = map[string]string{ + PnpmFrozenLockfileEnv: "false", + ConfigLevelEnv: "error", + CiEnv: "true", } func PnpmFilterCoordinateStyleDescriptorPaths(paths []string) []string { @@ -59,7 +55,7 @@ func (pnpm *PnpmPackageUpdater) UpdateDependency(fixDetails *FixDetails) error { } func (pnpm *PnpmPackageUpdater) updateDirectDependency(fixDetails *FixDetails) error { - descriptorPaths := pnpm.CollectVulnerabilityDescriptorPaths(fixDetails, []string{nodePackageJSONFileName}, []string{nodeModulesDirName}) + descriptorPaths := pnpm.CollectVulnerabilityDescriptorPaths(fixDetails, []string{NodePackageJSONFileName}, []string{NodeModulesDirName}) descriptorPaths = PnpmFilterCoordinateStyleDescriptorPaths(descriptorPaths) if len(descriptorPaths) == 0 { return fmt.Errorf("no descriptor evidence was found for package %s", fixDetails.ImpactedDependencyName) @@ -143,7 +139,7 @@ func (pnpm *PnpmPackageUpdater) runPnpmInstallLockOnly() error { //#nosec G204 -- False positive - the subprocess only runs after the user's approval cmd := exec.CommandContext(ctx, "pnpm", args...) - cmd.Env = EnvWithCorepackIntegrityWorkaround(pnpm.BuildEnvWithOverrides(PnpmLockfileInstallEnvOverrides())) + cmd.Env = EnvWithCorepackIntegrityWorkaround(pnpm.BuildEnvWithOverrides(PnpmLockfileInstallEnvOverrides)) output, err := cmd.CombinedOutput() if errors.Is(ctx.Err(), context.DeadlineExceeded) || errors.Is(err, context.DeadlineExceeded) { diff --git a/utils/remediation/packageupdaters/pnpmpackageupdater_test.go b/utils/remediation/packageupdaters/pnpmpackageupdater_test.go index 550f50858..bf9dabeb2 100644 --- a/utils/remediation/packageupdaters/pnpmpackageupdater_test.go +++ b/utils/remediation/packageupdaters/pnpmpackageupdater_test.go @@ -57,7 +57,7 @@ func TestPnpmCollectLeavesNpmParityThenPnpmFilterDropsCoordinates(t *testing.T) func TestPnpmLockRegenerationEnv(t *testing.T) { t.Parallel() pnpm := &PnpmPackageUpdater{} - env := EnvWithCorepackIntegrityWorkaround(pnpm.BuildEnvWithOverrides(PnpmLockfileInstallEnvOverrides())) + env := EnvWithCorepackIntegrityWorkaround(pnpm.BuildEnvWithOverrides(PnpmLockfileInstallEnvOverrides)) envMap := make(map[string]string) for _, e := range env { parts := strings.SplitN(e, "=", 2) diff --git a/utils/remediation/packageupdaters/types.go b/utils/remediation/packageupdaters/types.go index b7a414463..ecacfad50 100644 --- a/utils/remediation/packageupdaters/types.go +++ b/utils/remediation/packageupdaters/types.go @@ -7,7 +7,6 @@ import ( "github.com/jfrog/jfrog-cli-security/utils/techutils" ) -// FixDetails holds the minimal information an updater needs to fix a vulnerability. type FixDetails struct { ImpactedDependencyName string ImpactedDependencyVersion string @@ -21,14 +20,12 @@ type FixDetails struct { IssueId string } -// UnsupportedErrorType classifies the reason a fix is not supported. type UnsupportedErrorType string const ( IndirectDependencyFixNotSupported UnsupportedErrorType = "IndirectDependencyFixNotSupported" ) -// ErrUnsupportedFix is returned when a fix cannot be applied for a known reason. type ErrUnsupportedFix struct { PackageName string FixedVersion string @@ -39,10 +36,9 @@ func (err *ErrUnsupportedFix) Error() string { if err.ErrorType == IndirectDependencyFixNotSupported { return fmt.Sprintf("skipping fix of vulnerable package '%s' version '%s' - indirect dependency fix is not supported", err.PackageName, err.FixedVersion) } - return fmt.Sprintf("skipping fix of vulnerable package '%s' - '%s', version '%s' - build tools dependency fix is not supported", err.PackageName, err.PackageName, err.FixedVersion) + return fmt.Sprintf("skipping fix of vulnerable package '%s' version '%s' - build tools dependency fix is not supported", err.PackageName, err.FixedVersion) } -// PackageUpdater is the interface that all technology-specific updaters implement. type PackageUpdater interface { UpdateDependency(details *FixDetails) error } From 496ab8fd8d596cc5d25c25d7837e500ca011a136 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 14 Jun 2026 11:13:47 +0300 Subject: [PATCH 04/10] =?UTF-8?q?Fix=20typos:=20manger=E2=86=92manager,=20?= =?UTF-8?q?occured=E2=86=92occurred=20in=20error=20strings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- utils/remediation/packageupdaters/pythonpackageupdater.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/remediation/packageupdaters/pythonpackageupdater.go b/utils/remediation/packageupdaters/pythonpackageupdater.go index 1a3f4fcf0..837ea8151 100644 --- a/utils/remediation/packageupdaters/pythonpackageupdater.go +++ b/utils/remediation/packageupdaters/pythonpackageupdater.go @@ -45,7 +45,7 @@ func (py *PythonPackageUpdater) updateDirectDependency(fixDetails *FixDetails) ( case techutils.Pipenv: return py.CommonPackageUpdater.UpdateDependency(fixDetails, fixDetails.Technology.GetPackageInstallationCommand()) default: - return errors.New("unknown python package manger: " + fixDetails.Technology.GetPackageType()) + return errors.New("unknown python package manager: " + fixDetails.Technology.GetPackageType()) } } @@ -72,7 +72,7 @@ func (py *PythonPackageUpdater) handlePip(fixDetails *FixDetails) (err error) { } //#nosec G703 -- False positive - the path is determined by internal file scanning, not user input, and was already validated by the preceding Stat call. if err = os.WriteFile(py.pipRequirementsFile, []byte(fixedFile), 0600); err != nil { - err = fmt.Errorf("an error occured while writing the fixed version of %s to the requirements file:\n%s", fixDetails.SuggestedFixedVersion, err.Error()) + err = fmt.Errorf("an error occurred while writing the fixed version of %s to the requirements file:\n%s", fixDetails.SuggestedFixedVersion, err.Error()) } return } From 6239d9caf4a5144ac34174789e4e457b0e191b73 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 14 Jun 2026 11:56:04 +0300 Subject: [PATCH 05/10] Add nosec suppressions for gosec findings in commonpackageupdater --- utils/remediation/packageupdaters/commonpackageupdater.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/utils/remediation/packageupdaters/commonpackageupdater.go b/utils/remediation/packageupdaters/commonpackageupdater.go index 5550ff775..0e629ec9d 100644 --- a/utils/remediation/packageupdaters/commonpackageupdater.go +++ b/utils/remediation/packageupdaters/commonpackageupdater.go @@ -133,6 +133,7 @@ func (cph *CommonPackageUpdater) GetFixedPackageJSONManifest(content []byte, pac } func (cph *CommonPackageUpdater) UpdatePackageJSONDescriptor(descriptorPath, packageName, newVersion string) ([]byte, error) { + //#nosec G304 -- descriptorPath comes from descriptor discovery in the scanned repository. descriptorContent, err := os.ReadFile(descriptorPath) if err != nil { return nil, fmt.Errorf("failed to read file '%s': %w", descriptorPath, err) @@ -146,6 +147,7 @@ func (cph *CommonPackageUpdater) UpdatePackageJSONDescriptor(descriptorPath, pac return nil, fmt.Errorf("failed to update version in descriptor: %w", err) } + //#nosec G703 G306 -- descriptorPath from scan workflow; 0644 for VCS-tracked sources. if err = os.WriteFile(descriptorPath, updatedContent, 0644); err != nil { return nil, fmt.Errorf("failed to write updated descriptor '%s': %w", descriptorPath, err) } @@ -192,6 +194,7 @@ func (cph *CommonPackageUpdater) UpdateDependency(fixDetails *FixDetails, instal func runPackageMangerCommand(commandName string, techName string, commandArgs []string) error { fullCommand := commandName + " " + strings.Join(commandArgs, " ") log.Debug(fmt.Sprintf("Running '%s'", fullCommand)) + //#nosec G204 -- commandName is a known package manager binary, not user-controlled input. cmd := exec.Command(commandName, commandArgs...) if commandName == "pnpm" { cmd.Env = EnvWithCorepackIntegrityWorkaround(os.Environ()) From 1b80b27ca69909d5866b2021e486736bc46d07c5 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 14 Jun 2026 12:03:38 +0300 Subject: [PATCH 06/10] Fix gocritic captLocal: lowercase parameter names in BackupModuleFiles --- .../packageupdaters/gopackageupdater.go | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/utils/remediation/packageupdaters/gopackageupdater.go b/utils/remediation/packageupdaters/gopackageupdater.go index bc9ea0715..37f4d0ab2 100644 --- a/utils/remediation/packageupdaters/gopackageupdater.go +++ b/utils/remediation/packageupdaters/gopackageupdater.go @@ -109,29 +109,29 @@ func (gpu *GoPackageUpdater) buildGoCommandEnv() []string { return append(os.Environ(), goFlagModEditEnv, goWorkOffEnv) } -func (gpu *GoPackageUpdater) BackupModuleFiles(GoModPath string) (*GoModuleBackup, error) { +func (gpu *GoPackageUpdater) BackupModuleFiles(goModPath string) (*GoModuleBackup, error) { //#nosec G304 -- go.mod path from scan workflow. - GoModContent, err := os.ReadFile(GoModPath) + goModContent, err := os.ReadFile(goModPath) if err != nil { - return nil, fmt.Errorf("failed to read '%s': %w", GoModPath, err) + return nil, fmt.Errorf("failed to read '%s': %w", goModPath, err) } - descriptorDir := filepath.Dir(GoModPath) - GoSumPath := filepath.Join(descriptorDir, GoSumFileName) + descriptorDir := filepath.Dir(goModPath) + goSumPath := filepath.Join(descriptorDir, GoSumFileName) //#nosec G304 -- go.sum adjacent to go.mod from same scan workflow. - GoSumContent, err := os.ReadFile(GoSumPath) + goSumContent, err := os.ReadFile(goSumPath) if err != nil { - return nil, fmt.Errorf("failed to read '%s': %w", GoSumPath, err) + 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)), + GoModPath: goModPath, + GoModContent: make([]byte, len(goModContent)), + GoSumPath: goSumPath, + GoSumContent: make([]byte, len(goSumContent)), } - copy(backup.GoModContent, GoModContent) - copy(backup.GoSumContent, GoSumContent) + copy(backup.GoModContent, goModContent) + copy(backup.GoSumContent, goSumContent) return backup, nil } From f7c34f1289b131e1b50787878cc8452b6c478932 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Sun, 14 Jun 2026 12:15:51 +0300 Subject: [PATCH 07/10] Add nosec suppressions for G122/G703 in copyDir test helper --- utils/remediation/packageupdaters/commonpackageupdater_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/utils/remediation/packageupdaters/commonpackageupdater_test.go b/utils/remediation/packageupdaters/commonpackageupdater_test.go index 031785c27..36f690c79 100644 --- a/utils/remediation/packageupdaters/commonpackageupdater_test.go +++ b/utils/remediation/packageupdaters/commonpackageupdater_test.go @@ -970,11 +970,12 @@ func copyDir(src, dst string) error { if info.IsDir() { return os.MkdirAll(dstPath, info.Mode()) } + //#nosec G122 -- test helper; path comes from filepath.WalkDir over a known test directory. data, err := os.ReadFile(path) if err != nil { return err } - return os.WriteFile(dstPath, data, info.Mode()) + return os.WriteFile(dstPath, data, info.Mode()) //#nosec G703 -- test helper writing to a known temp directory. }) } From fd6cdbdfd1139fa3e0f1b59affb108e953349a0b Mon Sep 17 00:00:00 2001 From: Or Toren Date: Mon, 15 Jun 2026 10:25:39 +0300 Subject: [PATCH 08/10] Move package updaters to remediation/sca and testdata to tests/testdata - utils/remediation/packageupdaters/ -> remediation/sca/packageupdaters/ - utils/remediation/testdata/ -> tests/testdata/projects/remediation/ - Update testdata relative paths in test files - Fix TestGetAllDescriptorFilesFullPaths expected results to include multi1/pom.xml --- .../sca}/packageupdaters/commonpackageupdater.go | 0 .../sca}/packageupdaters/commonpackageupdater_test.go | 6 +++--- .../sca}/packageupdaters/gopackageupdater.go | 0 .../sca}/packageupdaters/gopackageupdater_test.go | 0 .../sca}/packageupdaters/mavenpackageupdater.go | 0 .../sca}/packageupdaters/mavenpackageupdater_test.go | 4 ++-- .../sca}/packageupdaters/npmpackageupdater.go | 0 .../sca}/packageupdaters/npmpackageupdater_test.go | 0 .../sca}/packageupdaters/pnpmpackageupdater.go | 0 .../sca}/packageupdaters/pnpmpackageupdater_test.go | 0 .../sca}/packageupdaters/pythonpackageupdater.go | 0 .../sca}/packageupdaters/pythonpackageupdater_test.go | 0 .../sca}/packageupdaters/types.go | 0 .../projects/remediation}/indirect-projects/go/go.mod.txt | 0 .../projects/remediation}/indirect-projects/go/go.sum.txt | 0 .../projects/remediation}/indirect-projects/go/main.go.txt | 0 .../projects/remediation}/indirect-projects/maven/pom.xml | 0 .../remediation}/indirect-projects/pip/requirements.txt | 0 .../projects/remediation}/indirect-projects/pip/setup.py | 0 .../projects/remediation}/indirect-projects/pipenv/Pipfile | 0 .../remediation}/indirect-projects/pipenv/Pipfile.lock | 0 .../remediation}/indirect-projects/poetry/pyproject.toml | 0 .../testdata/projects/remediation}/packageupdaters/pom.xml | 0 .../testdata/projects/remediation}/projects/go/go.mod.txt | 0 .../testdata/projects/remediation}/projects/go/go.sum.txt | 0 .../testdata/projects/remediation}/projects/go/main.go.txt | 0 .../projects/remediation}/projects/maven/multi1/pom.xml | 0 .../testdata/projects/remediation}/projects/maven/pom.xml | 0 .../remediation}/projects/noIssuesProject/package-lock.json | 0 .../remediation}/projects/noIssuesProject/package.json | 0 .../remediation}/projects/npm-rollback/package-lock.json | 0 .../remediation}/projects/npm-rollback/package.json | 0 .../projects/remediation}/projects/npm/package-lock.json | 0 .../projects/remediation}/projects/npm/package.json | 0 .../projects/remediation}/projects/pip/requirements.txt | 0 .../testdata/projects/remediation}/projects/pip/setup.py | 0 .../testdata/projects/remediation}/projects/pipenv/Pipfile | 0 .../projects/remediation}/projects/pipenv/Pipfile.lock | 0 .../projects/remediation}/projects/poetry/pyproject.toml | 0 39 files changed, 5 insertions(+), 5 deletions(-) rename {utils/remediation => remediation/sca}/packageupdaters/commonpackageupdater.go (100%) rename {utils/remediation => remediation/sca}/packageupdaters/commonpackageupdater_test.go (98%) rename {utils/remediation => remediation/sca}/packageupdaters/gopackageupdater.go (100%) rename {utils/remediation => remediation/sca}/packageupdaters/gopackageupdater_test.go (100%) rename {utils/remediation => remediation/sca}/packageupdaters/mavenpackageupdater.go (100%) rename {utils/remediation => remediation/sca}/packageupdaters/mavenpackageupdater_test.go (96%) rename {utils/remediation => remediation/sca}/packageupdaters/npmpackageupdater.go (100%) rename {utils/remediation => remediation/sca}/packageupdaters/npmpackageupdater_test.go (100%) rename {utils/remediation => remediation/sca}/packageupdaters/pnpmpackageupdater.go (100%) rename {utils/remediation => remediation/sca}/packageupdaters/pnpmpackageupdater_test.go (100%) rename {utils/remediation => remediation/sca}/packageupdaters/pythonpackageupdater.go (100%) rename {utils/remediation => remediation/sca}/packageupdaters/pythonpackageupdater_test.go (100%) rename {utils/remediation => remediation/sca}/packageupdaters/types.go (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/indirect-projects/go/go.mod.txt (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/indirect-projects/go/go.sum.txt (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/indirect-projects/go/main.go.txt (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/indirect-projects/maven/pom.xml (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/indirect-projects/pip/requirements.txt (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/indirect-projects/pip/setup.py (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/indirect-projects/pipenv/Pipfile (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/indirect-projects/pipenv/Pipfile.lock (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/indirect-projects/poetry/pyproject.toml (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/packageupdaters/pom.xml (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/go/go.mod.txt (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/go/go.sum.txt (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/go/main.go.txt (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/maven/multi1/pom.xml (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/maven/pom.xml (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/noIssuesProject/package-lock.json (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/noIssuesProject/package.json (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/npm-rollback/package-lock.json (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/npm-rollback/package.json (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/npm/package-lock.json (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/npm/package.json (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/pip/requirements.txt (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/pip/setup.py (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/pipenv/Pipfile (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/pipenv/Pipfile.lock (100%) rename {utils/remediation/testdata => tests/testdata/projects/remediation}/projects/poetry/pyproject.toml (100%) diff --git a/utils/remediation/packageupdaters/commonpackageupdater.go b/remediation/sca/packageupdaters/commonpackageupdater.go similarity index 100% rename from utils/remediation/packageupdaters/commonpackageupdater.go rename to remediation/sca/packageupdaters/commonpackageupdater.go diff --git a/utils/remediation/packageupdaters/commonpackageupdater_test.go b/remediation/sca/packageupdaters/commonpackageupdater_test.go similarity index 98% rename from utils/remediation/packageupdaters/commonpackageupdater_test.go rename to remediation/sca/packageupdaters/commonpackageupdater_test.go index 36f690c79..bdb0b41ba 100644 --- a/utils/remediation/packageupdaters/commonpackageupdater_test.go +++ b/remediation/sca/packageupdaters/commonpackageupdater_test.go @@ -224,7 +224,7 @@ func getTestDataDir(t *testing.T, directDependency bool) string { } else { projectDir = "indirect-projects" } - testdataDir, err := filepath.Abs(filepath.Join("..", "testdata", projectDir)) + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "..", "tests", "testdata", "projects", "remediation", projectDir)) assert.NoError(t, err) return testdataDir } @@ -326,7 +326,7 @@ func TestGetAllDescriptorFilesFullPaths(t *testing.T) { { testProjectRepo: "maven", suffixesToSearch: []string{"pom.xml"}, - expectedResultSuffixes: []string{"pom.xml"}, + expectedResultSuffixes: []string{"pom.xml", filepath.Join("multi1", "pom.xml")}, }, } @@ -336,7 +336,7 @@ func TestGetAllDescriptorFilesFullPaths(t *testing.T) { for _, testcase := range testcases { tmpDir, err := os.MkdirTemp("", "") assert.NoError(t, err) - assert.NoError(t, copyDir(filepath.Join("..", "testdata", "projects", testcase.testProjectRepo), tmpDir)) + assert.NoError(t, copyDir(filepath.Join("..", "..", "..", "tests", "testdata", "projects", "remediation", "projects", testcase.testProjectRepo), tmpDir)) assert.NoError(t, os.Chdir(tmpDir)) finalDirPath, err := os.Getwd() diff --git a/utils/remediation/packageupdaters/gopackageupdater.go b/remediation/sca/packageupdaters/gopackageupdater.go similarity index 100% rename from utils/remediation/packageupdaters/gopackageupdater.go rename to remediation/sca/packageupdaters/gopackageupdater.go diff --git a/utils/remediation/packageupdaters/gopackageupdater_test.go b/remediation/sca/packageupdaters/gopackageupdater_test.go similarity index 100% rename from utils/remediation/packageupdaters/gopackageupdater_test.go rename to remediation/sca/packageupdaters/gopackageupdater_test.go diff --git a/utils/remediation/packageupdaters/mavenpackageupdater.go b/remediation/sca/packageupdaters/mavenpackageupdater.go similarity index 100% rename from utils/remediation/packageupdaters/mavenpackageupdater.go rename to remediation/sca/packageupdaters/mavenpackageupdater.go diff --git a/utils/remediation/packageupdaters/mavenpackageupdater_test.go b/remediation/sca/packageupdaters/mavenpackageupdater_test.go similarity index 96% rename from utils/remediation/packageupdaters/mavenpackageupdater_test.go rename to remediation/sca/packageupdaters/mavenpackageupdater_test.go index b50fc9dfa..a6eca4225 100644 --- a/utils/remediation/packageupdaters/mavenpackageupdater_test.go +++ b/remediation/sca/packageupdaters/mavenpackageupdater_test.go @@ -14,7 +14,7 @@ import ( ) func TestMavenUpdateDependency(t *testing.T) { - testProjectPath := filepath.Join("..", "testdata", "packageupdaters") + testProjectPath := filepath.Join("..", "..", "..", "tests", "testdata", "projects", "remediation", "packageupdaters") currDir, err := os.Getwd() assert.NoError(t, err) @@ -146,7 +146,7 @@ func TestMavenUpdateDependency(t *testing.T) { } func TestMavenUpdateDependencyErrors(t *testing.T) { - testProjectPath := filepath.Join("..", "testdata", "packageupdaters") + testProjectPath := filepath.Join("..", "..", "..", "tests", "testdata", "projects", "remediation", "packageupdaters") currDir, err := os.Getwd() assert.NoError(t, err) diff --git a/utils/remediation/packageupdaters/npmpackageupdater.go b/remediation/sca/packageupdaters/npmpackageupdater.go similarity index 100% rename from utils/remediation/packageupdaters/npmpackageupdater.go rename to remediation/sca/packageupdaters/npmpackageupdater.go diff --git a/utils/remediation/packageupdaters/npmpackageupdater_test.go b/remediation/sca/packageupdaters/npmpackageupdater_test.go similarity index 100% rename from utils/remediation/packageupdaters/npmpackageupdater_test.go rename to remediation/sca/packageupdaters/npmpackageupdater_test.go diff --git a/utils/remediation/packageupdaters/pnpmpackageupdater.go b/remediation/sca/packageupdaters/pnpmpackageupdater.go similarity index 100% rename from utils/remediation/packageupdaters/pnpmpackageupdater.go rename to remediation/sca/packageupdaters/pnpmpackageupdater.go diff --git a/utils/remediation/packageupdaters/pnpmpackageupdater_test.go b/remediation/sca/packageupdaters/pnpmpackageupdater_test.go similarity index 100% rename from utils/remediation/packageupdaters/pnpmpackageupdater_test.go rename to remediation/sca/packageupdaters/pnpmpackageupdater_test.go diff --git a/utils/remediation/packageupdaters/pythonpackageupdater.go b/remediation/sca/packageupdaters/pythonpackageupdater.go similarity index 100% rename from utils/remediation/packageupdaters/pythonpackageupdater.go rename to remediation/sca/packageupdaters/pythonpackageupdater.go diff --git a/utils/remediation/packageupdaters/pythonpackageupdater_test.go b/remediation/sca/packageupdaters/pythonpackageupdater_test.go similarity index 100% rename from utils/remediation/packageupdaters/pythonpackageupdater_test.go rename to remediation/sca/packageupdaters/pythonpackageupdater_test.go diff --git a/utils/remediation/packageupdaters/types.go b/remediation/sca/packageupdaters/types.go similarity index 100% rename from utils/remediation/packageupdaters/types.go rename to remediation/sca/packageupdaters/types.go diff --git a/utils/remediation/testdata/indirect-projects/go/go.mod.txt b/tests/testdata/projects/remediation/indirect-projects/go/go.mod.txt similarity index 100% rename from utils/remediation/testdata/indirect-projects/go/go.mod.txt rename to tests/testdata/projects/remediation/indirect-projects/go/go.mod.txt diff --git a/utils/remediation/testdata/indirect-projects/go/go.sum.txt b/tests/testdata/projects/remediation/indirect-projects/go/go.sum.txt similarity index 100% rename from utils/remediation/testdata/indirect-projects/go/go.sum.txt rename to tests/testdata/projects/remediation/indirect-projects/go/go.sum.txt diff --git a/utils/remediation/testdata/indirect-projects/go/main.go.txt b/tests/testdata/projects/remediation/indirect-projects/go/main.go.txt similarity index 100% rename from utils/remediation/testdata/indirect-projects/go/main.go.txt rename to tests/testdata/projects/remediation/indirect-projects/go/main.go.txt diff --git a/utils/remediation/testdata/indirect-projects/maven/pom.xml b/tests/testdata/projects/remediation/indirect-projects/maven/pom.xml similarity index 100% rename from utils/remediation/testdata/indirect-projects/maven/pom.xml rename to tests/testdata/projects/remediation/indirect-projects/maven/pom.xml diff --git a/utils/remediation/testdata/indirect-projects/pip/requirements.txt b/tests/testdata/projects/remediation/indirect-projects/pip/requirements.txt similarity index 100% rename from utils/remediation/testdata/indirect-projects/pip/requirements.txt rename to tests/testdata/projects/remediation/indirect-projects/pip/requirements.txt diff --git a/utils/remediation/testdata/indirect-projects/pip/setup.py b/tests/testdata/projects/remediation/indirect-projects/pip/setup.py similarity index 100% rename from utils/remediation/testdata/indirect-projects/pip/setup.py rename to tests/testdata/projects/remediation/indirect-projects/pip/setup.py diff --git a/utils/remediation/testdata/indirect-projects/pipenv/Pipfile b/tests/testdata/projects/remediation/indirect-projects/pipenv/Pipfile similarity index 100% rename from utils/remediation/testdata/indirect-projects/pipenv/Pipfile rename to tests/testdata/projects/remediation/indirect-projects/pipenv/Pipfile diff --git a/utils/remediation/testdata/indirect-projects/pipenv/Pipfile.lock b/tests/testdata/projects/remediation/indirect-projects/pipenv/Pipfile.lock similarity index 100% rename from utils/remediation/testdata/indirect-projects/pipenv/Pipfile.lock rename to tests/testdata/projects/remediation/indirect-projects/pipenv/Pipfile.lock diff --git a/utils/remediation/testdata/indirect-projects/poetry/pyproject.toml b/tests/testdata/projects/remediation/indirect-projects/poetry/pyproject.toml similarity index 100% rename from utils/remediation/testdata/indirect-projects/poetry/pyproject.toml rename to tests/testdata/projects/remediation/indirect-projects/poetry/pyproject.toml diff --git a/utils/remediation/testdata/packageupdaters/pom.xml b/tests/testdata/projects/remediation/packageupdaters/pom.xml similarity index 100% rename from utils/remediation/testdata/packageupdaters/pom.xml rename to tests/testdata/projects/remediation/packageupdaters/pom.xml diff --git a/utils/remediation/testdata/projects/go/go.mod.txt b/tests/testdata/projects/remediation/projects/go/go.mod.txt similarity index 100% rename from utils/remediation/testdata/projects/go/go.mod.txt rename to tests/testdata/projects/remediation/projects/go/go.mod.txt diff --git a/utils/remediation/testdata/projects/go/go.sum.txt b/tests/testdata/projects/remediation/projects/go/go.sum.txt similarity index 100% rename from utils/remediation/testdata/projects/go/go.sum.txt rename to tests/testdata/projects/remediation/projects/go/go.sum.txt diff --git a/utils/remediation/testdata/projects/go/main.go.txt b/tests/testdata/projects/remediation/projects/go/main.go.txt similarity index 100% rename from utils/remediation/testdata/projects/go/main.go.txt rename to tests/testdata/projects/remediation/projects/go/main.go.txt diff --git a/utils/remediation/testdata/projects/maven/multi1/pom.xml b/tests/testdata/projects/remediation/projects/maven/multi1/pom.xml similarity index 100% rename from utils/remediation/testdata/projects/maven/multi1/pom.xml rename to tests/testdata/projects/remediation/projects/maven/multi1/pom.xml diff --git a/utils/remediation/testdata/projects/maven/pom.xml b/tests/testdata/projects/remediation/projects/maven/pom.xml similarity index 100% rename from utils/remediation/testdata/projects/maven/pom.xml rename to tests/testdata/projects/remediation/projects/maven/pom.xml diff --git a/utils/remediation/testdata/projects/noIssuesProject/package-lock.json b/tests/testdata/projects/remediation/projects/noIssuesProject/package-lock.json similarity index 100% rename from utils/remediation/testdata/projects/noIssuesProject/package-lock.json rename to tests/testdata/projects/remediation/projects/noIssuesProject/package-lock.json diff --git a/utils/remediation/testdata/projects/noIssuesProject/package.json b/tests/testdata/projects/remediation/projects/noIssuesProject/package.json similarity index 100% rename from utils/remediation/testdata/projects/noIssuesProject/package.json rename to tests/testdata/projects/remediation/projects/noIssuesProject/package.json diff --git a/utils/remediation/testdata/projects/npm-rollback/package-lock.json b/tests/testdata/projects/remediation/projects/npm-rollback/package-lock.json similarity index 100% rename from utils/remediation/testdata/projects/npm-rollback/package-lock.json rename to tests/testdata/projects/remediation/projects/npm-rollback/package-lock.json diff --git a/utils/remediation/testdata/projects/npm-rollback/package.json b/tests/testdata/projects/remediation/projects/npm-rollback/package.json similarity index 100% rename from utils/remediation/testdata/projects/npm-rollback/package.json rename to tests/testdata/projects/remediation/projects/npm-rollback/package.json diff --git a/utils/remediation/testdata/projects/npm/package-lock.json b/tests/testdata/projects/remediation/projects/npm/package-lock.json similarity index 100% rename from utils/remediation/testdata/projects/npm/package-lock.json rename to tests/testdata/projects/remediation/projects/npm/package-lock.json diff --git a/utils/remediation/testdata/projects/npm/package.json b/tests/testdata/projects/remediation/projects/npm/package.json similarity index 100% rename from utils/remediation/testdata/projects/npm/package.json rename to tests/testdata/projects/remediation/projects/npm/package.json diff --git a/utils/remediation/testdata/projects/pip/requirements.txt b/tests/testdata/projects/remediation/projects/pip/requirements.txt similarity index 100% rename from utils/remediation/testdata/projects/pip/requirements.txt rename to tests/testdata/projects/remediation/projects/pip/requirements.txt diff --git a/utils/remediation/testdata/projects/pip/setup.py b/tests/testdata/projects/remediation/projects/pip/setup.py similarity index 100% rename from utils/remediation/testdata/projects/pip/setup.py rename to tests/testdata/projects/remediation/projects/pip/setup.py diff --git a/utils/remediation/testdata/projects/pipenv/Pipfile b/tests/testdata/projects/remediation/projects/pipenv/Pipfile similarity index 100% rename from utils/remediation/testdata/projects/pipenv/Pipfile rename to tests/testdata/projects/remediation/projects/pipenv/Pipfile diff --git a/utils/remediation/testdata/projects/pipenv/Pipfile.lock b/tests/testdata/projects/remediation/projects/pipenv/Pipfile.lock similarity index 100% rename from utils/remediation/testdata/projects/pipenv/Pipfile.lock rename to tests/testdata/projects/remediation/projects/pipenv/Pipfile.lock diff --git a/utils/remediation/testdata/projects/poetry/pyproject.toml b/tests/testdata/projects/remediation/projects/poetry/pyproject.toml similarity index 100% rename from utils/remediation/testdata/projects/poetry/pyproject.toml rename to tests/testdata/projects/remediation/projects/poetry/pyproject.toml From 08d88b5240f567cf3f73f6aa82253b5bfe73efbd Mon Sep 17 00:00:00 2001 From: Or Toren Date: Wed, 17 Jun 2026 10:37:26 +0300 Subject: [PATCH 09/10] Add remediation integration test suite with CI job - Add --test.remediation flag to tests/config.go and InitRemediationTest helper to gate tests on the new flag - Replace ad-hoc JF_URL env check in TestUpdateDependency with InitRemediationTest so the test is properly controlled by the flag - Gate all unit-style tests in remediation/sca/packageupdaters/ with InitUnitTest so they run in the unit test suite - Add remediation_test.go at repo root with integration tests that exercise npm, maven, go, and rollback scenarios via the public API - Add Remediation_Integration_Tests CI job to test.yml Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/test.yml | 30 ++- .../commonpackageupdater_test.go | 11 +- .../packageupdaters/gopackageupdater_test.go | 3 + .../mavenpackageupdater_test.go | 51 +++-- .../packageupdaters/npmpackageupdater_test.go | 4 + .../pnpmpackageupdater_test.go | 5 + .../pythonpackageupdater_test.go | 2 + remediation_test.go | 209 ++++++++++++++++++ tests/config.go | 6 +- .../integration/test_integrationutils.go | 6 + 10 files changed, 298 insertions(+), 29 deletions(-) create mode 100644 remediation_test.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8c54d393e..8822748c9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -333,4 +333,32 @@ jobs: # Test - name: Run tests - run: go test ${{ env.GO_COMMON_TEST_ARGS }} --test.git --ci.runId=${{ runner.os }}-sec-test \ No newline at end of file + run: go test ${{ env.GO_COMMON_TEST_ARGS }} --test.git --ci.runId=${{ runner.os }}-sec-test + + Remediation_Integration_Tests: + name: "[${{ matrix.os }}] Remediation Integration Tests" + needs: Pretest + runs-on: ${{ matrix.os }}-latest + strategy: + fail-fast: false + matrix: + os: [ ubuntu, windows, macos ] + steps: + # Prepare the environment + - name: Checkout code + uses: actions/checkout@v5 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: Start FastCI Optimization + if: env.ENABLE_FASTCI_BOOST == 'true' + uses: jfrog/boost@v0 + with: + github_token: ${{secrets.GITHUB_TOKEN}} + + - name: Install and Setup Dependencies + uses: ./.github/actions/install-and-setup + + # Test + - name: Run tests + run: go test ${{ env.GO_COMMON_TEST_ARGS }} --test.remediation --ci.runId=${{ runner.os }}-sec-test \ No newline at end of file diff --git a/remediation/sca/packageupdaters/commonpackageupdater_test.go b/remediation/sca/packageupdaters/commonpackageupdater_test.go index bdb0b41ba..4daf186fb 100644 --- a/remediation/sca/packageupdaters/commonpackageupdater_test.go +++ b/remediation/sca/packageupdaters/commonpackageupdater_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/jfrog/build-info-go/tests" + "github.com/jfrog/jfrog-cli-security/tests/utils/integration" "github.com/jfrog/jfrog-cli-security/utils/formats" "github.com/jfrog/jfrog-cli-security/utils/techutils" "github.com/jfrog/jfrog-client-go/utils/io/fileutils" @@ -38,9 +39,7 @@ type pipPackageRegexTest struct { } func TestUpdateDependency(t *testing.T) { - if strings.TrimSuffix(os.Getenv("JF_URL"), "/") == "" { - t.Skipf("skipping: JF_URL is not set (package updater integration tests run in CI with platform credentials)") - } + integration.InitRemediationTest(t) testCases := [][]dependencyFixTest{ // Go test cases @@ -290,6 +289,7 @@ func verifyDependencyUpdate(t *testing.T, test dependencyFixTest) { } func TestGetFixedPackage(t *testing.T) { + integration.InitUnitTest(t) var testcases = []struct { impactedPackage string versionOperator string @@ -317,6 +317,7 @@ func TestGetFixedPackage(t *testing.T) { } func TestGetAllDescriptorFilesFullPaths(t *testing.T) { + integration.InitUnitTest(t) var testcases = []struct { testProjectRepo string suffixesToSearch []string @@ -358,6 +359,7 @@ func TestGetAllDescriptorFilesFullPaths(t *testing.T) { } func TestGetVulnerabilityLocations(t *testing.T) { + integration.InitUnitTest(t) testcases := []struct { name string fixDetails *FixDetails @@ -566,6 +568,7 @@ func TestGetVulnerabilityLocations(t *testing.T) { } func TestEnvWithCorepackIntegrityWorkaround(t *testing.T) { + integration.InitUnitTest(t) t.Parallel() base := []string{"FOO=1", "COREPACK_INTEGRITY_KEYS=old-value", "BAR=2"} out := EnvWithCorepackIntegrityWorkaround(base) @@ -587,6 +590,7 @@ func TestEnvWithCorepackIntegrityWorkaround(t *testing.T) { } func TestGetVulnerabilityRegexCompiler(t *testing.T) { + integration.InitUnitTest(t) // Sample format patterns from different package managers const ( npmPattern = `\s*"%s"\s*:\s*"[~^]?%s"` @@ -982,6 +986,7 @@ func copyDir(src, dst string) error { // TestGetCompatiblePackageUpdater verifies the factory routes every supported technology // to the correct updater type and returns (nil, false) for unsupported ones. func TestGetCompatiblePackageUpdater(t *testing.T) { + integration.InitUnitTest(t) tests := []struct { tech techutils.Technology supported bool diff --git a/remediation/sca/packageupdaters/gopackageupdater_test.go b/remediation/sca/packageupdaters/gopackageupdater_test.go index 67edb981b..86a9696c1 100644 --- a/remediation/sca/packageupdaters/gopackageupdater_test.go +++ b/remediation/sca/packageupdaters/gopackageupdater_test.go @@ -5,10 +5,12 @@ import ( "path/filepath" "testing" + "github.com/jfrog/jfrog-cli-security/tests/utils/integration" "github.com/stretchr/testify/assert" ) func TestBackupModuleFiles(t *testing.T) { + integration.InitUnitTest(t) testcases := []struct { name string goModContent []byte @@ -65,6 +67,7 @@ func TestBackupModuleFiles(t *testing.T) { } func TestHasVendorDirectory(t *testing.T) { + integration.InitUnitTest(t) testcases := []struct { name string setupVendor bool diff --git a/remediation/sca/packageupdaters/mavenpackageupdater_test.go b/remediation/sca/packageupdaters/mavenpackageupdater_test.go index a6eca4225..7aefddb2b 100644 --- a/remediation/sca/packageupdaters/mavenpackageupdater_test.go +++ b/remediation/sca/packageupdaters/mavenpackageupdater_test.go @@ -7,6 +7,7 @@ import ( "testing" biutils "github.com/jfrog/build-info-go/utils" + "github.com/jfrog/jfrog-cli-security/tests/utils/integration" "github.com/jfrog/jfrog-cli-security/utils/formats" "github.com/jfrog/jfrog-cli-security/utils/techutils" "github.com/jfrog/jfrog-client-go/utils/io/fileutils" @@ -14,6 +15,7 @@ import ( ) func TestMavenUpdateDependency(t *testing.T) { + integration.InitUnitTest(t) testProjectPath := filepath.Join("..", "..", "..", "tests", "testdata", "projects", "remediation", "packageupdaters") currDir, err := os.Getwd() assert.NoError(t, err) @@ -62,11 +64,11 @@ func TestMavenUpdateDependency(t *testing.T) { { name: "RegularDependency", fixDetails: &FixDetails{ - SuggestedFixedVersion: "1.1.5", - IsDirectDependency: true, - Technology: techutils.Maven, + SuggestedFixedVersion: "1.1.5", + IsDirectDependency: true, + Technology: techutils.Maven, ImpactedDependencyName: "org.jfrog.filespecs:file-specs-java", - Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, + Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, }, expectedContains: []string{"1.1.5"}, expectedNotContain: []string{"1.1.1"}, @@ -74,11 +76,11 @@ func TestMavenUpdateDependency(t *testing.T) { { name: "DependencyManagement", fixDetails: &FixDetails{ - SuggestedFixedVersion: "2.15.0", - IsDirectDependency: true, - Technology: techutils.Maven, + SuggestedFixedVersion: "2.15.0", + IsDirectDependency: true, + Technology: techutils.Maven, ImpactedDependencyName: "com.fasterxml.jackson.core:jackson-core", - Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, + Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, }, expectedContains: []string{"2.15.0"}, expectedNotContain: []string{"2.13.4"}, @@ -87,11 +89,11 @@ func TestMavenUpdateDependency(t *testing.T) { name: "PropertyVersion", customPOM: propertyPOM, fixDetails: &FixDetails{ - SuggestedFixedVersion: "2.13.0", - IsDirectDependency: true, - Technology: techutils.Maven, + SuggestedFixedVersion: "2.13.0", + IsDirectDependency: true, + Technology: techutils.Maven, ImpactedDependencyName: "com.fasterxml.jackson.core:jackson-databind", - Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, + Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, }, expectedContains: []string{"2.13.0"}, expectedNotContain: []string{"2.9.8"}, @@ -100,11 +102,11 @@ func TestMavenUpdateDependency(t *testing.T) { name: "ParentPOM", customPOM: parentPOM, fixDetails: &FixDetails{ - SuggestedFixedVersion: "2.7.0", - IsDirectDependency: true, - Technology: techutils.Maven, + SuggestedFixedVersion: "2.7.0", + IsDirectDependency: true, + Technology: techutils.Maven, ImpactedDependencyName: "org.springframework.boot:spring-boot-starter-parent", - Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, + Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, }, expectedContains: []string{"2.7.0"}, expectedNotContain: []string{"2.5.0"}, @@ -146,6 +148,7 @@ func TestMavenUpdateDependency(t *testing.T) { } func TestMavenUpdateDependencyErrors(t *testing.T) { + integration.InitUnitTest(t) testProjectPath := filepath.Join("..", "..", "..", "tests", "testdata", "projects", "remediation", "packageupdaters") currDir, err := os.Getwd() assert.NoError(t, err) @@ -159,11 +162,11 @@ func TestMavenUpdateDependencyErrors(t *testing.T) { { name: "DependencyNotFound", fixDetails: &FixDetails{ - SuggestedFixedVersion: "1.0.0", - IsDirectDependency: true, - Technology: techutils.Maven, + SuggestedFixedVersion: "1.0.0", + IsDirectDependency: true, + Technology: techutils.Maven, ImpactedDependencyName: "com.nonexistent:package", - Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, + Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, }, useTestData: true, assertErr: func(t *testing.T, err error) { @@ -174,11 +177,11 @@ func TestMavenUpdateDependencyErrors(t *testing.T) { { name: "IndirectDependencyNotSupported", fixDetails: &FixDetails{ - SuggestedFixedVersion: "1.0.0", - IsDirectDependency: false, - Technology: techutils.Maven, + SuggestedFixedVersion: "1.0.0", + IsDirectDependency: false, + Technology: techutils.Maven, ImpactedDependencyName: "org.springframework:spring-core", - Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, + Components: []formats.ComponentRow{{Evidences: []formats.Location{{File: "pom.xml"}}}}, }, useTestData: false, assertErr: func(t *testing.T, err error) { diff --git a/remediation/sca/packageupdaters/npmpackageupdater_test.go b/remediation/sca/packageupdaters/npmpackageupdater_test.go index 6e72e5f99..f97123c47 100644 --- a/remediation/sca/packageupdaters/npmpackageupdater_test.go +++ b/remediation/sca/packageupdaters/npmpackageupdater_test.go @@ -5,10 +5,12 @@ import ( "strings" "testing" + "github.com/jfrog/jfrog-cli-security/tests/utils/integration" "github.com/stretchr/testify/assert" ) func TestGetFixedDescriptor(t *testing.T) { + integration.InitUnitTest(t) npm := &NpmPackageUpdater{} testcases := []struct { @@ -148,6 +150,7 @@ func TestGetFixedDescriptor(t *testing.T) { } func TestEscapeJsonPathKey(t *testing.T) { + integration.InitUnitTest(t) testcases := []struct { name string input string @@ -194,6 +197,7 @@ func TestEscapeJsonPathKey(t *testing.T) { } func TestBuildIsolatedEnv(t *testing.T) { + integration.InitUnitTest(t) testcases := []struct { name string predefineEnv bool diff --git a/remediation/sca/packageupdaters/pnpmpackageupdater_test.go b/remediation/sca/packageupdaters/pnpmpackageupdater_test.go index bf9dabeb2..4cf2d0635 100644 --- a/remediation/sca/packageupdaters/pnpmpackageupdater_test.go +++ b/remediation/sca/packageupdaters/pnpmpackageupdater_test.go @@ -4,11 +4,13 @@ import ( "strings" "testing" + "github.com/jfrog/jfrog-cli-security/tests/utils/integration" "github.com/jfrog/jfrog-cli-security/utils/techutils" "github.com/stretchr/testify/assert" ) func TestEvidencePathLooksLikeNpmPackageCoordinate(t *testing.T) { + integration.InitUnitTest(t) t.Parallel() tests := []struct { path string @@ -32,6 +34,7 @@ func TestEvidencePathLooksLikeNpmPackageCoordinate(t *testing.T) { } func TestPnpmFilterCoordinateStyleDescriptorPaths(t *testing.T) { + integration.InitUnitTest(t) t.Parallel() in := []string{ "lodash@4.17.19/package.json", @@ -45,6 +48,7 @@ func TestPnpmFilterCoordinateStyleDescriptorPaths(t *testing.T) { } func TestPnpmCollectLeavesNpmParityThenPnpmFilterDropsCoordinates(t *testing.T) { + integration.InitUnitTest(t) t.Parallel() pnpm := &PnpmPackageUpdater{} vuln := createFixDetails(techutils.Pnpm, "lodash", "4.17.19", "4.17.21", true, @@ -55,6 +59,7 @@ func TestPnpmCollectLeavesNpmParityThenPnpmFilterDropsCoordinates(t *testing.T) } func TestPnpmLockRegenerationEnv(t *testing.T) { + integration.InitUnitTest(t) t.Parallel() pnpm := &PnpmPackageUpdater{} env := EnvWithCorepackIntegrityWorkaround(pnpm.BuildEnvWithOverrides(PnpmLockfileInstallEnvOverrides)) diff --git a/remediation/sca/packageupdaters/pythonpackageupdater_test.go b/remediation/sca/packageupdaters/pythonpackageupdater_test.go index 2fcfaa3b8..d369a5c03 100644 --- a/remediation/sca/packageupdaters/pythonpackageupdater_test.go +++ b/remediation/sca/packageupdaters/pythonpackageupdater_test.go @@ -5,10 +5,12 @@ import ( "strings" "testing" + "github.com/jfrog/jfrog-cli-security/tests/utils/integration" "github.com/stretchr/testify/assert" ) func TestPipPackageRegex(t *testing.T) { + integration.InitUnitTest(t) var pipPackagesRegexTests = []pipPackageRegexTest{ {"oslo.config", "oslo.config>=1.12.1,<1.13"}, {"oslo.utils", "oslo.utils<5.0,>=4.0.0"}, diff --git a/remediation_test.go b/remediation_test.go new file mode 100644 index 000000000..814741dfd --- /dev/null +++ b/remediation_test.go @@ -0,0 +1,209 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/jfrog/jfrog-cli-security/remediation/sca/packageupdaters" + integrationUtils "github.com/jfrog/jfrog-cli-security/tests/utils/integration" + "github.com/jfrog/jfrog-cli-security/utils/formats" + "github.com/jfrog/jfrog-cli-security/utils/techutils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// remediationTestDataDir returns the absolute path to the remediation test data directory. +func remediationTestDataDir(t *testing.T, subPath ...string) string { + t.Helper() + parts := append([]string{"tests", "testdata", "projects", "remediation"}, subPath...) + absPath, err := filepath.Abs(filepath.Join(parts...)) + require.NoError(t, err) + return absPath +} + +// copyProjectToTemp copies a named project from the remediation testdata and chdirs into it. +// Returns a cleanup function that restores the working directory and removes the temp dir. +func copyProjectToTemp(t *testing.T, projectDir string) (tmpDir string, cleanup func()) { + t.Helper() + tmpDir = t.TempDir() + require.NoError(t, copyDir(t, projectDir, tmpDir)) + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(tmpDir)) + return tmpDir, func() { + assert.NoError(t, os.Chdir(origDir)) + } +} + +// copyDir copies the directory tree rooted at src into dst. +func copyDir(t *testing.T, src, dst string) error { + t.Helper() + return filepath.Walk(src, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + rel, err := filepath.Rel(src, path) + if err != nil { + return err + } + target := filepath.Join(dst, rel) + if info.IsDir() { + return os.MkdirAll(target, info.Mode()) + } + //#nosec G304 -- path comes from filepath.Walk over a known test data directory. + data, err := os.ReadFile(path) + if err != nil { + return err + } + return os.WriteFile(target, data, info.Mode()) //#nosec G306 -- test helper writing to a temp directory. + }) +} + +// newFixDetails builds a FixDetails value for use in remediation integration tests. +func newFixDetails(tech techutils.Technology, pkg, currentVersion, fixedVersion string, isDirect bool, evidencePaths ...string) *packageupdaters.FixDetails { + var evidences []formats.Location + for _, p := range evidencePaths { + if p != "" { + evidences = append(evidences, formats.Location{File: p}) + } + } + return &packageupdaters.FixDetails{ + Technology: tech, + ImpactedDependencyName: pkg, + ImpactedDependencyVersion: currentVersion, + SuggestedFixedVersion: fixedVersion, + IsDirectDependency: isDirect, + Components: []formats.ComponentRow{ + { + Name: pkg, + Version: currentVersion, + Evidences: evidences, + }, + }, + } +} + +// TestRemediationNpm verifies that the npm package updater applies a direct-dependency fix to +// package.json and regenerates package-lock.json. +func TestRemediationNpm(t *testing.T) { + integrationUtils.InitRemediationTest(t) + + projectDir := remediationTestDataDir(t, "projects", "npm") + _, cleanup := copyProjectToTemp(t, projectDir) + defer cleanup() + + fix := newFixDetails(techutils.Npm, "minimist", "1.2.5", "1.2.6", true, "package.json", "package-lock.json") + updater, supported := packageupdaters.GetCompatiblePackageUpdater(fix) + require.True(t, supported) + + lockBefore, err := os.ReadFile("package-lock.json") + require.NoError(t, err) + + require.NoError(t, updater.UpdateDependency(fix)) + + descriptor, err := os.ReadFile("package.json") + require.NoError(t, err) + assert.Contains(t, string(descriptor), "1.2.6", "package.json should contain the fixed version") + + lockAfter, err := os.ReadFile("package-lock.json") + require.NoError(t, err) + assert.NotEqual(t, lockBefore, lockAfter, "package-lock.json should be regenerated") +} + +// TestRemediationMaven verifies that the maven package updater updates the pom.xml version. +func TestRemediationMaven(t *testing.T) { + integrationUtils.InitRemediationTest(t) + + projectDir := remediationTestDataDir(t, "projects", "maven") + _, cleanup := copyProjectToTemp(t, projectDir) + defer cleanup() + + fix := newFixDetails(techutils.Maven, "commons-io:commons-io", "", "2.7", true, filepath.Join("multi1", "pom.xml")) + updater, supported := packageupdaters.GetCompatiblePackageUpdater(fix) + require.True(t, supported) + + require.NoError(t, updater.UpdateDependency(fix)) + + pom, err := os.ReadFile(filepath.Join("multi1", "pom.xml")) + require.NoError(t, err) + assert.Contains(t, string(pom), "2.7", "pom.xml should contain the fixed version") +} + +// TestRemediationGo verifies that the go package updater updates go.mod and regenerates go.sum. +func TestRemediationGo(t *testing.T) { + integrationUtils.InitRemediationTest(t) + + projectDir := remediationTestDataDir(t, "projects", "go") + tmpDir, cleanup := copyProjectToTemp(t, projectDir) + defer cleanup() + + // The go test-data files are stored with a .txt suffix to avoid Go toolchain interference. + for _, txtFile := range []string{"go.mod.txt", "go.sum.txt", "main.go.txt"} { + target := strings.TrimSuffix(filepath.Join(tmpDir, txtFile), ".txt") + require.NoError(t, os.Rename(filepath.Join(tmpDir, txtFile), target)) + } + + fix := newFixDetails(techutils.Go, "golang.org/x/crypto", "", "0.0.0-20201216223049-8b5274cf687f", false, "go.mod") + updater, supported := packageupdaters.GetCompatiblePackageUpdater(fix) + require.True(t, supported) + + goSumBefore, err := os.ReadFile("go.sum") + require.NoError(t, err) + + require.NoError(t, updater.UpdateDependency(fix)) + + goMod, err := os.ReadFile("go.mod") + require.NoError(t, err) + assert.Contains(t, string(goMod), "0.0.0-20201216223049-8b5274cf687f", "go.mod should contain the fixed version") + + goSumAfter, err := os.ReadFile("go.sum") + require.NoError(t, err) + assert.NotEqual(t, goSumBefore, goSumAfter, "go.sum should be regenerated") +} + +// TestRemediationUnsupportedIndirect verifies that indirect dependency fix attempts return +// ErrUnsupportedFix for package managers that do not support it. +func TestRemediationUnsupportedIndirect(t *testing.T) { + integrationUtils.InitRemediationTest(t) + + for _, tech := range []techutils.Technology{techutils.Npm, techutils.Maven, techutils.Go} { + t.Run(tech.String(), func(t *testing.T) { + fix := newFixDetails(tech, "some-package", "1.0.0", "1.0.1", false) + updater, supported := packageupdaters.GetCompatiblePackageUpdater(fix) + if !supported { + // If not supported at factory level, that is acceptable. + return + } + err := updater.UpdateDependency(fix) + require.Error(t, err) + var unsupported *packageupdaters.ErrUnsupportedFix + assert.ErrorAs(t, err, &unsupported, "expected ErrUnsupportedFix for indirect dependency") + }) + } +} + +// TestRemediationNpmRollback verifies that the npm package updater rolls back package.json +// when npm install fails (e.g. invalid dependency graph in the rollback test project). +func TestRemediationNpmRollback(t *testing.T) { + integrationUtils.InitRemediationTest(t) + + projectDir := remediationTestDataDir(t, "projects", "npm-rollback") + _, cleanup := copyProjectToTemp(t, projectDir) + defer cleanup() + + descriptorBefore, err := os.ReadFile("package.json") + require.NoError(t, err) + + fix := newFixDetails(techutils.Npm, "minimist", "1.2.5", "1.2.6", true, "package.json", "package-lock.json") + updater, supported := packageupdaters.GetCompatiblePackageUpdater(fix) + require.True(t, supported) + + err = updater.UpdateDependency(fix) + require.Error(t, err, "expected an error from the rollback project") + + descriptorAfter, err := os.ReadFile("package.json") + require.NoError(t, err) + assert.Equal(t, descriptorBefore, descriptorAfter, "package.json should be rolled back to its original state") +} diff --git a/tests/config.go b/tests/config.go index 16b68fc7f..918bfc548 100644 --- a/tests/config.go +++ b/tests/config.go @@ -57,6 +57,8 @@ var ( TestAuditCocoapods *bool TestAuditSwift *bool + TestRemediation *bool + JfrogUrl *string JfrogUser *string JfrogPassword *string @@ -126,6 +128,7 @@ func init() { TestAuditCocoapods = flag.Bool("test.audit.Cocoapods", false, "Run Cocoapods technologies audit integration tests") TestAuditSwift = flag.Bool("test.audit.Swift", false, "Run Swift technologies audit integration tests") TestMaliciousScan = flag.Bool("test.maliciousScan", false, "Run Malicious scan command integration tests") + TestRemediation = flag.Bool("test.remediation", false, "Run package updater remediation integration tests") JfrogUrl = flag.String("jfrog.url", getTestUrlDefaultValue(), "JFrog platform url") JfrogUser = flag.String("jfrog.user", getTestUserDefaultValue(), "JFrog platform username") @@ -142,7 +145,7 @@ func init() { func InitTestFlags() { flag.Parse() // If no test types flags were set, run all types - shouldRunAllTests := !isAtLeastOneFlagSet(TestUnit, TestArtifactory, TestXray, TestXsc, TestAuditGeneral, TestAuditNewSca, TestAuditJas, TestAuditJavaScript, TestAuditJava, TestAuditCTypes, TestAuditGo, TestAuditPython, TestAuditCocoapods, TestAuditSwift, TestScan, TestDockerScan, TestCuration, TestEnrich, TestMaliciousScan, TestGit) + shouldRunAllTests := !isAtLeastOneFlagSet(TestUnit, TestArtifactory, TestXray, TestXsc, TestAuditGeneral, TestAuditNewSca, TestAuditJas, TestAuditJavaScript, TestAuditJava, TestAuditCTypes, TestAuditGo, TestAuditPython, TestAuditCocoapods, TestAuditSwift, TestScan, TestDockerScan, TestCuration, TestEnrich, TestMaliciousScan, TestGit, TestRemediation) if shouldRunAllTests { log.Info("Running all tests. To run only specific tests, please specify the desired test flags.") *TestUnit = true @@ -165,6 +168,7 @@ func InitTestFlags() { *TestEnrich = true *TestMaliciousScan = true *TestGit = true + *TestRemediation = true } } diff --git a/tests/utils/integration/test_integrationutils.go b/tests/utils/integration/test_integrationutils.go index 6122a92fe..f9cb3dbb8 100644 --- a/tests/utils/integration/test_integrationutils.go +++ b/tests/utils/integration/test_integrationutils.go @@ -44,6 +44,12 @@ func InitUnitTest(t *testing.T) { } } +func InitRemediationTest(t *testing.T) { + if !*configTests.TestRemediation { + t.Skip(getSkipTestMsg("Remediation integration", "--test.remediation")) + } +} + func InitArtifactoryTest(t *testing.T) { if !*configTests.TestArtifactory { t.Skip(getSkipTestMsg("Artifactory integration", "--test.artifactory")) From 043eb5e2a03a2783682b21ca2345a072ba3f52a1 Mon Sep 17 00:00:00 2001 From: Or Toren Date: Wed, 17 Jun 2026 12:01:54 +0300 Subject: [PATCH 10/10] Fix gosec nosec annotations in remediation_test.go Replace wrong rule IDs G304/G306 with the actual rules gosec flags: - G122 (symlink TOCTOU) on os.ReadFile - G703 (path traversal) on os.WriteFile Co-Authored-By: Claude Sonnet 4.6 --- remediation_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/remediation_test.go b/remediation_test.go index 814741dfd..8177a9443 100644 --- a/remediation_test.go +++ b/remediation_test.go @@ -52,12 +52,11 @@ func copyDir(t *testing.T, src, dst string) error { if info.IsDir() { return os.MkdirAll(target, info.Mode()) } - //#nosec G304 -- path comes from filepath.Walk over a known test data directory. - data, err := os.ReadFile(path) + data, err := os.ReadFile(path) //#nosec G122 -- path comes from filepath.Walk over a known test data directory; symlink TOCTOU is acceptable in tests. if err != nil { return err } - return os.WriteFile(target, data, info.Mode()) //#nosec G306 -- test helper writing to a temp directory. + return os.WriteFile(target, data, info.Mode()) //#nosec G703 -- target is derived from a controlled temp directory; path traversal is not a concern in tests. }) }