Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(nodejs): detect direct dependencies when using latest version for files yarn.lock + package.json #7110

Merged
merged 3 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions pkg/dependency/parser/nodejs/yarn/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"bytes"
"io"
"regexp"
"sort"
"strings"

"github.com/samber/lo"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 => [email protected]
// This is needed to update dependencies from `DependsOn`.
patternIDs := make(map[string]string)

// patternIDs holds mapping between package ID and patterns
// e.g. `@babel/[email protected]` => [`@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)
Expand All @@ -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
}
Expand All @@ -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 => [email protected]
Expand All @@ -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 => [email protected]
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 {
Expand Down
20 changes: 1 addition & 19 deletions pkg/dependency/parser/nodejs/yarn/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"dependencies": {
"debug": "latest"
},
"devDependencies" : {
"js-tokens": "^9.0.0"
}
}
Original file line number Diff line number Diff line change
@@ -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==

[email protected]:
version "2.1.2"
resolved "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz"
integrity sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==
46 changes: 29 additions & 17 deletions pkg/fanal/analyzer/language/nodejs/yarn/yarn.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ import (
"path"
"path/filepath"
"regexp"
"slices"
"sort"
"strings"

"github.com/hashicorp/go-multierror"
"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"
Expand All @@ -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
}
Expand All @@ -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

Expand All @@ -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 {
Expand All @@ -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))
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
}
Expand All @@ -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)
Expand All @@ -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).
knqyf263 marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down
55 changes: 55 additions & 0 deletions pkg/fanal/analyzer/language/nodejs/yarn/yarn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: "[email protected]",
Name: "debug",
Version: "4.3.5",
Relationship: types.RelationshipDirect,
Locations: []types.Location{
{
StartLine: 5,
EndLine: 10,
},
},
DependsOn: []string{
"[email protected]",
},
},
{
ID: "[email protected]",
Name: "js-tokens",
Version: "9.0.0",
Relationship: types.RelationshipDirect,
Dev: true,
Locations: []types.Location{
{
StartLine: 12,
EndLine: 15,
},
},
},
{
ID: "[email protected]",
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",
Expand Down