From 54bb8bdfb934d114b5570005853bf4bc0d40c609 Mon Sep 17 00:00:00 2001 From: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com> Date: Tue, 30 Jul 2024 12:31:39 +0600 Subject: [PATCH] fix(nodejs): detect direct dependencies when using `latest` version for files `yarn.lock` + `package.json` (#7110) --- pkg/dependency/parser/nodejs/yarn/parse.go | 27 ++++++--- .../parser/nodejs/yarn/parse_test.go | 20 +------ .../yarn/testdata/latest-version/package.json | 8 +++ .../yarn/testdata/latest-version/yarn.lock | 20 +++++++ .../analyzer/language/nodejs/yarn/yarn.go | 46 ++++++++++------ .../language/nodejs/yarn/yarn_test.go | 55 +++++++++++++++++++ 6 files changed, 132 insertions(+), 44 deletions(-) create mode 100644 pkg/fanal/analyzer/language/nodejs/yarn/testdata/latest-version/package.json create mode 100644 pkg/fanal/analyzer/language/nodejs/yarn/testdata/latest-version/yarn.lock diff --git a/pkg/dependency/parser/nodejs/yarn/parse.go b/pkg/dependency/parser/nodejs/yarn/parse.go index 813554730bea..fbe77913a1e4 100644 --- a/pkg/dependency/parser/nodejs/yarn/parse.go +++ b/pkg/dependency/parser/nodejs/yarn/parse.go @@ -5,6 +5,7 @@ import ( "bytes" "io" "regexp" + "sort" "strings" "github.com/samber/lo" @@ -127,7 +128,7 @@ func ignoreProtocol(protocol string) bool { return false } -func parseResults(patternIDs map[string]string, dependsOn map[string][]string) (deps []ftypes.Dependency) { +func parseResults(patternIDs map[string]string, dependsOn map[string][]string) (deps ftypes.Dependencies) { // find dependencies by patterns for pkgID, depPatterns := range dependsOn { depIDs := lo.Map(depPatterns, func(pattern string, index int) string { @@ -269,14 +270,20 @@ func parseDependency(line string) (string, error) { } } -func (p *Parser) Parse(r xio.ReadSeekerAt) ([]ftypes.Package, []ftypes.Dependency, error) { +func (p *Parser) Parse(r xio.ReadSeekerAt) ([]ftypes.Package, []ftypes.Dependency, map[string][]string, error) { lineNumber := 1 - var pkgs []ftypes.Package + var pkgs ftypes.Packages - // patternIDs holds mapping between patterns and library IDs + // patternIDs holds mapping between patterns and package IDs // e.g. ajv@^6.5.5 => ajv@6.10.0 + // This is needed to update dependencies from `DependsOn`. patternIDs := make(map[string]string) + // patternIDs holds mapping between package ID and patterns + // e.g. `@babel/helper-regex@7.4.4` => [`@babel/helper-regex@^7.0.0`, `@babel/helper-regex@^7.4.4`] + // This is needed to compare package patterns with patterns from package.json files in `fanal` package. + pkgIDPatterns := make(map[string][]string) + scanner := bufio.NewScanner(r) scanner.Split(p.scanBlocks) dependsOn := make(map[string][]string) @@ -285,7 +292,7 @@ func (p *Parser) Parse(r xio.ReadSeekerAt) ([]ftypes.Package, []ftypes.Dependenc lib, deps, newLine, err := p.parseBlock(block, lineNumber) lineNumber = newLine + 2 if err != nil { - return nil, nil, err + return nil, nil, nil, err } else if lib.Name == "" { continue } @@ -298,6 +305,7 @@ func (p *Parser) Parse(r xio.ReadSeekerAt) ([]ftypes.Package, []ftypes.Dependenc Locations: []ftypes.Location{lib.Location}, }) + pkgIDPatterns[pkgID] = lib.Patterns for _, pattern := range lib.Patterns { // e.g. // combined-stream@^1.0.6 => combined-stream@1.0.8 @@ -310,13 +318,16 @@ func (p *Parser) Parse(r xio.ReadSeekerAt) ([]ftypes.Package, []ftypes.Dependenc } if err := scanner.Err(); err != nil { - return nil, nil, xerrors.Errorf("failed to scan yarn.lock, got scanner error: %s", err.Error()) + return nil, nil, nil, xerrors.Errorf("failed to scan yarn.lock, got scanner error: %s", err.Error()) } - // Replace dependency patterns with library IDs + // Replace dependency patterns with package IDs // e.g. ajv@^6.5.5 => ajv@6.10.0 deps := parseResults(patternIDs, dependsOn) - return pkgs, deps, nil + + sort.Sort(pkgs) + sort.Sort(deps) + return pkgs, deps, pkgIDPatterns, nil } func packageID(name, version string) string { diff --git a/pkg/dependency/parser/nodejs/yarn/parse_test.go b/pkg/dependency/parser/nodejs/yarn/parse_test.go index 275514351954..e5122882636d 100644 --- a/pkg/dependency/parser/nodejs/yarn/parse_test.go +++ b/pkg/dependency/parser/nodejs/yarn/parse_test.go @@ -301,32 +301,14 @@ func TestParse(t *testing.T) { f, err := os.Open(tt.file) require.NoError(t, err) - got, deps, err := NewParser().Parse(f) + got, deps, _, err := NewParser().Parse(f) require.NoError(t, err) - sortPkgs(got) - sortPkgs(tt.want) assert.Equal(t, tt.want, got) if tt.wantDeps != nil { - sortDeps(deps) - sortDeps(tt.wantDeps) assert.Equal(t, tt.wantDeps, deps) } }) } } - -func sortPkgs(pkgs ftypes.Packages) { - sort.Sort(pkgs) - for _, pkg := range pkgs { - sort.Sort(pkg.Locations) - } -} - -func sortDeps(deps ftypes.Dependencies) { - sort.Sort(deps) - for _, dep := range deps { - sort.Strings(dep.DependsOn) - } -} diff --git a/pkg/fanal/analyzer/language/nodejs/yarn/testdata/latest-version/package.json b/pkg/fanal/analyzer/language/nodejs/yarn/testdata/latest-version/package.json new file mode 100644 index 000000000000..211be0da7db9 --- /dev/null +++ b/pkg/fanal/analyzer/language/nodejs/yarn/testdata/latest-version/package.json @@ -0,0 +1,8 @@ +{ + "dependencies": { + "debug": "latest" + }, + "devDependencies" : { + "js-tokens": "^9.0.0" + } +} \ No newline at end of file diff --git a/pkg/fanal/analyzer/language/nodejs/yarn/testdata/latest-version/yarn.lock b/pkg/fanal/analyzer/language/nodejs/yarn/testdata/latest-version/yarn.lock new file mode 100644 index 000000000000..9f7b3d06857c --- /dev/null +++ b/pkg/fanal/analyzer/language/nodejs/yarn/testdata/latest-version/yarn.lock @@ -0,0 +1,20 @@ +# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 + + +debug@latest: + version "4.3.5" + resolved "https://registry.npmjs.org/debug/-/debug-4.3.5.tgz" + integrity sha512-pt0bNEmneDIvdL1Xsd9oDQ/wrQRkXDT4AUWlNZNPKvW5x/jyO9VFXkJUP07vQ2upmw5PlaITaPKc31jK13V+jg== + dependencies: + ms "2.1.2" + +js-tokens@^9.0.0: + version "9.0.0" + resolved "https://registry.npmjs.org/js-tokens/-/js-tokens-9.0.0.tgz" + integrity sha512-WriZw1luRMlmV3LGJaR6QOJjWwgLUTf89OwT2lUOyjX2dJGBwgmIkbcz+7WFZjrZM635JOIR517++e/67CP9dQ== + +ms@2.1.2: + version "2.1.2" + resolved "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz" + integrity sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w== \ No newline at end of file diff --git a/pkg/fanal/analyzer/language/nodejs/yarn/yarn.go b/pkg/fanal/analyzer/language/nodejs/yarn/yarn.go index 687d147a63bc..984f72983ec9 100644 --- a/pkg/fanal/analyzer/language/nodejs/yarn/yarn.go +++ b/pkg/fanal/analyzer/language/nodejs/yarn/yarn.go @@ -10,6 +10,7 @@ import ( "path" "path/filepath" "regexp" + "slices" "sort" "strings" @@ -17,6 +18,7 @@ import ( "github.com/samber/lo" "golang.org/x/xerrors" + "github.com/aquasecurity/trivy/pkg/dependency" "github.com/aquasecurity/trivy/pkg/dependency/parser/nodejs/packagejson" "github.com/aquasecurity/trivy/pkg/dependency/parser/nodejs/yarn" "github.com/aquasecurity/trivy/pkg/detector/library/compare/npm" @@ -42,7 +44,6 @@ var fragmentRegexp = regexp.MustCompile(`(\S+):(@?.*?)(@(.*?)|)$`) type yarnAnalyzer struct { logger *log.Logger packageJsonParser *packagejson.Parser - lockParser language.Parser comparer npm.Comparer license *license.License } @@ -51,12 +52,21 @@ func newYarnAnalyzer(opt analyzer.AnalyzerOptions) (analyzer.PostAnalyzer, error return &yarnAnalyzer{ logger: log.WithPrefix("yarn"), packageJsonParser: packagejson.NewParser(), - lockParser: yarn.NewParser(), comparer: npm.Comparer{}, license: license.NewLicense(opt.LicenseScannerOption.ClassifierConfidenceLevel), }, nil } +type parserWithPatterns struct { + patterns map[string][]string +} + +func (p *parserWithPatterns) Parse(r xio.ReadSeekerAt) ([]types.Package, []types.Dependency, error) { + pkgs, deps, patterns, err := yarn.NewParser().Parse(r) + p.patterns = patterns + return pkgs, deps, err +} + func (a yarnAnalyzer) PostAnalyze(_ context.Context, input analyzer.PostAnalysisInput) (*analyzer.AnalysisResult, error) { var apps []types.Application @@ -65,8 +75,9 @@ func (a yarnAnalyzer) PostAnalyze(_ context.Context, input analyzer.PostAnalysis } err := fsutils.WalkDir(input.FS, ".", required, func(filePath string, d fs.DirEntry, r io.Reader) error { + parser := &parserWithPatterns{} // Parse yarn.lock - app, err := a.parseYarnLock(filePath, r) + app, err := language.Parse(types.Yarn, filePath, r, parser) if err != nil { return xerrors.Errorf("parse error: %w", err) } else if app == nil { @@ -79,7 +90,7 @@ func (a yarnAnalyzer) PostAnalyze(_ context.Context, input analyzer.PostAnalysis } // Parse package.json alongside yarn.lock to find direct deps and mark dev deps - if err = a.analyzeDependencies(input.FS, path.Dir(filePath), app); err != nil { + if err = a.analyzeDependencies(input.FS, path.Dir(filePath), app, parser.patterns); err != nil { a.logger.Warn("Unable to parse package.json to remove dev dependencies", log.FilePath(path.Join(path.Dir(filePath), types.NpmPkg)), log.Err(err)) } @@ -147,13 +158,9 @@ func (a yarnAnalyzer) Version() int { return version } -func (a yarnAnalyzer) parseYarnLock(filePath string, r io.Reader) (*types.Application, error) { - return language.Parse(types.Yarn, filePath, r, a.lockParser) -} - // analyzeDependencies analyzes the package.json file next to yarn.lock, // distinguishing between direct and transitive dependencies as well as production and development dependencies. -func (a yarnAnalyzer) analyzeDependencies(fsys fs.FS, dir string, app *types.Application) error { +func (a yarnAnalyzer) analyzeDependencies(fsys fs.FS, dir string, app *types.Application, patterns map[string][]string) error { packageJsonPath := path.Join(dir, types.NpmPkg) directDeps, directDevDeps, err := a.parsePackageJsonDependencies(fsys, packageJsonPath) if errors.Is(err, fs.ErrNotExist) { @@ -170,13 +177,13 @@ func (a yarnAnalyzer) analyzeDependencies(fsys fs.FS, dir string, app *types.App }) // Walk prod dependencies - pkgs, err := a.walkDependencies(app.Packages, pkgIDs, directDeps, false) + pkgs, err := a.walkDependencies(app.Packages, pkgIDs, directDeps, patterns, false) if err != nil { return xerrors.Errorf("unable to walk dependencies: %w", err) } // Walk dev dependencies - devPkgs, err := a.walkDependencies(app.Packages, pkgIDs, directDevDeps, true) + devPkgs, err := a.walkDependencies(app.Packages, pkgIDs, directDevDeps, patterns, true) if err != nil { return xerrors.Errorf("unable to walk dependencies: %w", err) } @@ -194,7 +201,7 @@ func (a yarnAnalyzer) analyzeDependencies(fsys fs.FS, dir string, app *types.App } func (a yarnAnalyzer) walkDependencies(pkgs []types.Package, pkgIDs map[string]types.Package, - directDeps map[string]string, dev bool) (map[string]types.Package, error) { + directDeps map[string]string, patterns map[string][]string, dev bool) (map[string]types.Package, error) { // Identify direct dependencies directPkgs := make(map[string]types.Package) @@ -211,11 +218,16 @@ func (a yarnAnalyzer) walkDependencies(pkgs []types.Package, pkgIDs map[string]t constraint = m[4] } - // npm has own comparer to compare versions - if match, err := a.comparer.MatchVersion(pkg.Version, constraint); err != nil { - return nil, xerrors.Errorf("unable to match version for %s", pkg.Name) - } else if !match { - continue + // Try to find an exact match to the pattern. + // In some cases, patterns from yarn.lock and package.json may not match (e.g., yarn v2 uses the allowed version for ID). + // Therefore, if the patterns don't match - compare versions. + if pkgPatterns, found := patterns[pkg.ID]; !found || !slices.Contains(pkgPatterns, dependency.ID(types.Yarn, pkg.Name, constraint)) { + // npm has own comparer to compare versions + if match, err := a.comparer.MatchVersion(pkg.Version, constraint); err != nil { + return nil, xerrors.Errorf("unable to match version for %s", pkg.Name) + } else if !match { + continue + } } // Mark as a direct dependency diff --git a/pkg/fanal/analyzer/language/nodejs/yarn/yarn_test.go b/pkg/fanal/analyzer/language/nodejs/yarn/yarn_test.go index 201629335b7a..6684f0fa4740 100644 --- a/pkg/fanal/analyzer/language/nodejs/yarn/yarn_test.go +++ b/pkg/fanal/analyzer/language/nodejs/yarn/yarn_test.go @@ -354,6 +354,61 @@ func Test_yarnLibraryAnalyzer_Analyze(t *testing.T) { }, }, }, + { + name: "package uses `latest` version", + dir: "testdata/latest-version", + want: &analyzer.AnalysisResult{ + Applications: []types.Application{ + { + Type: types.Yarn, + FilePath: "yarn.lock", + Packages: types.Packages{ + { + ID: "debug@4.3.5", + Name: "debug", + Version: "4.3.5", + Relationship: types.RelationshipDirect, + Locations: []types.Location{ + { + StartLine: 5, + EndLine: 10, + }, + }, + DependsOn: []string{ + "ms@2.1.2", + }, + }, + { + ID: "js-tokens@9.0.0", + Name: "js-tokens", + Version: "9.0.0", + Relationship: types.RelationshipDirect, + Dev: true, + Locations: []types.Location{ + { + StartLine: 12, + EndLine: 15, + }, + }, + }, + { + ID: "ms@2.1.2", + Name: "ms", + Version: "2.1.2", + Indirect: true, + Relationship: types.RelationshipIndirect, + Locations: []types.Location{ + { + StartLine: 17, + EndLine: 20, + }, + }, + }, + }, + }, + }, + }, + }, { name: "happy path with alias rewrite", dir: "testdata/alias",