[BUG]: package normalization might cause crashes

[sresch4b]

Hi,

we're trying to setup your plugin, but are running into issues with some dependencies like "@apollo/client". This might not the only one, but is the first which seems to break with a package naming pattern.

Error: Invalid name: "@apollo/client/core"
    at ensureValidName (/Users/abc/Develop/project_xy/node_modules/normalize-package-data/lib/fixer.js:370:11)
    at Object.fixNameField (/Users/abc/Develop/project_xy/node_modules/normalize-package-data/lib/fixer.js:233:5)
    at /Users/abc/Develop/project_xy/node_modules/normalize-package-data/lib/normalize.js:41:38
    at Array.forEach (<anonymous>)
    at normalize (/Users/abc/Develop/project_xy/node_modules/normalize-package-data/lib/normalize.js:40:15)
    at Extractor.generateComponents (/Users/abc/Develop/project_xy/node_modules/@cyclonedx/webpack-plugin/dist/extractor.js:66:54)
    at /Users/abc/Develop/project_xy/node_modules/@cyclonedx/webpack-plugin/dist/plugin.js:129:43
    at Hook.eval [as call] (eval at create (/Users/abc/Develop/project_xy/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:7:1)
    at Hook.CALL_DELEGATE [as _call] (/Users/abc/Develop/project_xy/node_modules/tapable/lib/Hook.js:14:14)
    at /Users/abc/Develop/project_xy/node_modules/webpack/lib/Compilation.js:2966:33

Apollo seems to deliver a package.json for each of their sub-modules, where they extend the name with the module name (directory). Whether this is correct or not, I cannot judge, but acknowledging the problem with an error seems to me to be the wrong solution.
Perhaps there must be a way to configure this or it is valid to ignore them, cause it is (only) a sub-module of a package.

You can find a small example in the following repo:
https://github.com/sresch4b/cyclonedx-issue

I am curious about your perspective on this topic.

[jkowalleck]

Thanks for reporting.
I appreciate that you even prepared an example setup.

From the call stack you posted, it appears that not this webpack plugin is causing the error, but normalize-package-data.

@sresch4b, Could you check with the developers of normalize-package-data what is going on?

[jkowalleck]

some background:

• description of "name" in a package.json: https://nodejs.org/api/packages.html#name
• basically there are no rules o them. Every character is allowed. Slashes(/) are treated as directory-separator by node's module resolution system.

will check if it normalize-package-data can be slackened somehow, so it is less strict. in https://github.com/npm/normalize-package-data/blob/cb13a34f17f0874c1c56d741b6448621efb2bcce/lib/fixer.js#L216-L237
cannot be slacked. well, that is odd.

[sresch4b]

@jkowalleck
I've created an issue for normalize-package-data to get more information o this topic.
npm/normalize-package-data#175

[wraithgar]

That is not a valid package name and npm cannot support it because it is not anything that an npm registry will recognize. It is valid as a require but not as a package.json entry.

[jkowalleck]

i might implement a fallback for this
and i might drop you a release-candiate/preview version to try if it solves your issue.

[jkowalleck]

@sresch4b, could you see if the following meets your expectations?
#754 (comment)

[sresch4b]

@jkowalleck
Hi, it looks promising but to be honest I don't know the bom format well enough. We want to use it as input for the osv-scanner.
Would it be possible to get access to a release or test candidate for further tests with an internal project?

Thx for your support, we really appreciate it!

[jkowalleck]

@sresch4b here you are: https://github.com/CycloneDX/cyclonedx-webpack-plugin/releases/tag/v3.4.1-rc.1

[sresch4b]

@jkowalleck
Thx for the test candidate.

I tested the version with two internal projects and the build ran through without breaking.
However, I noticed the following:

• The "luxon" package is not listed. This project also has another "package.json" in the "src" subfolder, which probably leads to problems. I've updated my test repo, if you want to check it: https://github.com/sresch4b/cyclonedx-issue
• Regarding the package "apollo-client", I would understand wraithgar to mean that their subfolders should not actually be listed in the BOM file, as these are not packages in the sense of npm and as such cannot be obtained from the npm registry.
• The last point now concerns the concrete adaptation. This outputs the complete stack trace, which is a bit much context for a warning without explicitly requesting it, such as with a "verbose" flag. But that may be personal taste and part of a new "feature" request.

I will discuss the generated bom files with colleagues tomorrow, just in case they notice something else.

[sresch4b]

I've asked another colleague to check the generated bom file for his project and it seems to be ok, with one exception that "luxon" is missing.

[jkowalleck]

luxon is missing, because you are not using it.
therefore, it appears, webpack is not using any "luxon", not packing it, so it is omitted in the build and in the BOM.
read https://webpack.js.org/guides/tree-shaking/

this answer was based on https://github.com/sresch4b/cyclonedx-issue/tree/a7060b1180e738ffe20b70da0100a14b231aa059

[jkowalleck]

some remarks - as this issue was resolved, now.

re: #745 (comment)

---

The "luxon" package is not listed. This project also has another "package.json" in the "src" subfolder, which probably leads to problems. I've updated my test repo, if you want to check it: https://github.com/sresch4b/cyclonedx-issue

answered here: #745 (comment)

---

Regarding the package "apollo-client", I would understand wraithgar to mean that their subfolders should not actually be listed in the BOM file, as these are not packages in the sense of npm and as such cannot be obtained from the npm registry.

No, they should be in the SBOM.
This is webpack, this is not NPM. Webpack does not know any http://npmjs.org registry or others, nor does it care for it. Webpack uses Node as a runtime, and not as a build system.
Webpack is file-based, not package-based. But since it runs on Node, we adhere to Node's rules: each directory with a package.json in it is a package in all means.
This is important due to webpack's tree-shaking. Webpack tries to pack as little as possible, and this is reflected in the SBOM.

---

The last point now concerns the concrete adaptation. This outputs the complete stack trace, which is a bit much context for a warning without explicitly requesting it, such as with a "verbose" flag. But that may be personal taste and part of a new "feature" request.

Webpack's logging rules are completely followed.
Feel free to configure your setup's log visibility with https://webpack.js.org/configuration/stats/

[sresch4b]

@jkowalleck

Thx for the remarks and the explaination.
Excuse me for asking again here, but I'm wondering how webpack can tree-shaking "luxon", when I'm definetly using it in my code.

Regarding to my example https://github.com/sresch4b/cyclonedx-issue I have the following code, where I'm importing and using DateTime from luxon:

// src/index.js 
import {ApolloClient, InMemoryCache} from "@apollo/client";
import {DateTime} from "luxon";

function component() {
  const client = new ApolloClient({
    uri: 'https://example.com/',
    cache: new InMemoryCache(),
  });
  const now = DateTime.now();
  client.query({query: ''}).then(() => console.log('test'));
  const element = document.createElement('div');
  element.textContent = now.toISODate();
  return element;
}

document.body.appendChild(component());

If I search for "luxon" in the generated "dist/main.js" I also have some findings. The class name "DateTime" has been renamed for minification, but I can still find the code from above in the generated file. Therefore, I would argue that luxon is part of the generated code and should also be part of the BOM. However, you are welcome to correct me again. ;)

