Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enables Artifactory integration during scan-repository fix #648

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
github.com/go-git/go-git/v5 v5.11.0
github.com/golang/mock v1.6.0
github.com/google/go-github/v45 v45.2.0
github.com/jfrog/build-info-go v1.9.21
github.com/jfrog/build-info-go v1.9.23
github.com/jfrog/froggit-go v1.14.6
github.com/jfrog/gofrog v1.6.0
github.com/jfrog/jfrog-cli-core/v2 v2.47.12
Expand All @@ -24,7 +24,7 @@ require (
require (
dario.cat/mergo v1.0.0 // indirect
github.com/BurntSushi/toml v1.3.2 // indirect
github.com/CycloneDX/cyclonedx-go v0.7.2 // indirect
github.com/CycloneDX/cyclonedx-go v0.8.0 // indirect
github.com/Microsoft/go-winio v0.6.1 // indirect
github.com/ProtonMail/go-crypto v1.0.0 // indirect
github.com/andybalholm/brotli v1.1.0 // indirect
Expand Down Expand Up @@ -119,10 +119,10 @@ require (
gopkg.in/warnings.v0 v0.1.2 // indirect
)

replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.31.1-0.20240214142246-bb1e61c953ac
replace github.com/jfrog/jfrog-cli-core/v2 => github.com/eranturgeman/jfrog-cli-core/v2 v2.0.0-20240225082957-57a59e31b87f

replace github.com/jfrog/jfrog-cli-security => github.com/jfrog/jfrog-cli-security v1.0.2-0.20240214165055-5ca5e6374e6c
replace github.com/jfrog/jfrog-cli-security => github.com/eranturgeman/jfrog-cli-security v0.0.0-20240225083411-ef3f087d58d3

// replace github.com/jfrog/build-info-go => github.com/jfrog/build-info-go dev
replace github.com/jfrog/build-info-go => github.com/eranturgeman/build-info-go v0.0.0-20240225082441-c0213bb4fbda

replace github.com/jfrog/jfrog-client-go => github.com/jfrog/jfrog-client-go v1.28.1-0.20240214150718-c734e234d315
16 changes: 8 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,8 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03
github.com/BurntSushi/toml v1.3.2 h1:o7IhLm0Msx3BaB+n3Ag7L8EVlByGnpq14C4YWiu/gL8=
github.com/BurntSushi/toml v1.3.2/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
github.com/CycloneDX/cyclonedx-go v0.7.2 h1:kKQ0t1dPOlugSIYVOMiMtFqeXI2wp/f5DBIdfux8gnQ=
github.com/CycloneDX/cyclonedx-go v0.7.2/go.mod h1:K2bA+324+Og0X84fA8HhN2X066K7Bxz4rpMQ4ZhjtSk=
github.com/CycloneDX/cyclonedx-go v0.8.0 h1:FyWVj6x6hoJrui5uRQdYZcSievw3Z32Z88uYzG/0D6M=
github.com/CycloneDX/cyclonedx-go v0.8.0/go.mod h1:K2bA+324+Og0X84fA8HhN2X066K7Bxz4rpMQ4ZhjtSk=
github.com/JohnCGriffin/overflow v0.0.0-20211019200055-46fa312c352c/go.mod h1:X0CRv0ky0k6m906ixxpzmDRLvX58TFUKS2eePweuyxk=
github.com/Microsoft/go-winio v0.5.2/go.mod h1:WpS1mjBmmwHBEWmogvA2mj8546UReBk4v8QkMxJ6pZY=
github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow=
Expand Down Expand Up @@ -706,6 +706,12 @@ github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7
github.com/envoyproxy/protoc-gen-validate v0.6.7/go.mod h1:dyJXwwfPK2VSqiB9Klm1J6romD608Ba7Hij42vrOBCo=
github.com/envoyproxy/protoc-gen-validate v0.9.1/go.mod h1:OKNgG7TCp5pF4d6XftA0++PMirau2/yoOwVac3AbF2w=
github.com/envoyproxy/protoc-gen-validate v0.10.0/go.mod h1:DRjgyB0I43LtJapqN6NiRwroiAU2PaFuvk/vjgh61ss=
github.com/eranturgeman/build-info-go v0.0.0-20240225082441-c0213bb4fbda h1:i1OkEeNXNee0WeeQnYJHB6dYeKrbWg+F/Crz+kChVjA=
github.com/eranturgeman/build-info-go v0.0.0-20240225082441-c0213bb4fbda/go.mod h1:QHcKuesY4MrBVBuEwwBz4uIsX6mwYuMEDV09ng4AvAU=
github.com/eranturgeman/jfrog-cli-core/v2 v2.0.0-20240225082957-57a59e31b87f h1:RiMoMytLZSoQB35tYLa37QciMb1H/iwXBRra06GYR3Y=
github.com/eranturgeman/jfrog-cli-core/v2 v2.0.0-20240225082957-57a59e31b87f/go.mod h1:gnO6E1UQKfpTiYFZH/Fc2JFEBfsnnGsS8iydtlZ35dM=
github.com/eranturgeman/jfrog-cli-security v0.0.0-20240225083411-ef3f087d58d3 h1:X0wK/Xs7Dds9p+zoQi48yIcB1GqM5aEZDY+Y4cbMoHU=
github.com/eranturgeman/jfrog-cli-security v0.0.0-20240225083411-ef3f087d58d3/go.mod h1:el+7A/QX5KciLPihQTOyvr1jaWVv2pvpS2ZCD0mV2YE=
github.com/fatih/color v1.14.1 h1:qfhVLaG5s+nCROl1zJsZRxFeYrHLqWroPOQ8BWiNb4w=
github.com/fatih/color v1.14.1/go.mod h1:2oHN61fhTpgcxD3TSWCgKDiH1+x4OiDVVGH8WlgGZGg=
github.com/fogleman/gg v1.2.1-0.20190220221249-0403632d5b90/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k=
Expand Down Expand Up @@ -890,18 +896,12 @@ github.com/jedib0t/go-pretty/v6 v6.5.4 h1:gOGo0613MoqUcf0xCj+h/V3sHDaZasfv152G6/
github.com/jedib0t/go-pretty/v6 v6.5.4/go.mod h1:5LQIxa52oJ/DlDSLv0HEkWOFMDGoWkJb9ss5KqPpJBg=
github.com/jfrog/archiver/v3 v3.6.0 h1:OVZ50vudkIQmKMgA8mmFF9S0gA47lcag22N13iV3F1w=
github.com/jfrog/archiver/v3 v3.6.0/go.mod h1:fCAof46C3rAXgZurS8kNRNdSVMKBbZs+bNNhPYxLldI=
github.com/jfrog/build-info-go v1.9.21 h1:bcD0SEC2lEilhjE+aDB3xlvA8zsr4Kw/bFzvr9Tcj9I=
github.com/jfrog/build-info-go v1.9.21/go.mod h1:Vxv6zmx4e1NWsx40OHaDWCCYDeYAq2yXzpJ4nsDChbE=
github.com/jfrog/froggit-go v1.14.6 h1:7Z2KgDgdjGAyVt1GROe2PwzllgJhw2s2g64dA9MoYCU=
github.com/jfrog/froggit-go v1.14.6/go.mod h1:TEJSzgiV+3D/GVGE8Y6j46ut1jrBLD1FL6WdMdKwwCE=
github.com/jfrog/gofrog v1.6.0 h1:jOwb37nHY2PnxePNFJ6e6279Pgkr3di05SbQQw47Mq8=
github.com/jfrog/gofrog v1.6.0/go.mod h1:SZ1EPJUruxrVGndOzHd+LTiwWYKMlHqhKD+eu+v5Hqg=
github.com/jfrog/jfrog-apps-config v1.0.1 h1:mtv6k7g8A8BVhlHGlSveapqf4mJfonwvXYLipdsOFMY=
github.com/jfrog/jfrog-apps-config v1.0.1/go.mod h1:8AIIr1oY9JuH5dylz2S6f8Ym2MaadPLR6noCBO4C22w=
github.com/jfrog/jfrog-cli-core/v2 v2.31.1-0.20240214142246-bb1e61c953ac h1:dOPjCfPHtCdpp2e3yGeZZ7EieFxvlY6E45XGVqto5Mg=
github.com/jfrog/jfrog-cli-core/v2 v2.31.1-0.20240214142246-bb1e61c953ac/go.mod h1:xKh03WzFK7YlZtHNJdOzkMvFjyq75yeUWW5EKNKV2Qk=
github.com/jfrog/jfrog-cli-security v1.0.2-0.20240214165055-5ca5e6374e6c h1:oWlaEVqbkrbe3sX281EKE+lM8Y0OMep1y8X9S1Vl3HA=
github.com/jfrog/jfrog-cli-security v1.0.2-0.20240214165055-5ca5e6374e6c/go.mod h1:CQo/eRqwB+O31pCnJv6tdvrTLWP2K7fmK4k4Oh+gebQ=
github.com/jfrog/jfrog-client-go v1.28.1-0.20240214150718-c734e234d315 h1:A/pujr4z8wSFK5Atrr5NHnsl1Y758zP2++R99qNKDyE=
github.com/jfrog/jfrog-client-go v1.28.1-0.20240214150718-c734e234d315/go.mod h1:fV5wrs86ihQkFKfMKpGxMbNf3mbVT4LUf320C1T9C2M=
github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible h1:jdpOPRN1zP63Td1hDQbZW73xKmzDvZHzVdNYxhnTMDA=
Expand Down
44 changes: 41 additions & 3 deletions packagehandlers/nugetpackagehandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ package packagehandlers
import (
"errors"
"fmt"
bidotnet "github.com/jfrog/build-info-go/build/utils/dotnet"
bifileutils "github.com/jfrog/build-info-go/utils"
"github.com/jfrog/frogbot/v2/utils"
"github.com/jfrog/jfrog-cli-core/v2/artifactory/commands/dotnet"
"github.com/jfrog/jfrog-client-go/utils/io/fileutils"
"io/fs"
"os"
"path"
Expand Down Expand Up @@ -35,6 +39,7 @@ func (nph *NugetPackageHandler) UpdateDependency(vulnDetails *utils.Vulnerabilit
}
}

// TODO ERAN fix comments
func (nph *NugetPackageHandler) updateDirectDependency(vulnDetails *utils.VulnerabilityDetails) (err error) {
var assetsFilePaths []string
assetsFilePaths, err = getAssetsFilesPaths()
Expand All @@ -48,12 +53,25 @@ func (nph *NugetPackageHandler) updateDirectDependency(vulnDetails *utils.Vulner
return
}

var installCommandArgs []string
if nph.depsRepo != "" {
// The toolType specified here is consistently 'dotnet' since Frogbot exclusively executes fixes using .NET CLI.
dotnetToolType := bidotnet.ConvertNameToToolType("dotnet")
var clearResolutionServerFunc func() error
if installCommandArgs, clearResolutionServerFunc, err = dotnet.SetArtifactoryAsResolutionServer(nph.serverDetails, nph.depsRepo, wd, dotnetToolType, false); err != nil {
return
}
defer func() {
err = errors.Join(err, clearResolutionServerFunc())
}()
}

vulnRegexpCompiler := getVulnerabilityRegexCompiler(vulnDetails.ImpactedDependencyName, vulnDetails.ImpactedDependencyVersion)
var isAnyFileChanged bool

for _, assetFilePath := range assetsFilePaths {
var isFileChanged bool
isFileChanged, err = nph.fixVulnerabilityIfExists(vulnDetails, assetFilePath, vulnRegexpCompiler, wd)
isFileChanged, err = nph.fixVulnerabilityIfExists(vulnDetails, assetFilePath, vulnRegexpCompiler, wd, installCommandArgs)
if err != nil {
err = fmt.Errorf("failed to update asset file '%s': %s", assetFilePath, err.Error())
return
Expand Down Expand Up @@ -88,7 +106,7 @@ func getAssetsFilesPaths() (assetsFilePaths []string, err error) {
return
}

func (nph *NugetPackageHandler) fixVulnerabilityIfExists(vulnDetails *utils.VulnerabilityDetails, assetFilePath string, vulnRegexpCompiler *regexp.Regexp, originalWd string) (isFileChanged bool, err error) {
func (nph *NugetPackageHandler) fixVulnerabilityIfExists(vulnDetails *utils.VulnerabilityDetails, assetFilePath string, vulnRegexpCompiler *regexp.Regexp, originalWd string, installCommandArgs []string) (isFileChanged bool, err error) {
modulePath := path.Dir(assetFilePath)

var fileData []byte
Expand All @@ -109,7 +127,27 @@ func (nph *NugetPackageHandler) fixVulnerabilityIfExists(vulnDetails *utils.Vuln
err = errors.Join(err, os.Chdir(originalWd))
}()

err = nph.CommonPackageHandler.UpdateDependency(vulnDetails, vulnDetails.Technology.GetPackageInstallationCommand(), dotnetUpdateCmdPackageExtraArg, dotnetNoRestoreFlag)
if nph.depsRepo == "" {
// If integration with Artifactory is not defined we want to update dependencies without running 'restore'
installCommandArgs = append([]string{dotnetUpdateCmdPackageExtraArg, dotnetNoRestoreFlag}, installCommandArgs...)
} else {
installCommandArgs = append([]string{dotnetUpdateCmdPackageExtraArg}, installCommandArgs...)

// In integration with Artifactory is defined we must restore in order to push the newly resolved version to Artifactory
objDirPath := filepath.Join(modulePath, "obj")
var exists bool
if exists, err = fileutils.IsDirExists(objDirPath, false); err != nil {
return
}
if !exists {
// If we don't have an 'obj' directory we want to remove it after executing update command with 'restore'
defer func() {
err = bifileutils.RemoveTempDir(objDirPath)
}()
}
}

err = nph.CommonPackageHandler.UpdateDependency(vulnDetails, vulnDetails.Technology.GetPackageInstallationCommand(), installCommandArgs...)
if err != nil {
return
}
Expand Down
98 changes: 67 additions & 31 deletions packagehandlers/packagehandlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -668,59 +668,95 @@ func uniquePackageManagerChecks(t *testing.T, test dependencyFixTest) {
}
}

// TODO ERAN fix comments
func TestNugetFixVulnerabilityIfExists(t *testing.T) {
var testcases = []struct {
vulnerabilityDetails *utils.VulnerabilityDetails
vulnerabilityDetails *utils.VulnerabilityDetails
setDepsRepo bool
oldDepLine string
expectedUpdatedDepLine string
}{
// Basic check
{
vulnerabilityDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "1.1.1",
IsDirectDependency: true,
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Nuget, ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ImpactedDependencyName: "snappier", ImpactedDependencyVersion: "1.1.0"}}},
oldDepLine: "<PackageReference Include=\"snappier\" Version=\"1.1.0\" />",
expectedUpdatedDepLine: "<PackageReference Include=\"snappier\" Version=\"1.1.1\" />",
},
// This testcase checks a fix with a vulnerability that has '.' in the dependency's group and name + more complex version, including letters, to check that the regexp captures them correctly
{
vulnerabilityDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "7.0.11",
IsDirectDependency: true,
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Nuget, ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ImpactedDependencyName: "Microsoft.Bcl.AsyncInterfaces", ImpactedDependencyVersion: "8.0.0-rc.1.23419.4"}}},
oldDepLine: "<PackageReference Include=\"Microsoft.Bcl.AsyncInterfaces\" Version=\"8.0.0-rc.1.23419.4\" />",
expectedUpdatedDepLine: "<PackageReference Include=\"Microsoft.Bcl.AsyncInterfaces\" Version=\"7.0.11\" />",
},
// This test case checks fix execution with 'restore' (in case we have depsRepo set, we must execute 'restore' in order to push new resolved versions to Artifactory) and verifies 'obj' directory is removed if wasn't existed
{
vulnerabilityDetails: &utils.VulnerabilityDetails{
SuggestedFixedVersion: "13.0.3",
IsDirectDependency: true,
VulnerabilityOrViolationRow: formats.VulnerabilityOrViolationRow{Technology: coreutils.Nuget, ImpactedDependencyDetails: formats.ImpactedDependencyDetails{ImpactedDependencyName: "Newtonsoft.Json", ImpactedDependencyVersion: "13.0.2"}}},
setDepsRepo: true,
oldDepLine: "<PackageReference Include=\"Newtonsoft.Json\" Version=\"13.0.2\" />",
expectedUpdatedDepLine: "<PackageReference Include=\"Newtonsoft.Json\" Version=\"13.0.3\" />",
},
}
testRootDir, err := os.Getwd()
assert.NoError(t, err)

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

assetFiles, err := getAssetsFilesPaths()
assert.NoError(t, err)
testedAssetFile := assetFiles[0]

nph := &NugetPackageHandler{}
testRootDir, outerErr := os.Getwd()
assert.NoError(t, outerErr)

for _, testcase := range testcases {
vulnRegexpCompiler := getVulnerabilityRegexCompiler(testcase.vulnerabilityDetails.ImpactedDependencyName, testcase.vulnerabilityDetails.ImpactedDependencyVersion)
var isFileChanged bool
isFileChanged, err = nph.fixVulnerabilityIfExists(testcase.vulnerabilityDetails, testedAssetFile, vulnRegexpCompiler, tmpDir)
assert.NoError(t, err)
assert.True(t, isFileChanged)
t.Run(fmt.Sprintf("fix %s, depsRepo: %t", testcase.vulnerabilityDetails.ImpactedDependencyName, testcase.setDepsRepo),
func(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "")
assert.NoError(t, err)
defer func() {
assert.NoError(t, fileutils.RemoveTempDir(tmpDir))
}()
assert.NoError(t, biutils.CopyDir(filepath.Join("..", "testdata", "projects", "dotnet"), tmpDir, true, nil))
assert.NoError(t, os.Chdir(tmpDir))
defer func() {
assert.NoError(t, os.Chdir(testRootDir))
}()

assetFiles, err := getAssetsFilesPaths()
assert.NoError(t, err)
testedAssetFile := assetFiles[0]

var nph *NugetPackageHandler
if testcase.setDepsRepo {
nph = &NugetPackageHandler{CommonPackageHandler{
depsRepo: "myRepo",
}}
} else {
nph = &NugetPackageHandler{}
}

vulnRegexpCompiler := getVulnerabilityRegexCompiler(testcase.vulnerabilityDetails.ImpactedDependencyName, testcase.vulnerabilityDetails.ImpactedDependencyVersion)
var isFileChanged bool
isFileChanged, err = nph.fixVulnerabilityIfExists(testcase.vulnerabilityDetails, testedAssetFile, vulnRegexpCompiler, tmpDir, []string{})
assert.NoError(t, err)
assert.True(t, isFileChanged)

var fixedFileContent []byte
fixedFileContent, err = os.ReadFile(testedAssetFile)
fixedFileContentString := string(fixedFileContent)
assert.NoError(t, err)
assert.NotContains(t, fixedFileContentString, testcase.oldDepLine)
assert.Contains(t, fixedFileContentString, testcase.expectedUpdatedDepLine)

if testcase.setDepsRepo {
var objExists bool
objExists, err = fileutils.IsDirExists(filepath.Join(tmpDir, "obj"), false)
assert.NoError(t, err)
assert.False(t, objExists)
}
})
}

var fixedFileContent []byte
fixedFileContent, err = os.ReadFile(testedAssetFile)
fixedFileContentString := string(fixedFileContent)

assert.NoError(t, err)
assert.NotContains(t, fixedFileContentString, "<PackageReference Include=\"snappier\" Version=\"1.1.0\" />")
assert.Contains(t, fixedFileContentString, "<PackageReference Include=\"snappier\" Version=\"1.1.1\" />")
assert.NotContains(t, fixedFileContentString, "<PackageReference Include=\"Microsoft.Bcl.AsyncInterfaces\" Version=\"8.0.0-rc.1.23419.4\" />")
assert.Contains(t, fixedFileContentString, "<PackageReference Include=\"Microsoft.Bcl.AsyncInterfaces\" Version=\"7.0.11\" />")
}

func TestGetFixedPackage(t *testing.T) {
Expand Down
Loading