From 1c319eba0e4d668449f963372bb956135896163f Mon Sep 17 00:00:00 2001 From: Abhisek Datta Date: Thu, 8 Jan 2026 00:15:02 +0530 Subject: [PATCH] fix: Proxy flow should respect trusted packages (#96) * fix: Handle trusted packages in proxy flow * perf: Pre-parse trusted PURLs * fix: Code review fixes * fix: Remove unused config --- config/config.go | 14 ++ config/trusted.go | 74 ++++++++ config/trusted_test.go | 267 ++++++++++++++++++++++++++++ guard/guard.go | 40 +---- guard/guard_test.go | 259 --------------------------- internal/flows/common_flow.go | 1 - proxy/interceptors/base_registry.go | 22 ++- 7 files changed, 373 insertions(+), 304 deletions(-) create mode 100644 config/trusted.go create mode 100644 config/trusted_test.go diff --git a/config/config.go b/config/config.go index 7496ec1..00acf5a 100644 --- a/config/config.go +++ b/config/config.go @@ -8,6 +8,9 @@ import ( "strconv" _ "embed" + + packagev1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/package/v1" + "github.com/safedep/dry/log" ) const ( @@ -62,6 +65,13 @@ type Config struct { type TrustedPackage struct { Purl string `mapstructure:"purl"` Reason string `mapstructure:"reason"` + + // Pre-parsed PURL components (not serialized, computed at load time) + // These fields avoid repeated PURL parsing on every IsTrustedPackage() call + parsed bool + ecosystem packagev1.Ecosystem + name string + version string } // RuntimeConfig is the configuration that is used at runtime. It contains static configuration @@ -152,6 +162,10 @@ func initConfig() { globalConfig.eventLogDir = eventLogDir loadConfig() + + if err := preprocessTrustedPackages(&globalConfig.Config); err != nil { + log.Warnf("Failed to preprocess trusted packages: %v", err) + } } // loadConfig loads the configuration from the config file. diff --git a/config/trusted.go b/config/trusted.go new file mode 100644 index 0000000..7b327c9 --- /dev/null +++ b/config/trusted.go @@ -0,0 +1,74 @@ +package config + +import ( + packagev1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/package/v1" + "github.com/safedep/dry/api/pb" + "github.com/safedep/dry/log" +) + +// IsTrustedPackage checks if a package version is trusted based on global configuration. +// This is the primary API that should be used by guard and proxy flows. +// It returns true if the package is in the trusted packages list, false otherwise. +func IsTrustedPackage(pkgVersion *packagev1.PackageVersion) bool { + return isTrustedPackageVersion(Get().Config.TrustedPackages, pkgVersion) +} + +// preprocessTrustedPackages pre-parses all PURL strings in trusted packages. +// This is called once during config load to avoid repeated parsing during +// trusted package checks. Invalid PURLs are logged but not fatal. +func preprocessTrustedPackages(cfg *Config) error { + for i := range cfg.TrustedPackages { + tp := &cfg.TrustedPackages[i] + + parsedPurl, err := pb.NewPurlPackageVersion(tp.Purl) + if err != nil { + log.Warnf("Failed to parse trusted package PURL: %s: %v", tp.Purl, err) + tp.parsed = false + continue + } + + tp.parsed = true + tp.ecosystem = parsedPurl.Ecosystem() + tp.name = parsedPurl.Name() + tp.version = parsedPurl.Version() + } + + return nil +} + +// isTrustedPackageVersion checks if a package version is in the trusted packages list. +// +// It matches based on ecosystem, package name, and optionally version. +// If the trusted package PURL doesn't specify a version, all versions of that package are trusted. +// Returns false if pkgVersion is nil or if trustedPackages is empty. +func isTrustedPackageVersion(trustedPackages []TrustedPackage, pkgVersion *packagev1.PackageVersion) bool { + if pkgVersion == nil { + return false + } + + if len(trustedPackages) == 0 { + return false + } + + for _, v := range trustedPackages { + if !v.parsed { + continue + } + + if v.ecosystem != pkgVersion.GetPackage().GetEcosystem() { + continue + } + + if v.name != pkgVersion.GetPackage().GetName() { + continue + } + + if v.version != "" && v.version != pkgVersion.GetVersion() { + continue + } + + return true + } + + return false +} diff --git a/config/trusted_test.go b/config/trusted_test.go new file mode 100644 index 0000000..ecafbd7 --- /dev/null +++ b/config/trusted_test.go @@ -0,0 +1,267 @@ +package config + +import ( + "testing" + + packagev1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/package/v1" + "github.com/stretchr/testify/assert" +) + +func TestIsTrustedPackageVersion(t *testing.T) { + tests := []struct { + name string + trustedPackages []TrustedPackage + pkgVersion *packagev1.PackageVersion + want bool + }{ + { + name: "nil package version returns false", + trustedPackages: []TrustedPackage{}, + pkgVersion: nil, + want: false, + }, + { + name: "empty trusted packages list returns false", + trustedPackages: []TrustedPackage{}, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "express", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, + }, + Version: "4.18.0", + }, + want: false, + }, + { + name: "exact match with version returns true", + trustedPackages: []TrustedPackage{ + { + Purl: "pkg:npm/express@4.18.0", + Reason: "trusted by team", + }, + }, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "express", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, + }, + Version: "4.18.0", + }, + want: true, + }, + { + name: "match without version in trusted package returns true", + trustedPackages: []TrustedPackage{ + { + Purl: "pkg:npm/express", + Reason: "all versions trusted", + }, + }, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "express", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, + }, + Version: "4.18.0", + }, + want: true, + }, + { + name: "version mismatch returns false", + trustedPackages: []TrustedPackage{ + { + Purl: "pkg:npm/express@4.17.0", + Reason: "old version trusted", + }, + }, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "express", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, + }, + Version: "4.18.0", + }, + want: false, + }, + { + name: "name mismatch returns false", + trustedPackages: []TrustedPackage{ + { + Purl: "pkg:npm/react@18.0.0", + Reason: "trusted package", + }, + }, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "express", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, + }, + Version: "4.18.0", + }, + want: false, + }, + { + name: "ecosystem mismatch returns false", + trustedPackages: []TrustedPackage{ + { + Purl: "pkg:pypi/requests@2.28.0", + Reason: "trusted package", + }, + }, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "requests", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, + }, + Version: "2.28.0", + }, + want: false, + }, + { + name: "pypi package exact match returns true", + trustedPackages: []TrustedPackage{ + { + Purl: "pkg:pypi/requests@2.28.0", + Reason: "trusted http library", + }, + }, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "requests", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_PYPI, + }, + Version: "2.28.0", + }, + want: true, + }, + { + name: "multiple trusted packages finds correct match", + trustedPackages: []TrustedPackage{ + { + Purl: "pkg:npm/lodash@4.17.21", + Reason: "utility library", + }, + { + Purl: "pkg:npm/express@4.18.0", + Reason: "web framework", + }, + { + Purl: "pkg:pypi/requests@2.28.0", + Reason: "http library", + }, + }, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "express", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, + }, + Version: "4.18.0", + }, + want: true, + }, + { + name: "multiple trusted packages no match returns false", + trustedPackages: []TrustedPackage{ + { + Purl: "pkg:npm/lodash@4.17.21", + Reason: "utility library", + }, + { + Purl: "pkg:npm/react@18.0.0", + Reason: "ui library", + }, + }, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "express", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, + }, + Version: "4.18.0", + }, + want: false, + }, + { + name: "invalid purl in trusted packages skips and returns false", + trustedPackages: []TrustedPackage{ + { + Purl: "invalid-purl-format", + Reason: "malformed", + }, + }, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "express", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, + }, + Version: "4.18.0", + }, + want: false, + }, + { + name: "invalid purl skipped but valid match found", + trustedPackages: []TrustedPackage{ + { + Purl: "invalid-purl-format", + Reason: "malformed", + }, + { + Purl: "pkg:npm/express@4.18.0", + Reason: "valid trusted package", + }, + }, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "express", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, + }, + Version: "4.18.0", + }, + want: true, + }, + { + name: "package version without version field matches versionless trusted package", + trustedPackages: []TrustedPackage{ + { + Purl: "pkg:npm/express", + Reason: "all versions trusted", + }, + }, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "express", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, + }, + Version: "", + }, + want: true, + }, + { + name: "package version without version field does not match versioned trusted package", + trustedPackages: []TrustedPackage{ + { + Purl: "pkg:npm/express@4.18.0", + Reason: "specific version trusted", + }, + }, + pkgVersion: &packagev1.PackageVersion{ + Package: &packagev1.Package{ + Name: "express", + Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, + }, + Version: "", + }, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Pre-process trusted packages to populate pre-parsed fields + cfg := &Config{TrustedPackages: tt.trustedPackages} + _ = preprocessTrustedPackages(cfg) + + got := isTrustedPackageVersion(cfg.TrustedPackages, tt.pkgVersion) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/guard/guard.go b/guard/guard.go index 0fda6b4..2d43343 100644 --- a/guard/guard.go +++ b/guard/guard.go @@ -10,7 +10,6 @@ import ( "time" packagev1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/package/v1" - "github.com/safedep/dry/api/pb" "github.com/safedep/dry/log" "github.com/safedep/pmg/analyzer" "github.com/safedep/pmg/config" @@ -45,7 +44,6 @@ type PackageManagerGuardConfig struct { AnalysisTimeout time.Duration DryRun bool InsecureInstallation bool - TrustedPackages []config.TrustedPackage } func DefaultPackageManagerGuardConfig() PackageManagerGuardConfig { @@ -55,45 +53,9 @@ func DefaultPackageManagerGuardConfig() PackageManagerGuardConfig { AnalysisTimeout: 5 * time.Minute, DryRun: false, InsecureInstallation: false, - TrustedPackages: []config.TrustedPackage{}, } } -func (c *PackageManagerGuardConfig) IsTrustedPackageVersion(pkgVersion *packagev1.PackageVersion) bool { - if pkgVersion == nil { - return false - } - - trustedPkgs := c.TrustedPackages - if len(trustedPkgs) == 0 { - return false - } - - for _, v := range trustedPkgs { - purlTrustedPackageVersion, err := pb.NewPurlPackageVersion(v.Purl) - if err != nil { - log.Warnf("failed to parse trusted package version: %s: %v", v.Purl, err) - continue - } - - if purlTrustedPackageVersion.Version() != "" && purlTrustedPackageVersion.Version() != pkgVersion.GetVersion() { - continue - } - - if purlTrustedPackageVersion.Name() != pkgVersion.GetPackage().GetName() { - continue - } - - if purlTrustedPackageVersion.Ecosystem() != pkgVersion.GetPackage().GetEcosystem() { - continue - } - - return true - } - - return false -} - type packageManagerGuard struct { config PackageManagerGuardConfig interaction PackageManagerGuardInteraction @@ -293,7 +255,7 @@ func (g *packageManagerGuard) concurrentAnalyzePackages(ctx context.Context, // Queue all packages for analysis for _, pkg := range packages { - if g.config.IsTrustedPackageVersion(pkg) { + if config.IsTrustedPackage(pkg) { log.Debugf("Skipping trusted package: %s/%s@%s", pkg.GetPackage().GetEcosystem(), pkg.GetPackage().GetName(), pkg.GetVersion()) diff --git a/guard/guard_test.go b/guard/guard_test.go index 5abf59c..391e518 100644 --- a/guard/guard_test.go +++ b/guard/guard_test.go @@ -6,7 +6,6 @@ import ( packagev1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/package/v1" "github.com/safedep/pmg/analyzer" - "github.com/safedep/pmg/config" "github.com/safedep/pmg/internal/ui" "github.com/safedep/pmg/packagemanager" "github.com/stretchr/testify/assert" @@ -270,261 +269,3 @@ func TestGuardInsecureInstallation(t *testing.T) { }) } -func TestGuardIsTrustedPackageVersion(t *testing.T) { - tests := []struct { - name string - trustedPackages []config.TrustedPackage - pkgVersion *packagev1.PackageVersion - want bool - }{ - { - name: "nil package version returns false", - trustedPackages: []config.TrustedPackage{}, - pkgVersion: nil, - want: false, - }, - { - name: "empty trusted packages list returns false", - trustedPackages: []config.TrustedPackage{}, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "express", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, - }, - Version: "4.18.0", - }, - want: false, - }, - { - name: "exact match with version returns true", - trustedPackages: []config.TrustedPackage{ - { - Purl: "pkg:npm/express@4.18.0", - Reason: "trusted by team", - }, - }, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "express", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, - }, - Version: "4.18.0", - }, - want: true, - }, - { - name: "match without version in trusted package returns true", - trustedPackages: []config.TrustedPackage{ - { - Purl: "pkg:npm/express", - Reason: "all versions trusted", - }, - }, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "express", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, - }, - Version: "4.18.0", - }, - want: true, - }, - { - name: "version mismatch returns false", - trustedPackages: []config.TrustedPackage{ - { - Purl: "pkg:npm/express@4.17.0", - Reason: "old version trusted", - }, - }, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "express", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, - }, - Version: "4.18.0", - }, - want: false, - }, - { - name: "name mismatch returns false", - trustedPackages: []config.TrustedPackage{ - { - Purl: "pkg:npm/react@18.0.0", - Reason: "trusted package", - }, - }, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "express", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, - }, - Version: "4.18.0", - }, - want: false, - }, - { - name: "ecosystem mismatch returns false", - trustedPackages: []config.TrustedPackage{ - { - Purl: "pkg:pypi/requests@2.28.0", - Reason: "trusted package", - }, - }, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "requests", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, - }, - Version: "2.28.0", - }, - want: false, - }, - { - name: "pypi package exact match returns true", - trustedPackages: []config.TrustedPackage{ - { - Purl: "pkg:pypi/requests@2.28.0", - Reason: "trusted http library", - }, - }, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "requests", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_PYPI, - }, - Version: "2.28.0", - }, - want: true, - }, - { - name: "multiple trusted packages finds correct match", - trustedPackages: []config.TrustedPackage{ - { - Purl: "pkg:npm/lodash@4.17.21", - Reason: "utility library", - }, - { - Purl: "pkg:npm/express@4.18.0", - Reason: "web framework", - }, - { - Purl: "pkg:pypi/requests@2.28.0", - Reason: "http library", - }, - }, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "express", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, - }, - Version: "4.18.0", - }, - want: true, - }, - { - name: "multiple trusted packages no match returns false", - trustedPackages: []config.TrustedPackage{ - { - Purl: "pkg:npm/lodash@4.17.21", - Reason: "utility library", - }, - { - Purl: "pkg:npm/react@18.0.0", - Reason: "ui library", - }, - }, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "express", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, - }, - Version: "4.18.0", - }, - want: false, - }, - { - name: "invalid purl in trusted packages skips and returns false", - trustedPackages: []config.TrustedPackage{ - { - Purl: "invalid-purl-format", - Reason: "malformed", - }, - }, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "express", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, - }, - Version: "4.18.0", - }, - want: false, - }, - { - name: "invalid purl skipped but valid match found", - trustedPackages: []config.TrustedPackage{ - { - Purl: "invalid-purl-format", - Reason: "malformed", - }, - { - Purl: "pkg:npm/express@4.18.0", - Reason: "valid trusted package", - }, - }, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "express", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, - }, - Version: "4.18.0", - }, - want: true, - }, - { - name: "package version without version field matches versionless trusted package", - trustedPackages: []config.TrustedPackage{ - { - Purl: "pkg:npm/express", - Reason: "all versions trusted", - }, - }, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "express", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, - }, - Version: "", - }, - want: true, - }, - { - name: "package version without version field does not match versioned trusted package", - trustedPackages: []config.TrustedPackage{ - { - Purl: "pkg:npm/express@4.18.0", - Reason: "specific version trusted", - }, - }, - pkgVersion: &packagev1.PackageVersion{ - Package: &packagev1.Package{ - Name: "express", - Ecosystem: packagev1.Ecosystem_ECOSYSTEM_NPM, - }, - Version: "", - }, - want: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - config := PackageManagerGuardConfig{ - TrustedPackages: tt.trustedPackages, - } - - got := config.IsTrustedPackageVersion(tt.pkgVersion) - assert.Equal(t, tt.want, got) - }) - } -} diff --git a/internal/flows/common_flow.go b/internal/flows/common_flow.go index 59d3a01..ddbf36c 100644 --- a/internal/flows/common_flow.go +++ b/internal/flows/common_flow.go @@ -57,7 +57,6 @@ func (f *commonFlow) Run(ctx context.Context, args []string, parsedCmd *packagem guardConfig := guard.DefaultPackageManagerGuardConfig() guardConfig.DryRun = config.DryRun guardConfig.InsecureInstallation = config.InsecureInstallation - guardConfig.TrustedPackages = config.Config.TrustedPackages proxy, err := guard.NewPackageManagerGuard(guardConfig, f.pm, f.packageResolver, analyzers, interaction) if err != nil { diff --git a/proxy/interceptors/base_registry.go b/proxy/interceptors/base_registry.go index f327594..a4f73ac 100644 --- a/proxy/interceptors/base_registry.go +++ b/proxy/interceptors/base_registry.go @@ -9,6 +9,7 @@ import ( packagev1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/package/v1" "github.com/safedep/dry/log" "github.com/safedep/pmg/analyzer" + "github.com/safedep/pmg/config" "github.com/safedep/pmg/guard" "github.com/safedep/pmg/proxy" ) @@ -47,11 +48,7 @@ func (b *baseRegistryInterceptor) analyzePackage( packageName string, packageVersion string, ) (*analyzer.PackageVersionAnalysisResult, error) { - if cached, ok := b.cache.Get(ecosystem.String(), packageName, packageVersion); ok { - log.Debugf("[%s] Using cached analysis result for %s@%s", ctx.RequestID, packageName, packageVersion) - return cached, nil - } - + // Check if package is trusted before analyzing pkgVersion := &packagev1.PackageVersion{ Package: &packagev1.Package{ Ecosystem: ecosystem, @@ -60,6 +57,21 @@ func (b *baseRegistryInterceptor) analyzePackage( Version: packageVersion, } + if config.IsTrustedPackage(pkgVersion) { + log.Debugf("[%s] Skipping trusted package: %s/%s@%s", + ctx.RequestID, ecosystem.String(), packageName, packageVersion) + + return &analyzer.PackageVersionAnalysisResult{ + PackageVersion: pkgVersion, + Action: analyzer.ActionAllow, + }, nil + } + + if cached, ok := b.cache.Get(ecosystem.String(), packageName, packageVersion); ok { + log.Debugf("[%s] Using cached analysis result for %s@%s", ctx.RequestID, packageName, packageVersion) + return cached, nil + } + log.Debugf("[%s] Analyzing package %s@%s", ctx.RequestID, packageName, packageVersion) analysisCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)