Thank you for your efforts and support.

[jkowalleck]

Thanks for getting back, @sresch4b .
Please open a new issue regarding the missing "luxon" and i will have a look at the details.
Hopefully I can find an explanation and publish a bugfix.

[jkowalleck]

@sresch4b you can always set webpack's state to detailed and you will see a lot of debug information that might help identify why a component was missing.

you might find something like

LOG from CycloneDxWebpackPlugin/ComponentFetcher/Extractor
    start building Components from modules...
    try to build new Component from PkgPath /tmp/dadsad/cyclonedx-issue/package.json
    try to build new Component from PkgPath /tmp/dadsad/cyclonedx-issue/node_modules/@apollo/client/package.json
    try to build new Component from PkgPath /tmp/dadsad/cyclonedx-issue/node_modules/luxon/src/package.json
    try to build new Component from PkgPath /tmp/dadsad/cyclonedx-issue/node_modules/luxon/src/package.json

[rkg-mm]

@jkowalleck I run into the same issue with the latest version, because in your plugin.js you have another call to normalizePackageJson in "makeRootComponent".

cyclonedx-webpack-plugin/src/plugin.ts

Lines 298 to 309 in 58a4381

	#makeRootComponent (
	path: string,
	builder: CDX.Builders.FromNodePackageJson.ComponentBuilder,
	logger: WebpackLogger
	): CDX.Models.Component | undefined {
	const thisPackageJson = this.rootComponentAutodetect
	? getPackageDescription(path)?.packageJson
	: { name: this.rootComponentName, version: this.rootComponentVersion }
	if (thisPackageJson === undefined) { return undefined }
	normalizePackageJson(thisPackageJson, w => { logger.debug('normalizePackageJson from PkgPath', path, 'caused:', w) })
	return builder.makeComponent(thisPackageJson)
	}

