From 7bd47c2713e29f16eb1b772e48f5deeeace5cc4c Mon Sep 17 00:00:00 2001 From: Eran Turgeman <81029514+eranturgeman@users.noreply.github.com> Date: Tue, 1 Oct 2024 01:35:05 +0300 Subject: [PATCH] Skip install in dependencies map calculation if requested (#277) --- .../testdata/npm/noBuildProject/package.json | 17 +++++++ build/utils/npm.go | 47 +++++++++++++++---- build/utils/npm_test.go | 18 ++++++- utils/error.go | 9 ++++ 4 files changed, 80 insertions(+), 11 deletions(-) create mode 100644 build/testdata/npm/noBuildProject/package.json diff --git a/build/testdata/npm/noBuildProject/package.json b/build/testdata/npm/noBuildProject/package.json new file mode 100644 index 00000000..9a4a6cbb --- /dev/null +++ b/build/testdata/npm/noBuildProject/package.json @@ -0,0 +1,17 @@ +{ + "name": "npm_test2", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC", + "dependencies": { + "lightweight": "^0.1.0", + "minimist": "^0.1.0", + "underscore": "^1.13.6", + "cors.js": "0.0.1-security" + } +} diff --git a/build/utils/npm.go b/build/utils/npm.go index 289ecadb..3d4ae3ff 100644 --- a/build/utils/npm.go +++ b/build/utils/npm.go @@ -27,7 +27,7 @@ func CalculateNpmDependenciesList(executablePath, srcPath, moduleId string, npmP log = &utils.NullLog{} } // Calculate npm dependency tree using 'npm ls...'. - dependenciesMap, err := CalculateDependenciesMap(executablePath, srcPath, moduleId, npmParams, log) + dependenciesMap, err := CalculateDependenciesMap(executablePath, srcPath, moduleId, npmParams, log, false) if err != nil { return nil, err } @@ -92,7 +92,7 @@ type dependencyInfo struct { // Run 'npm list ...' command and parse the returned result to create a dependencies map of. // The dependencies map looks like name:version -> entities.Dependency. -func CalculateDependenciesMap(executablePath, srcPath, moduleId string, npmListParams NpmTreeDepListParam, log utils.Log) (map[string]*dependencyInfo, error) { +func CalculateDependenciesMap(executablePath, srcPath, moduleId string, npmListParams NpmTreeDepListParam, log utils.Log, skipInstall bool) (map[string]*dependencyInfo, error) { dependenciesMap := make(map[string]*dependencyInfo) // These arguments must be added at the end of the command, to override their other values (if existed in nm.npmArgs). npmVersion, err := GetNpmVersion(executablePath, log) @@ -108,7 +108,7 @@ func CalculateDependenciesMap(executablePath, srcPath, moduleId string, npmListP if nodeModulesExist && !npmListParams.IgnoreNodeModules { data = runNpmLsWithNodeModules(executablePath, srcPath, npmListParams.Args, log) } else { - data, err = runNpmLsWithoutNodeModules(executablePath, srcPath, npmListParams, log, npmVersion) + data, err = runNpmLsWithoutNodeModules(executablePath, srcPath, npmListParams, log, npmVersion, skipInstall) if err != nil { return nil, err } @@ -135,13 +135,14 @@ func runNpmLsWithNodeModules(executablePath, srcPath string, npmArgs []string, l return } -func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams NpmTreeDepListParam, log utils.Log, npmVersion *version.Version) ([]byte, error) { - isPackageLockExist, isDirExistsErr := utils.IsFileExists(filepath.Join(srcPath, "package-lock.json"), false) - if isDirExistsErr != nil { - return nil, isDirExistsErr +func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams NpmTreeDepListParam, log utils.Log, npmVersion *version.Version, skipInstall bool) ([]byte, error) { + installRequired, err := isInstallRequired(srcPath, npmListParams, log, skipInstall) + if err != nil { + return nil, err } - if !isPackageLockExist || (npmListParams.OverwritePackageLock && checkIfLockFileShouldBeUpdated(srcPath, log)) { - err := installPackageLock(executablePath, srcPath, npmListParams.InstallCommandArgs, npmListParams.Args, log, npmVersion) + + if installRequired { + err = installPackageLock(executablePath, srcPath, npmListParams.InstallCommandArgs, npmListParams.Args, log, npmVersion) if err != nil { return nil, err } @@ -156,6 +157,34 @@ func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams Np return data, nil } +// This function determines whether a project installation is required by evaluating the following criteria: +// 1) Checks if the "package-lock.json" file exists in the project directory. +// 2) Verifies if an installation command was provided by the user. +// 3) Checks if the lock file should be updated due to an override request. +// +// Conditions for triggering installation: +// - If the user provided an installation command, installation is required. +// - If the "package-lock.json" file is missing or an override request to update the lock file exists, installation is required, unless the user explicitly requested to skip the installation. +// +// If installation is required but skipped by the user's request, an error is returned. +func isInstallRequired(srcPath string, npmListParams NpmTreeDepListParam, log utils.Log, skipInstall bool) (bool, error) { + isPackageLockExist, err := utils.IsFileExists(filepath.Join(srcPath, "package-lock.json"), false) + if err != nil { + return false, err + } + + if len(npmListParams.InstallCommandArgs) > 0 { + return true, nil + } + if !isPackageLockExist || (npmListParams.OverwritePackageLock && checkIfLockFileShouldBeUpdated(srcPath, log)) { + if skipInstall { + return false, &utils.ErrProjectNotInstalled{UninstalledDir: srcPath} + } + return true, nil + } + return false, nil +} + 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") diff --git a/build/utils/npm_test.go b/build/utils/npm_test.go index 8fc958ec..a06438cc 100644 --- a/build/utils/npm_test.go +++ b/build/utils/npm_test.go @@ -2,6 +2,7 @@ package utils import ( "encoding/json" + "errors" "os" "path/filepath" "strings" @@ -233,7 +234,7 @@ func TestDependencyWithNoIntegrity(t *testing.T) { assert.Greaterf(t, len(dependencies), 0, "Error: dependencies are not found!") } -// This test case verifies that CalculateNpmDependenciesList correctly handles the exclusion of 'node_modules' +// This test case verifies that CalculateDependenciesMap correctly handles the exclusion of 'node_modules' // and updates 'package-lock.json' as required, based on the 'IgnoreNodeModules' and 'OverwritePackageLock' parameters. func TestDependencyPackageLockOnly(t *testing.T) { npmVersion, _, err := GetNpmVersionAndExecPath(logger) @@ -249,12 +250,25 @@ func TestDependencyPackageLockOnly(t *testing.T) { // Calculate dependencies. dependencies, err := CalculateDependenciesMap("npm", path, "jfrogtest", - NpmTreeDepListParam{Args: []string{}, IgnoreNodeModules: true, OverwritePackageLock: true}, logger) + NpmTreeDepListParam{Args: []string{}, IgnoreNodeModules: true, OverwritePackageLock: true}, logger, false) assert.NoError(t, err) var expectedRes = getExpectedRespForTestDependencyPackageLockOnly() assert.Equal(t, expectedRes, dependencies) } +func TestCalculateDependenciesMapWithProhibitedInstallation(t *testing.T) { + path, cleanup := tests.CreateTestProject(t, filepath.Join("..", "testdata", "npm", "noBuildProject")) + defer cleanup() + + dependencies, err := CalculateDependenciesMap("npm", path, "jfrogtest", + NpmTreeDepListParam{Args: []string{}, IgnoreNodeModules: false, OverwritePackageLock: false}, logger, true) + + assert.Nil(t, dependencies) + assert.Error(t, err) + var installForbiddenErr *utils.ErrProjectNotInstalled + assert.True(t, errors.As(err, &installForbiddenErr)) +} + func getExpectedRespForTestDependencyPackageLockOnly() map[string]*dependencyInfo { return map[string]*dependencyInfo{ "underscore:1.13.6": { diff --git a/utils/error.go b/utils/error.go index 8d14cbd1..de5dd74c 100644 --- a/utils/error.go +++ b/utils/error.go @@ -1,6 +1,7 @@ package utils import ( + "fmt" "strings" ) @@ -28,6 +29,14 @@ func NewForbiddenError() *ForbiddenError { return &ForbiddenError{} } +type ErrProjectNotInstalled struct { + UninstalledDir string +} + +func (err *ErrProjectNotInstalled) Error() string { + return fmt.Sprintf("Directory '%s' is not installed. Skipping SCA scan in this directory...", err.UninstalledDir) +} + // IsForbiddenOutput checks whether the provided output includes a 403 Forbidden. The various package managers have their own forbidden output formats. func IsForbiddenOutput(tech PackageManager, cmdOutput string) bool { switch tech {