From c8422dff40ee1343076a97b38f32273cea8e80b4 Mon Sep 17 00:00:00 2001 From: Eran Turgeman Date: Wed, 25 Oct 2023 17:24:46 +0300 Subject: [PATCH 1/4] enabled passing arguments to install command during dep tree build to improve 'audit' --- build/utils/npm.go | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/build/utils/npm.go b/build/utils/npm.go index d086cc11..07f80503 100644 --- a/build/utils/npm.go +++ b/build/utils/npm.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "golang.org/x/exp/slices" "os" "os/exec" "path/filepath" @@ -16,6 +17,8 @@ import ( "github.com/jfrog/gofrog/version" ) +const npmInstallCommand = "install" + // CalculateNpmDependenciesList gets an npm project's dependencies. func CalculateNpmDependenciesList(executablePath, srcPath, moduleId string, npmParams NpmTreeDepListParam, calculateChecksums bool, log utils.Log) ([]entities.Dependency, error) { if log == nil { @@ -136,12 +139,12 @@ func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams Np return nil, isDirExistsErr } if !isPackageLockExist || (npmListParams.OverwritePackageLock && checkIfLockFileShouldBeUpdated(srcPath, log)) { - err := installPackageLock(executablePath, srcPath, npmListParams.Args, log, npmVersion) + err := installPackageLock(executablePath, srcPath, npmListParams.InstallCommandArgs, npmListParams.Args, log, npmVersion) if err != nil { return nil, err } } - npmListParams.Args = append(npmListParams.Args, "--json", "--all", "--long", "--package-lock-only") + npmListParams.Args = append(npmListParams.Args, "--json", "--all", "--long", "--package-lock-only", "--no-audit") data, errData, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(npmListParams.Args, "ls"), log) if err != nil { log.Warn(err.Error()) @@ -151,9 +154,10 @@ func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams Np return data, nil } -func installPackageLock(executablePath, srcPath string, npmArgs []string, log utils.Log, npmVersion *version.Version) error { +func installPackageLock(executablePath, srcPath string, npmInstallCommandArgs []string, npmArgs []string, log utils.Log, npmVersion *version.Version) error { if npmVersion.AtLeast("6.0.0") { npmArgs = append(npmArgs, "--package-lock-only") + npmArgs = append(npmArgs, filterUniqueArgs(npmInstallCommandArgs, npmArgs)...) // Installing package-lock to generate the dependencies map. _, _, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(npmArgs, "install"), log) if err != nil { @@ -164,6 +168,20 @@ func installPackageLock(executablePath, srcPath string, npmArgs []string, log ut return errors.New("it looks like you’re using version " + npmVersion.GetVersion() + " of the npm client. Versions below 6.0.0 require running `npm install` before running this command") } +// filters out all args from argsToFilter that already in existingArgs. In addition, filters out npm install command and leave only flags within the final returned args +func filterUniqueArgs(argsToFilter []string, existingArgs []string) []string { + var filteredArgs []string + for _, arg := range argsToFilter { + if arg == npmInstallCommand { + continue + } + if !slices.Contains(existingArgs, arg) { + filteredArgs = append(filteredArgs, arg) + } + } + return filteredArgs +} + // Check if package.json has been modified. // This might indicate the addition of new packages to package.json that haven't been reflected in package-lock.json. func checkIfLockFileShouldBeUpdated(srcPath string, log utils.Log) bool { @@ -192,7 +210,8 @@ func GetNpmVersion(executablePath string, log utils.Log) (*version.Version, erro } type NpmTreeDepListParam struct { - Args []string + Args []string + InstallCommandArgs []string // Ignore the node_modules folder if exists, using the '--package-lock-only' flag IgnoreNodeModules bool // Rewrite package-lock.json, if exists. From 66c0ccd9156921eed7b1f46c8e6410e58562e925 Mon Sep 17 00:00:00 2001 From: Eran Turgeman Date: Thu, 26 Oct 2023 18:07:46 +0300 Subject: [PATCH 2/4] removed un-necessary flag --- build/utils/npm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/utils/npm.go b/build/utils/npm.go index 07f80503..980d6d10 100644 --- a/build/utils/npm.go +++ b/build/utils/npm.go @@ -144,7 +144,7 @@ func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams Np return nil, err } } - npmListParams.Args = append(npmListParams.Args, "--json", "--all", "--long", "--package-lock-only", "--no-audit") + npmListParams.Args = append(npmListParams.Args, "--json", "--all", "--long", "--package-lock-only") data, errData, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(npmListParams.Args, "ls"), log) if err != nil { log.Warn(err.Error()) From 839a635bb8d2813957cdf04d23791053a469e863 Mon Sep 17 00:00:00 2001 From: Eran Turgeman Date: Mon, 30 Oct 2023 13:47:50 +0200 Subject: [PATCH 3/4] added documentations and test --- build/utils/npm.go | 10 +++++++--- build/utils/npm_test.go | 41 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/build/utils/npm.go b/build/utils/npm.go index 980d6d10..c5c85acf 100644 --- a/build/utils/npm.go +++ b/build/utils/npm.go @@ -154,9 +154,10 @@ func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams Np return data, nil } -func installPackageLock(executablePath, srcPath string, npmInstallCommandArgs []string, npmArgs []string, log utils.Log, npmVersion *version.Version) error { +func installPackageLock(executablePath, srcPath string, npmInstallCommandArgs, npmArgs []string, log utils.Log, npmVersion *version.Version) error { if npmVersion.AtLeast("6.0.0") { npmArgs = append(npmArgs, "--package-lock-only") + // Adding 'install' command flags provided by the user, if such were provided in previous steps of the flow, and making sure no duplicates will be inserted npmArgs = append(npmArgs, filterUniqueArgs(npmInstallCommandArgs, npmArgs)...) // Installing package-lock to generate the dependencies map. _, _, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(npmArgs, "install"), log) @@ -168,7 +169,7 @@ func installPackageLock(executablePath, srcPath string, npmInstallCommandArgs [] return errors.New("it looks like you’re using version " + npmVersion.GetVersion() + " of the npm client. Versions below 6.0.0 require running `npm install` before running this command") } -// filters out all args from argsToFilter that already in existingArgs. In addition, filters out npm install command and leave only flags within the final returned args +// Filters out all args from argsToFilter that already in existingArgs. In addition, filters out npm install command and leave only flags within the final returned args func filterUniqueArgs(argsToFilter []string, existingArgs []string) []string { var filteredArgs []string for _, arg := range argsToFilter { @@ -210,7 +211,10 @@ func GetNpmVersion(executablePath string, log utils.Log) (*version.Version, erro } type NpmTreeDepListParam struct { - Args []string + // Required for 'install' and 'ls' commands that might be executed while building NPM dependency tree + Args []string + // 'Install' command args provided by the user. These are optional arguments, which are addable only from certain entry points. + // These arguments might be used while building NPM dependency tree, that might require running 'npm install...' InstallCommandArgs []string // Ignore the node_modules folder if exists, using the '--package-lock-only' flag IgnoreNodeModules bool diff --git a/build/utils/npm_test.go b/build/utils/npm_test.go index f8363c6e..7ef9ad67 100644 --- a/build/utils/npm_test.go +++ b/build/utils/npm_test.go @@ -25,7 +25,7 @@ func TestReadPackageInfo(t *testing.T) { return } - tests := []struct { + testcases := []struct { json string pi *PackageInfo }{ @@ -35,7 +35,7 @@ func TestReadPackageInfo(t *testing.T) { &PackageInfo{Name: "build-info-go-tests", Version: "1.0.0", Scope: "@jfrog"}}, {`{}`, &PackageInfo{}}, } - for _, test := range tests { + for _, test := range testcases { t.Run(test.json, func(t *testing.T) { packInfo, err := ReadPackageInfo([]byte(test.json), npmVersion) assert.NoError(t, err) @@ -94,14 +94,14 @@ func TestReadPackageInfoFromPackageJsonIfExistErr(t *testing.T) { } func TestGetDeployPath(t *testing.T) { - tests := []struct { + testcases := []struct { expectedPath string pi *PackageInfo }{ {`build-info-go-tests/-/build-info-go-tests-1.0.0.tgz`, &PackageInfo{Name: "build-info-go-tests", Version: "1.0.0", Scope: ""}}, {`@jfrog/build-info-go-tests/-/build-info-go-tests-1.0.0.tgz`, &PackageInfo{Name: "build-info-go-tests", Version: "1.0.0", Scope: "@jfrog"}}, } - for _, test := range tests { + for _, test := range testcases { t.Run(test.expectedPath, func(t *testing.T) { assert.Equal(t, test.expectedPath, test.pi.GetDeployPath()) }) @@ -430,3 +430,36 @@ func validateDependencies(t *testing.T, projectPath string, npmArgs []string) { // Asserting there is at least one dependency. assert.Greater(t, len(dependencies), 0, "Error: dependencies are not found!") } + +func TestFilterUniqueArgs(t *testing.T) { + var testcases = []struct { + argsToFilter []string + alreadyExists []string + expectedResult []string + }{ + { + argsToFilter: []string{"install"}, + alreadyExists: []string{}, + expectedResult: nil, + }, + { + argsToFilter: []string{"install", "--flagA"}, + alreadyExists: []string{"--flagA"}, + expectedResult: nil, + }, + { + argsToFilter: []string{"install", "--flagA", "--flagB"}, + alreadyExists: []string{"--flagA"}, + expectedResult: []string{"--flagB"}, + }, + { + argsToFilter: []string{"install", "--flagA", "--flagB"}, + alreadyExists: []string{"--flagA", "--flagC"}, + expectedResult: []string{"--flagB"}, + }, + } + + for _, testcase := range testcases { + assert.Equal(t, testcase.expectedResult, filterUniqueArgs(testcase.argsToFilter, testcase.alreadyExists)) + } +} From fe2b56c06f87d4e33edde9d0319b6d15f585f506 Mon Sep 17 00:00:00 2001 From: Eran Turgeman Date: Tue, 31 Oct 2023 15:39:29 +0200 Subject: [PATCH 4/4] fixed comments --- build/utils/npm.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/build/utils/npm.go b/build/utils/npm.go index c5c85acf..b962f8c9 100644 --- a/build/utils/npm.go +++ b/build/utils/npm.go @@ -157,7 +157,7 @@ func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams Np func installPackageLock(executablePath, srcPath string, npmInstallCommandArgs, npmArgs []string, log utils.Log, npmVersion *version.Version) error { if npmVersion.AtLeast("6.0.0") { npmArgs = append(npmArgs, "--package-lock-only") - // Adding 'install' command flags provided by the user, if such were provided in previous steps of the flow, and making sure no duplicates will be inserted + // Including any 'install' command flags that were supplied by the user in preceding steps of the process, while ensuring that duplicates are avoided. npmArgs = append(npmArgs, filterUniqueArgs(npmInstallCommandArgs, npmArgs)...) // Installing package-lock to generate the dependencies map. _, _, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(npmArgs, "install"), log) @@ -169,7 +169,7 @@ func installPackageLock(executablePath, srcPath string, npmInstallCommandArgs, n return errors.New("it looks like you’re using version " + npmVersion.GetVersion() + " of the npm client. Versions below 6.0.0 require running `npm install` before running this command") } -// Filters out all args from argsToFilter that already in existingArgs. In addition, filters out npm install command and leave only flags within the final returned args +// Removes any arguments from argsToFilter that are already present in existingArgs. Furthermore, excludes the "install" command and retains only the flags in the resulting argument list. func filterUniqueArgs(argsToFilter []string, existingArgs []string) []string { var filteredArgs []string for _, arg := range argsToFilter { @@ -211,10 +211,9 @@ func GetNpmVersion(executablePath string, log utils.Log) (*version.Version, erro } type NpmTreeDepListParam struct { - // Required for 'install' and 'ls' commands that might be executed while building NPM dependency tree + // Required for the 'install' and 'ls' commands that could be triggered during the construction of the NPM dependency tree Args []string - // 'Install' command args provided by the user. These are optional arguments, which are addable only from certain entry points. - // These arguments might be used while building NPM dependency tree, that might require running 'npm install...' + // Optional user-supplied arguments for the 'install' command. These arguments are not available from all entry points. They may be employed when constructing the NPM dependency tree, which could necessitate the execution of 'npm install...' InstallCommandArgs []string // Ignore the node_modules folder if exists, using the '--package-lock-only' flag IgnoreNodeModules bool