fix proxy mode failing for GH private packages (#137)

* fix proxy mode failing for GH private packages

* skip analysis for private packages for proxy mode

* introduce npmRegistryConfig and support for handling multiple parsers in future

* refactor name and unexport npm config functions

* rm unused function

* rename & unexport npmRegistryURLParser

* add e2e for malicious pkg blocked using proxy mode

Author: Sahilb315

.github/workflows/pmg-e2e.yml

@@ -372,6 +372,27 @@ jobs:
           ! pmg npm install nyc-config@10.0.0 || echo "Malicious package correctly blocked"
           cd .. && rm -rf malicious-test
 
+      - name: Test safedep-test-pkg is Blocked using Proxy mode
+        run: |
+          echo "Testing that safedep-test-pkg is blocked..."
+          mkdir safedep-test-pkg-test && cd safedep-test-pkg-test
+          pmg npm init -y
+          # Attempt to install safedep-test-pkg - should fail
+          if pmg --experimental-proxy-mode npm --no-cache --prefer-online i safedep-test-pkg@0.1.3; then
+            echo "ERROR: safedep-test-pkg was not blocked!"
+            exit 1
+          else
+            echo "SUCCESS: safedep-test-pkg correctly blocked"
+          fi
+          # Verify package is not installed locally
+          if [ -d "node_modules/safedep-test-pkg" ]; then
+            echo "ERROR: safedep-test-pkg found in node_modules!"
+            exit 1
+          else
+            echo "SUCCESS: safedep-test-pkg not present in node_modules"
+          fi
+          cd .. && rm -rf safedep-test-pkg-test
+
       - name: Test PMG Modes
         run: |
           echo "Testing different PMG modes..."

docs/demo/pmg-intro.tape

@@ -21,4 +21,3 @@ Enter
 Type "npm install safedep-test-pkg"
 Enter
 Sleep 5s
-

proxy/interceptors/npm_registry.go

@@ -9,12 +9,28 @@ import (
 	"github.com/safedep/pmg/proxy"
 )
 
-var (
-	npmRegistryDomains = []string{
-		"registry.npmjs.org",
-		"registry.yarnpkg.com",
-	}
-)
+var npmRegistryDomains = map[string]*npmRegistryConfig{
+	"registry.npmjs.org": {
+		Host:                 "registry.npmjs.org",
+		SupportedForAnalysis: true,
+		RegistryParser:       npmParser{},
+	},
+	"registry.yarnpkg.com": {
+		Host:                 "registry.yarnpkg.com",
+		SupportedForAnalysis: true,
+		RegistryParser:       npmParser{},
+	},
+	"npm.pkg.github.com": {
+		Host:                 "npm.pkg.github.com",
+		SupportedForAnalysis: false, // Skip analysis for now (private packages, auth complexity)
+		RegistryParser:       githubParser{},
+	},
+	"pkg-npm.githubusercontent.com": {
+		Host:                 "pkg-npm.githubusercontent.com",
+		SupportedForAnalysis: false, // Skip analysis (blob storage, redirected downloads)
+		RegistryParser:       githubBlobParser{},
+	},
+}
 
 // NpmRegistryInterceptor intercepts NPM registry requests and analyzes packages for malware
 // It embeds baseRegistryInterceptor to reuse ecosystem agnostic functionality
@@ -46,8 +62,13 @@ func (i *NpmRegistryInterceptor) Name() string {
 
 // ShouldIntercept determines if this interceptor should handle the given request
 func (i *NpmRegistryInterceptor) ShouldIntercept(ctx *proxy.RequestContext) bool {
-	for _, domain := range npmRegistryDomains {
-		if ctx.Hostname == domain || strings.HasSuffix(ctx.Hostname, "."+domain) {
+	if _, exists := npmRegistryDomains[ctx.Hostname]; exists {
+		return true
+	}
+
+	// Check subdomain match
+	for domain := range npmRegistryDomains {
+		if strings.HasSuffix(ctx.Hostname, "."+domain) {
 			return true
 		}
 	}
@@ -60,9 +81,26 @@ func (i *NpmRegistryInterceptor) ShouldIntercept(ctx *proxy.RequestContext) bool
 func (i *NpmRegistryInterceptor) HandleRequest(ctx *proxy.RequestContext) (*proxy.InterceptorResponse, error) {
 	log.Debugf("[%s] Handling NPM registry request: %s", ctx.RequestID, ctx.URL.Path)
 
-	pkgInfo, err := parseNpmRegistryURL(ctx.URL.Path)
+	// Get registry configuration
+	config := getNpmRegistryConfigForHostname(ctx.Hostname)
+	if config == nil {
+		// Shouldn't happen if ShouldIntercept is working correctly
+		log.Warnf("[%s] No registry config found for hostname: %s", ctx.RequestID, ctx.Hostname)
+		return &proxy.InterceptorResponse{Action: proxy.ActionAllow}, nil
+	}
+
+	// Skip analysis for registries that are not supported for analysis
+	if !config.SupportedForAnalysis {
+		log.Debugf("[%s] Skipping analysis for %s registry (not supported for analysis): %s",
+			ctx.RequestID, config.Host, ctx.URL.String())
+		return &proxy.InterceptorResponse{Action: proxy.ActionAllow}, nil
+	}
+
+	// Parse URL using registry-specific strategy
+	pkgInfo, err := config.RegistryParser.ParseURL(ctx.URL.Path)
 	if err != nil {
-		log.Warnf("[%s] Failed to parse NPM registry URL %s: %v", ctx.RequestID, ctx.URL.Path, err)
+		log.Warnf("[%s] Failed to parse NPM registry URL %s for %s: %v",
+			ctx.RequestID, ctx.URL.Path, config.Host, err)
 		return &proxy.InterceptorResponse{Action: proxy.ActionAllow}, nil
 	}
 

proxy/interceptors/npm_registry_config.go

@@ -0,0 +1,32 @@
+package interceptors
+
+import "strings"
+
+// npmRegistryConfig defines configuration for npm registry endpoints
+type npmRegistryConfig struct {
+	// Hostname
+	Host string
+
+	// Whether this registry is supported for malware analysis
+	SupportedForAnalysis bool
+
+	// Parser for the registry
+	RegistryParser npmRegistryURLParser
+}
+
+// getNpmRegistryConfigForHostname returns the configuration for a hostname (with subdomain matching)
+func getNpmRegistryConfigForHostname(hostname string) *npmRegistryConfig {
+	// Check exact match first
+	if config, exists := npmRegistryDomains[hostname]; exists {
+		return config
+	}
+
+	// Check subdomain match: hostname could be "cdn.registry.npmjs.org" matching "registry.npmjs.org"
+	for endpoint, config := range npmRegistryDomains {
+		if strings.HasSuffix(hostname, "."+endpoint) {
+			return config
+		}
+	}
+
+	return nil
+}

proxy/interceptors/npm_url_parser.go

@@ -13,7 +13,15 @@ type npmPackageInfo struct {
 	IsScoped  bool
 }
 
-// parseNpmRegistryURL parses an NPM registry URL path to extract package information
+// npmRegistryURLParser defines the interface for parsing registry-specific URLs
+type npmRegistryURLParser interface {
+	ParseURL(urlPath string) (*npmPackageInfo, error)
+}
+
+type npmParser struct{}
+
+// parseNpmRegistryURL parses standard NPM registry URL paths (registry.npmjs.org, registry.yarnpkg.com)
+// This function handles the standard npm registry URL format.
 //
 // Supported URL patterns:
 // - /package                               -> {Name: "package", Version: ""}
@@ -22,7 +30,7 @@ type npmPackageInfo struct {
 // - /@scope/package/1.0.0                  -> {Name: "@scope/package", Version: "1.0.0", IsScoped: true}
 // - /package/-/package-1.0.0.tgz          -> {Name: "package", Version: "1.0.0", IsTarball: true}
 // - /@scope/package/-/@scope-package-1.0.0.tgz -> {Name: "@scope/package", Version: "1.0.0", IsTarball: true, IsScoped: true}
-func parseNpmRegistryURL(urlPath string) (*npmPackageInfo, error) {
+func (n npmParser) ParseURL(urlPath string) (*npmPackageInfo, error) {
 	// Remove leading and trailing slashes
 	urlPath = strings.Trim(urlPath, "/")
 
@@ -43,6 +51,32 @@ func parseNpmRegistryURL(urlPath string) (*npmPackageInfo, error) {
 	return parseUnscopedPackageURL(segments)
 }
 
+type githubParser struct{}
+
+// ParseURL implements RegistryURLParser for GitHub npm registry
+func (g githubParser) ParseURL(urlPath string) (*npmPackageInfo, error) {
+	// For now, just allow all GitHub npm registry requests through without analysis
+	// TODO: Implement proper GitHub npm registry URL parsing when analysis is enabled
+	// GitHub URLs follow patterns:
+	// - /download/@owner/package/version/hash.tgz -> {Name: "package", Version: "1.0.0", IsTarball: true}
+	// - /@owner/package (metadata requests)
+	return &npmPackageInfo{
+		IsTarball: false, // Mark as non-tarball to skip analysis
+	}, nil
+}
+
+type githubBlobParser struct{}
+
+// ParseURL implements RegistryURLParser for GitHub blob storage
+func (g githubBlobParser) ParseURL(urlPath string) (*npmPackageInfo, error) {
+	// For now, just allow all GitHub blob storage requests through without analysis
+	// TODO: Implement proper GitHub blob storage URL parsing when analysis is enabled
+	// Pattern: /npmregistryv2prod/blobs/{blob_id}/{package_name}/{version}/***
+	return &npmPackageInfo{
+		IsTarball: false, // Mark as non-tarball to skip analysis
+	}, nil
+}
+
 // parseScopedPackageURL parses a scoped package URL
 // Patterns:
 // - [@scope, package]                    -> @scope/package

proxy/interceptors/npm_url_parser_test.go

@@ -265,7 +265,8 @@ func TestParseNpmRegistryURL(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := parseNpmRegistryURL(tt.urlPath)
+			parser := npmParser{}
+			got, err := parser.ParseURL(tt.urlPath)
 
 			if tt.wantErr {
 				assert.Error(t, err)
@@ -280,3 +281,71 @@ func TestParseNpmRegistryURL(t *testing.T) {
 		})
 	}
 }
+
+func TestGithubParser_ParseURL(t *testing.T) {
+	tests := []struct {
+		name          string
+		urlPath       string
+		wantIsTarball bool
+		wantErr       bool
+	}{
+		{
+			name:          "github metadata request",
+			urlPath:       "/@owner/package",
+			wantIsTarball: false,
+			wantErr:       false,
+		},
+		{
+			name:          "github download request",
+			urlPath:       "/download/@owner/package/1.0.0/abc123.tgz",
+			wantIsTarball: false,
+			wantErr:       false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			parser := githubParser{}
+			got, err := parser.ParseURL(tt.urlPath)
+
+			if tt.wantErr {
+				assert.Error(t, err)
+				return
+			}
+
+			assert.NoError(t, err)
+			assert.Equal(t, tt.wantIsTarball, got.IsTarball)
+		})
+	}
+}
+
+func TestGithubBlobParser_ParseURL(t *testing.T) {
+	tests := []struct {
+		name          string
+		urlPath       string
+		wantIsTarball bool
+		wantErr       bool
+	}{
+		{
+			name:          "github blob storage request",
+			urlPath:       "/npmregistryv2prod/blobs/132160241/gh-npm-pkg/1.0.0/abc123",
+			wantIsTarball: false,
+			wantErr:       false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			parser := githubBlobParser{}
+			got, err := parser.ParseURL(tt.urlPath)
+
+			if tt.wantErr {
+				assert.Error(t, err)
+				return
+			}
+
+			assert.NoError(t, err)
+			assert.Equal(t, tt.wantIsTarball, got.IsTarball)
+		})
+	}
+}