search

CycloneDX / cyclonedx-node-yarn

Create CycloneDX Software Bill of Materials (SBOM) from Node.js Yarn projects.
Apache License 2.0
18 stars 5 forks source link

[BUG] unexpected package version expression causes crash #133

Open jkowalleck opened 4 months ago

jkowalleck commented 4 months ago

Describe the bug

When attempting to generate the SBOM and a package.json contains for example "version": "1.0-dev"it now crashes in a function called Object.fixVersionField. This concerns me as I would not expect the SBOM generation to adjust version numbers and instead just take them verbatim.

To Reproduce

package.json

{
  "name": "jretret",
  "version": "1.0-dev",
  "packageManager": "yarn@4.3.0",
  "dependencies": {
    "is-sorted": "^1.0.5"
  }
}

Expected behavior

no crashes

Screenshots or output-paste

$ yarn dlx --quiet @cyclonedx/yarn-plugin-cyclonedx -vvv                                                                             

YARN_PLUGINS='/tmp/xfs-8c459013/dlx-363278/.yarn/unplugged/@cyclonedx-yarn-plugin-cyclonedx-npm-1.0.0-rc.7-922b640b2d/node_modules/@cyclonedx/yarn-plugin-cyclonedx/yarn-plugin-cyclonedx.cjs' yarn cyclonedx -vvv

DEBUG | YARN_VERSION: [ 4, 3, 0 ]
DEBUG | options: {"specVersion":"1.5","outputFormat":"JSON","outputFile":"-","production":false,"mcType":"application","shortPURLs":false,"outputReproducible":false,"verbosity":4,"projectDir":"/tmp/jretret"}
INFO  | gathering project & workspace ...
DEBUG | project: /tmp/jretret
DEBUG | workspace: /tmp/jretret
LOG   | gathering BOM data ...
Internal Error: Invalid version: "1.0-dev"
    at Object.fixVersionField (/tmp/xfs-8c459013/dlx-363278/.yarn/unplugged/@cyclonedx-yarn-plugin-cyclonedx-npm-1.0.0-rc.7-922b640b2d/node_modules/@cyclonedx/yarn-plugin-cyclonedx/yarn-plugin-cyclonedx.cjs:48:3853)
    at /tmp/xfs-8c459013/dlx-363278/.yarn/unplugged/@cyclonedx-yarn-plugin-cyclonedx-npm-1.0.0-rc.7-922b640b2d/node_modules/@cyclonedx/yarn-plugin-cyclonedx/yarn-plugin-cyclonedx.cjs:48:10076
    at Array.forEach (<anonymous>)
    at Jx (/tmp/xfs-8c459013/dlx-363278/.yarn/unplugged/@cyclonedx-yarn-plugin-cyclonedx-npm-1.0.0-rc.7-922b640b2d/node_modules/@cyclonedx/yarn-plugin-cyclonedx/yarn-plugin-cyclonedx.cjs:48:10041)
    at Xf.makeComponent (/tmp/xfs-8c459013/dlx-363278/.yarn/unplugged/@cyclonedx-yarn-plugin-cyclonedx-npm-1.0.0-rc.7-922b640b2d/node_modules/@cyclonedx/yarn-plugin-cyclonedx/yarn-plugin-cyclonedx.cjs:48:13143)
    at Xf.makeComponentFromWorkspace (/tmp/xfs-8c459013/dlx-363278/.yarn/unplugged/@cyclonedx-yarn-plugin-cyclonedx-npm-1.0.0-rc.7-922b640b2d/node_modules/@cyclonedx/yarn-plugin-cyclonedx/yarn-plugin-cyclonedx.cjs:48:12462)
    at Xf.buildFromWorkspace (/tmp/xfs-8c459013/dlx-363278/.yarn/unplugged/@cyclonedx-yarn-plugin-cyclonedx-npm-1.0.0-rc.7-922b640b2d/node_modules/@cyclonedx/yarn-plugin-cyclonedx/yarn-plugin-cyclonedx.cjs:48:11711)
    at async Is.execute (/tmp/xfs-8c459013/dlx-363278/.yarn/unplugged/@cyclonedx-yarn-plugin-cyclonedx-npm-1.0.0-rc.7-922b640b2d/node_modules/@cyclonedx/yarn-plugin-cyclonedx/yarn-plugin-cyclonedx.cjs:55:989)
    at async Is.validateAndExecute (.../.cache/node/corepack/v1/yarn/4.3.0/yarn.js:94:787)
    at async ls.run (/home/flow/.cache/node/corepack/v1/yarn/4.3.0/yarn.js:98:3250)

Environment

Additional context

as i've learned, is an invalid version identifier according to npm/yarn standards (which appears to adhere to semver), and causes crashes everywhere. So this is somehow expected.

But still, an option could be: in case a crash because of version happens, remove version and try again, and add back the version afterwards ... Or find a way to ignore the version when running manifest normalization ...

AugustusKling commented 4 months ago

I'd argue it's okay to refuse to generate an SBOM if any version number not confirming to semver is encountered. This is because https://docs.npmjs.com/cli/v10/configuring-npm/package-json#version explicitly says "version must be parseable by node-semver"

What it should not do is trying to fix version numbers in any way. In my opinion it's better to not receive an SBOM than to receive one that you cannot trust. What do you think about catching the error and aborting with a message explaining to the user which package had the invalid version number?

If you look in clean.js within https://www.npmjs.com/package/semver?activeTab=code which is used internally by cyclonedx-node-yarn, you see it strips whitespace, = characters and the character v from the version number. Hopefully, repositories such as NPM work the same way when ensuring package versions are unique.