perf(javascript): cache npm registry license lookups across runs (#5297)

When javascript.search-remote-licenses is enabled, the lock catalogers
query registry.npmjs.org once per package occurrence, on every run, with
nothing in front of the network call. The same name@version is fetched
again for every lockfile that mentions it, and again on the next scan.

This adds a javascriptLicenseResolver that resolves licenses through
syft's on-disk cache, keyed on "<name>/<version>" and namespaced as
javascript/v1, and routes the package-lock v1, package-lock v2, pnpm,
yarn and bun package constructors through it, leaving
getLicenseFromNpmRegistry itself unchanged. Failed lookups are not
cached: a transient registry failure or a name that 404s must not be able
to blank out licenses for the lifetime of the cache entry, since a
missing license silently weakens downstream license policy checks. A
successful lookup that returns an empty license is a real answer and is
still cached.

Signed-off-by: Anže Pečar <anze@pecar.me>

syft/pkg/cataloger/javascript/license.go

@@ -0,0 +1,53 @@
+package javascript
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/anchore/syft/internal/cache"
+	"github.com/anchore/syft/internal/log"
+	"github.com/anchore/syft/syft/pkg"
+)
+
+// javascriptLicenseResolver resolves licenses for a package from the npm registry, backed by syft's
+// on-disk cache so that the same package/version is only fetched once -- both within a single run
+// (the same dependency is commonly present in several lockfiles) and across runs.
+//
+// This mirrors the python cataloger's license resolver (see syft/pkg/cataloger/python/license.go)
+// with one deliberate difference: errors are not cached (cache.GetResolver rather than
+// cache.GetResolverCachingErrors). A missing license silently weakens downstream license policy
+// checks, so a transient registry failure (a 429, an outage) or a private package name that 404s
+// against the public registry must not be able to blank out licenses for the lifetime of the cache
+// entry. A successful lookup that returns an empty license is a real answer and is still cached.
+type javascriptLicenseResolver struct {
+	catalogerConfig CatalogerConfig
+	licenseCache    cache.Resolver[string]
+}
+
+func newJavascriptLicenseResolver(config CatalogerConfig) javascriptLicenseResolver {
+	return javascriptLicenseResolver{
+		licenseCache:    cache.GetResolver[string]("javascript", "v1"),
+		catalogerConfig: config,
+	}
+}
+
+func (lr *javascriptLicenseResolver) getLicenses(ctx context.Context, packageName string, packageVersion string) pkg.LicenseSet {
+	var licenseSet pkg.LicenseSet
+
+	if lr.catalogerConfig.SearchRemoteLicenses {
+		license, err := lr.getLicensesFromRemote(packageName, packageVersion)
+		if err == nil && license != "" {
+			licenseSet = pkg.NewLicenseSet(pkg.NewLicensesFromValuesWithContext(ctx, license)...)
+		}
+		if err != nil {
+			log.Debugf("unable to extract licenses from npm registry for package %s:%s: %+v", packageName, packageVersion, err)
+		}
+	}
+	return licenseSet
+}
+
+func (lr *javascriptLicenseResolver) getLicensesFromRemote(packageName string, packageVersion string) (string, error) {
+	return lr.licenseCache.Resolve(fmt.Sprintf("%s/%s", packageName, packageVersion), func() (string, error) {
+		return getLicenseFromNpmRegistry(lr.catalogerConfig.NPMBaseURL, packageName, packageVersion)
+	})
+}

syft/pkg/cataloger/javascript/license_test.go

