diff --git a/README.md b/README.md index aadb43e..1edb4bc 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,7 @@ pnpm add - [Malicious Package Detection](#malicious-package-detection) - [Bulk Package Analysis](#bulk-package-analysis) - [Contributing](#contributing) + - [Limitations](#limitations) ## Features @@ -134,3 +135,17 @@ pmg --debug npm install ## Contributing Refer to [CONTRIBUTING.md](CONTRIBUTING.md) + +## Limitations + +
+Approximate dependency version resolution +`pmg` resolves the transitive dependencies of a package to be installed. It does it by querying +package registry APIs such as `npmjs` and `pypi`. However, almost always, dependency versions are +specified as ranges instead of specific version. Different package managers have different ways of +resolving these ranges. It also depends on peer or host dependencies already available in the application. + +`pmg` is required to block a malicious package *before* it is installed. Hence it applies its own heuristic +to choose a version from a version range for evaluation. This is fine when all versions of a given package +is malicious. However, there is a possibility of inconsistency when a specific version of a package is malicious. +
diff --git a/cmd/npm/common.go b/cmd/npm/common.go index 87230f6..e9f3ddf 100644 --- a/cmd/npm/common.go +++ b/cmd/npm/common.go @@ -15,6 +15,7 @@ func executeCommonFlow(ctx context.Context, config config.Config, pm packagemana packageResolverConfig := packagemanager.NewDefaultNpmDependencyResolverConfig() packageResolverConfig.IncludeTransitiveDependencies = config.Transitive packageResolverConfig.TransitiveDepth = config.TransitiveDepth + packageResolverConfig.IncludeDevDependencies = config.IncludeDevDependencies packageResolver, err := packagemanager.NewNpmDependencyResolver(packageResolverConfig) if err != nil { diff --git a/config/config.go b/config/config.go index 8196c72..13796c3 100644 --- a/config/config.go +++ b/config/config.go @@ -12,8 +12,9 @@ type contextValue struct { // Global configuration type Config struct { - Transitive bool - TransitiveDepth int + Transitive bool + TransitiveDepth int + IncludeDevDependencies bool } // Inject config into context while protecting against context poisoning diff --git a/main.go b/main.go index 61400b5..c0c212e 100644 --- a/main.go +++ b/main.go @@ -56,8 +56,10 @@ func main() { cmd.PersistentFlags().BoolVar(&verbose, "verbose", false, "Verbose mode for more information") cmd.PersistentFlags().BoolVar(&debug, "debug", false, "Enable debug logging (defaults to stdout)") cmd.PersistentFlags().BoolVar(&globalConfig.Transitive, "transitive", true, "Resolve transitive dependencies") - cmd.PersistentFlags().IntVar(&globalConfig.TransitiveDepth, "transitive-depth", 20, + cmd.PersistentFlags().IntVar(&globalConfig.TransitiveDepth, "transitive-depth", 5, "Maximum depth of transitive dependencies to resolve") + cmd.PersistentFlags().BoolVar(&globalConfig.IncludeDevDependencies, "include-dev-dependencies", false, + "Include dev dependencies in the dependency graph (slows down resolution)") cmd.AddCommand(npm.NewNpmCommand()) cmd.AddCommand(npm.NewPnpmCommand()) diff --git a/packagemanager/dependency_resolver.go b/packagemanager/dependency_resolver.go index 7378e1c..f0a9ebd 100644 --- a/packagemanager/dependency_resolver.go +++ b/packagemanager/dependency_resolver.go @@ -3,6 +3,7 @@ package packagemanager import ( "context" "fmt" + "slices" packagev1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/package/v1" "github.com/safedep/dry/log" @@ -71,7 +72,7 @@ func (r *dependencyResolver) resolvePackageDependenciesRecursive( return err } - log.Warnf("error resolving package dependencies: %w", err) + log.Warnf("error resolving package dependencies: %s", err) return nil } @@ -103,28 +104,20 @@ func (r *dependencyResolver) resolvePackageDependenciesRecursive( dependencies = append(dependencies, dependencyList.DevDependencies...) } - // Create package version objects for all dependencies + // Create package version objects for all dependencies and clean versions resolvedDependencies := make([]*packagev1.PackageVersion, 0, len(dependencies)) for _, dependency := range dependencies { - depPackageVersion := &packagev1.PackageVersion{ + resolvedDependencies = append(resolvedDependencies, &packagev1.PackageVersion{ Package: &packagev1.Package{ Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, Name: dependency.Name, }, Version: npmCleanVersion(dependency.VersionSpec), - } - - // Check if this dependency is already in the result to avoid duplicates - depKey := r.packageKey(depPackageVersion) - if !visitedPackages[depKey] { - resolvedDependencies = append(resolvedDependencies, depPackageVersion) - *result = append(*result, depPackageVersion) - visitedPackages[depKey] = true - } + }) } // Process transitive dependencies if enabled - if r.config.IncludeTransitiveDependencies { + if r.config.IncludeTransitiveDependencies && depth < r.config.TransitiveDepth { for _, dependency := range resolvedDependencies { err := r.resolvePackageDependenciesRecursive(ctx, pd, dependency, depth+1, visitedPackages, result) if err != nil { @@ -133,6 +126,13 @@ func (r *dependencyResolver) resolvePackageDependenciesRecursive( } } + // Finally add resolved dependencies to the result + for _, dependency := range resolvedDependencies { + if !slices.Contains(*result, dependency) { + *result = append(*result, dependency) + } + } + return nil } diff --git a/packagemanager/npm_resolver.go b/packagemanager/npm_resolver.go index 4ce30d2..ab44fd2 100644 --- a/packagemanager/npm_resolver.go +++ b/packagemanager/npm_resolver.go @@ -20,7 +20,7 @@ type NpmDependencyResolverConfig struct { func NewDefaultNpmDependencyResolverConfig() NpmDependencyResolverConfig { return NpmDependencyResolverConfig{ - IncludeDevDependencies: true, + IncludeDevDependencies: false, IncludeTransitiveDependencies: true, TransitiveDepth: 5, FailFast: false, diff --git a/packagemanager/npm_resolver_test.go b/packagemanager/npm_resolver_test.go index 7af7414..8dd98f3 100644 --- a/packagemanager/npm_resolver_test.go +++ b/packagemanager/npm_resolver_test.go @@ -91,8 +91,16 @@ func TestNpmDependencyResolver_ResolveDependencies(t *testing.T) { assertFn: func(t *testing.T, dependencies []*packagev1.PackageVersion, err error) { require.NoError(t, err) require.Equal(t, 2, len(dependencies)) - require.Equal(t, "loose-envify", dependencies[0].Package.Name) - require.Equal(t, "react-dom", dependencies[1].Package.Name) + + packageNames := []string{} + for _, dep := range dependencies { + packageNames = append(packageNames, dep.Package.Name) + } + + require.ElementsMatch(t, []string{ + "loose-envify", + "js-tokens", + }, packageNames) }, }, {