fix: pip args & manifest files handling (#41)

* fix: pip args & manifest files handling

* chore: add pflag dependency

* chore: word fixes

* fix: multiple refs for suspicious packages

* refactor: introduce block config

* fix: makefile for windows build

* test: add test cases for manifest based installation

* refactor: Makefile

* refactor: remove .exe for windows build
This commit is contained in:
Sahil Bansal
2025-06-24 12:44:12 +05:30
committed by GitHub
parent b64f158654
commit 3f85282935
8 changed files with 179 additions and 221 deletions
+8 -2
View File
@@ -12,6 +12,7 @@ import (
type ExtractorConfig struct {
ExtractorPackageManager PackageManagerName
ScanDir string
ManifestFiles []string
}
type extractor struct {
@@ -21,7 +22,8 @@ type extractor struct {
func NewDefaultExtractorConfig() *ExtractorConfig {
return &ExtractorConfig{
ScanDir: ".",
ScanDir: ".",
ManifestFiles: []string{},
}
}
@@ -36,7 +38,11 @@ func (e *extractor) ExtractManifest() ([]*packagev1.PackageVersion, error) {
packagesToAnalyze := []*packagev1.PackageVersion{}
// Get the list of lockfiles to check based on ecosystem
filesToCheck := e.getFilesToCheck()
filesToCheck := e.Config.ManifestFiles
if len(filesToCheck) == 0 {
filesToCheck = e.getFilesToCheck()
}
for _, filename := range filesToCheck {
filePath := filepath.Join(e.Config.ScanDir, filename)
+1 -1
View File
@@ -13,6 +13,7 @@ require (
github.com/jedib0t/go-pretty/v6 v6.6.7
github.com/safedep/dry v0.0.0-20250514080944-bb77f30c7175
github.com/spf13/cobra v1.9.1
github.com/spf13/pflag v1.0.6
github.com/stretchr/testify v1.10.0
google.golang.org/grpc v1.72.0
)
@@ -170,7 +171,6 @@ require (
github.com/sourcegraph/go-diff v0.7.0 // indirect
github.com/spf13/afero v1.14.0 // indirect
github.com/spf13/cast v1.7.1 // indirect
github.com/spf13/pflag v1.0.6 // indirect
github.com/spf13/viper v1.20.1 // indirect
github.com/ssgreg/nlreturn/v2 v2.2.1 // indirect
github.com/stbenjam/no-sprintf-host-port v0.2.0 // indirect
+23 -11
View File
@@ -13,6 +13,7 @@ import (
"github.com/safedep/dry/log"
"github.com/safedep/pmg/analyzer"
"github.com/safedep/pmg/extractor"
"github.com/safedep/pmg/internal/ui"
"github.com/safedep/pmg/packagemanager"
)
@@ -29,7 +30,7 @@ type PackageManagerGuardInteraction struct {
// Block is called to block the installation of the malware packages. One or more malicious
// packages are passed as arguments. These are the packages that were detected as malicious.
// Client code must perform the necessary error handling and termination of the process.
Block func(...*analyzer.PackageVersionAnalysisResult) error
Block func(config *ui.BlockConfig) error
}
type PackageManagerGuardConfig struct {
@@ -85,6 +86,8 @@ func (g *packageManagerGuard) Run(ctx context.Context, args []string, parsedComm
return g.continueExecution(ctx, parsedCommand)
}
blockConfig := ui.NewDefaultBlockConfig()
// TODO: We should track the dependency tree here so that we can trace a
// dependency to one of the parent packages from install targets
@@ -95,7 +98,7 @@ func (g *packageManagerGuard) Run(ctx context.Context, args []string, parsedComm
log.Debugf("Found %d install targets", len(parsedCommand.InstallTargets))
g.setStatus(fmt.Sprintf("Resolving dependencies for %d packages", len(parsedCommand.InstallTargets)))
g.setStatus(fmt.Sprintf("Resolving dependencies for %d package(s)", len(parsedCommand.InstallTargets)))
if g.config.ResolveDependencies {
for _, pkg := range parsedCommand.InstallTargets {
@@ -125,7 +128,7 @@ func (g *packageManagerGuard) Run(ctx context.Context, args []string, parsedComm
log.Debugf("Checking %d packages for malware", len(packagesToAnalyze))
g.setStatus(fmt.Sprintf("Analyzing %d packages for malware", len(packagesToAnalyze)))
g.setStatus(fmt.Sprintf("Analyzing %d dependencies for malware", len(packagesToAnalyze)))
analysisResults, err := g.concurrentAnalyzePackages(ctx, packagesToAnalyze)
if err != nil {
@@ -135,7 +138,8 @@ func (g *packageManagerGuard) Run(ctx context.Context, args []string, parsedComm
confirmableMalwarePackages := []*analyzer.PackageVersionAnalysisResult{}
for _, result := range analysisResults {
if result.Action == analyzer.ActionBlock {
return g.blockInstallation(result)
blockConfig.MalwarePackages = append(blockConfig.MalwarePackages, result)
return g.blockInstallation(blockConfig)
}
if result.Action == analyzer.ActionConfirm {
@@ -150,7 +154,9 @@ func (g *packageManagerGuard) Run(ctx context.Context, args []string, parsedComm
}
if !confirmed {
return g.blockInstallation(confirmableMalwarePackages...)
blockConfig.ShowReference = false
blockConfig.MalwarePackages = confirmableMalwarePackages
return g.blockInstallation(blockConfig)
}
}
@@ -261,12 +267,12 @@ func (g *packageManagerGuard) setStatus(status string) {
g.interaction.SetStatus(status)
}
func (g *packageManagerGuard) blockInstallation(malwarePackages ...*analyzer.PackageVersionAnalysisResult) error {
func (g *packageManagerGuard) blockInstallation(config *ui.BlockConfig) error {
if g.interaction.Block == nil {
return nil
}
return g.interaction.Block(malwarePackages...)
return g.interaction.Block(config)
}
func (g *packageManagerGuard) clearStatus() {
@@ -280,6 +286,7 @@ func (g *packageManagerGuard) clearStatus() {
func (g *packageManagerGuard) handleManifestInstallation(ctx context.Context, parsedCommand *packagemanager.ParsedCommand) error {
extractorConfig := extractor.NewDefaultExtractorConfig()
extractorConfig.ExtractorPackageManager = extractor.PackageManagerName(g.packageManager.Name())
extractorConfig.ManifestFiles = parsedCommand.ManifestFiles
packageExtractor := extractor.New(*extractorConfig)
@@ -288,6 +295,8 @@ func (g *packageManagerGuard) handleManifestInstallation(ctx context.Context, pa
return fmt.Errorf("failed to extract packages from manifest files: %w", err)
}
blockConfig := ui.NewDefaultBlockConfig()
if len(packages) == 0 {
log.Debugf("No packages found in manifest files, continuing execution")
return g.continueExecution(ctx, parsedCommand)
@@ -303,7 +312,7 @@ func (g *packageManagerGuard) handleManifestInstallation(ctx context.Context, pa
// Only resolve dependencies for requirements.txt because other lockfiles dependencies are already resolved
if g.config.ResolveDependencies && slices.Contains(parsedCommand.ManifestFiles, "requirements.txt") {
g.setStatus(fmt.Sprintf("Resolving dependencies for %d packages", len(packages)))
g.setStatus(fmt.Sprintf("Resolving dependencies for %d package(s)", len(packages)))
for _, pkg := range packages {
if pkg.GetVersion() == "" {
@@ -332,7 +341,7 @@ func (g *packageManagerGuard) handleManifestInstallation(ctx context.Context, pa
log.Debugf("Checking %d packages for malware", len(packagesToAnalyze))
g.setStatus(fmt.Sprintf("Analyzing %d packages from manifest files", len(packagesToAnalyze)))
g.setStatus(fmt.Sprintf("Analyzing %d dependencies from manifest files", len(packagesToAnalyze)))
analysisResults, err := g.concurrentAnalyzePackages(ctx, packagesToAnalyze)
if err != nil {
@@ -342,7 +351,8 @@ func (g *packageManagerGuard) handleManifestInstallation(ctx context.Context, pa
confirmableMalwarePackages := []*analyzer.PackageVersionAnalysisResult{}
for _, result := range analysisResults {
if result.Action == analyzer.ActionBlock {
return g.blockInstallation(result)
blockConfig.MalwarePackages = append(blockConfig.MalwarePackages, result)
return g.blockInstallation(blockConfig)
}
if result.Action == analyzer.ActionConfirm {
@@ -357,7 +367,9 @@ func (g *packageManagerGuard) handleManifestInstallation(ctx context.Context, pa
}
if !confirmed {
return g.blockInstallation(confirmableMalwarePackages...)
blockConfig.ShowReference = false
blockConfig.MalwarePackages = confirmableMalwarePackages
return g.blockInstallation(blockConfig)
}
}
+18 -2
View File
@@ -26,6 +26,20 @@ const (
VerbosityLevelVerbose
)
type BlockConfig struct {
// ShowReference determines whether to show detailed information for suspicious packages.
// If false, the details are omitted to avoid repeating information already shown to the user.
ShowReference bool
MalwarePackages []*analyzer.PackageVersionAnalysisResult
}
func NewDefaultBlockConfig() *BlockConfig {
return &BlockConfig{
ShowReference: true,
}
}
var verbosityLevel VerbosityLevel = VerbosityLevelNormal
func SetVerbosityLevel(level VerbosityLevel) {
@@ -37,13 +51,15 @@ func ClearStatus() {
fmt.Print("\r")
}
func Block(malwarePackages ...*analyzer.PackageVersionAnalysisResult) error {
func Block(config *BlockConfig) error {
StopSpinner()
fmt.Println()
fmt.Println(Colors.Red("❌ Malicious package blocked!"))
printMaliciousPackagesList(malwarePackages)
if config.ShowReference {
printMaliciousPackagesList(config.MalwarePackages)
}
fmt.Println()
os.Exit(1)
+2 -8
View File
@@ -83,7 +83,7 @@ func (npm *npmPackageManager) ParseCommand(args []string) (*ParsedCommand, error
}
// If install command was found but no explicit packages,
// this is a manifest-based installation (install from package.json)
// this is a manifest-based installation
if foundInstallCmd && len(packages) == 0 {
isManifestInstall = true
}
@@ -119,17 +119,11 @@ 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,
IsManifestInstall: isManifestInstall,
ManifestFiles: manifestFiles,
ManifestFiles: []string{},
}, nil
}
+27 -87
View File
@@ -109,6 +109,33 @@ func TestNpmParseCommand(t *testing.T) {
assert.Equal(t, "1.2.3", parsedCommand.InstallTargets[1].PackageVersion.Version)
},
},
{
name: "manifest installation (bare install)",
command: "npm install",
assert: func(t *testing.T, parsedCommand *ParsedCommand, err error) {
assert.NoError(t, err)
assert.Equal(t, 0, len(parsedCommand.InstallTargets))
assert.Equal(t, true, parsedCommand.IsManifestInstall)
},
},
{
name: "manifest installation (short form)",
command: "npm i",
assert: func(t *testing.T, parsedCommand *ParsedCommand, err error) {
assert.NoError(t, err)
assert.Equal(t, 0, len(parsedCommand.InstallTargets))
assert.Equal(t, true, parsedCommand.IsManifestInstall)
},
},
{
name: "npm install with flags but no packages",
command: "npm install --save-dev",
assert: func(t *testing.T, parsedCommand *ParsedCommand, err error) {
assert.NoError(t, err)
assert.Equal(t, 0, len(parsedCommand.InstallTargets))
assert.Equal(t, true, parsedCommand.IsManifestInstall)
},
},
}
for _, tc := range cases {
@@ -121,90 +148,3 @@ 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")
})
}
}
+44 -61
View File
@@ -2,10 +2,13 @@ package packagemanager
import (
"fmt"
"io"
"slices"
"strconv"
"strings"
"github.com/spf13/pflag"
packagev1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/package/v1"
)
@@ -42,78 +45,61 @@ func (pip *pipPackageManager) Ecosystem() packagev1.Ecosystem {
}
func (pip *pipPackageManager) ParseCommand(args []string) (*ParsedCommand, error) {
// Remove 'pip' if it's the first argument
if len(args) > 0 && args[0] == "pip" {
args = args[1:]
}
command := Command{Exe: pip.Config.CommandName, Args: args}
// Since manifest-based installs like 'npm i' are now valid commands
if len(args) < 1 {
return &ParsedCommand{Command: command}, nil
}
// Find the install command
var installCmdIndex = -1
for idx, arg := range args {
if slices.Contains(pip.Config.InstallCommands, arg) {
installCmdIndex = idx
break
}
}
if installCmdIndex == -1 {
// No install command found, return as-is
return &ParsedCommand{Command: command}, nil
}
// Extract arguments after the install command
installArgs := args[installCmdIndex+1:]
fs := pflag.NewFlagSet("pip", pflag.ContinueOnError)
fs.SetOutput(io.Discard)
// Define flags
var requirementFiles []string
fs.StringArrayVarP(&requirementFiles, "requirement", "r", nil, "Install from requirement file")
// Parse arguments (supports interleaved flags + positional args)
err := fs.Parse(installArgs)
if err != nil {
return &ParsedCommand{
Command: command,
}, nil
}
var packages []string
var manifestFiles []string
var isManifestInstall bool
var foundInstallCmd bool
// Get remaining arguments (package names)
packages := fs.Args()
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++ {
currentArg := args[i]
// Determine if this is a manifest install
isManifestInstall := len(requirementFiles) > 0
// 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
}
// 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")
}
// Combine all manifest files
var allManifestFiles []string
allManifestFiles = append(allManifestFiles, requirementFiles...)
// Process packages
var installTargets []*PackageInstallTarget
for _, pkg := range packages {
packageName, version, extras, err := pipParsePackageInfo(pkg)
if err != nil {
@@ -121,12 +107,9 @@ func (pip *pipPackageManager) ParseCommand(args []string) (*ParsedCommand, error
}
if version != "" {
// If exact version provided just trim it. If not get a version that satisfies a given version specifier
if strings.HasPrefix(version, "==") {
// Exact version, just trim
version = strings.TrimPrefix(version, "==")
} else {
// Version range, resolve from PyPI
version, err = pipGetMatchingVersion(packageName, version)
if err != nil {
return nil, fmt.Errorf("error resolving version for %s: %s", packageName, err.Error())
@@ -150,7 +133,7 @@ func (pip *pipPackageManager) ParseCommand(args []string) (*ParsedCommand, error
Command: command,
InstallTargets: installTargets,
IsManifestInstall: isManifestInstall,
ManifestFiles: manifestFiles,
ManifestFiles: allManifestFiles,
}, nil
}
+56 -49
View File
@@ -111,72 +111,79 @@ func TestPipParsePackageInfo(t *testing.T) {
}
}
func TestPipParseCommand_ManifestInstallation(t *testing.T) {
func TestPipParseCommand(t *testing.T) {
pm, err := NewPipPackageManager(DefaultPipPackageManagerConfig())
assert.NoError(t, err)
cases := []struct {
name string
args []string
expectedManifest bool
expectedFiles []string
expectedTargets int
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 -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 -r flag with different filename",
args: []string{"install", "-r", "requirements-dev.txt"},
expectedManifest: true,
expectedFiles: []string{"requirements-dev.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 with --requirement flag",
args: []string{"install", "--requirement", "requirements.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 combined -r flag",
args: []string{"install", "-rrequirements.txt"},
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 without args",
args: []string{"install"},
expectedManifest: false,
expectedFiles: nil,
expectedTargets: 0,
},
{
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 explicit package",
args: []string{"install", "django"},
expectedManifest: false,
expectedFiles: nil,
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: "pip install with mixed args",
args: []string{"install", "django", "-r", "requirements.txt"},
expectedManifest: true,
expectedFiles: []string{"requirements.txt"},
expectedTargets: 1,
},
{
name: "non-install command",
args: []string{"list"},
expectedManifest: false,
expectedFiles: nil,
expectedTargets: 0,
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,
},
}
@@ -184,14 +191,14 @@ func TestPipParseCommand_ManifestInstallation(t *testing.T) {
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")
})