cyclonedx-node-npm icon indicating copy to clipboard operation
cyclonedx-node-npm copied to clipboard

[BUG] `--omit=dev` and `--omit=optional` flags don't exclude `devOptional` dependencies, causing misleading output

Open hakandilek opened this issue 5 months ago • 10 comments

Describe the bug

With #1307, dependency tree resolution is mainly based on the npm ls output. The npm ls command's --omit=dev and --omit=optional flags are misleading when dealing with devOptional dependencies. These dependencies are not excluded by either flag individually or in combination, despite their development-oriented nature.

npm documentation states:

dev, optional, devOptional: If the package is strictly part of the devDependencies tree, then dev will be true. If it is strictly part of the optionalDependencies tree, then optional will be set. If it is both a dev dependency and an optional dependency of a non-dev dependency, then devOptional will be set. (An optional dependency of a dev dependency will have both dev and optional set.)

Following this logic, I'd expect, using --omit=dev and --omit=optional at the same time, would also omit devOptionals but it does not.

To Reproduce

Scanning cyclonedx-node-npm includes the devOptional dependency @isaacs/[email protected]:

  • git clone https://github.com/CycloneDX/cyclonedx-node-npm
  • pushd cyclonedx-node-npm
  • npm install
  • popd
  • npx @cyclonedx/[email protected] --verbose --verbose --verbose --flatten-components --short-PURLs --spec-version 1.6 --output-format JSON --output-file - --omit dev --omit optional -- cyclonedx-node-npm/package.json > cyclonedx-node-npm/cyclonedx-node-npm-4.cdx.json

Produces the cyclonedx-node-npm-4.cdx.json file with the following [email protected] entry:

