fix: Bug with transitive dependency resolution

README.md

@@ -36,6 +36,7 @@ pnpm add <package-name>
       - [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 <package-name>
 ## Contributing
 
 Refer to [CONTRIBUTING.md](CONTRIBUTING.md)
+
+## Limitations
+
+<details>
+<summary>Approximate dependency version resolution</summary>
+`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.
+</details>

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 {

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

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())

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
 }
 

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,

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)
 			},
 		},
 		{