Skip to content

Commit

Permalink
fix: Accept trusted URL base for LFP analyser
Browse files Browse the repository at this point in the history
Signed-off-by: abhisek <[email protected]>
  • Loading branch information
abhisek committed Aug 9, 2024
1 parent 26d68d4 commit 95cc1e3
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 18 deletions.
50 changes: 37 additions & 13 deletions pkg/analyzer/lfp_npm.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"net/url"
"os"
"slices"
"strings"

jsonreportspec "github.com/safedep/vet/gen/jsonreport"
Expand Down Expand Up @@ -146,7 +147,7 @@ func (npm *npmLockfilePoisoningAnalyzer) Analyze(manifest *models.PackageManifes
})
}

if !npmIsUrlFollowsPathConvention(lockfilePackage.Resolved, packageName) {
if !npmIsUrlFollowsPathConvention(lockfilePackage.Resolved, packageName, trustedRegistryUrls) {
logger.Debugf("npmLockfilePoisoningAnalyzer: Package [%s] resolved to an unconventional URL [%s]",
packageName, lockfilePackage.Resolved)

Expand Down Expand Up @@ -180,14 +181,7 @@ func (npm *npmLockfilePoisoningAnalyzer) Analyze(manifest *models.PackageManifes

// Analyze the artifact URL and determine if the source is trusted
func npmIsTrustedSource(sourceUrl string, trusteUrls []string) bool {
// Go url parser cannot handle git+ssh://host:project/repo.git#commit
if len(sourceUrl) > 10 && strings.EqualFold(sourceUrl[0:10], "git+ssh://") {
if cIndex := strings.Index(sourceUrl[10:], ":"); cIndex != -1 {
sourceUrl = sourceUrl[0:10+cIndex] + "/" + sourceUrl[10+cIndex+1:]
}
}

parsedUrl, err := url.Parse(sourceUrl)
parsedUrl, err := npmParseSourceUrl(sourceUrl)
if err != nil {
logger.Errorf("npmIsTrustedSource: Failed to parse URL %s: %v",
sourceUrl, err)
Expand All @@ -206,7 +200,7 @@ func npmIsTrustedSource(sourceUrl string, trusteUrls []string) bool {

// Compare with trusted URLs
for _, trusteUrl := range trusteUrls {
parsedTrustedUrl, err := url.Parse(trusteUrl)
parsedTrustedUrl, err := npmParseSourceUrl(trusteUrl)
if err != nil {
logger.Errorf("npmIsTrustedSource: Failed to parse trusted URL %s: %v",
trusteUrl, err)
Expand Down Expand Up @@ -235,16 +229,27 @@ func npmIsTrustedSource(sourceUrl string, trusteUrls []string) bool {
return false
}

func npmParseSourceUrl(sourceUrl string) (*url.URL, error) {
// Go url parser cannot handle git+ssh://host:project/repo.git#commit
if len(sourceUrl) > 10 && strings.EqualFold(sourceUrl[0:10], "git+ssh://") {
if cIndex := strings.Index(sourceUrl[10:], ":"); cIndex != -1 {
sourceUrl = sourceUrl[0:10+cIndex] + "/" + sourceUrl[10+cIndex+1:]
}
}

return url.Parse(sourceUrl)
}

// Extract the package name from the node_modules filesystem path
func npmNodeModulesPackagePathToName(path string) string {
return utils.NpmNodeModulesPackagePathToName(path)
}

// 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) bool {
func npmIsUrlFollowsPathConvention(sourceUrl string, pkg string, trustedUrls []string) bool {
// Example: https://registry.npmjs.org/express/-/express-4.17.1.tgz
parsedUrl, err := url.Parse(sourceUrl)
parsedUrl, err := npmParseSourceUrl(sourceUrl)
if err != nil {
logger.Errorf("npmIsUrlFollowsPathConvention: Failed to parse URL %s: %v",
sourceUrl, err)
Expand All @@ -260,6 +265,25 @@ func npmIsUrlFollowsPathConvention(sourceUrl string, pkg string) bool {
path = path[1:]
}

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)
continue
}

trustedBase := parsedTrustedUrl.Path
trustedBase = strings.TrimPrefix(trustedBase, "/")
trustedBase = strings.TrimSuffix(trustedBase, "/")

acceptablePackageNames = append(acceptablePackageNames,
fmt.Sprintf("%s/%s", trustedBase, pkg))
}

// Example: @angular/core from https://registry.npmjs.org/@angular/core/-/core-1.0.0.tgz
scopedPackageName := strings.Split(path, "/-/")[0]
return scopedPackageName == pkg

return slices.Contains(acceptablePackageNames, scopedPackageName)
}
55 changes: 50 additions & 5 deletions pkg/analyzer/lfp_npm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ func TestNpmIsTrustedSource(t *testing.T) {
[]string{"https://registry.npmjs.org", "git+ssh://github.com/safedep"},
false,
},
{
"source is trusted when trusted url has a base path",
"https://registry.example.org/base/a/b/-/c.tgz",
[]string{"https://registry.example.org/base"},
true,
},
}

for _, test := range cases {
Expand All @@ -85,34 +91,73 @@ func TestNpmIsTrustedSource(t *testing.T) {

func TestNpmIsUrlFollowsPathConvention(t *testing.T) {
cases := []struct {
name string
url string
pkgName string
expected bool
name string
url string
pkgName string
trustedUrls []string
expected bool
}{
{
"package name matches url path",
"https://registry.npmjs.org/package-name/-/package-name-1.0.0.tgz",
"package-name",
[]string{},
true,
},
{
"package name matches scoped url path",
"https://registry.npmjs.org/@angular/core/-/core-1.0.0.tgz",
"@angular/core",
[]string{},
true,
},
{
"package name does not match scoped url path",
"https://registry.npmjs.org/@angular/core/-/core-1.0.0.tgz",
"@someother/core",
[]string{},
false,
},
{
"package path matches trusted url path",
"https://registry.npmjs.org/base/package-name/-/package-name-1.0.0.tgz",
"package-name",
[]string{"https://registry.npmjs.org/base"},
true,
},
{
"package path matches trusted url path with trailing slash",
"https://registry.npmjs.org/base/package-name/-/package-name-1.0.0.tgz",
"package-name",
[]string{"https://registry.npmjs.org/base/"},
true,
},
{
"package path matches trusted url path prefix",
"https://registry.npmjs.org/base/package-name/-/package-name-1.0.0.tgz",
"package-name",
[]string{"https://registry.npmjs.org/base1/base2"},
false,
},
{
"package path has base without trusted url",
"https://registry.npmjs.org/base/package-name/-/package-name-1.0.0.tgz",
"package-name",
[]string{},
false,
},
{
"package path matches one of the trusted url base",
"https://registry.npmjs.org/base/package-name/-/package-name-1.0.0.tgz",
"package-name",
[]string{"https://registry.npmjs.org/base", "https://registry.npmjs.org/base1"},
true,
},
}

for _, test := range cases {
t.Run(test.name, func(t *testing.T) {
actual := npmIsUrlFollowsPathConvention(test.url, test.pkgName)
actual := npmIsUrlFollowsPathConvention(test.url, test.pkgName, test.trustedUrls)
assert.Equal(t, test.expected, actual)
})
}
Expand Down

0 comments on commit 95cc1e3

Please sign in to comment.