From 7d083bc890eccc3bf32765c6d7e922cab2e2ef94 Mon Sep 17 00:00:00 2001 From: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com> Date: Wed, 5 Jun 2024 13:38:42 +0600 Subject: [PATCH] fix(nodejs): fix infinity loops for `pnpm` with cyclic imports (#6857) --- pkg/dependency/parser/nodejs/pnpm/parse.go | 13 ++-- .../parser/nodejs/pnpm/parse_test.go | 12 ++++ .../parser/nodejs/pnpm/parse_testcase.go | 64 ++++++++++++++++++ .../testdata/pnpm-lock_v9_cyclic_import.yaml | 67 +++++++++++++++++++ 4 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 pkg/dependency/parser/nodejs/pnpm/testdata/pnpm-lock_v9_cyclic_import.yaml diff --git a/pkg/dependency/parser/nodejs/pnpm/parse.go b/pkg/dependency/parser/nodejs/pnpm/parse.go index aad4e138b9fe..bbe6c5a57aab 100644 --- a/pkg/dependency/parser/nodejs/pnpm/parse.go +++ b/pkg/dependency/parser/nodejs/pnpm/parse.go @@ -183,7 +183,7 @@ func (p *Parser) parseV9(lockFile LockFile) ([]ftypes.Package, []ftypes.Dependen if dep, ok := lockFile.Importers.Root.DevDependencies[name]; ok && dep.Version == ver { relationship = ftypes.RelationshipDirect } - if dep, ok := lockFile.Importers.Root.Dependencies[name]; ok && dep.Version == ver { + if dep, ok := lockFile.Importers.Root.Dependencies[name]; ok && p.trimPeerDeps(dep.Version, lockVer) == ver { relationship = ftypes.RelationshipDirect dev = false // mark root direct deps to update `dev` field of their child deps. } @@ -208,10 +208,11 @@ func (p *Parser) parseV9(lockFile LockFile) ([]ftypes.Package, []ftypes.Dependen } } + visited := make(map[string]struct{}) // Overwrite the `Dev` field for dev deps and their child dependencies. for _, pkg := range resolvedPkgs { if !pkg.Dev { - p.markRootPkgs(pkg.ID, resolvedPkgs, resolvedDeps) + p.markRootPkgs(pkg.ID, resolvedPkgs, resolvedDeps, visited) } } @@ -219,7 +220,10 @@ func (p *Parser) parseV9(lockFile LockFile) ([]ftypes.Package, []ftypes.Dependen } // markRootPkgs sets `Dev` to false for non dev dependency. -func (p *Parser) markRootPkgs(id string, pkgs map[string]ftypes.Package, deps map[string]ftypes.Dependency) { +func (p *Parser) markRootPkgs(id string, pkgs map[string]ftypes.Package, deps map[string]ftypes.Dependency, visited map[string]struct{}) { + if _, ok := visited[id]; ok { + return + } pkg, ok := pkgs[id] if !ok { return @@ -227,10 +231,11 @@ func (p *Parser) markRootPkgs(id string, pkgs map[string]ftypes.Package, deps ma pkg.Dev = false pkgs[id] = pkg + visited[id] = struct{}{} // Update child deps for _, depID := range deps[id].DependsOn { - p.markRootPkgs(depID, pkgs, deps) + p.markRootPkgs(depID, pkgs, deps, visited) } return } diff --git a/pkg/dependency/parser/nodejs/pnpm/parse_test.go b/pkg/dependency/parser/nodejs/pnpm/parse_test.go index 317b468020c7..ec869d6ff492 100644 --- a/pkg/dependency/parser/nodejs/pnpm/parse_test.go +++ b/pkg/dependency/parser/nodejs/pnpm/parse_test.go @@ -59,6 +59,18 @@ func TestParse(t *testing.T) { want: pnpmV9, wantDeps: pnpmV9Deps, }, + { + name: "v9", + file: "testdata/pnpm-lock_v9.yaml", + want: pnpmV9, + wantDeps: pnpmV9Deps, + }, + { + name: "v9 with cyclic dependencies import", + file: "testdata/pnpm-lock_v9_cyclic_import.yaml", + want: pnpmV9CyclicImport, + wantDeps: pnpmV9CyclicImportDeps, + }, } for _, tt := range tests { diff --git a/pkg/dependency/parser/nodejs/pnpm/parse_testcase.go b/pkg/dependency/parser/nodejs/pnpm/parse_testcase.go index 52b69ce79ee3..3c9383bd1e25 100644 --- a/pkg/dependency/parser/nodejs/pnpm/parse_testcase.go +++ b/pkg/dependency/parser/nodejs/pnpm/parse_testcase.go @@ -900,4 +900,68 @@ var ( }, }, } + + pnpmV9CyclicImport = []ftypes.Package{ + { + ID: "update-browserslist-db@1.0.16", + Name: "update-browserslist-db", + Version: "1.0.16", + Relationship: ftypes.RelationshipDirect, + }, + { + ID: "browserslist@4.23.0", + Name: "browserslist", + Version: "4.23.0", + Relationship: ftypes.RelationshipIndirect, + }, + { + ID: "caniuse-lite@1.0.30001627", + Name: "caniuse-lite", + Version: "1.0.30001627", + Relationship: ftypes.RelationshipIndirect, + }, + { + ID: "electron-to-chromium@1.4.789", + Name: "electron-to-chromium", + Version: "1.4.789", + Relationship: ftypes.RelationshipIndirect, + }, + { + ID: "escalade@3.1.2", + Name: "escalade", + Version: "3.1.2", + Relationship: ftypes.RelationshipIndirect, + }, + { + ID: "node-releases@2.0.14", + Name: "node-releases", + Version: "2.0.14", + Relationship: ftypes.RelationshipIndirect, + }, + { + ID: "picocolors@1.0.1", + Name: "picocolors", + Version: "1.0.1", + Relationship: ftypes.RelationshipIndirect, + }, + } + pnpmV9CyclicImportDeps = []ftypes.Dependency{ + { + ID: "browserslist@4.23.0", + DependsOn: []string{ + "caniuse-lite@1.0.30001627", + "electron-to-chromium@1.4.789", + "node-releases@2.0.14", + "update-browserslist-db@1.0.16", + }, + }, + { + ID: "update-browserslist-db@1.0.16", + DependsOn: []string{ + "browserslist@4.23.0", + "escalade@3.1.2", + "picocolors@1.0.1", + }, + }, + } ) diff --git a/pkg/dependency/parser/nodejs/pnpm/testdata/pnpm-lock_v9_cyclic_import.yaml b/pkg/dependency/parser/nodejs/pnpm/testdata/pnpm-lock_v9_cyclic_import.yaml new file mode 100644 index 000000000000..08a7991b0e79 --- /dev/null +++ b/pkg/dependency/parser/nodejs/pnpm/testdata/pnpm-lock_v9_cyclic_import.yaml @@ -0,0 +1,67 @@ +lockfileVersion: '9.0' + +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + +importers: + + .: + dependencies: + update-browserslist-db: + specifier: 1.0.16 + version: 1.0.16(browserslist@4.23.0) + +packages: + + browserslist@4.23.0: + resolution: {integrity: sha512-QW8HiM1shhT2GuzkvklfjcKDiWFXHOeFCIA/huJPwHsslwcydgk7X+z2zXpEijP98UCY7HbubZt5J2Zgvf0CaQ==} + engines: {node: ^6 || ^7 || ^8 || ^9 || ^10 || ^11 || ^12 || >=13.7} + hasBin: true + + caniuse-lite@1.0.30001627: + resolution: {integrity: sha512-4zgNiB8nTyV/tHhwZrFs88ryjls/lHiqFhrxCW4qSTeuRByBVnPYpDInchOIySWknznucaf31Z4KYqjfbrecVw==} + + electron-to-chromium@1.4.789: + resolution: {integrity: sha512-0VbyiaXoT++Fi2vHGo2ThOeS6X3vgRCWrjPeO2FeIAWL6ItiSJ9BqlH8LfCXe3X1IdcG+S0iLoNaxQWhfZoGzQ==} + + escalade@3.1.2: + resolution: {integrity: sha512-ErCHMCae19vR8vQGe50xIsVomy19rg6gFu3+r3jkEO46suLMWBksvVyoGgQV+jOfl84ZSOSlmv6Gxa89PmTGmA==} + engines: {node: '>=6'} + + node-releases@2.0.14: + resolution: {integrity: sha512-y10wOWt8yZpqXmOgRo77WaHEmhYQYGNA6y421PKsKYWEK8aW+cqAphborZDhqfyKrbZEN92CN1X2KbafY2s7Yw==} + + picocolors@1.0.1: + resolution: {integrity: sha512-anP1Z8qwhkbmu7MFP5iTt+wQKXgwzf7zTyGlcdzabySa9vd0Xt392U0rVmz9poOaBj0uHJKyyo9/upk0HrEQew==} + + update-browserslist-db@1.0.16: + resolution: {integrity: sha512-KVbTxlBYlckhF5wgfyZXTWnMn7MMZjMu9XG8bPlliUOP9ThaF4QnhP8qrjrH7DRzHfSk0oQv1wToW+iA5GajEQ==} + hasBin: true + peerDependencies: + browserslist: '>= 4.21.0' + +snapshots: + + browserslist@4.23.0: + dependencies: + caniuse-lite: 1.0.30001627 + electron-to-chromium: 1.4.789 + node-releases: 2.0.14 + update-browserslist-db: 1.0.16(browserslist@4.23.0) + + caniuse-lite@1.0.30001627: {} + + electron-to-chromium@1.4.789: {} + + escalade@3.1.2: {} + + node-releases@2.0.14: {} + + picocolors@1.0.1: {} + + update-browserslist-db@1.0.16(browserslist@4.23.0): + dependencies: + browserslist: 4.23.0 + escalade: 3.1.2 + picocolors: 1.0.1