@@ -0,0 +1,207 @@
+package javascript
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/anchore/syft/internal/cache"
+	"github.com/anchore/syft/syft/pkg"
+)
+
+// useInMemoryCache installs a real (in-memory) cache manager for the duration of the test. By
+// default syft's cache manager is bypassed, which is what keeps the rest of the cataloger tests
+// isolated from one another; these tests need a functioning cache to observe caching behavior.
+func useInMemoryCache(t *testing.T) {
+	t.Helper()
+	prev := cache.GetManager()
+	cache.SetManager(cache.NewInMemory(time.Hour))
+	t.Cleanup(func() { cache.SetManager(prev) })
+}
+
+// npmRegistryStub is a stand-in for registry.npmjs.org that counts the requests it serves.
+type npmRegistryStub struct {
+	*httptest.Server
+	requests atomic.Int64
+}
+
+func newNpmRegistryStub(t *testing.T, handler http.HandlerFunc) *npmRegistryStub {
+	t.Helper()
+	stub := &npmRegistryStub{}
+	stub.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		stub.requests.Add(1)
+		handler(w, r)
+	}))
+	t.Cleanup(stub.Close)
+	return stub
+}
+
+func (s *npmRegistryStub) config() CatalogerConfig {
+	return CatalogerConfig{SearchRemoteLicenses: true, NPMBaseURL: s.URL}
+}
+
+func TestJavascriptLicenseResolver_cachesSuccessfulLookups(t *testing.T) {
+	useInMemoryCache(t)
+
+	stub := newNpmRegistryStub(t, func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintf(w, `{"license": "MIT"}`)
+	})
+
+	lr := newJavascriptLicenseResolver(stub.config())
+
+	for i := 0; i < 3; i++ {
+		license, err := lr.getLicensesFromRemote("@babel/code-frame", "7.10.4")
+		require.NoError(t, err)
+		assert.Equal(t, "MIT", license)
+	}
+
+	assert.Equal(t, int64(1), stub.requests.Load(), "expected only the first lookup to reach the registry")
+}
+
+func TestJavascriptLicenseResolver_cacheIsSharedAcrossResolvers(t *testing.T) {
+	useInMemoryCache(t)
+
+	stub := newNpmRegistryStub(t, func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintf(w, `{"license": "MIT"}`)
+	})
+
+	// the same package/version is commonly present in more than one lockfile, each of which is
+	// parsed by a different adapter with its own resolver -- the cache is still shared
+	first := newJavascriptLicenseResolver(stub.config())
+	second := newJavascriptLicenseResolver(stub.config())
+
+	license, err := first.getLicensesFromRemote("lodash", "4.17.21")
+	require.NoError(t, err)
+	assert.Equal(t, "MIT", license)
+
+	license, err = second.getLicensesFromRemote("lodash", "4.17.21")
+	require.NoError(t, err)
+	assert.Equal(t, "MIT", license)
+
+	assert.Equal(t, int64(1), stub.requests.Load())
+}
+
+func TestJavascriptLicenseResolver_cachesEmptyLicense(t *testing.T) {
+	useInMemoryCache(t)
+
+	// a package that genuinely publishes no license field is a real answer, not a failure
+	stub := newNpmRegistryStub(t, func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintf(w, `{"name": "no-license-pkg"}`)
+	})
+
+	lr := newJavascriptLicenseResolver(stub.config())
+
+	for i := 0; i < 2; i++ {
+		license, err := lr.getLicensesFromRemote("no-license-pkg", "1.0.0")
+		require.NoError(t, err)
+		assert.Equal(t, "", license)
+	}
+
+	assert.Equal(t, int64(1), stub.requests.Load())
+}
+
+func TestJavascriptLicenseResolver_doesNotCacheErrors(t *testing.T) {
+	useInMemoryCache(t)
+
+	// unlike the python cataloger we intentionally do not cache failed lookups: a transient
+	// registry failure must not be able to blank out licenses for the lifetime of the cache entry
+	stub := newNpmRegistryStub(t, func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusTooManyRequests)
+		fmt.Fprintf(w, `too many requests`) // not valid JSON
+	})
+
+	lr := newJavascriptLicenseResolver(stub.config())
+
+	for i := 0; i < 3; i++ {
+		_, err := lr.getLicensesFromRemote("@babel/code-frame", "7.10.4")
+		require.Error(t, err)
+	}
+
+	assert.Equal(t, int64(3), stub.requests.Load(), "expected every lookup to be retried against the registry")
+}
+
+func TestJavascriptLicenseResolver_keyIncludesNameAndVersion(t *testing.T) {
+	useInMemoryCache(t)
+
+	stub := newNpmRegistryStub(t, func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintf(w, `{"license": %q}`, r.URL.Path)
+	})
+
+	lr := newJavascriptLicenseResolver(stub.config())
+
+	tests := []struct {
+		name     string
+		version  string
+		expected string
+	}{
+		{name: "lodash", version: "4.17.21", expected: "/lodash/4.17.21"},
+		{name: "lodash", version: "4.17.20", expected: "/lodash/4.17.20"},
+		{name: "@babel/code-frame", version: "7.10.4", expected: "/@babel/code-frame/7.10.4"},
+	}
+
+	for _, tt := range tests {
+		license, err := lr.getLicensesFromRemote(tt.name, tt.version)
+		require.NoError(t, err)
+		assert.Equal(t, tt.expected, license)
+	}
+
+	assert.Equal(t, int64(len(tests)), stub.requests.Load())
+}
+
+func TestJavascriptLicenseResolver_getLicenses(t *testing.T) {
+	ctx := context.TODO()
+
+	tests := []struct {
+		name             string
+		searchRemote     bool
+		response         string
+		expected         pkg.LicenseSet
+		expectedRequests int64
+	}{
+		{
+			name:             "remote search disabled makes no request",
+			searchRemote:     false,
+			response:         `{"license": "MIT"}`,
+			expected:         pkg.LicenseSet{},
+			expectedRequests: 0,
+		},
+		{
+			name:             "remote search returns the license from the registry",
+			searchRemote:     true,
+			response:         `{"license": "MIT"}`,
+			expected:         pkg.NewLicenseSet(pkg.NewLicenseWithContext(ctx, "MIT")),
+			expectedRequests: 1,
+		},
+		{
+			name:             "an empty license yields an empty license set",
+			searchRemote:     true,
+			response:         `{"name": "no-license-pkg"}`,
+			expected:         pkg.LicenseSet{},
+			expectedRequests: 1,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			useInMemoryCache(t)
+
+			stub := newNpmRegistryStub(t, func(w http.ResponseWriter, r *http.Request) {
+				fmt.Fprint(w, tt.response)
+			})
+
+			cfg := stub.config()
+			cfg.SearchRemoteLicenses = tt.searchRemote
+			lr := newJavascriptLicenseResolver(cfg)
+
+			assert.Equal(t, tt.expected, lr.getLicenses(ctx, "some-pkg", "1.0.0"))
+			assert.Equal(t, tt.expectedRequests, stub.requests.Load())
+		})
+	}
+}

