From 0b234b3256cd36dd1365487463f1a4833ec5a3cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C5=BEe=20Pe=C4=8Dar?= Date: Wed, 16 Sep 2026 02:32:28 +0500 Subject: [PATCH] perf(javascript): cache npm registry license lookups across runs (#5297) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 "/" 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 --- syft/pkg/cataloger/javascript/license.go | 53 +++++ syft/pkg/cataloger/javascript/license_test.go | 207 ++++++++++++++++++ syft/pkg/cataloger/javascript/package.go | 65 +----- .../cataloger/javascript/parse_bun_lock.go | 8 +- .../javascript/parse_package_lock.go | 10 +- .../cataloger/javascript/parse_pnpm_lock.go | 8 +- .../cataloger/javascript/parse_yarn_lock.go | 8 +- 7 files changed, 292 insertions(+), 67 deletions(-) create mode 100644 syft/pkg/cataloger/javascript/license.go create mode 100644 syft/pkg/cataloger/javascript/license_test.go diff --git a/syft/pkg/cataloger/javascript/license.go b/syft/pkg/cataloger/javascript/license.go new file mode 100644 index 000000000..bb3f5b70e --- /dev/null +++ b/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) + }) +} diff --git a/syft/pkg/cataloger/javascript/license_test.go b/syft/pkg/cataloger/javascript/license_test.go new file mode 100644 index 000000000..2bd827d5e --- /dev/null +++ b/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()) + }) + } +} diff --git a/syft/pkg/cataloger/javascript/package.go b/syft/pkg/cataloger/javascript/package.go index 27b0590fb..085f186dd 100644 --- a/syft/pkg/cataloger/javascript/package.go +++ b/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, diff --git a/syft/pkg/cataloger/javascript/parse_bun_lock.go b/syft/pkg/cataloger/javascript/parse_bun_lock.go index 50afd59d7..1ef6c00ba 100644 --- a/syft/pkg/cataloger/javascript/parse_bun_lock.go +++ b/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) diff --git a/syft/pkg/cataloger/javascript/parse_package_lock.go b/syft/pkg/cataloger/javascript/parse_package_lock.go index 4d12410ae..66c447d25 100644 --- a/syft/pkg/cataloger/javascript/parse_package_lock.go +++ b/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{}{} diff --git a/syft/pkg/cataloger/javascript/parse_pnpm_lock.go b/syft/pkg/cataloger/javascript/parse_pnpm_lock.go index eede2ccc8..707193957 100644 --- a/syft/pkg/cataloger/javascript/parse_pnpm_lock.go +++ b/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")) diff --git a/syft/pkg/cataloger/javascript/parse_yarn_lock.go b/syft/pkg/cataloger/javascript/parse_yarn_lock.go index 7c53bc556..9213be760 100644 --- a/syft/pkg/cataloger/javascript/parse_yarn_lock.go +++ b/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)