feat: add manifest-based package installation detection

cmd/pypi/pip.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 
-	packagev1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/package/v1"
 	"github.com/safedep/dry/log"
 	"github.com/safedep/pmg/config"
 	"github.com/safedep/pmg/internal/flows"
@@ -52,12 +51,6 @@ func executePipFlow(ctx context.Context, args []string) error {
 	packageResolverConfig.IncludeDevDependencies = config.IncludeDevDependencies
 	packageResolverConfig.PackageInstallTargets = parsedCommand.InstallTargets
 
-	extractorConfig := packagemanager.NewDefaultExtractorConfig()
-	extractorConfig.ExtractorEcosystem = packagev1.Ecosystem_ECOSYSTEM_PYPI
-	extractorConfig.ExtractorsName = packagemanager.PyPiExtractors
-
-	// extractor := packagemanager.NewExtractor(*extractorConfig)
-
 	packageResolver, err := packagemanager.NewPypiDependencyResolver(packageResolverConfig)
 	if err != nil {
 		ui.Fatalf("Failed to create dependency resolver: %s", err)

code-review-checklist.md

@@ -1,55 +0,0 @@
-# Code Review Checklist: Package Extractor
-
-## 🔍 Code Structure & Organization
-- [ ] Are the responsibilities clearly separated between configuration and extraction logic?
-- [ ] Could the extractor struct be made more testable by extracting interfaces?
-- [ ] Is the naming consistent throughout the file (e.g., `ExtractorConfig` vs `extractor`)?
-- [ ] Should the global variables (`NpmExtractors`, `PyPiExtractors`) be constants or part of a configuration?
-
-## 🛡️ Error Handling & Robustness
-- [ ] What happens if `e.Config.context` is nil? Should there be validation?
-- [ ] Are all potential error cases from the OSV-Scalibr library handled appropriately?
-- [ ] Could the error message in `ExtractManifestFiles()` be more descriptive?
-- [ ] What if `scanResult.Inventory.Packages` is nil or empty?
-
-## 📊 Data Validation & Edge Cases
-- [ ] Should `ExtractorConfig` validate that `ExtractorsName` and `ExtractorType` are compatible?
-- [ ] What happens if `ScanDir` doesn't exist or isn't readable?
-- [ ] Are there any assumptions about package name/version formats that could break?
-- [ ] Should there be limits on the number of packages processed?
-
-## 🚀 Performance & Resource Management
-- [ ] Is the scanner being reused efficiently, or should it be cached/pooled?
-- [ ] Could memory usage be optimized when processing large package lists?
-- [ ] Are there any potential goroutine leaks in the scanning process?
-- [ ] Should there be timeout handling for long-running scans?
-
-## 🧪 Testing & Maintainability
-- [ ] How would you unit test the `ExtractManifestFiles()` method?
-- [ ] Are the dependencies (OSV-Scalibr) easily mockable for testing?
-- [ ] Could the configuration creation be simplified or made more fluent?
-- [ ] Is the code following Go conventions for package structure?
-
-## 🔧 API Design Questions
-- [ ] Should `DefaultExtractorConfig()` return a pointer or a value?
-- [ ] Is the `NewExtractor` constructor providing enough validation?
-- [ ] Could the extractor support multiple ecosystems in a single scan?
-- [ ] Should there be a way to filter or transform packages during extraction?
-
-## 💭 Think About These Scenarios
-- [ ] What if a manifest file is corrupted or has unexpected format?
-- [ ] How would this code behave with very large codebases (1000+ dependencies)?
-- [ ] What happens if the scan is interrupted or cancelled via context?
-- [ ] Should there be logging or progress reporting for long scans?
-
-## 🎯 Next Steps to Explore
-1. **Research Question**: Look up Go best practices for factory patterns - is `NewExtractor` following them?
-2. **Deep Dive**: Investigate the OSV-Scalibr documentation - what other configuration options might be useful?
-3. **Design Pattern**: Consider the Single Responsibility Principle - is this struct doing too much?
-4. **Error Handling**: Research Go error wrapping patterns - could `fmt.Errorf` be improved?
-
-## 🤔 Questions for Self-Reflection
-- How would you explain what this code does to someone unfamiliar with package management?
-- If you had to add support for a new package manager, how much code would you need to change?
-- What's the most fragile part of this implementation, and why?
-- How confident would you feel deploying this code to production?

guard/guard.go

@@ -73,6 +73,12 @@ func (g *packageManagerGuard) Run(ctx context.Context, args []string, parsedComm
 	log.Debugf("Running package manager guard with args: %v", args)
 
 	if !parsedCommand.HasInstallTarget() {
+		// Check if this is a manifest-based installation
+		if parsedCommand.ShouldExtractFromManifest() {
+			log.Debugf("Detected manifest-based installation, extracting packages from manifest files")
+			return g.handleManifestInstallation(ctx, parsedCommand)
+		}
+		
 		log.Debugf("No install target found, continuing execution")
 		return g.continueExecution(ctx, parsedCommand)
 	}
@@ -268,3 +274,77 @@ func (g *packageManagerGuard) clearStatus() {
 
 	g.interaction.ClearStatus()
 }
+
+func (g *packageManagerGuard) handleManifestInstallation(ctx context.Context, parsedCommand *packagemanager.ParsedCommand) error {
+	g.setStatus("Extracting packages from manifest files")
+	
+	// Create extractor with appropriate ecosystem
+	var ecosystem packagev1.Ecosystem
+	switch g.packageManager.Name() {
+	case "pip":
+		ecosystem = packagev1.Ecosystem_ECOSYSTEM_PYPI
+	case "npm", "pnpm":
+		ecosystem = packagev1.Ecosystem_ECOSYSTEM_NPM
+	default:
+		return fmt.Errorf("unsupported package manager for manifest extraction: %s", g.packageManager.Name())
+	}
+	
+	extractorConfig := packagemanager.NewDefaultExtractorConfig()
+	extractorConfig.ExtractorEcosystem = ecosystem
+	
+	switch ecosystem {
+	case packagev1.Ecosystem_ECOSYSTEM_PYPI:
+		extractorConfig.ExtractorsName = packagemanager.PyPiExtractors
+	case packagev1.Ecosystem_ECOSYSTEM_NPM:
+		extractorConfig.ExtractorsName = packagemanager.NpmExtractors
+	}
+	
+	extractor := packagemanager.NewExtractor(*extractorConfig)
+	
+	packages, err := extractor.ExtractManifestFiles()
+	if err != nil {
+		return fmt.Errorf("failed to extract packages from manifest files: %w", err)
+	}
+	
+	if len(packages) == 0 {
+		log.Debugf("No packages found in manifest files, continuing execution")
+		return g.continueExecution(ctx, parsedCommand)
+	}
+	
+	log.Debugf("Extracted %d packages from manifest files", len(packages))
+	
+	// Analyze the extracted packages
+	g.setStatus(fmt.Sprintf("Analyzing %d packages from manifest files", len(packages)))
+	
+	analysisResults, err := g.concurrentAnalyzePackages(ctx, packages)
+	if err != nil {
+		return fmt.Errorf("failed to analyze packages: %w", err)
+	}
+	
+	confirmableMalwarePackages := []*analyzer.PackageVersionAnalysisResult{}
+	for _, result := range analysisResults {
+		if result.Action == analyzer.ActionBlock {
+			return g.blockInstallation(result)
+		}
+		
+		if result.Action == analyzer.ActionConfirm {
+			confirmableMalwarePackages = append(confirmableMalwarePackages, result)
+		}
+	}
+	
+	if len(confirmableMalwarePackages) > 0 {
+		confirmed, err := g.getConfirmationOnMalware(ctx, confirmableMalwarePackages)
+		if err != nil {
+			return fmt.Errorf("failed to get confirmation on malware: %w", err)
+		}
+		
+		if !confirmed {
+			return g.blockInstallation(confirmableMalwarePackages...)
+		}
+	}
+	
+	log.Debugf("No malicious packages found in manifest files, continuing execution")
+	
+	g.clearStatus()
+	return g.continueExecution(ctx, parsedCommand)
+}

packagemanager/npm.go

@@ -51,7 +51,7 @@ func (npm *npmPackageManager) ParseCommand(args []string) (*ParsedCommand, error
 	command := Command{Exe: npm.Config.CommandName, Args: args}
 
 	// No command specified
-	if len(args) < 2 {
+	if len(args) < 1 {
 		return &ParsedCommand{
 			Command: command,
 		}, nil
@@ -59,8 +59,12 @@ func (npm *npmPackageManager) ParseCommand(args []string) (*ParsedCommand, error
 
 	// Extract packages from args
 	var packages []string
+	var isManifestInstall bool
+	var foundInstallCmd bool
+	
 	for idx, arg := range args {
 		if slices.Contains(npm.Config.InstallCommands, arg) {
+			foundInstallCmd = true
 			// All subsequent args are packages except for flags
 			for i := idx + 1; i < len(args); i++ {
 				if strings.HasPrefix(args[i], "-") {
@@ -74,8 +78,14 @@ func (npm *npmPackageManager) ParseCommand(args []string) (*ParsedCommand, error
 		}
 	}
 
-	// No packages found
-	if len(packages) == 0 {
+	// If install command was found but no explicit packages,
+	// this is a manifest-based installation (install from package.json)
+	if foundInstallCmd && len(packages) == 0 {
+		isManifestInstall = true
+	}
+
+	// No packages found and not a manifest install
+	if len(packages) == 0 && !isManifestInstall {
 		return &ParsedCommand{
 			Command: command,
 		}, nil
@@ -105,9 +115,17 @@ func (npm *npmPackageManager) ParseCommand(args []string) (*ParsedCommand, error
 		})
 	}
 
+	var manifestFiles []string
+	if isManifestInstall {
+		// npm/pnpm installs from package.json by default
+		manifestFiles = append(manifestFiles, "package.json")
+	}
+
 	return &ParsedCommand{
-		Command:        command,
-		InstallTargets: installTargets,
+		Command:           command,
+		InstallTargets:    installTargets,
+		IsManifestInstall: isManifestInstall,
+		ManifestFiles:     manifestFiles,
 	}, nil
 }
 

packagemanager/npm_test.go

@@ -121,3 +121,90 @@ func TestNpmParseCommand(t *testing.T) {
 		})
 	}
 }
+
+func TestNpmParseCommand_ManifestInstallation(t *testing.T) {
+	pm, err := NewNpmPackageManager(DefaultNpmPackageManagerConfig())
+	assert.NoError(t, err)
+
+	cases := []struct {
+		name              string
+		args              []string
+		expectedManifest  bool
+		expectedFiles     []string
+		expectedTargets   int
+	}{
+		{
+			name:              "npm install without args (bare install)",
+			args:              []string{"install"},
+			expectedManifest:  true,
+			expectedFiles:     []string{"package.json"},
+			expectedTargets:   0,
+		},
+		{
+			name:              "npm i without args (short form)",
+			args:              []string{"i"},
+			expectedManifest:  true,
+			expectedFiles:     []string{"package.json"},
+			expectedTargets:   0,
+		},
+		{
+			name:              "npm install with explicit package",
+			args:              []string{"install", "react"},
+			expectedManifest:  false,
+			expectedFiles:     nil,
+			expectedTargets:   1,
+		},
+		{
+			name:              "npm install with multiple packages",
+			args:              []string{"install", "react", "vue"},
+			expectedManifest:  false,
+			expectedFiles:     nil,
+			expectedTargets:   2,
+		},
+		{
+			name:              "npm install with flags but no packages",
+			args:              []string{"install", "--save-dev"},
+			expectedManifest:  true,
+			expectedFiles:     []string{"package.json"},
+			expectedTargets:   0,
+		},
+		{
+			name:              "npm install with mixed args",
+			args:              []string{"install", "react", "--save"},
+			expectedManifest:  false,
+			expectedFiles:     nil,
+			expectedTargets:   1,
+		},
+		{
+			name:              "non-install command",
+			args:              []string{"run", "build"},
+			expectedManifest:  false,
+			expectedFiles:     nil,
+			expectedTargets:   0,
+		},
+		{
+			name:              "pnpm install without args",
+			args:              []string{"install"},
+			expectedManifest:  true,
+			expectedFiles:     []string{"package.json"},
+			expectedTargets:   0,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			parsed, err := pm.ParseCommand(tc.args)
+			assert.NoError(t, err)
+			
+			assert.Equal(t, tc.expectedManifest, parsed.IsManifestInstall, "IsManifestInstall mismatch")
+			assert.Equal(t, tc.expectedFiles, parsed.ManifestFiles, "ManifestFiles mismatch")
+			assert.Equal(t, tc.expectedTargets, len(parsed.InstallTargets), "InstallTargets count mismatch")
+			
+			// Test helper methods
+			assert.Equal(t, tc.expectedManifest, parsed.HasManifestInstall(), "HasManifestInstall mismatch")
+			
+			expectedShouldExtract := tc.expectedManifest && tc.expectedTargets == 0
+			assert.Equal(t, expectedShouldExtract, parsed.ShouldExtractFromManifest(), "ShouldExtractFromManifest mismatch")
+		})
+	}
+}

packagemanager/packagemanager.go

@@ -30,12 +30,28 @@ type ParsedCommand struct {
 
 	// Parsed install target if this is an install command
 	InstallTargets []*PackageInstallTarget
+
+	// IsManifestInstall indicates if this is a manifest-based installation
+	// (e.g., npm install, pip install -r requirements.txt)
+	IsManifestInstall bool
+
+	// ManifestFiles contains the list of manifest files to install from
+	// (e.g., ["requirements.txt"] for pip install -r requirements.txt)
+	ManifestFiles []string
 }
 
 func (pc *ParsedCommand) HasInstallTarget() bool {
 	return len(pc.InstallTargets) > 0
 }
 
+func (pc *ParsedCommand) HasManifestInstall() bool {
+	return pc.IsManifestInstall
+}
+
+func (pc *ParsedCommand) ShouldExtractFromManifest() bool {
+	return pc.IsManifestInstall && !pc.HasInstallTarget()
+}
+
 // PackageManager is the contract for implementing a package manager
 type PackageManager interface {
 	// Name of the package manager implementation

packagemanager/pypi.go

@@ -43,24 +43,70 @@ func (pip *pipPackageManager) ParseCommand(args []string) (*ParsedCommand, error
 	}
 	command := Command{Exe: pip.Config.CommandName, Args: args}
 
-	if len(args) < 2 {
+	if len(args) < 1 {
 		return &ParsedCommand{
 			Command: command,
 		}, nil
 	}
 
 	var packages []string
+	var manifestFiles []string
+	var isManifestInstall bool
+	var foundInstallCmd bool
+
 	for idx, arg := range args {
 		if slices.Contains(pip.Config.InstallCommands, arg) {
+			foundInstallCmd = true
+			// Check for manifest-based installation flags
 			for i := idx + 1; i < len(args); i++ {
-				if strings.HasPrefix(args[i], "-") {
+				currentArg := args[i]
+				
+				// Handle -r/--requirement flags
+				if currentArg == "-r" || currentArg == "--requirement" {
+					isManifestInstall = true
+					if i+1 < len(args) {
+						manifestFiles = append(manifestFiles, args[i+1])
+						i++ // skip the filename
+					}
 					continue
 				}
-				packages = append(packages, args[i])
+				
+				// Handle combined -r flag (e.g., -rrequirements.txt)
+				if strings.HasPrefix(currentArg, "-r") && len(currentArg) > 2 {
+					isManifestInstall = true
+					manifestFiles = append(manifestFiles, currentArg[2:])
+					continue
+				}
+				
+				// Handle other flags that indicate manifest installation
+				if currentArg == "-e" || currentArg == "--editable" ||
+				   currentArg == "-c" || currentArg == "--constraint" {
+					if i+1 < len(args) {
+						i++ // skip the next argument
+					}
+					continue
+				}
+				
+				// If it's a flag, skip it
+				if strings.HasPrefix(currentArg, "-") {
+					continue
+				}
+				
+				// Otherwise, it's a package name
+				packages = append(packages, currentArg)
 			}
 			break
 		}
 	}
+
+	// If install command was found but no explicit packages and no manifest flags,
+	// check if it's a bare "pip install" (which should look for default manifest files)
+	if foundInstallCmd && len(packages) == 0 && len(manifestFiles) == 0 {
+		isManifestInstall = true
+		// pip install without args typically looks for requirements.txt
+		manifestFiles = append(manifestFiles, "requirements.txt")
+	}
+
 	var installTargets []*PackageInstallTarget
 
 	for _, pkg := range packages {
@@ -96,8 +142,10 @@ func (pip *pipPackageManager) ParseCommand(args []string) (*ParsedCommand, error
 	}
 
 	return &ParsedCommand{
-		Command:        command,
-		InstallTargets: installTargets,
+		Command:           command,
+		InstallTargets:    installTargets,
+		IsManifestInstall: isManifestInstall,
+		ManifestFiles:     manifestFiles,
 	}, nil
 }
 

packagemanager/pypi_test.go

@@ -111,6 +111,93 @@ func TestPipParsePackageInfo(t *testing.T) {
 	}
 }
 
+func TestPipParseCommand_ManifestInstallation(t *testing.T) {
+	pm, err := NewPipPackageManager(DefaultPipPackageManagerConfig())
+	assert.NoError(t, err)
+
+	cases := []struct {
+		name              string
+		args              []string
+		expectedManifest  bool
+		expectedFiles     []string
+		expectedTargets   int
+	}{
+		{
+			name:              "pip install with -r flag",
+			args:              []string{"install", "-r", "requirements.txt"},
+			expectedManifest:  true,
+			expectedFiles:     []string{"requirements.txt"},
+			expectedTargets:   0,
+		},
+		{
+			name:              "pip install with --requirement flag",
+			args:              []string{"install", "--requirement", "requirements.txt"},
+			expectedManifest:  true,
+			expectedFiles:     []string{"requirements.txt"},
+			expectedTargets:   0,
+		},
+		{
+			name:              "pip install with combined -r flag",
+			args:              []string{"install", "-rrequirements.txt"},
+			expectedManifest:  true,
+			expectedFiles:     []string{"requirements.txt"},
+			expectedTargets:   0,
+		},
+		{
+			name:              "pip install without args (bare install)",
+			args:              []string{"install"},
+			expectedManifest:  true,
+			expectedFiles:     []string{"requirements.txt"},
+			expectedTargets:   0,
+		},
+		{
+			name:              "pip install with explicit package",
+			args:              []string{"install", "django"},
+			expectedManifest:  false,
+			expectedFiles:     nil,
+			expectedTargets:   1,
+		},
+		{
+			name:              "pip install with mixed args",
+			args:              []string{"install", "django", "-r", "requirements.txt"},
+			expectedManifest:  true,
+			expectedFiles:     []string{"requirements.txt"},
+			expectedTargets:   1,
+		},
+		{
+			name:              "pip install with multiple -r flags",
+			args:              []string{"install", "-r", "requirements.txt", "-r", "dev-requirements.txt"},
+			expectedManifest:  true,
+			expectedFiles:     []string{"requirements.txt", "dev-requirements.txt"},
+			expectedTargets:   0,
+		},
+		{
+			name:              "non-install command",
+			args:              []string{"list"},
+			expectedManifest:  false,
+			expectedFiles:     nil,
+			expectedTargets:   0,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			parsed, err := pm.ParseCommand(tc.args)
+			assert.NoError(t, err)
+			
+			assert.Equal(t, tc.expectedManifest, parsed.IsManifestInstall, "IsManifestInstall mismatch")
+			assert.Equal(t, tc.expectedFiles, parsed.ManifestFiles, "ManifestFiles mismatch")
+			assert.Equal(t, tc.expectedTargets, len(parsed.InstallTargets), "InstallTargets count mismatch")
+			
+			// Test helper methods
+			assert.Equal(t, tc.expectedManifest, parsed.HasManifestInstall(), "HasManifestInstall mismatch")
+			
+			expectedShouldExtract := tc.expectedManifest && tc.expectedTargets == 0
+			assert.Equal(t, expectedShouldExtract, parsed.ShouldExtractFromManifest(), "ShouldExtractFromManifest mismatch")
+		})
+	}
+}
+
 func TestPipConvertCompatibleRelease(t *testing.T) {
 	cases := []struct {
 		name     string