Files
pmg/guard/guard_test.go
T
365deb1897 feat: Add proxy_install_only config to restrict proxy to download commands (#222)
* feat: Add proxy_install_only config to restrict proxy to download commands

Introduces proxy_install_only (default: false) which, when enabled,
skips the proxy for package manager commands that do not download
packages (e.g. npm ls, pip list), avoiding unnecessary MITM overhead.

- Add ProxyInstallOnly to Config and config template
- Add IsKnownDownloadCommand / MayDownloadPackages to ParsedCommand
- Add DownloadCommands to npm and pypi PM configs covering update,
  ci, audit, dlx, exec, x, download, run and equivalents per PM
- Extract shared runner.Execute used by both proxy flow and guard
- Proxy flow short-circuits to runner.Execute for non-download commands
  when proxy_install_only=true

* refactor: Inject CommandExecutor into guard to fix dependency direction

guard depended on internal/runner, which inverted the intended layer
hierarchy. Now guard defines a CommandExecutor function type and accepts
it as a constructor argument. internal/flows (the composition root)
creates the executor closure wrapping runner.Execute and injects it,
keeping guard free of internal/ dependencies.

* refactor: Invert proxy_install_only logic to use known non-download commands

Replace the DownloadCommands allowlist (opt-in, fail-open) with a
NonDownloadCommands denylist (opt-out, fail-safe). The proxy now runs
for all commands except those explicitly known to not download packages.
Unknown or future package manager subcommands default to running with
the proxy.

Includes script runners (run, start, test, stop, restart) that can spin
up local servers — setting proxy env vars on these breaks them without
providing any security benefit. Also covers removal commands and local
operations that never contact the registry.

* fix: Support PMG_* env vars regardless of config file state

AutomaticEnv only resolves env vars for keys Viper already knows about
via AllKeys(). When a key is absent from the config file (commented out,
new key added after last setup, or no config file at all), Viper had no
knowledge of it and silently skipped the env var.

Fix by registering all Config struct fields as Viper defaults via
reflection (using mapstructure tags) before reading the config file.
This ensures PMG_* env vars work in all cases.

Precedence: cobra flags > env vars > config file > defaults.
SetDefault is used (not Set) so env vars and config file can still
override the Go defaults freely.

Tests added covering all precedence levels including the key-absent-
from-config-file case that was the original bug report.

* fix: Only check first non-flag arg against NonDownloadCommands

Scanning all args caused false proxy bypasses when package names or
script arguments matched a NonDownloadCommands entry. For example:
- npm exec test → "test" matched, proxy incorrectly skipped
- npm update config → "config" matched, proxy skipped
- npm publish --tag version → "version" matched, proxy skipped

Fix by checking only the first non-flag argument (the subcommand).
If it is not in NonDownloadCommands we break immediately, so trailing
args never influence the classification. Applied to all four parsers:
npm, pip/pip3, uv, and poetry.

Regression tests added for the false positive cases.

* refactor: Replace reflection-based Viper defaults with embedded template

Load the embedded config template as the Viper base so all keys are
registered upfront, enabling PMG_* env vars to work regardless of
whether a key exists in the user's config file.

* fix: Restore trusted_packages template entry and revert DefaultConfig change

* docs: Document environment variable overrides for config keys

* update npm test cmd

* refactor: extract shared non-download command detection helper

Replaces duplicated first-non-flag-arg detection loops in npm.go and
pypi.go (pip + poetry parsers) with a shared isFirstNonFlagArgInList
helper in packagemanager.go.

https://claude.ai/code/session_01AHaKF3vc2Haj9tK3jgUBAs

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-04-17 01:13:30 +05:30

266 lines
9.1 KiB
Go

package guard
import (
"context"
"testing"
packagev1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/package/v1"
"github.com/safedep/pmg/analyzer"
"github.com/safedep/pmg/internal/ui"
"github.com/safedep/pmg/packagemanager"
"github.com/stretchr/testify/assert"
)
// noopExecutor is a no-op executor for use in tests that set DryRun=true
// or otherwise don't reach actual command execution.
var noopExecutor CommandExecutor = func(_ context.Context, _ *packagemanager.ParsedCommand) error {
return nil
}
func TestGuardConcurrentlyAnalyzePackagesMalwareQueryService(t *testing.T) {
mq, err := analyzer.NewMalysisQueryAnalyzer(analyzer.MalysisQueryAnalyzerConfig{})
if err != nil {
t.Fatalf("failed to create mq: %v", err)
}
pg, err := NewPackageManagerGuard(DefaultPackageManagerGuardConfig(), nil, nil,
[]analyzer.PackageVersionAnalyzer{mq}, PackageManagerGuardInteraction{
ShowWarning: func(message string) {},
}, noopExecutor)
if err != nil {
t.Fatalf("failed to create pg: %v", err)
}
t.Run("should resolve a single known malicious package version", func(t *testing.T) {
r, trustedSkipped, err := pg.concurrentAnalyzePackages(context.Background(), []*packagev1.PackageVersion{
{
Package: &packagev1.Package{
Name: "nyc-config",
Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM,
},
Version: "10.0.0",
},
})
if err != nil {
t.Fatalf("failed to analyze packages: %v", err)
}
assert.Equal(t, 0, trustedSkipped)
assert.Equal(t, 1, len(r))
assert.Equal(t, "nyc-config", r[0].PackageVersion.GetPackage().GetName())
assert.Equal(t, "10.0.0", r[0].PackageVersion.GetVersion())
assert.Equal(t, packagev1.Ecosystem_ECOSYSTEM_NPM, r[0].PackageVersion.GetPackage().GetEcosystem())
assert.NotEmpty(t, r[0].ReferenceURL)
assert.NotEmpty(t, r[0].Summary)
assert.NotNil(t, r[0].Data)
assert.Equal(t, analyzer.ActionBlock, r[0].Action)
})
}
func TestGuardInsecureInstallation(t *testing.T) {
mq, err := analyzer.NewMalysisQueryAnalyzer(analyzer.MalysisQueryAnalyzerConfig{})
if err != nil {
t.Fatalf("failed to create mq: %v", err)
}
t.Run("should bypass malware blocking when InsecureInstallation is enabled", func(t *testing.T) {
// Create guard with InsecureInstallation enabled
config := DefaultPackageManagerGuardConfig()
config.InsecureInstallation = true
config.DryRun = true // Enable dry run to avoid actual command execution
config.ResolveDependencies = false // Disable dependency resolution to avoid nil pointer issues
blockCalled := false
warningCalled := false
var warningMessage string
interaction := PackageManagerGuardInteraction{
ShowWarning: func(message string) {
warningCalled = true
warningMessage = message
},
Block: func(config *ui.BlockConfig) error {
blockCalled = true
return nil
},
}
pg, err := NewPackageManagerGuard(config, nil, nil,
[]analyzer.PackageVersionAnalyzer{mq}, interaction, noopExecutor)
if err != nil {
t.Fatalf("failed to create pg: %v", err)
}
// Create a parsed command with a known malicious package
parsedCommand := &packagemanager.ParsedCommand{
Command: packagemanager.Command{
Exe: "npm",
Args: []string{"install", "nyc-config@10.0.0"},
},
InstallTargets: []*packagemanager.PackageInstallTarget{
{
PackageVersion: &packagev1.PackageVersion{
Package: &packagev1.Package{
Name: "nyc-config",
Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM,
},
Version: "10.0.0",
},
},
},
}
_, err = pg.Run(context.Background(), []string{"npm", "install", "nyc-config@10.0.0"}, parsedCommand)
// With dry run enabled, we expect no error even though we're bypassing execution
assert.NoError(t, err)
// Block should not be called because InsecureInstallation bypasses the analysis
assert.False(t, blockCalled, "Block should not be called when InsecureInstallation is enabled")
// Warning should be called to inform user about insecure installation
assert.True(t, warningCalled, "Warning should be called when InsecureInstallation is enabled")
assert.Contains(t, warningMessage, "INSECURE INSTALLATION MODE", "Warning message should mention insecure installation")
})
t.Run("should block malware when InsecureInstallation is disabled", func(t *testing.T) {
// Create guard with InsecureInstallation disabled (default)
config := DefaultPackageManagerGuardConfig()
config.InsecureInstallation = false
config.DryRun = true
config.ResolveDependencies = false // Disable dependency resolution to avoid nil pointer issues
interaction := PackageManagerGuardInteraction{
ShowWarning: func(message string) {},
}
pg, err := NewPackageManagerGuard(config, nil, nil,
[]analyzer.PackageVersionAnalyzer{mq}, interaction, noopExecutor)
if err != nil {
t.Fatalf("failed to create pg: %v", err)
}
// Create a parsed command with a known malicious package
parsedCommand := &packagemanager.ParsedCommand{
Command: packagemanager.Command{
Exe: "npm",
Args: []string{"install", "nyc-config@10.0.0"},
},
InstallTargets: []*packagemanager.PackageInstallTarget{
{
PackageVersion: &packagev1.PackageVersion{
Package: &packagev1.Package{
Name: "nyc-config",
Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM,
},
Version: "10.0.0",
},
},
},
}
r, err := pg.Run(context.Background(), []string{"npm", "install", "nyc-config@10.0.0"}, parsedCommand)
// We expect no error from the guard itself (blocking is handled via the Block callback)
assert.NoError(t, err)
// Verify that the malicious package was detected and blocked
assert.NotEmpty(t, r.BlockedPackages, "Blocked packages should not be empty")
assert.Greater(t, r.BlockedCount, 0)
assert.Equal(t, "nyc-config", r.BlockedPackages[0].PackageVersion.GetPackage().GetName())
assert.Equal(t, "10.0.0", r.BlockedPackages[0].PackageVersion.GetVersion())
assert.Equal(t, analyzer.ActionBlock, r.BlockedPackages[0].Action)
})
t.Run("should continue execution for commands without install targets when InsecureInstallation is enabled", func(t *testing.T) {
// Create guard with InsecureInstallation enabled
config := DefaultPackageManagerGuardConfig()
config.InsecureInstallation = true
config.DryRun = true
config.ResolveDependencies = false // Disable dependency resolution to avoid nil pointer issues
blockCalled := false
interaction := PackageManagerGuardInteraction{
ShowWarning: func(message string) {},
Block: func(config *ui.BlockConfig) error {
blockCalled = true
return nil
},
}
pg, err := NewPackageManagerGuard(config, nil, nil,
[]analyzer.PackageVersionAnalyzer{mq}, interaction, noopExecutor)
if err != nil {
t.Fatalf("failed to create pg: %v", err)
}
// Create a parsed command without install targets (e.g., npm list)
parsedCommand := &packagemanager.ParsedCommand{
Command: packagemanager.Command{
Exe: "npm",
Args: []string{"list"},
},
InstallTargets: []*packagemanager.PackageInstallTarget{}, // No install targets
}
_, err = pg.Run(context.Background(), []string{"npm", "list"}, parsedCommand)
// Should not error since there are no install targets to analyze
assert.NoError(t, err)
// Block should not be called since there are no packages to analyze
assert.False(t, blockCalled, "Block should not be called when there are no install targets")
})
t.Run("should handle manifest-based installation when InsecureInstallation is enabled", func(t *testing.T) {
// Create guard with InsecureInstallation enabled
config := DefaultPackageManagerGuardConfig()
config.InsecureInstallation = true
config.DryRun = true
config.ResolveDependencies = false // Disable dependency resolution to avoid nil pointer issues
blockCalled := false
interaction := PackageManagerGuardInteraction{
ShowWarning: func(message string) {},
Block: func(config *ui.BlockConfig) error {
blockCalled = true
return nil
},
}
pg, err := NewPackageManagerGuard(config, nil, nil,
[]analyzer.PackageVersionAnalyzer{mq}, interaction, noopExecutor)
if err != nil {
t.Fatalf("failed to create pg: %v", err)
}
// Create a parsed command for manifest-based installation
parsedCommand := &packagemanager.ParsedCommand{
Command: packagemanager.Command{
Exe: "npm",
Args: []string{"install"},
},
InstallTargets: []*packagemanager.PackageInstallTarget{}, // No direct install targets
IsManifestInstall: true,
ManifestFiles: []string{"package.json"},
}
_, err = pg.Run(context.Background(), []string{"npm", "install"}, parsedCommand)
// Should not error and should bypass malware checking
assert.NoError(t, err)
// Block should not be called because InsecureInstallation bypasses analysis
assert.False(t, blockCalled, "Block should not be called when InsecureInstallation is enabled for manifest installation")
})
t.Run("should verify InsecureInstallation defaults to false", func(t *testing.T) {
config := DefaultPackageManagerGuardConfig()
// Verify that InsecureInstallation defaults to false
assert.False(t, config.InsecureInstallation, "InsecureInstallation should default to false")
})
}