fix(node): Tolerate unused malformed package.json files under node_modules

MarcelBochtler · fviernau · MarcelBochtler · commit 8f0c2c003c6a · 2025-05-08T12:21:24.000+02:00
As a side effect of cd56fa7 the Yarn analyzer now tries to parse `package.json` files from test directories, which were previously not part of the `node_modules` directory. This also applies to Yarn2. This causes an error if the installed module contains additional `package.json` files which are unused, such as for test project like e.g. [1]. [1]: https://github.com/browserify/resolve/blob/v1.22.2/test/resolver/malformed_package_json/package.json Co-authored-by: Frank Viernau <frank_viernau@epam.com> Signed-off-by: Marcel Bochtler <marcel.bochtler@bosch.com>
diff --git a/plugins/package-managers/node/src/funTest/assets/projects/synthetic/yarn/invalid-package-json-expected-output.yml b/plugins/package-managers/node/src/funTest/assets/projects/synthetic/yarn/invalid-package-json-expected-output.yml
@@ -0,0 +1,217 @@
+---
+project:
+  id: "Yarn::yarn-project-with-dep-with-invalid-package-json:1.0.0"
+  definition_file_path: "plugins/package-managers/node/src/funTest/assets/projects/synthetic/yarn/invalid-package-json/package.json"
+  authors:
+  - "The Author"
+  declared_licenses:
+  - "Apache-2.0"
+  declared_licenses_processed:
+    spdx_expression: "Apache-2.0"
+  vcs:
+    type: "Git"
+    url: "https://github.com/oss-review-toolkit/ort.git"
+    revision: ""
+    path: ""
+  vcs_processed:
+    type: "Git"
+    url: "<REPLACE_URL_PROCESSED>"
+    revision: "<REPLACE_REVISION>"
+    path: "<REPLACE_PATH>"
+  description: "Yarn test project with yarn.lock."
+  homepage_url: ""
+  scopes:
+  - name: "dependencies"
+    dependencies:
+    - id: "NPM::resolve:1.22.2"
+      dependencies:
+      - id: "NPM::is-core-module:2.16.1"
+        dependencies:
+        - id: "NPM::hasown:2.0.2"
+          dependencies:
+          - id: "NPM::function-bind:1.1.2"
+      - id: "NPM::path-parse:1.0.7"
+      - id: "NPM::supports-preserve-symlinks-flag:1.0.0"
+packages:
+- id: "NPM::function-bind:1.1.2"
+  purl: "pkg:npm/function-bind@1.1.2"
+  authors:
+  - "Raynos"
+  declared_licenses:
+  - "MIT"
+  declared_licenses_processed:
+    spdx_expression: "MIT"
+  description: "Implementation of Function.prototype.bind"
+  homepage_url: "https://github.com/Raynos/function-bind"
+  binary_artifact:
+    url: ""
+    hash:
+      value: ""
+      algorithm: ""
+  source_artifact:
+    url: "https://registry.npmjs.org/function-bind/-/function-bind-1.1.2.tgz"
+    hash:
+      value: "2c02d864d97f3ea6c8830c464cbd11ab6eab7a1c"
+      algorithm: "SHA-1"
+  vcs:
+    type: "Git"
+    url: "https://github.com/Raynos/function-bind.git"
+    revision: "40197beb5f4cf89dd005f0b268256c1e4716ea81"
+    path: ""
+  vcs_processed:
+    type: "Git"
+    url: "https://github.com/Raynos/function-bind.git"
+    revision: "40197beb5f4cf89dd005f0b268256c1e4716ea81"
+    path: ""
+- id: "NPM::hasown:2.0.2"
+  purl: "pkg:npm/hasown@2.0.2"
+  authors:
+  - "Jordan Harband"
+  declared_licenses:
+  - "MIT"
+  declared_licenses_processed:
+    spdx_expression: "MIT"
+  description: "A robust, ES3 compatible, \"has own property\" predicate."
+  homepage_url: "https://github.com/inspect-js/hasOwn#readme"
+  binary_artifact:
+    url: ""
+    hash:
+      value: ""
+      algorithm: ""
+  source_artifact:
+    url: "https://registry.npmjs.org/hasown/-/hasown-2.0.2.tgz"
+    hash:
+      value: "003eaf91be7adc372e84ec59dc37252cedb80003"
+      algorithm: "SHA-1"
+  vcs:
+    type: "Git"
+    url: "git+https://github.com/inspect-js/hasOwn.git"
+    revision: "d00d35005baf16a33d691a13f8ad627f35040742"
+    path: ""
+  vcs_processed:
+    type: "Git"
+    url: "https://github.com/inspect-js/hasOwn.git"
+    revision: "d00d35005baf16a33d691a13f8ad627f35040742"
+    path: ""
+- id: "NPM::is-core-module:2.16.1"
+  purl: "pkg:npm/is-core-module@2.16.1"
+  authors:
+  - "Jordan Harband"
+  declared_licenses:
+  - "MIT"
+  declared_licenses_processed:
+    spdx_expression: "MIT"
+  description: "Is this specifier a node.js core module?"
+  homepage_url: "https://github.com/inspect-js/is-core-module"
+  binary_artifact:
+    url: ""
+    hash:
+      value: ""
+      algorithm: ""
+  source_artifact:
+    url: "https://registry.npmjs.org/is-core-module/-/is-core-module-2.16.1.tgz"
+    hash:
+      value: "2a98801a849f43e2add644fbb6bc6229b19a4ef4"
+      algorithm: "SHA-1"
+  vcs:
+    type: "Git"
+    url: "git+https://github.com/inspect-js/is-core-module.git"
+    revision: "7c4284853357b2fcd49d42010d5a2b8a8420905f"
+    path: ""
+  vcs_processed:
+    type: "Git"
+    url: "https://github.com/inspect-js/is-core-module.git"
+    revision: "7c4284853357b2fcd49d42010d5a2b8a8420905f"
+    path: ""
+- id: "NPM::path-parse:1.0.7"
+  purl: "pkg:npm/path-parse@1.0.7"
+  authors:
+  - "Javier Blanco <http://jbgutierrez.info>"
+  declared_licenses:
+  - "MIT"
+  declared_licenses_processed:
+    spdx_expression: "MIT"
+  description: "Node.js path.parse() ponyfill"
+  homepage_url: "https://github.com/jbgutierrez/path-parse#readme"
+  binary_artifact:
+    url: ""
+    hash:
+      value: ""
+      algorithm: ""
+  source_artifact:
+    url: "https://registry.npmjs.org/path-parse/-/path-parse-1.0.7.tgz"
+    hash:
+      value: "fbc114b60ca42b30d9daf5858e4bd68bbedb6735"
+      algorithm: "SHA-1"
+  vcs:
+    type: "Git"
+    url: "https://github.com/jbgutierrez/path-parse.git"
+    revision: "9f1db2802ffbc572e6b447f66126697e33b0055e"
+    path: ""
+  vcs_processed:
+    type: "Git"
+    url: "https://github.com/jbgutierrez/path-parse.git"
+    revision: "9f1db2802ffbc572e6b447f66126697e33b0055e"
+    path: ""
+- id: "NPM::resolve:1.22.2"
+  purl: "pkg:npm/resolve@1.22.2"
+  authors:
+  - "James Halliday"
+  declared_licenses:
+  - "MIT"
+  declared_licenses_processed:
+    spdx_expression: "MIT"
+  description: "resolve like require.resolve() on behalf of files asynchronously and\
+    \ synchronously"
+  homepage_url: "https://github.com/browserify/resolve#readme"
+  binary_artifact:
+    url: ""
+    hash:
+      value: ""
+      algorithm: ""
+  source_artifact:
+    url: "https://registry.npmjs.org/resolve/-/resolve-1.22.2.tgz"
+    hash:
+      value: "0ed0943d4e301867955766c9f3e1ae6d01c6845f"
+      algorithm: "SHA-1"
+  vcs:
+    type: "Git"
+    url: "git://github.com/browserify/resolve.git"
+    revision: "c2f9ce254f0157b5e2e53e9aee0403c510909f7d"
+    path: ""
+  vcs_processed:
+    type: "Git"
+    url: "https://github.com/browserify/resolve.git"
+    revision: "c2f9ce254f0157b5e2e53e9aee0403c510909f7d"
+    path: ""
+- id: "NPM::supports-preserve-symlinks-flag:1.0.0"
+  purl: "pkg:npm/supports-preserve-symlinks-flag@1.0.0"
+  authors:
+  - "Jordan Harband"
+  declared_licenses:
+  - "MIT"
+  declared_licenses_processed:
+    spdx_expression: "MIT"
+  description: "Determine if the current node version supports the `--preserve-symlinks`\
+    \ flag."
+  homepage_url: "https://github.com/inspect-js/node-supports-preserve-symlinks-flag#readme"
+  binary_artifact:
+    url: ""
+    hash:
+      value: ""
+      algorithm: ""
+  source_artifact:
+    url: "https://registry.npmjs.org/supports-preserve-symlinks-flag/-/supports-preserve-symlinks-flag-1.0.0.tgz"
+    hash:
+      value: "6eda4bd344a3c94aea376d4cc31bc77311039e09"
+      algorithm: "SHA-1"
+  vcs:
+    type: "Git"
+    url: "git+https://github.com/inspect-js/node-supports-preserve-symlinks-flag.git"
+    revision: "1f7cac19c0c298cf40b3f2f3c735477ad579ac61"
+    path: ""
+  vcs_processed:
+    type: "Git"
+    url: "https://github.com/inspect-js/node-supports-preserve-symlinks-flag.git"
+    revision: "1f7cac19c0c298cf40b3f2f3c735477ad579ac61"
+    path: ""
diff --git a/plugins/package-managers/node/src/funTest/assets/projects/synthetic/yarn/invalid-package-json/package.json b/plugins/package-managers/node/src/funTest/assets/projects/synthetic/yarn/invalid-package-json/package.json
@@ -0,0 +1,15 @@
+{
+  "name": "yarn-project-with-dep-with-invalid-package-json",
+  "version": "1.0.0",
+  "description": "Yarn test project with yarn.lock.",
+  "repository": {
+    "type": "git",
+    "url": "https://github.com/oss-review-toolkit/ort.git"
+  },
+  "license": "Apache-2.0",
+  "author": "The Author <the.author@mail.example> (https://www.the.author.example/index.html)",
+  "private": true,
+  "dependencies": {
+    "resolve": "1.22.2"
+  }
+}
diff --git a/plugins/package-managers/node/src/funTest/assets/projects/synthetic/yarn/invalid-package-json/yarn.lock b/plugins/package-managers/node/src/funTest/assets/projects/synthetic/yarn/invalid-package-json/yarn.lock
@@ -0,0 +1,41 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+# yarn lockfile v1
+
+
+function-bind@^1.1.2:
+  version "1.1.2"
+  resolved "https://registry.yarnpkg.com/function-bind/-/function-bind-1.1.2.tgz#2c02d864d97f3ea6c8830c464cbd11ab6eab7a1c"
+  integrity sha512-7XHNxH7qX9xG5mIwxkhumTox/MIRNcOgDrxWsMt2pAr23WHp6MrRlN7FBSFpCpr+oVO0F744iUgR82nJMfG2SA==
+
+hasown@^2.0.2:
+  version "2.0.2"
+  resolved "https://registry.yarnpkg.com/hasown/-/hasown-2.0.2.tgz#003eaf91be7adc372e84ec59dc37252cedb80003"
+  integrity sha512-0hJU9SCPvmMzIBdZFqNPXWa6dqh7WdH0cII9y+CyS8rG3nL48Bclra9HmKhVVUHyPWNH5Y7xDwAB7bfgSjkUMQ==
+  dependencies:
+    function-bind "^1.1.2"
+
+is-core-module@^2.11.0:
+  version "2.16.1"
+  resolved "https://registry.yarnpkg.com/is-core-module/-/is-core-module-2.16.1.tgz#2a98801a849f43e2add644fbb6bc6229b19a4ef4"
+  integrity sha512-UfoeMA6fIJ8wTYFEUjelnaGI67v6+N7qXJEvQuIGa99l4xsCruSYOVSQ0uPANn4dAzm8lkYPaKLrrijLq7x23w==
+  dependencies:
+    hasown "^2.0.2"
+
+path-parse@^1.0.7:
+  version "1.0.7"
+  resolved "https://registry.yarnpkg.com/path-parse/-/path-parse-1.0.7.tgz#fbc114b60ca42b30d9daf5858e4bd68bbedb6735"
+  integrity sha512-LDJzPVEEEPR+y48z93A0Ed0yXb8pAByGWo/k5YYdYgpY2/2EsOsksJrq7lOHxryrVOn1ejG6oAp8ahvOIQD8sw==
+
+resolve@1.22.2:
+  version "1.22.2"
+  resolved "https://registry.yarnpkg.com/resolve/-/resolve-1.22.2.tgz#0ed0943d4e301867955766c9f3e1ae6d01c6845f"
+  integrity sha512-Sb+mjNHOULsBv818T40qSPeRiuWLyaGMa5ewydRLFimneixmVy2zdivRl+AF6jaYPC8ERxGDmFSiqui6SfPd+g==
+  dependencies:
+    is-core-module "^2.11.0"
+    path-parse "^1.0.7"
+    supports-preserve-symlinks-flag "^1.0.0"
+
+supports-preserve-symlinks-flag@^1.0.0:
+  version "1.0.0"
+  resolved "https://registry.yarnpkg.com/supports-preserve-symlinks-flag/-/supports-preserve-symlinks-flag-1.0.0.tgz#6eda4bd344a3c94aea376d4cc31bc77311039e09"
+  integrity sha512-ot0WnXS9fgdkgIcePe6RHNk1WA8+muPa6cSjeR3V8K27q9BB1rTE3R1p7Hv0z1ZyAc8s6Vvv8DIyWf681MAt0w==
diff --git a/plugins/package-managers/node/src/funTest/kotlin/yarn/YarnFunTest.kt b/plugins/package-managers/node/src/funTest/kotlin/yarn/YarnFunTest.kt
@@ -20,6 +20,7 @@
 package org.ossreviewtoolkit.plugins.packagemanagers.node.yarn
 
 import io.kotest.core.spec.style.StringSpec
