Skip to content

Commit

Permalink
fix: Handle known false positives with npm LFP
Browse files Browse the repository at this point in the history
  • Loading branch information
abhisek committed Feb 25, 2025
1 parent c313485 commit 1d5bcd0
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 4 deletions.
2 changes: 1 addition & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"editor.formatOnSave": true,
"editor.defaultFormatter": "golang.go",
"editor.codeActionsOnSave": {
"source.organizeImports": true
"source.organizeImports": "explicit"
}
},
"gopls": {
Expand Down
29 changes: 27 additions & 2 deletions pkg/analyzer/lfp_npm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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())

Expand Down Expand Up @@ -70,7 +81,6 @@ func (npm *npmLockfilePoisoningAnalyzer) Analyze(manifest *models.PackageManifes
pkgMap[p.Name] = p
return nil
})

if err != nil {
return err
}
Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
9 changes: 8 additions & 1 deletion pkg/analyzer/lfp_npm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}
Expand Down

0 comments on commit 1d5bcd0

Please # to comment.