From 359712bee05d7b3dad0fc2c02ef00a488f2bd2b1 Mon Sep 17 00:00:00 2001
From: Abhisek Datta <abhisek.datta@gmail.com>
Date: Tue, 25 Feb 2025 11:25:58 +0530
Subject: [PATCH] fix: Handle known false positives with npm LFP (#359)

---
 .vscode/settings.json        |  2 +-
 pkg/analyzer/lfp_npm.go      | 29 +++++++++++++++++++++++++++--
 pkg/analyzer/lfp_npm_test.go |  9 ++++++++-
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/.vscode/settings.json b/.vscode/settings.json
index 8e7634a..433dcd4 100644
--- a/.vscode/settings.json
+++ b/.vscode/settings.json
@@ -3,7 +3,7 @@
     "editor.formatOnSave": true,
     "editor.defaultFormatter": "golang.go",
     "editor.codeActionsOnSave": {
-      "source.organizeImports": true
+      "source.organizeImports": "explicit"
     }
   },
   "gopls": {
diff --git a/pkg/analyzer/lfp_npm.go b/pkg/analyzer/lfp_npm.go
index ca8c760..a1a38b2 100644
--- a/pkg/analyzer/lfp_npm.go
+++ b/pkg/analyzer/lfp_npm.go
@@ -18,6 +18,16 @@ import (
 
 const npmRegistryTrustedUrlBase = "https://registry.npmjs.org"
 
+// List of packages that are known to have inconsistent URLs in the package-lock.json
+// file. For example, `strip-ansi-cjs` resolvs to URL that does not follow path convention
+// Here we maintain a map of such package names so that users don't have to manually
+// add them to the trusted registry URLs in the config file.
+var npmRegistryKnownInconsistentPackageUrls = map[string]string{
+	"strip-ansi-cjs":   "https://registry.npmjs.org/strip-ansi/-/",
+	"wrap-ansi-cjs":    "https://registry.npmjs.org/wrap-ansi/-/",
+	"string-width-cjs": "https://registry.npmjs.org/string-width/-/",
+}
+
 type npmPackageLockPackage struct {
 	Version   string `json:"version"`
 	License   string `json:"license"`
@@ -41,7 +51,8 @@ type npmLockfilePoisoningAnalyzer struct {
 }
 
 func (npm *npmLockfilePoisoningAnalyzer) Analyze(manifest *models.PackageManifest,
-	handler AnalyzerEventHandler) error {
+	handler AnalyzerEventHandler,
+) error {
 	logger.Debugf("npmLockfilePoisoningAnalyzer: Analyzing [%s] %s",
 		manifest.GetSpecEcosystem(), manifest.GetDisplayPath())
 
@@ -70,7 +81,6 @@ func (npm *npmLockfilePoisoningAnalyzer) Analyze(manifest *models.PackageManifes
 		pkgMap[p.Name] = p
 		return nil
 	})
-
 	if err != nil {
 		return err
 	}
@@ -116,6 +126,7 @@ func (npm *npmLockfilePoisoningAnalyzer) Analyze(manifest *models.PackageManifes
 		trustedRegistryUrls := []string{npmRegistryTrustedUrlBase}
 		trustedRegistryUrls = append(trustedRegistryUrls, npm.config.TrustedRegistryUrls...)
 		userTrustUrls := npm.config.TrustedRegistryUrls
+
 		logger.Debugf("npmLockfilePoisoningAnalyzer: Analyzing package [%s] with %d trusted registry URLs in config",
 			packageName, len(trustedRegistryUrls))
 
@@ -248,6 +259,10 @@ func npmNodeModulesPackagePathToName(path string) string {
 // Test if URL follows the pkg name path convention as per NPM package registry
 // specification https://docs.npmjs.com/cli/v10/configuring-npm/package-lock-json
 func npmIsUrlFollowsPathConvention(sourceUrl string, pkg string, trustedUrls []string, userTrustedUrls []string) bool {
+	if npmIsPackageKnownToHaveInconsistentUrl(pkg, sourceUrl) {
+		return true
+	}
+
 	// Parse the source URL
 	parsedUrl, err := npmParseSourceUrl(sourceUrl)
 	if err != nil {
@@ -293,3 +308,13 @@ func npmIsUrlFollowsPathConvention(sourceUrl string, pkg string, trustedUrls []s
 	// Default fallback
 	return false
 }
+
+func npmIsPackageKnownToHaveInconsistentUrl(pkg, url string) bool {
+	knownUrl, ok := npmRegistryKnownInconsistentPackageUrls[pkg]
+	if !ok {
+		return false
+	}
+
+	// IMPORTANT: We must check that the URL is indeed a prefix of the known URL
+	return strings.HasPrefix(url, knownUrl)
+}
diff --git a/pkg/analyzer/lfp_npm_test.go b/pkg/analyzer/lfp_npm_test.go
index c32e537..e48dfc3 100644
--- a/pkg/analyzer/lfp_npm_test.go
+++ b/pkg/analyzer/lfp_npm_test.go
@@ -164,7 +164,14 @@ func TestNpmIsUrlFollowsPathConvention(t *testing.T) {
 			"strip_ansi_cjs package path matches trusted url path",
 			"https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.1.tgz",
 			"strip-ansi-cjs",
-			[]string{"https://registry.npmjs.org/strip-ansi"},
+			[]string{},
+			true,
+		},
+		{
+			"wrap_ansi_cjs package path matches trusted url path",
+			"https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-7.0.0.tgz",
+			"wrap-ansi-cjs",
+			[]string{},
 			true,
 		},
 	}