syft/pkg/cataloger/javascript/package.go

@@ -85,7 +85,7 @@ func newPackageJSONPackage(ctx context.Context, resolver file.Resolver, u packag
 	return p
 }
 
-func newPackageLockV1Package(ctx context.Context, cfg CatalogerConfig, resolver file.Resolver, location file.Location, name string, u lockDependency) pkg.Package {
+func newPackageLockV1Package(ctx context.Context, licenseResolver javascriptLicenseResolver, resolver file.Resolver, location file.Location, name string, u lockDependency) pkg.Package {
 	version := u.Version
 
 	const aliasPrefixPackageLockV1 = "npm:"
@@ -101,17 +101,7 @@ func newPackageLockV1Package(ctx context.Context, cfg CatalogerConfig, resolver
 		version = canonicalPackageAndVersion[versionSeparator+1:]
 	}
 
-	var licenseSet pkg.LicenseSet
-
-	if cfg.SearchRemoteLicenses {
-		license, err := getLicenseFromNpmRegistry(cfg.NPMBaseURL, name, version)
-		if err == nil && license != "" {
-			licenseSet = pkg.NewLicenseSet(pkg.NewLicensesFromValuesWithContext(ctx, license)...)
-		}
-		if err != nil {
-			log.Debugf("unable to extract licenses from javascript package-lock.json for package %s:%s: %+v", name, version, err)
-		}
-	}
+	licenseSet := licenseResolver.getLicenses(ctx, name, version)
 
 	return finalizeLockPkg(
 		ctx,
@@ -130,19 +120,13 @@ func newPackageLockV1Package(ctx context.Context, cfg CatalogerConfig, resolver
 	)
 }
 
-func newPackageLockV2Package(ctx context.Context, cfg CatalogerConfig, resolver file.Resolver, location file.Location, name string, u lockPackage) pkg.Package {
+func newPackageLockV2Package(ctx context.Context, licenseResolver javascriptLicenseResolver, resolver file.Resolver, location file.Location, name string, u lockPackage) pkg.Package {
 	var licenseSet pkg.LicenseSet
 
 	if u.License != nil {
 		licenseSet = pkg.NewLicenseSet(pkg.NewLicensesFromLocationWithContext(ctx, location, u.License...)...)
-	} else if cfg.SearchRemoteLicenses {
-		license, err := getLicenseFromNpmRegistry(cfg.NPMBaseURL, name, u.Version)
-		if err == nil && license != "" {
-			licenseSet = pkg.NewLicenseSet(pkg.NewLicensesFromValuesWithContext(ctx, license)...)
-		}
-		if err != nil {
-			log.Debugf("unable to extract licenses from javascript package-lock.json for package %s:%s: %+v", name, u.Version, err)
-		}
+	} else {
+		licenseSet = licenseResolver.getLicenses(ctx, name, u.Version)
 	}
 
 	return finalizeLockPkg(
@@ -162,18 +146,9 @@ func newPackageLockV2Package(ctx context.Context, cfg CatalogerConfig, resolver
 	)
 }
 
-func newPnpmPackage(ctx context.Context, cfg CatalogerConfig, resolver file.Resolver, location file.Location, name, version string, integrity string, dependencies map[string]string) pkg.Package {
-	var licenseSet pkg.LicenseSet
+func newPnpmPackage(ctx context.Context, licenseResolver javascriptLicenseResolver, resolver file.Resolver, location file.Location, name, version string, integrity string, dependencies map[string]string) pkg.Package {
+	licenseSet := licenseResolver.getLicenses(ctx, name, version)
 
-	if cfg.SearchRemoteLicenses {
-		license, err := getLicenseFromNpmRegistry(cfg.NPMBaseURL, name, version)
-		if err == nil && license != "" {
-			licenseSet = pkg.NewLicenseSet(pkg.NewLicensesFromValuesWithContext(ctx, license)...)
-		}
-		if err != nil {
-			log.Debugf("unable to extract licenses from javascript pnpm-lock.yaml for package %s:%s: %+v", name, version, err)
-		}
-	}
 	return finalizeLockPkg(
 		ctx,
 		resolver,
@@ -191,18 +166,9 @@ func newPnpmPackage(ctx context.Context, cfg CatalogerConfig, resolver file.Reso
 	)
 }
 
-func newYarnLockPackage(ctx context.Context, cfg CatalogerConfig, resolver file.Resolver, location file.Location, name, version string, resolved string, integrity string, dependencies map[string]string) pkg.Package {
-	var licenseSet pkg.LicenseSet
+func newYarnLockPackage(ctx context.Context, licenseResolver javascriptLicenseResolver, resolver file.Resolver, location file.Location, name, version string, resolved string, integrity string, dependencies map[string]string) pkg.Package {
+	licenseSet := licenseResolver.getLicenses(ctx, name, version)
 
-	if cfg.SearchRemoteLicenses {
-		license, err := getLicenseFromNpmRegistry(cfg.NPMBaseURL, name, version)
-		if err == nil && license != "" {
-			licenseSet = pkg.NewLicenseSet(pkg.NewLicensesFromValuesWithContext(ctx, license)...)
-		}
-		if err != nil {
-			log.Debugf("unable to extract licenses from javascript yarn.lock for package %s:%s: %+v", name, version, err)
-		}
-	}
 	return finalizeLockPkg(
 		ctx,
 		resolver,
@@ -220,18 +186,9 @@ func newYarnLockPackage(ctx context.Context, cfg CatalogerConfig, resolver file.
 	)
 }
 
-func newBunPackage(ctx context.Context, cfg CatalogerConfig, resolver file.Resolver, location file.Location, name, version string, integrity string, metadata bunPackageMetadata) pkg.Package {
-	var licenseSet pkg.LicenseSet
+func newBunPackage(ctx context.Context, licenseResolver javascriptLicenseResolver, resolver file.Resolver, location file.Location, name, version string, integrity string, metadata bunPackageMetadata) pkg.Package {
+	licenseSet := licenseResolver.getLicenses(ctx, name, version)
 
-	if cfg.SearchRemoteLicenses {
-		license, err := getLicenseFromNpmRegistry(cfg.NPMBaseURL, name, version)
-		if err == nil && license != "" {
-			licenseSet = pkg.NewLicenseSet(pkg.NewLicensesFromValuesWithContext(ctx, license)...)
-		}
-		if err != nil {
-			log.Debugf("unable to extract licenses from javascript bun.lock for package %s:%s: %+v", name, version, err)
-		}
-	}
 	return finalizeLockPkg(
 		ctx,
 		resolver,

syft/pkg/cataloger/javascript/parse_bun_lock.go

@@ -53,12 +53,14 @@ type bunWorkspace struct {
 }
 
 type genericBunLockAdapter struct {
-	cfg CatalogerConfig
+	cfg             CatalogerConfig
+	licenseResolver javascriptLicenseResolver
 }
 
 func newGenericBunLockAdapter(cfg CatalogerConfig) genericBunLockAdapter {
 	return genericBunLockAdapter{
-		cfg: cfg,
+		cfg:             cfg,
+		licenseResolver: newJavascriptLicenseResolver(cfg),
 	}
 }
 
@@ -106,7 +108,7 @@ func (a genericBunLockAdapter) parseBunLock(ctx context.Context, resolver file.R
 		if devOnlyPkgs[p.Name] && !a.cfg.IncludeDevDependencies {
 			continue
 		}
-		packages = append(packages, newBunPackage(ctx, a.cfg, resolver, reader.Location, p.Name, p.Version, p.Integrity, p.Metadata))
+		packages = append(packages, newBunPackage(ctx, a.licenseResolver, resolver, reader.Location, p.Name, p.Version, p.Integrity, p.Metadata))
 	}
 
 	pkg.Sort(packages)

syft/pkg/cataloger/javascript/parse_package_lock.go

@@ -48,12 +48,14 @@ type lockPackage struct {
 type packageLockLicense []string
 
 type genericPackageLockAdapter struct {
-	cfg CatalogerConfig
+	cfg             CatalogerConfig
+	licenseResolver javascriptLicenseResolver
 }
 
 func newGenericPackageLockAdapter(cfg CatalogerConfig) genericPackageLockAdapter {
 	return genericPackageLockAdapter{
-		cfg: cfg,
+		cfg:             cfg,
+		licenseResolver: newJavascriptLicenseResolver(cfg),
 	}
 }
 
@@ -100,7 +102,7 @@ func (a genericPackageLockAdapter) parsePackageLock(ctx context.Context, resolve
 				name = pkgMeta.Name
 			}
 
-			newPkg := newPackageLockV2Package(ctx, a.cfg, resolver, reader.Location, getNameFromPath(name), pkgMeta)
+			newPkg := newPackageLockV2Package(ctx, a.licenseResolver, resolver, reader.Location, getNameFromPath(name), pkgMeta)
 			pkgs = append(pkgs, newPkg)
 		}
 	}
@@ -122,7 +124,7 @@ func (a genericPackageLockAdapter) packageLockV1Packages(ctx context.Context, re
 				continue
 			}
 
-			p := newPackageLockV1Package(ctx, a.cfg, resolver, location, name, pkgMeta)
+			p := newPackageLockV1Package(ctx, a.licenseResolver, resolver, location, name, pkgMeta)
 			if _, exists := seen[p.PURL]; !exists {
 				pkgs = append(pkgs, p)
 				seen[p.PURL] = struct{}{}

syft/pkg/cataloger/javascript/parse_pnpm_lock.go

@@ -74,12 +74,14 @@ type pnpmV9LockYaml struct {
 }
 
 type genericPnpmLockAdapter struct {
-	cfg CatalogerConfig
+	cfg             CatalogerConfig
+	licenseResolver javascriptLicenseResolver
 }
 
 func newGenericPnpmLockAdapter(cfg CatalogerConfig) genericPnpmLockAdapter {
 	return genericPnpmLockAdapter{
-		cfg: cfg,
+		cfg:             cfg,
+		licenseResolver: newJavascriptLicenseResolver(cfg),
 	}
 }
 
@@ -216,7 +218,7 @@ func (a genericPnpmLockAdapter) parsePnpmLock(ctx context.Context, resolver file
 		if p.Dev && !a.cfg.IncludeDevDependencies {
 			continue
 		}
-		packages = append(packages, newPnpmPackage(ctx, a.cfg, resolver, reader.Location, p.Name, p.Version, p.Integrity, p.Dependencies))
+		packages = append(packages, newPnpmPackage(ctx, a.licenseResolver, resolver, reader.Location, p.Name, p.Version, p.Integrity, p.Dependencies))
 	}
 
 	errs = unknown.Join(errs, unknown.IfEmptyf(packages, "unable to determine packages"))

syft/pkg/cataloger/javascript/parse_yarn_lock.go

@@ -67,12 +67,14 @@ type yarnV2PackageEntry struct {
 }
 
 type genericYarnLockAdapter struct {
-	cfg CatalogerConfig
+	cfg             CatalogerConfig
+	licenseResolver javascriptLicenseResolver
 }
 
 func newGenericYarnLockAdapter(cfg CatalogerConfig) genericYarnLockAdapter {
 	return genericYarnLockAdapter{
-		cfg: cfg,
+		cfg:             cfg,
+		licenseResolver: newJavascriptLicenseResolver(cfg),
 	}
 }
 
@@ -307,7 +309,7 @@ func (a genericYarnLockAdapter) parseYarnLock(ctx context.Context, resolver file
 		if devOnlyPkgs[p.Name] && !a.cfg.IncludeDevDependencies {
 			continue
 		}
-		packages = append(packages, newYarnLockPackage(ctx, a.cfg, resolver, reader.Location, p.Name, p.Version, p.Resolved, p.Integrity, p.Dependencies))
+		packages = append(packages, newYarnLockPackage(ctx, a.licenseResolver, resolver, reader.Location, p.Name, p.Version, p.Resolved, p.Integrity, p.Dependencies))
 	}
 
 	pkg.Sort(packages)