Hi again
Two more things I noted while looking into this: It seems that the current implementation (reading the dependencies
property) doesn't consider transitive dependencies at all. Take the package chalk
as a example. I have one package that has a requirement of the form "chalk": "^2.0.0"
, which is resolved to the following dependency
"chalk": {
"version": "2.4.2",
"resolved": "https://registry.npmjs.org/chalk/-/chalk-2.4.2.tgz",
"integrity": "sha512-Mti+f9lpJNcwF4tWV8/OrTTtF1gZi+f8FqlyAdouralcFWFQWF2+NgCHShjkCb+IFBLq9buZwE1xckQU4peSuQ==",
"requires": {
"ansi-styles": "^3.2.1",
"escape-string-regexp": "^1.0.5",
"supports-color": "^5.3.0"
}
However, I have some more packages with requirements like "chalk": "^4.1.0"
, which are resolved to sub-dependencies like this:
"chalk": {
"version": "4.1.2",
"resolved": "https://registry.npmjs.org/chalk/-/chalk-4.1.2.tgz",
"integrity": "sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA==",
"dev": true,
"requires": {
"ansi-styles": "^4.1.0",
"supports-color": "^7.1.0"
}
Those sub-dependencies are not scanned at all, if I'm reading the current implementation of NpmDependencyScanner
correctly. This results to only version 2.4.2 being reported into our SCA reports. Version 4.1.2 is not listed.
The packages
property of the new file versions 2 and 3 seems to list all packages in a flat list, putting information about relations between packages on the top level. This leads to entries like "node_modules/chalk"
(resolving the version 2.4.2), "node_modules/critters/node_modules/chalk"
(resolving the version 4.1.2), "node_modules/eslint/node_modules/chalk"
(4.1.2 as well), ...
So if we extract the correct name of the package and add to corresponding version to it, our SCA reports should now (correctly?) report both versions of the chalk
package.
The second thing I noticed here: we will get multiple entries of the same package/version combination. This probably won't bother ProGet, as I guess those entries will be considered identical, but I guess it wouldn't hurt to put a Distinct()
in somewhere on the client side. Like, maybe changing the line
return new[] { new ScannedProject(projectName, ReadDependencies(doc)) };
to
return new[] { new ScannedProject(projectName, ReadDependencies(doc).Distinct()) };
or maybe changing to code of the ScannedProject
constructor from
this.Dependencies = dependencies.ToList();
to
this.Dependencies = dependencies.Distinct().ToList();
Cheers,
Sebastian