+import io.kotest.matchers.collections.beEmpty
 import io.kotest.matchers.should
 
 import org.ossreviewtoolkit.analyzer.analyze
@@ -51,6 +52,16 @@ class YarnFunTest : StringSpec({
         result.toYaml() should matchExpectedResult(expectedResultFile, definitionFile)
     }
 
+    "Resolve dependencies for a project which installs a module with an invalid unused 'package.json'" {
+        val definitionFile = getAssetFile("projects/synthetic/yarn/invalid-package-json/package.json")
+        val expectedResultFile = getAssetFile("projects/synthetic/yarn/invalid-package-json-expected-output.yml")
+
+        val result = YarnFactory.create().resolveSingleProject(definitionFile, resolveScopes = true)
+
+        result.issues should beEmpty()
+        result.toYaml() should matchExpectedResult(expectedResultFile, definitionFile)
+    }
+
     "Resolve dependencies for a project depending on Babel correctly" {
         val definitionFile = getAssetFile("projects/synthetic/yarn/babel/package.json")
         val expectedResultFile = getAssetFile("projects/synthetic/yarn/babel-expected-output.yml")
diff --git a/plugins/package-managers/node/src/main/kotlin/Utils.kt b/plugins/package-managers/node/src/main/kotlin/Utils.kt
@@ -20,6 +20,7 @@
 package org.ossreviewtoolkit.plugins.packagemanagers.node
 
 import java.io.File
+import java.util.LinkedList
 
 import org.ossreviewtoolkit.analyzer.PackageManager.Companion.processPackageVcs
 import org.ossreviewtoolkit.analyzer.parseAuthorString
@@ -245,7 +246,37 @@ internal val PackageJson.moduleId: String get() =
 /**
  * Return the directories of all modules which have been installed in the 'node_modules' dir within [moduleDir].
  */
-internal fun getInstalledModulesDirs(moduleDir: File): Set<File> =
-    moduleDir.resolve("node_modules").walk().filter {
-        it.isFile && it.name == NodePackageManagerType.DEFINITION_FILE
-    }.mapTo(mutableSetOf()) { it.parentFile.realFile }
+internal fun getInstalledModulesDirs(projectDir: File): Set<File> {
+    val modulesDirsToProcess = LinkedList<File>().apply { add(projectDir.realFile) }
+    val discoveredModulesDirs = mutableSetOf<File>()
+
+    while (modulesDirsToProcess.isNotEmpty()) {
+        val currentModule = modulesDirsToProcess.removeFirst()
+        if (!discoveredModulesDirs.add(currentModule)) continue
+
+        modulesDirsToProcess += getChildModuleDirs(currentModule)
+    }
+
+    return discoveredModulesDirs
+}
+
+/**
+* Find all direct dependency module directories within the node_modules directory of the given [moduleDir].
+* This handles both regular modules and namespaced (@organization) modules.
+*/
+private fun getChildModuleDirs(moduleDir: File): Set<File> {
+    val nodeModulesDir = moduleDir.resolve("node_modules").takeIf { it.isDirectory } ?: return emptySet()
+
+    fun File.isModuleDir(): Boolean =
+        isDirectory && !isHidden && !name.startsWith("@") && resolve(NodePackageManagerType.DEFINITION_FILE).isFile
+
+    val nodeModulesDirFiles = nodeModulesDir.walk().maxDepth(1)
+    val childModuleDirsWithoutNamespace = nodeModulesDirFiles.filter { it.isModuleDir() }
+    val childModuleDirsWithNamespace = nodeModulesDirFiles.filter {
+        it.isDirectory && !it.isHidden && it.name.startsWith("@")
+    }.flatMap { namespaceDir ->
+        namespaceDir.walk().maxDepth(1).filter { it.isModuleDir() }
+    }
+
+    return (childModuleDirsWithoutNamespace + childModuleDirsWithNamespace).mapTo(mutableSetOf()) { it.realFile }
+}
diff --git a/plugins/package-managers/node/src/test/kotlin/NodePackageManagerDetectionTest.kt b/plugins/package-managers/node/src/test/kotlin/NodePackageManagerDetectionTest.kt
@@ -242,6 +242,7 @@ class NodePackageManagerDetectionTest : WordSpec({
 
             filteredFiles.map { it.relativeTo(projectDir).invariantSeparatorsPath } should containExactlyInAnyOrder(
                 "yarn/babel/package.json",
+                "yarn/invalid-package-json/package.json",
                 "yarn/project-with-lockfile/package.json",
                 "yarn/workspaces/package.json"
             )

Original file line number	Diff line number	Diff line change
`@@ -242,6 +242,7 @@ class NodePackageManagerDetectionTest : WordSpec({`
`242`	`242`
`243`	`243`	`filteredFiles.map { it.relativeTo(projectDir).invariantSeparatorsPath } should containExactlyInAnyOrder(`
`244`	`244`	`"yarn/babel/package.json",`
	`245`	`+ "yarn/invalid-package-json/package.json",`
`245`	`246`	`"yarn/project-with-lockfile/package.json",`
`246`	`247`	`"yarn/workspaces/package.json"`
`247`	`248`	`)`