Skip to content

Commit decf73e

Browse files
soul2zimateclaude
soul2zimate
and
claude
committed
fix: include peer, optional, and bundled deps in JS stack/component analysis
pnpm, yarn classic, and yarn berry providers were missing peer and/or optional dependencies from the SBOM, while yarn berry additionally included devDependencies. This caused inconsistent vulnerability reports across package managers for the same package.json. Root causes: - Manifest.loadDependencies() only read content.dependencies - base_javascript _getRootDependencies() and _addDependenciesToSbom() ignored pnpm's separate optionalDependencies key - yarn berry had no mechanism to filter out devDependencies from yarn info --all output Fixes: - Manifest now loads peer, optional, and bundled dependencies - Base JS provider merges optionalDependencies from dep tree output - ensurePeerAndOptionalDeps() injects manifest-declared peer/optional deps when the package manager does not resolve them - Yarn berry processor filters root dependencies against manifest Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
1 parent b8af0f8 commit decf73e

25 files changed

Lines changed: 802 additions & 13 deletions

File tree

‎src/providers/base_javascript.js‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -264,18 +264,43 @@ export default class Base_javascript {
264264
sbom.addRoot(mainComponent, license);
265265

266266
this._addDependenciesToSbom(sbom, depsObject);
267+
this.#ensurePeerAndOptionalDeps(sbom);
267268
sbom.filterIgnoredDeps(this.#manifest.ignored);
268269
return sbom.getAsJsonString(opts);
269270
}
270271

272+
/**
273+
* Ensures peer and optional dependencies declared in the manifest are
274+
* present in the SBOM, even when the package manager does not resolve them
275+
* (e.g. yarn does not include peer deps in its dependency listing).
276+
* @param {Sbom} sbom - The SBOM to supplement
277+
* @private
278+
*/
279+
#ensurePeerAndOptionalDeps(sbom) {
280+
const rootPurl = toPurl(purlType, this.#manifest.name, this.#manifest.version);
281+
const rootComponent = sbom.getRoot();
282+
const depSources = [this.#manifest.peerDependencies, this.#manifest.optionalDependencies];
283+
for (const source of depSources) {
284+
for (const [name, version] of Object.entries(source)) {
285+
if (!sbom.checkIfPackageInsideDependsOnList(rootComponent, name)) {
286+
const target = toPurl(purlType, name, version);
287+
sbom.addDependency(rootPurl, target);
288+
}
289+
}
290+
}
291+
}
292+
271293
/**
272294
* Recursively builds the Sbom from the JSON that npm listing returns
273295
* @param {Sbom} sbom - The SBOM object to add dependencies to
274296
* @param {Object} depTree - The current dependency tree
275297
* @protected
276298
*/
277299
_addDependenciesToSbom(sbom, depTree) {
278-
const dependencies = depTree["dependencies"] || {};
300+
const dependencies = {
301+
...depTree["dependencies"],
302+
...depTree["optionalDependencies"],
303+
};
279304

280305
Object.entries(dependencies)
281306
.forEach(entry => {
@@ -330,6 +355,7 @@ export default class Base_javascript {
330355
const rootPurl = toPurlFromString(sbom.getRoot().purl);
331356
sbom.addDependency(rootPurl, rootDeps.get(key));
332357
}
358+
this.#ensurePeerAndOptionalDeps(sbom);
333359
sbom.filterIgnoredDeps(this.#manifest.ignored);
334360
return sbom.getAsJsonString(opts);
335361
}
@@ -341,12 +367,16 @@ export default class Base_javascript {
341367
* @protected
342368
*/
343369
_getRootDependencies(depTree) {
344-
if (!depTree.dependencies) {
370+
const allDeps = {
371+
...depTree.dependencies,
372+
...depTree.optionalDependencies,
373+
};
374+
if (Object.keys(allDeps).length === 0) {
345375
return new Map();
346376
}
347377

348378
return new Map(
349-
Object.entries(depTree.dependencies).map(
379+
Object.entries(allDeps).map(
350380
([key, value]) => [key, toPurl(purlType, key, value.version)]
351381
)
352382
);

‎src/providers/manifest.js‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ export default class Manifest {
1212
this.manifestPath = manifestPath;
1313
const content = this.loadManifest();
1414
this.dependencies = this.loadDependencies(content);
15+
this.peerDependencies = content.peerDependencies || {};
16+
this.optionalDependencies = content.optionalDependencies || {};
1517
this.name = content.name;
1618
this.version = content.version || DEFAULT_VERSION;
1719
this.ignored = this.loadIgnored(content);
@@ -30,11 +32,27 @@ export default class Manifest {
3032

3133
loadDependencies(content) {
3234
let deps = [];
33-
if(!content.dependencies) {
34-
return deps;
35+
const depSources = [
36+
content.dependencies,
37+
content.peerDependencies,
38+
content.optionalDependencies,
39+
];
40+
for (const source of depSources) {
41+
if (source) {
42+
for (let dep in source) {
43+
if (!deps.includes(dep)) {
44+
deps.push(dep);
45+
}
46+
}
47+
}
3548
}
36-
for(let dep in content.dependencies) {
37-
deps.push(dep);
49+
// bundledDependencies is an array of package names (subset of dependencies)
50+
if (Array.isArray(content.bundledDependencies)) {
51+
for (const dep of content.bundledDependencies) {
52+
if (!deps.includes(dep)) {
53+
deps.push(dep);
54+
}
55+
}
3856
}
3957
return deps;
4058
}

‎src/providers/processors/yarn_berry_processor.js‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,15 @@ export default class Yarn_berry_processor extends Yarn_processor {
5858
}
5959

6060
return new Map(
61-
depTree.filter(dep => !this.#isRoot(dep.value)).map(
62-
dep => {
61+
depTree.filter(dep => !this.#isRoot(dep.value))
62+
.map(dep => {
6363
const depName = dep.value;
6464
const idx = depName.lastIndexOf('@');
6565
const name = depName.substring(0, idx);
6666
const version = dep.children.Version;
6767
return [name, toPurl(purlType, name, version)];
68-
}
69-
)
68+
})
69+
.filter(([name]) => this._manifest.dependencies.includes(name))
7070
);
7171
}
7272

@@ -93,14 +93,25 @@ export default class Yarn_berry_processor extends Yarn_processor {
9393
return;
9494
}
9595

96+
// Collect names of production dependencies to filter out devDeps from root
97+
const prodDeps = new Set(this._manifest.dependencies);
98+
9699
depTree.forEach(n => {
97100
const depName = n.value;
98-
const from = this.#isRoot(depName) ? toPurlFromString(sbom.getRoot().purl) : this.#purlFromNode(depName, n);
101+
const isRoot = this.#isRoot(depName);
102+
const from = isRoot ? toPurlFromString(sbom.getRoot().purl) : this.#purlFromNode(depName, n);
99103
const deps = n.children?.Dependencies;
100104
if(!deps) {return;}
101105
deps.forEach(d => {
102106
const to = this.#purlFromLocator(d.locator);
103107
if(to) {
108+
// For root node, only add production dependencies (exclude devDeps)
109+
if (isRoot) {
110+
const fullName = to.namespace ? `${to.namespace}/${to.name}` : to.name;
111+
if (!prodDeps.has(fullName)) {
112+
return;
113+
}
114+
}
104115
sbom.addDependency(from, to);
105116
}
106117
});

‎test/providers/javascript.test.js‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ suite('testing the javascript-npm data provider', async () => {
7575
});
7676
['npm', 'pnpm', 'yarn-classic', 'yarn-berry'].flatMap(providerName => [
7777
"package_json_deps_without_exhortignore_object",
78-
"package_json_deps_with_exhortignore_object"
78+
"package_json_deps_with_exhortignore_object",
79+
"package_json_deps_with_mixed_dep_types"
7980
].map(testCase => ({ providerName, testCase }))).forEach(({ providerName, testCase }) => {
8081
let scenario = testCase.replace('package_json_deps_', '').replaceAll('_', ' ')
8182
test(`verify package.json data provided for ${providerName} - stack analysis - ${scenario}`, async () => {
@@ -148,6 +149,21 @@ suite('testing the javascript-npm data provider', async () => {
148149
expect(m.ignored).to.be.empty;
149150
});
150151

152+
test('loads a manifest with mixed dependency types (peer, optional, bundled)', () => {
153+
const testCase = 'package_json_deps_with_mixed_dep_types';
154+
const manifestPath = `test/providers/tst_manifests/npm/${testCase}/package.json`;
155+
const m = new Manifest(manifestPath);
156+
expect(m.name).to.be.equals('mixed-deps-test');
157+
expect(m.version).to.be.equals('1.0.0');
158+
expect(m.dependencies).to.have.all.members([
159+
'express', 'axios', 'minimist', 'lodash']);
160+
expect(m.dependencies).to.not.include('jest');
161+
expect(m.dependencies).to.not.include('eslint');
162+
expect(m.peerDependencies).to.deep.equal({ minimist: '1.2.0' });
163+
expect(m.optionalDependencies).to.deep.equal({ lodash: '4.17.19' });
164+
expect(m.ignored).to.be.empty;
165+
});
166+
151167
test('fails when the manifest does not exist', () => {
152168
const testCase = 'wrong_folder';
153169
const manifestPath = `test/providers/tst_manifests/npm/${testCase}/package.json`;

‎test/providers/tst_manifests/js-common/package_json_deps_with_mixed_dep_types/component_expected_sbom.json‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
{
2+
"bomFormat": "CycloneDX",
3+
"specVersion": "1.4",
4+
"version": 1,
5+
"metadata": {
6+
"timestamp": "2023-08-07T00:00:00.000Z",
7+
"component": {
8+
"name": "mixed-deps-test",
9+
"version": "1.0.0",
10+
"purl": "pkg:npm/mixed-deps-test@1.0.0",
11+
"type": "application",
12+
"bom-ref": "pkg:npm/mixed-deps-test@1.0.0"
13+
}
14+
},
15+
"components": [
16+
{
17+
"name": "axios",
18+
"version": "0.19.0",
19+
"purl": "pkg:npm/axios@0.19.0",
20+
"type": "library",
21+
"bom-ref": "pkg:npm/axios@0.19.0"
22+
},
23+
{
24+
"name": "express",
25+
"version": "4.17.1",
26+
"purl": "pkg:npm/express@4.17.1",
27+
"type": "library",
28+
"bom-ref": "pkg:npm/express@4.17.1"
29+
},
30+
{
31+
"name": "lodash",
32+
"version": "4.17.19",
33+
"purl": "pkg:npm/lodash@4.17.19",
34+
"type": "library",
35+
"bom-ref": "pkg:npm/lodash@4.17.19"
36+
},
37+
{
38+
"name": "minimist",
39+
"version": "1.2.0",
40+
"purl": "pkg:npm/minimist@1.2.0",
41+
"type": "library",
42+
"bom-ref": "pkg:npm/minimist@1.2.0"
43+
}
44+
],
45+
"dependencies": [
46+
{
47+
"ref": "pkg:npm/mixed-deps-test@1.0.0",
48+
"dependsOn": [
49+
"pkg:npm/axios@0.19.0",
50+
"pkg:npm/express@4.17.1",
51+
"pkg:npm/lodash@4.17.19",
52+
"pkg:npm/minimist@1.2.0"
53+
]
54+
},
55+
{
56+
"ref": "pkg:npm/axios@0.19.0",
57+
"dependsOn": []
58+
},
59+
{
60+
"ref": "pkg:npm/express@4.17.1",
61+
"dependsOn": []
62+
},
63+
{
64+
"ref": "pkg:npm/lodash@4.17.19",
65+
"dependsOn": []
66+
},
67+
{
68+
"ref": "pkg:npm/minimist@1.2.0",
69+
"dependsOn": []
70+
}
71+
]
72+
}

‎test/providers/tst_manifests/npm/package_json_deps_with_mixed_dep_types/listing_component.json‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
{
2+
"version": "1.0.0",
3+
"name": "mixed-deps-test",
4+
"dependencies": {
5+
"express": {
6+
"version": "4.17.1",
7+
"resolved": "https://registry.npmjs.org/express/-/express-4.17.1.tgz",
8+
"overridden": false
9+
},
10+
"axios": {
11+
"version": "0.19.0",
12+
"resolved": "https://registry.npmjs.org/axios/-/axios-0.19.0.tgz",
13+
"overridden": false
14+
},
15+
"minimist": {
16+
"version": "1.2.0",
17+
"resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz",
18+
"overridden": false
19+
},
20+
"lodash": {
21+
"version": "4.17.19",
22+
"resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.19.tgz",
23+
"overridden": false
24+
}
25+
}
26+
}

‎test/providers/tst_manifests/npm/package_json_deps_with_mixed_dep_types/listing_stack.json‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
{
2+
"version": "1.0.0",
3+
"name": "mixed-deps-test",
4+
"dependencies": {
5+
"express": {
6+
"version": "4.17.1",
7+
"resolved": "https://registry.npmjs.org/express/-/express-4.17.1.tgz",
8+
"overridden": false,
9+
"dependencies": {
10+
"body-parser": {
11+
"version": "1.19.0",
12+
"resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.19.0.tgz",
13+
"overridden": false
14+
}
15+
}
16+
},
17+
"axios": {
18+
"version": "0.19.0",
19+
"resolved": "https://registry.npmjs.org/axios/-/axios-0.19.0.tgz",
20+
"overridden": false,
21+
"dependencies": {
22+
"follow-redirects": {
23+
"version": "1.5.10",
24+
"resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.5.10.tgz",
25+
"overridden": false
26+
}
27+
}
28+
},
29+
"minimist": {
30+
"version": "1.2.0",
31+
"resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz",
32+
"overridden": false
33+
},
34+
"lodash": {
35+
"version": "4.17.19",
36+
"resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.19.tgz",
37+
"overridden": false
38+
}
39+
}
40+
}

‎test/providers/tst_manifests/npm/package_json_deps_with_mixed_dep_types/package-lock.json‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/providers/tst_manifests/npm/package_json_deps_with_mixed_dep_types/package.json‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"name": "mixed-deps-test",
3+
"version": "1.0.0",
4+
"dependencies": {
5+
"express": "4.17.1",
6+
"axios": "0.19.0"
7+
},
8+
"peerDependencies": {
9+
"minimist": "1.2.0"
10+
},
11+
"optionalDependencies": {
12+
"lodash": "4.17.19"
13+
},
14+
"bundledDependencies": [
15+
"express"
16+
],
17+
"devDependencies": {
18+
"jest": "26.0.0",
19+
"eslint": "7.0.0"
20+
}
21+
}

0 commit comments

Comments
 (0)