From 01396c12434f885aec4739bdcf880439f6390d32 Mon Sep 17 00:00:00 2001
From: Manav sethi <mail@manav.co.in>
Date: Tue, 14 Jan 2025 15:23:12 +0530
Subject: [PATCH] Minor bugfix to handle cases where the package name doesn't
resolve properly (#305)
* Minor bugfix to handle cases where the package name doesn't resolve to a proper URL
* Remove accidental changes
* PR review fixes
* Pass the trustedURLs directly instead of ignoring the npm configured URL
* Updated the test cases
---
pkg/analyzer/lfp_npm.go | 38 +++++++++++++++++++++---------------
pkg/analyzer/lfp_npm_test.go | 9 ++++++++-
2 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/pkg/analyzer/lfp_npm.go b/pkg/analyzer/lfp_npm.go
index 1577dfc2..ca8c7604 100644
--- a/pkg/analyzer/lfp_npm.go
+++ b/pkg/analyzer/lfp_npm.go
@@ -115,7 +115,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))
@@ -147,7 +147,7 @@ func (npm *npmLockfilePoisoningAnalyzer) Analyze(manifest *models.PackageManifes
})
}
- if !npmIsUrlFollowsPathConvention(lockfilePackage.Resolved, packageName, trustedRegistryUrls) {
+ if !npmIsUrlFollowsPathConvention(lockfilePackage.Resolved, packageName, trustedRegistryUrls, userTrustUrls) {
logger.Debugf("npmLockfilePoisoningAnalyzer: Package [%s] resolved to an unconventional URL [%s]",
packageName, lockfilePackage.Resolved)
@@ -247,12 +247,11 @@ 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) bool {
- // Example: https://registry.npmjs.org/express/-/express-4.17.1.tgz
+func npmIsUrlFollowsPathConvention(sourceUrl string, pkg string, trustedUrls []string, userTrustedUrls []string) bool {
+ // Parse the source URL
parsedUrl, err := npmParseSourceUrl(sourceUrl)
if err != nil {
- logger.Errorf("npmIsUrlFollowsPathConvention: Failed to parse URL %s: %v",
- sourceUrl, err)
+ logger.Errorf("npmIsUrlFollowsPathConvention: Failed to parse URL %s: %v", sourceUrl, err)
return false
}
@@ -265,25 +264,32 @@ func npmIsUrlFollowsPathConvention(sourceUrl string, pkg string, trustedUrls []s
path = path[1:]
}
+ // Build a list of acceptable package names
acceptablePackageNames := []string{pkg}
for _, trustedUrl := range trustedUrls {
parsedTrustedUrl, err := npmParseSourceUrl(trustedUrl)
if err != nil {
- logger.Errorf("npmIsUrlFollowsPathConvention: Failed to parse trusted URL %s: %v",
- trustedUrl, err)
+ logger.Errorf("npmIsUrlFollowsPathConvention: Failed to parse trusted URL %s: %v", trustedUrl, err)
continue
}
- trustedBase := parsedTrustedUrl.Path
- trustedBase = strings.TrimPrefix(trustedBase, "/")
- trustedBase = strings.TrimSuffix(trustedBase, "/")
-
- acceptablePackageNames = append(acceptablePackageNames,
- fmt.Sprintf("%s/%s", trustedBase, pkg))
+ trustedBase := strings.Trim(parsedTrustedUrl.Path, "/")
+ acceptablePackageNames = append(acceptablePackageNames, fmt.Sprintf("%s/%s", trustedBase, pkg))
}
- // Example: @angular/core from https://registry.npmjs.org/@angular/core/-/core-1.0.0.tgz
+ // Extract the scoped package name
scopedPackageName := strings.Split(path, "/-/")[0]
+ if slices.Contains(acceptablePackageNames, scopedPackageName) {
+ return true
+ }
- return slices.Contains(acceptablePackageNames, scopedPackageName)
+ // Check if the source URL starts with any trusted URL except the NPM trusted base URL
+ for _, trustedUrl := range userTrustedUrls {
+ if strings.HasPrefix(sourceUrl, trustedUrl) {
+ return true
+ }
+ }
+
+ // Default fallback
+ return false
}
diff --git a/pkg/analyzer/lfp_npm_test.go b/pkg/analyzer/lfp_npm_test.go
index d5cee319..c32e5373 100644
--- a/pkg/analyzer/lfp_npm_test.go
+++ b/pkg/analyzer/lfp_npm_test.go
@@ -160,11 +160,18 @@ func TestNpmIsUrlFollowsPathConvention(t *testing.T) {
[]string{"https://registry.npmjs.org/base", "https://registry.npmjs.org/base1"},
true,
},
+ {
+ "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"},
+ true,
+ },
}
for _, test := range cases {
t.Run(test.name, func(t *testing.T) {
- actual := npmIsUrlFollowsPathConvention(test.url, test.pkgName, test.trustedUrls)
+ actual := npmIsUrlFollowsPathConvention(test.url, test.pkgName, test.trustedUrls, test.trustedUrls)
assert.Equal(t, test.expected, actual)
})
}