Stack trace like

Error: Invalid name: "REDACTED REDACTED"
    at ensureValidName ([REDACTED]\MobileApp\node_modules\normalize-package-data\lib\fixer.js:370:11)
    at Object.fixNameField ([REDACTED]\MobileApp\node_modules\normalize-package-data\lib\fixer.js:233:5)
    at [REDACTED]\MobileApp\node_modules\normalize-package-data\lib\normalize.js:41:38
    at Array.forEach (<anonymous>)
    at normalize ([REDACTED]\MobileApp\node_modules\normalize-package-data\lib\normalize.js:40:15)
    at CycloneDxWebpackPlugin._CycloneDxWebpackPlugin_makeRootComponent ([REDACTED]\MobileApp\node_modules\@cyclonedx\webpack-plugin\dist\plugin.js:186:5)
    at CycloneDxWebpackPlugin._CycloneDxWebpackPlugin_compilationHook ([REDACTED]\MobileApp\node_modules\@cyclonedx\webpack-plugin\dist\plugin.js:79:142)
    at Hook.eval [as call] (eval at create ([REDACTED]\MobileApp\node_modules\tapable\lib\HookCodeFactory.js:19:10), <anonymous>:36:1)
    at Hook.CALL_DELEGATE [as _call] ([REDACTED]\MobileApp\node_modules\tapable\lib\Hook.js:14:14)
    at Compiler.newCompilation ([REDACTED]\MobileApp\node_modules\webpack\lib\Compiler.js:1125:30)

[jkowalleck]

re #745 (comment)

    at CycloneDxWebpackPlugin._CycloneDxWebpackPlugin_makeRootComponent ([REDACTED]\MobileApp\node_modules\@cyclonedx\webpack-plugin\dist\plugin.js:186:5)

in https://github.com/CycloneDX/cyclonedx-webpack-plugin/releases/download/v3.8.2/cyclonedx-webpack-plugin-3.8.2.tgz

is

    normalizePackageJson(thisPackageJson, w => { logger.debug('normalizePackageJson from PkgPath', path, 'caused:', w); });

inside of

function _CycloneDxWebpackPlugin_makeRootComponent(path, builder, logger) {}

so assumption is about right.

---

The issue is thrown only when data you have under your control appears to be unexpected. You could just change your package.js file, right?
Other than data from dependencies, that you do not control.

What do you think, @rkg-mm

[rkg-mm]

@jkowalleck after discussing with the responsible team I can, but the actual restriction is unnecessary here, or? since in this case its the name of a hybrid mobile app and not a package name in terms of something that is published on a package manager, so why restrict it and force changes on a project just to be able to generate a BOM file?

[jkowalleck]

The discussed normalization does not only affect the name.
It checks for basic rules of a package.json and tries hard to bring everything in a standard format.

why restrict it and force changes on a project just to be able to generate a BOM file?

Standards, for documentation’s sake.
... for the same reason you'd expect this tool to adhere to CycloneDX document standards, this tool expects the input to adhere package.json standards. And since i expect users to make mistakes, a "normalizer" takes care of it and tries hard to make sense of the provided package.json data.

If you do not agree, just

1. create a new issue report, describing the scenario and behavior you would expect.
2. drop me a pullrequest including changes and (regression) tests, so that a desired behavioral change might be considered.

[jkowalleck]

related to #1284