{
// ...
  "components": [
// ...
    {
      "type": "library",
      "name": "cliui",
      "group": "@isaacs",
      "version": "8.0.2",
      "bom-ref": "@cyclonedx/[email protected]|@isaacs/[email protected]",
      "author": "Ben Coe",
      "description": "easily create complex multi-column command-line-interfaces",
      "licenses": ...,
      "purl": "pkg:npm/%40isaacs/[email protected]",
      "externalReferences": ...
      "properties": [
        {
          "name": "cdx:npm:package:development",
          "value": "true"
        },
        {
          "name": "cdx:npm:package:path",
          "value": "node_modules/@isaacs/cliui"
        }
      ]
    },
// ...
  "dependencies": [
// ...
    {
      "ref": "@cyclonedx/[email protected]|[email protected]",
      "dependsOn": [
        "@cyclonedx/[email protected]|@isaacs/[email protected]",
        "@cyclonedx/[email protected]|@pkgjs/[email protected]"
      ]
    },
// ...
}

Expected behavior

Analyzing the whole dependency path for @isaacs/cliui,

@cyclonedx/cyclonedx-npm (root package)
└── @cyclonedx/[email protected]
    └── [email protected] (optional dependency)
        └── [email protected]
            └── [email protected]
                └── [email protected]
                    └── [email protected]
                        └── [email protected]
                            └── @isaacs/[email protected]

shows that, @isaacs/cliui has been pulled through a long tail of dependencies, but mainly by [email protected] optional dependency of the @cyclonedx/[email protected]. I'd expect that it would not show up after all.

Screenshots or output-paste

CLI output of the npx @cyclonedx/[email protected] --verbose --verbose --verbose --flatten-components --short-PURLs --spec-version 1.6 --output-format JSON --output-file - --omit dev --omit optional -- cyclonedx-node-npm/package.json > cyclonedx-node-npm/cyclonedx-node-npm-4.cdx.json given above is:

DEBUG | options: {"ignoreNpmErrors":false,"packageLockOnly":false,"omit":["dev","optional"],"workspace":[],"workspaces":true,"gatherLicenseTexts":false,"flattenComponents":true,"shortPURLs":true,"specVersion":"1.6","outputFormat":"JSON","outputFile":"-","mcType":"application","verbose":3}
DEBUG | makeNpmRunner caused execFileSync "/opt/homebrew/Cellar/node@22/22.17.1/bin/node" with  "-- /opt/homebrew/Cellar/node@22/22.17.1/lib/node_modules/npm/bin/npm-cli.js"
DEBUG | found NPM version "10.9.2"
DEBUG | packageFile: /Users/addiha5/work/cyclonedx-node-npm/package.json
INFO  | projectDir: /Users/addiha5/work/cyclonedx-node-npm
INFO  | detected a `node_modules` dir
LOG   | gathering BOM data ...
INFO  | gathering dependency tree ...
DEBUG | npm-ls: run npm with ["ls","--json","--long","--all","--omit=dev","--omit=optional"] in "/Users/addiha5/work/cyclonedx-node-npm"
INFO  | building BOM ...
LOG   | serializing BOM ...
LOG   | try validating BOM result ...
INFO  | BOM result appears valid
LOG   | writing BOM to -
INFO  | wrote 366448 bytes to -

Indicating that npm ls --json --long --all --omit=dev --omit=optional has been called under the hood. Calling npm ls --json --long --all --omit=dev --omit=optional > npm-ls-no-dev-op.json would give me npm-ls-no-dev-op.json.

Environment

  • @cyclonedx/cyclonedx-npm version: v4.0.0
  • NPM version: 10.9.2
  • Node version: v22.17.1
  • OS: macOS 15.5

Additional context

Add any other context about the problem here.

Contribution

  • [x] I am willing to provide a fix (given that someone give some pointers to the right direction)
  • [ ] I will wait until somebody else fixes it

hakandilek avatar Jul 24 '25 07:07 hakandilek

Thanks for the report. Will have a look at it.

jkowalleck avatar Jul 24 '25 17:07 jkowalleck

we do have test data for optional and dev-dependencies, already:

  • https://github.com/CycloneDX/cyclonedx-node-npm/tree/main/tests/_data/npm-ls_demo-results/dev-dependencies/CI_results -they are based on https://github.com/CycloneDX/cyclonedx-node-npm/tree/main/demo/dev-dependencies/project
  • https://github.com/CycloneDX/cyclonedx-node-npm/tree/main/tests/_data/dummy_projects/with-prepared

will investigate the actual test cases further, and might add new ones, so that the fix for the bug is solved for good.

jkowalleck avatar Jul 30 '25 17:07 jkowalleck

some research remarks:

Following this logic, I'd expect, using --omit=dev and --omit=optional at the same time, would also omit devOptionals but it does not.

the omit switches are logical or-concatenation - meaning the expectation would be that having at least one of these would omit any optional, transitive dev-dependencies that are.

the thing is, that no direct dev-dependency can be optional. optional dependencies are runtime-dependencies, not dev-dependencies. to get an "optional" dev-dependency O , you need to have a dev-dependency D; where D has an optional runtime-dependency O.

But this "optional dev-dependency" is NOT the use case of a "devOptional". what we see in https://github.com/CycloneDX/cyclonedx-node-npm/blob/main/tests/_data/dummy_projects/with-prepared:

  • uuidv4 is direct, optional-dep -- see package.json
  • uuid is direct, dev-dep -- see package.json
  • uuid is transitive dep, via uuidv4 -- per NPM lock file
  • uuid is a "devOptional" per NPM lock file

will add tests to document the current state of implementation.

PS: added tests for omitting peer/dev/optional - https://github.com/CycloneDX/cyclonedx-node-npm/pull/1329

jkowalleck avatar Jul 31 '25 15:07 jkowalleck

  • I am willing to provide a fix (given that someone give some pointers to the right direction)

@hakandilek i am still working on some test setups so that the needed changes and their effect are visible, after a fix was provided. Will let you know, when everything is in place, so you could work on a fix that suites your needs.

jkowalleck avatar Jul 31 '25 20:07 jkowalleck

@hakandilek i am still working on some test setups so that the needed changes and their effect are visible, after a fix was provided. Will let you know, when everything is in place, so you could work on a fix that suites your needs.

Thanks for the detailed analysis and preparing all these. Please let me know when you're ready.

hakandilek avatar Aug 01 '25 07:08 hakandilek

Feel free to pull request the needed changes and tests accordingly.

In #1329 you'd find the test beds that might need changes to showcase the results. Instructions on how to run tests and update snapshots can be found here: https://github.com/CycloneDX/cyclonedx-node-npm/blob/main/tests/README.md

jkowalleck avatar Aug 01 '25 13:08 jkowalleck

Just discovered another strange behavior which may be related:

[email protected] is an optional dependency of @cyclonedx/[email protected] but it somehow gets listed with the --omit=optional

https://github.com/CycloneDX/cyclonedx-node-npm/blob/3d7480537ae75f5a5b86f35546ee51184ca9a035/tests/_data/sbom_dummy-results/omit-optional/with-prepared.snap.json#L784-L787

hakandilek avatar Aug 20 '25 09:08 hakandilek

[email protected] is an optional dependency of @cyclonedx/[email protected] but it somehow gets listed with the --omit=optional

npm ls --long --all --json puts [email protected] under optionalDependencies, _dependencies and dependencies:

  "dependencies": {
    "@cyclonedx/cyclonedx-library": {
      ...
      "optionalDependencies": {
        ...
        "ajv-formats-draft2019": "^1.6.1",
        ...
      },
      ...
      "_dependencies": {
        ...
        "ajv-formats-draft2019": "^1.6.1",
        ...
      },
      ...
      "dependencies": {
        "ajv-formats-draft2019": {
          "version": "1.6.1",
          "resolved": "https://registry.npmjs.org/ajv-formats-draft2019/-/ajv-formats-draft2019-1.6.1.tgz",
          "overridden": false,
          "name": "ajv-formats-draft2019",
          ...
        },
        ...

Do you think this is an upstream npm ls issue?

hakandilek avatar Aug 20 '25 10:08 hakandilek

This is also true for the @cyclonedx/[email protected] -> [email protected] optional dependency:

https://github.com/CycloneDX/cyclonedx-node-npm/blob/3d7480537ae75f5a5b86f35546ee51184ca9a035/tests/_data/sbom_dummy-results/omit-optional/with-prepared.snap.json#L6351-L6355

Where npm ls --long --all --json list it under optionalDependencies, _dependencies and dependencies:

  "dependencies": {
    "@cyclonedx/cyclonedx-library": {
      ...
      "optionalDependencies": {
        ...
        "libxmljs2": "^0.31 || ^0.32 || ^0.33 || ^0.35",
        ...
      },
      ...
      "_dependencies": {
        ...
        "libxmljs2": "^0.31 || ^0.32 || ^0.33 || ^0.35",
        ...
      },
      ...
      "dependencies": {
        ...
        "libxmljs2": {
          "version": "0.35.0",
          "resolved": "https://registry.npmjs.org/libxmljs2/-/libxmljs2-0.35.0.tgz",
          "overridden": false,
          "name": "libxmljs2",
          ...
        },
        ...

hakandilek avatar Aug 20 '25 11:08 hakandilek

related or caused by the underlying issue: https://github.com/npm/cli/issues/8485

jkowalleck avatar Oct 29 '25 11:10 jkowalleck

@jkowalleck Given that NPM won't fix the issue, is there a way to provide a workaround for this? Currently we are publishing SBOMs which claim we ship dependencies having security issues which in fact are not even included in our software. This raises questions with our users who analyze those SBOMs. Is there another way cyclonedx-node-npm can retrieve the required information to filter out the optional und dev dependencies? Thank you very much.

spfeiffer-iem avatar Nov 25 '25 10:11 spfeiffer-iem

@jkowalleck Given that NPM won't fix the issue, is there a way to provide a workaround for this? Currently we are publishing SBOMs which claim we ship dependencies having security issues which in fact are not even included in our software. This raises questions with our users who analyze those SBOMs. Is there another way cyclonedx-node-npm can retrieve the required information to filter out the optional und dev dependencies? Thank you very much.

for now, running with the --package-lock-only switch should report/exclude the dev/optionals properly in the SBOM.

But if I were you, I would open yet another ticket for NPM, and ask them to fix the broken reporting on their end, as they have changed behavior others relied on - not only this tool. Tell NPM developers, why a correct result set is critical for you and the ecosystem, regardless of ingesting tools like this.

jkowalleck avatar Nov 25 '25 14:11 jkowalleck

for now, running with the --package-lock-only switch should report/exclude the dev/optionals properly in the SBOM.

Thanks, i was not aware this is also an option to the plugin itself, i thought it is an option to the npm-ls command only. I will try that.

spfeiffer-iem avatar Nov 25 '25 14:11 spfeiffer-iem