From 24a3e547d9d2c44e505590e16f4354a09bb4a056 Mon Sep 17 00:00:00 2001 From: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com> Date: Sun, 23 Jul 2023 19:07:49 +0600 Subject: [PATCH] feat(nodejs): add support for include-dev-deps flag for yarn (#4812) * add support for include-dev-deps flag * remove go.mod replace * refactor * bump go-dep-parser --------- Co-authored-by: knqyf263 --- .../configuration/cli/trivy_filesystem.md | 2 +- .../configuration/cli/trivy_repository.md | 1 + .../scanner/vulnerability/language/nodejs.md | 2 + go.mod | 2 +- go.sum | 4 +- pkg/commands/app.go | 1 - .../analyzer/language/nodejs/yarn/yarn.go | 70 +++++++++------ .../language/nodejs/yarn/yarn_test.go | 85 +++++++++++++++++++ pkg/flag/scan_flags.go | 2 +- 9 files changed, 139 insertions(+), 30 deletions(-) diff --git a/docs/docs/references/configuration/cli/trivy_filesystem.md b/docs/docs/references/configuration/cli/trivy_filesystem.md index 7b1f799fb87..c5f53c9e429 100644 --- a/docs/docs/references/configuration/cli/trivy_filesystem.md +++ b/docs/docs/references/configuration/cli/trivy_filesystem.md @@ -43,7 +43,7 @@ trivy filesystem [flags] PATH --ignore-unfixed display only fixed vulnerabilities --ignored-licenses strings specify a list of license to ignore --ignorefile string specify .trivyignore file (default ".trivyignore") - --include-dev-deps include development dependencies in the report (supported: npm) + --include-dev-deps include development dependencies in the report (supported: npm, yarn) --include-non-failures include successes and exceptions, available with '--scanners config' --java-db-repository string OCI repository to retrieve trivy-java-db from (default "ghcr.io/aquasecurity/trivy-java-db") --license-confidence-level float specify license classifier's confidence level (default 0.9) diff --git a/docs/docs/references/configuration/cli/trivy_repository.md b/docs/docs/references/configuration/cli/trivy_repository.md index a8b93f903ad..7fbf3fd768c 100644 --- a/docs/docs/references/configuration/cli/trivy_repository.md +++ b/docs/docs/references/configuration/cli/trivy_repository.md @@ -41,6 +41,7 @@ trivy repository [flags] REPO_URL --ignore-unfixed display only fixed vulnerabilities --ignored-licenses strings specify a list of license to ignore --ignorefile string specify .trivyignore file (default ".trivyignore") + --include-dev-deps include development dependencies in the report (supported: npm, yarn) --include-non-failures include successes and exceptions, available with '--scanners config' --java-db-repository string OCI repository to retrieve trivy-java-db from (default "ghcr.io/aquasecurity/trivy-java-db") --license-confidence-level float specify license classifier's confidence level (default 0.9) diff --git a/docs/docs/scanner/vulnerability/language/nodejs.md b/docs/docs/scanner/vulnerability/language/nodejs.md index 99fa4d444a8..f39c4cad2b1 100644 --- a/docs/docs/scanner/vulnerability/language/nodejs.md +++ b/docs/docs/scanner/vulnerability/language/nodejs.md @@ -35,6 +35,8 @@ By default, Trivy doesn't report development dependencies. Use the `--include-de Trivy parses `yarn.lock`, which doesn't contain information about development dependencies. To exclude devDependencies, `package.json` also needs to be present next to `yarn.lock`. +By default, Trivy doesn't report development dependencies. Use the `--include-dev-deps` flag to include them. + ### pnpm Trivy parses `pnpm-lock.yaml`, then finds production dependencies and builds a [tree] of dependencies with vulnerabilities. diff --git a/go.mod b/go.mod index 2e822a65563..2f9304c680d 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/alicebob/miniredis/v2 v2.30.4 github.com/aquasecurity/bolt-fixtures v0.0.0-20200903104109-d34e7f983986 github.com/aquasecurity/defsec v0.90.4-0.20230716083016-931764ac907f - github.com/aquasecurity/go-dep-parser v0.0.0-20230627073354-fb7eb3159bd5 + github.com/aquasecurity/go-dep-parser v0.0.0-20230713131216-85ebd0d79cd3 github.com/aquasecurity/go-gem-version v0.0.0-20201115065557-8eed6fe000ce github.com/aquasecurity/go-npm-version v0.0.0-20201110091526-0b796d180798 github.com/aquasecurity/go-pep440-version v0.0.0-20210121094942-22b2f8951d46 diff --git a/go.sum b/go.sum index ead16ba9468..e0d37db5efc 100644 --- a/go.sum +++ b/go.sum @@ -323,8 +323,8 @@ github.com/aquasecurity/bolt-fixtures v0.0.0-20200903104109-d34e7f983986 h1:2a30 github.com/aquasecurity/bolt-fixtures v0.0.0-20200903104109-d34e7f983986/go.mod h1:NT+jyeCzXk6vXR5MTkdn4z64TgGfE5HMLC8qfj5unl8= github.com/aquasecurity/defsec v0.90.4-0.20230716083016-931764ac907f h1:JQnhl5zK5cBJKPbCLdvK0ialSkwvp+z1B9rY61SRxNI= github.com/aquasecurity/defsec v0.90.4-0.20230716083016-931764ac907f/go.mod h1:VPkgjZz3dx3znIIVLZgbtFhSzN9aZC2409s5V5Oqb7o= -github.com/aquasecurity/go-dep-parser v0.0.0-20230627073354-fb7eb3159bd5 h1:FA5XM/KP1l+PYH+QafFzzBjdsT+WxWTWsYGPzKrMeAQ= -github.com/aquasecurity/go-dep-parser v0.0.0-20230627073354-fb7eb3159bd5/go.mod h1:VjG2wX19QDny5yKN+he0v9wuZjF0k+00173mh0FJCVU= +github.com/aquasecurity/go-dep-parser v0.0.0-20230713131216-85ebd0d79cd3 h1:btZmyXc4e4wDNBEI4guYzpCMeNPM0f8p0F/IzSsoP0M= +github.com/aquasecurity/go-dep-parser v0.0.0-20230713131216-85ebd0d79cd3/go.mod h1:Cl6aYro+Ddzh1MB451j/C6rvwKdn/Ifa7z98sFirJ9I= github.com/aquasecurity/go-gem-version v0.0.0-20201115065557-8eed6fe000ce h1:QgBRgJvtEOBtUXilDb1MLi1p1MWoyFDXAu5DEUl5nwM= github.com/aquasecurity/go-gem-version v0.0.0-20201115065557-8eed6fe000ce/go.mod h1:HXgVzOPvXhVGLJs4ZKO817idqr/xhwsTcj17CLYY74s= github.com/aquasecurity/go-mock-aws v0.0.0-20230328195059-5bf52338aec3 h1:Vt9y1gZS5JGY3tsL9zc++Cg4ofX51CG7PaMyC5SXWPg= diff --git a/pkg/commands/app.go b/pkg/commands/app.go index cc9991af7d4..97f3fb6bef1 100644 --- a/pkg/commands/app.go +++ b/pkg/commands/app.go @@ -463,7 +463,6 @@ func NewRepositoryCommand(globalFlags *flag.GlobalFlagGroup) *cobra.Command { repoFlags.ReportFlagGroup.ReportFormat = nil // TODO: support --report summary repoFlags.ReportFlagGroup.Compliance = nil // disable '--compliance' repoFlags.ReportFlagGroup.ExitOnEOL = nil // disable '--exit-on-eol' - repoFlags.ScanFlagGroup.IncludeDevDeps = nil // disable '--include-dev-deps' cmd := &cobra.Command{ Use: "repository [flags] REPO_URL", diff --git a/pkg/fanal/analyzer/language/nodejs/yarn/yarn.go b/pkg/fanal/analyzer/language/nodejs/yarn/yarn.go index aafb69c0183..3d9582189b1 100644 --- a/pkg/fanal/analyzer/language/nodejs/yarn/yarn.go +++ b/pkg/fanal/analyzer/language/nodejs/yarn/yarn.go @@ -60,8 +60,8 @@ func (a yarnAnalyzer) PostAnalyze(_ context.Context, input analyzer.PostAnalysis return nil } - // Parse package.json alongside yarn.lock to remove dev dependencies - if err = a.removeDevDependencies(input.FS, filepath.Dir(path), app); err != nil { + // Parse package.json alongside yarn.lock to find direct deps and mark dev deps + if err = a.analyzeDependencies(input.FS, filepath.Dir(path), app); err != nil { log.Logger.Warnf("Unable to parse %q to remove dev dependencies: %s", filepath.Join(filepath.Dir(path), types.NpmPkg), err) } apps = append(apps, *app) @@ -94,9 +94,11 @@ func (a yarnAnalyzer) parseYarnLock(path string, r dio.ReadSeekerAt) (*types.App return language.Parse(types.Yarn, path, r, a.lockParser) } -func (a yarnAnalyzer) removeDevDependencies(fsys fs.FS, dir string, app *types.Application) error { +// 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 { packageJsonPath := filepath.Join(dir, types.NpmPkg) - directDeps, err := a.parsePackageJsonDependencies(fsys, packageJsonPath) + directDeps, directDevDeps, err := a.parsePackageJsonDependencies(fsys, packageJsonPath) if errors.Is(err, fs.ErrNotExist) { log.Logger.Debugf("Yarn: %s not found", packageJsonPath) return nil @@ -110,38 +112,55 @@ func (a yarnAnalyzer) removeDevDependencies(fsys fs.FS, dir string, app *types.A return pkg.ID, pkg }) + // Walk prod dependencies + pkgs, err := a.walkDependencies(app.Libraries, pkgIDs, directDeps, false) + if err != nil { + return xerrors.Errorf("unable to walk dependencies: %w", err) + } + + // Walk dev dependencies + devPkgs, err := a.walkDependencies(app.Libraries, pkgIDs, directDevDeps, true) + if err != nil { + return xerrors.Errorf("unable to walk dependencies: %w", err) + } + + // Merge prod and dev dependencies. + // If the same package is found in both prod and dev dependencies, use the one in prod. + pkgs = lo.Assign(devPkgs, pkgs) + + pkgSlice := maps.Values(pkgs) + sort.Sort(types.Packages(pkgSlice)) + + // Save libraries + app.Libraries = pkgSlice + return nil +} + +func (a yarnAnalyzer) walkDependencies(libs []types.Package, pkgIDs map[string]types.Package, + directDeps map[string]string, dev bool) (map[string]types.Package, error) { + // Identify direct dependencies pkgs := map[string]types.Package{} - for name, constraint := range directDeps { - for _, pkg := range app.Libraries { - if pkg.Name != name { - continue - } - + for _, pkg := range libs { + if constraint, ok := directDeps[pkg.Name]; ok { // npm has own comparer to compare versions if match, err := a.comparer.MatchVersion(pkg.Version, constraint); err != nil { - return xerrors.Errorf("unable to match version for %s", pkg.Name) + return nil, xerrors.Errorf("unable to match version for %s", pkg.Name) } else if match { // Mark as a direct dependency pkg.Indirect = false + pkg.Dev = dev pkgs[pkg.ID] = pkg - break } } } // Walk indirect dependencies - // Since it starts from direct dependencies, devDependencies will not appear in this walk. for _, pkg := range pkgs { a.walkIndirectDependencies(pkg, pkgIDs, pkgs) } - pkgSlice := maps.Values(pkgs) - sort.Sort(types.Packages(pkgSlice)) - - // Save only prod libraries - app.Libraries = pkgSlice - return nil + return pkgs, nil } func (a yarnAnalyzer) walkIndirectDependencies(pkg types.Package, pkgIDs map[string]types.Package, deps map[string]types.Package) { @@ -156,38 +175,41 @@ func (a yarnAnalyzer) walkIndirectDependencies(pkg types.Package, pkgIDs map[str } dep.Indirect = true + dep.Dev = pkg.Dev deps[dep.ID] = dep a.walkIndirectDependencies(dep, pkgIDs, deps) } } -func (a yarnAnalyzer) parsePackageJsonDependencies(fsys fs.FS, path string) (map[string]string, error) { +func (a yarnAnalyzer) parsePackageJsonDependencies(fsys fs.FS, path string) (map[string]string, map[string]string, error) { // Parse package.json f, err := fsys.Open(path) if err != nil { - return nil, xerrors.Errorf("file open error: %w", err) + return nil, nil, xerrors.Errorf("file open error: %w", err) } defer func() { _ = f.Close() }() rootPkg, err := a.packageJsonParser.Parse(f) if err != nil { - return nil, xerrors.Errorf("parse error: %w", err) + return nil, nil, xerrors.Errorf("parse error: %w", err) } // Merge dependencies and optionalDependencies dependencies := lo.Assign(rootPkg.Dependencies, rootPkg.OptionalDependencies) + devDependencies := rootPkg.DevDependencies if len(rootPkg.Workspaces) > 0 { pkgs, err := a.traverseWorkspaces(fsys, rootPkg.Workspaces) if err != nil { - return nil, xerrors.Errorf("traverse workspaces error: %w", err) + return nil, nil, xerrors.Errorf("traverse workspaces error: %w", err) } for _, pkg := range pkgs { dependencies = lo.Assign(dependencies, pkg.Dependencies, pkg.OptionalDependencies) + devDependencies = lo.Assign(devDependencies, pkg.DevDependencies) } } - return dependencies, nil + return dependencies, devDependencies, nil } func (a yarnAnalyzer) traverseWorkspaces(fsys fs.FS, workspaces []string) ([]packagejson.Package, error) { diff --git a/pkg/fanal/analyzer/language/nodejs/yarn/yarn_test.go b/pkg/fanal/analyzer/language/nodejs/yarn/yarn_test.go index f7f7c65ffb0..78789b0d7ff 100644 --- a/pkg/fanal/analyzer/language/nodejs/yarn/yarn_test.go +++ b/pkg/fanal/analyzer/language/nodejs/yarn/yarn_test.go @@ -77,6 +77,36 @@ func Test_yarnLibraryAnalyzer_Analyze(t *testing.T) { }, }, }, + { + ID: "prop-types@15.7.2", + Name: "prop-types", + Version: "15.7.2", + Dev: true, + Locations: []types.Location{ + { + StartLine: 27, + EndLine: 34, + }, + }, + DependsOn: []string{ + "loose-envify@1.4.0", + "object-assign@4.1.1", + "react-is@16.13.1", + }, + }, + { + ID: "react-is@16.13.1", + Name: "react-is", + Version: "16.13.1", + Dev: true, + Indirect: true, + Locations: []types.Location{ + { + StartLine: 36, + EndLine: 39, + }, + }, + }, { ID: "scheduler@0.13.6", Name: "scheduler", @@ -310,6 +340,61 @@ func Test_yarnLibraryAnalyzer_Analyze(t *testing.T) { }, }, }, + { + ID: "object-assign@4.1.1", + Name: "object-assign", + Version: "4.1.1", + Indirect: true, + Dev: true, + Locations: []types.Location{ + { + StartLine: 64, + EndLine: 69, + }, + }, + }, + { + ID: "prettier@2.8.8", + Name: "prettier", + Version: "2.8.8", + Dev: true, + Locations: []types.Location{ + { + StartLine: 87, + EndLine: 94, + }, + }, + }, + { + ID: "prop-types@15.8.1", + Name: "prop-types", + Version: "15.8.1", + Dev: true, + Locations: []types.Location{ + { + StartLine: 96, + EndLine: 105, + }, + }, + DependsOn: []string{ + "loose-envify@1.4.0", + "object-assign@4.1.1", + "react-is@16.13.1", + }, + }, + { + ID: "react-is@16.13.1", + Name: "react-is", + Version: "16.13.1", + Dev: true, + Indirect: true, + Locations: []types.Location{ + { + StartLine: 107, + EndLine: 112, + }, + }, + }, { ID: "scheduler@0.23.0", Name: "scheduler", diff --git a/pkg/flag/scan_flags.go b/pkg/flag/scan_flags.go index 0dd8fb1ca34..e031c68e9c3 100644 --- a/pkg/flag/scan_flags.go +++ b/pkg/flag/scan_flags.go @@ -74,7 +74,7 @@ var ( Name: "include-dev-deps", ConfigName: "include-dev-deps", Default: false, - Usage: "include development dependencies in the report (supported: npm)", + Usage: "include development dependencies in the report (supported: npm, yarn)", } )