Skip to content

Commit 14fce6d

Browse files
soul2zimateclaude
soul2zimate
and
claude
authored
fix: handle peerDependencies and optionalDependencies consistently ac… (guacsec#392)
…ross JS providers JavaScript providers (npm, pnpm, yarn classic, yarn berry) produced inconsistent dependency analysis results when package.json contains peerDependencies, optionalDependencies, and devDependencies. - pnpm: lodash (optionalDependency) was missing because pnpm outputs optional deps under a separate "optionalDependencies" key, but addDependenciesToSbom and getRootDependencies only read the "dependencies" key. - Yarn Berry: devDependencies were included in the scan because yarn info has no --prod flag and the processor did not filter at root level. - Yarn Classic: lodash was missing because Manifest.loadDependencies only read the "dependencies" key, causing the filter in addDependenciesToSbom to exclude it. - All yarn providers: minimist (peerDependency) was missing because yarn does not include peer deps in its output, and no backfill mechanism existed. Fixes applied (matching trustify-da-javascript-client approach): - Manifest: loadDependencies now includes optionalDependencies and peerDependencies names. Added peerDependencies and optionalDependencies as Map<String, String> fields. - JavaScriptProvider: addDependenciesToSbom and getRootDependencies now also read the "optionalDependencies" key from dep tree output. Added ensurePeerAndOptionalDeps to backfill missing peer/optional deps using declared versions from package.json. - YarnBerryProcessor: addDependenciesToSbom now filters root node dependencies to only include production deps (those in manifest.dependencies). Fixes: guacsec#391 Jira issue: https://redhat.atlassian.net/browse/TC-3977 --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7d88afb commit 14fce6d

29 files changed

Lines changed: 926 additions & 56 deletions

File tree

‎src/main/java/io/github/guacsec/trustifyda/providers/JavaScriptProvider.java‎

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -159,20 +159,29 @@ private Sbom getDependencySbom() throws IOException {
159159
var sbom = SbomFactory.newInstance();
160160
sbom.addRoot(manifest.root, readLicenseFromManifest());
161161
addDependenciesToSbom(sbom, depTree);
162+
ensurePeerAndOptionalDeps(sbom);
162163
sbom.filterIgnoredDeps(manifest.ignored);
163164
return sbom;
164165
}
165166

166167
protected void addDependenciesToSbom(Sbom sbom, JsonNode depTree) {
167-
var deps = depTree.get("dependencies");
168+
addDependenciesFromKey(sbom, depTree, "dependencies");
169+
addDependenciesFromKey(sbom, depTree, "optionalDependencies");
170+
}
171+
172+
private void addDependenciesFromKey(Sbom sbom, JsonNode depTree, String key) {
173+
var deps = depTree.get(key);
168174
if (deps == null) {
169175
return;
170176
}
171177
deps.fields()
172178
.forEachRemaining(
173179
e -> {
174-
var version = e.getValue().get("version").asText();
175-
var target = toPurl(e.getKey(), version);
180+
JsonNode versionNode = e.getValue().get("version");
181+
if (versionNode == null || versionNode.isNull()) {
182+
return; // skip entries without a resolved version
183+
}
184+
var target = toPurl(e.getKey(), versionNode.asText());
176185
sbom.addDependency(manifest.root, target, null);
177186
addDependenciesOf(sbom, target, e.getValue());
178187
});
@@ -187,6 +196,7 @@ private Sbom getDirectDependencySbom() throws IOException {
187196
.filter(e -> manifest.dependencies.contains(e.getKey()))
188197
.map(Entry::getValue)
189198
.forEach(p -> sbom.addDependency(manifest.root, p, null));
199+
ensurePeerAndOptionalDeps(sbom);
190200
sbom.filterIgnoredDeps(manifest.ignored);
191201
return sbom;
192202
}
@@ -195,9 +205,18 @@ private Sbom getDirectDependencySbom() throws IOException {
195205
// axios -> pkg:npm/axios@0.19.2
196206
protected Map<String, PackageURL> getRootDependencies(JsonNode depTree) {
197207
Map<String, PackageURL> direct = new TreeMap<>();
198-
depTree
199-
.get("dependencies")
200-
.fields()
208+
addRootDependenciesFromKey(direct, depTree, "dependencies");
209+
addRootDependenciesFromKey(direct, depTree, "optionalDependencies");
210+
return direct;
211+
}
212+
213+
private void addRootDependenciesFromKey(
214+
Map<String, PackageURL> direct, JsonNode depTree, String key) {
215+
var node = depTree.get(key);
216+
if (node == null) {
217+
return;
218+
}
219+
node.fields()
201220
.forEachRemaining(
202221
e -> {
203222
String name = e.getKey();
@@ -208,7 +227,21 @@ protected Map<String, PackageURL> getRootDependencies(JsonNode depTree) {
208227
direct.put(name, purl);
209228
}
210229
});
211-
return direct;
230+
}
231+
232+
private void ensurePeerAndOptionalDeps(Sbom sbom) {
233+
var rootComponent = sbom.getRoot();
234+
var depSources = new Map[] {manifest.peerDependencies, manifest.optionalDependencies};
235+
for (var source : depSources) {
236+
@SuppressWarnings("unchecked")
237+
Map<String, String> deps = source;
238+
deps.forEach(
239+
(name, version) -> {
240+
if (!sbom.checkIfPackageInsideDependsOnList(rootComponent, name)) {
241+
sbom.addDependency(manifest.root, toPurl(name, version), null);
242+
}
243+
});
244+
}
212245
}
213246

214247
protected JsonNode buildDependencyTree(boolean includeTransitive) throws JsonProcessingException {

‎src/main/java/io/github/guacsec/trustifyda/providers/YarnBerryProcessor.java‎

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@
2323
import io.github.guacsec.trustifyda.sbom.Sbom;
2424
import io.github.guacsec.trustifyda.tools.Operations;
2525
import java.nio.file.Path;
26+
import java.util.ArrayDeque;
27+
import java.util.HashMap;
28+
import java.util.HashSet;
2629
import java.util.Map;
30+
import java.util.Set;
2731
import java.util.TreeMap;
2832
import java.util.regex.Pattern;
2933

@@ -108,15 +112,77 @@ void addDependenciesToSbom(Sbom sbom, JsonNode depTree) {
108112
if (depTree == null) {
109113
return;
110114
}
115+
116+
Map<String, JsonNode> nodeIndex = new HashMap<>();
117+
depTree.forEach(n -> nodeIndex.put(n.get("value").asText(), n));
118+
119+
Set<String> prodDeps = manifest.dependencies;
120+
Set<String> reachable = new HashSet<>();
121+
var queue = new ArrayDeque<String>();
122+
123+
for (JsonNode n : depTree) {
124+
var depName = n.get("value").asText();
125+
if (isRoot(depName)) {
126+
var deps = (ArrayNode) n.get("children").get("Dependencies");
127+
if (deps != null) {
128+
for (JsonNode d : deps) {
129+
var locator = d.get("locator").asText();
130+
var target = purlFromlocator(locator);
131+
if (target != null) {
132+
var fullName = purlToFullName(target);
133+
if (prodDeps.contains(fullName)) {
134+
queue.add(locator);
135+
}
136+
}
137+
}
138+
}
139+
break;
140+
}
141+
}
142+
143+
Set<String> reachableNodeValues = new HashSet<>();
144+
while (!queue.isEmpty()) {
145+
var locator = queue.poll();
146+
if (reachable.contains(locator)) {
147+
continue;
148+
}
149+
reachable.add(locator);
150+
151+
var nodeValue = nodeValueFromLocator(locator);
152+
reachableNodeValues.add(nodeValue);
153+
154+
var node = nodeIndex.get(nodeValue);
155+
if (node != null) {
156+
var deps = (ArrayNode) node.get("children").get("Dependencies");
157+
if (deps != null) {
158+
for (JsonNode d : deps) {
159+
var childLocator = d.get("locator").asText();
160+
if (!reachable.contains(childLocator)) {
161+
queue.add(childLocator);
162+
}
163+
}
164+
}
165+
}
166+
}
167+
111168
depTree.forEach(
112169
n -> {
113170
var depName = n.get("value").asText();
114-
var from = isRoot(depName) ? sbom.getRoot() : purlFromNode(depName, n);
171+
var isRootNode = isRoot(depName);
172+
if (!isRootNode && !reachableNodeValues.contains(depName)) {
173+
return;
174+
}
175+
176+
var from = isRootNode ? sbom.getRoot() : purlFromNode(depName, n);
115177
var deps = (ArrayNode) n.get("children").get("Dependencies");
116178
if (deps != null && !deps.isEmpty()) {
117179
deps.forEach(
118180
d -> {
119-
var target = purlFromlocator(d.get("locator").asText());
181+
var locator = d.get("locator").asText();
182+
if (!reachable.contains(locator)) {
183+
return;
184+
}
185+
var target = purlFromlocator(locator);
120186
if (target != null) {
121187
sbom.addDependency(from, target, null);
122188
}
@@ -125,6 +191,20 @@ void addDependenciesToSbom(Sbom sbom, JsonNode depTree) {
125191
});
126192
}
127193

194+
private String nodeValueFromLocator(String locator) {
195+
var matcher = VIRTUAL_LOCATOR_PATTERN.matcher(locator);
196+
if (matcher.matches()) {
197+
return matcher.group(1) + "@npm:" + matcher.group(2);
198+
}
199+
return locator;
200+
}
201+
202+
private static String purlToFullName(PackageURL purl) {
203+
return purl.getNamespace() != null
204+
? purl.getNamespace() + "/" + purl.getName()
205+
: purl.getName();
206+
}
207+
128208
private PackageURL purlFromlocator(String locator) {
129209
if (locator == null) return null;
130210
var matcher = LOCATOR_PATTERN.matcher(locator);

‎src/main/java/io/github/guacsec/trustifyda/providers/javascript/model/Manifest.java‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@
2525
import java.nio.file.Files;
2626
import java.nio.file.Path;
2727
import java.util.Collections;
28+
import java.util.HashMap;
2829
import java.util.HashSet;
30+
import java.util.Map;
2931
import java.util.Set;
3032

3133
public class Manifest {
@@ -35,6 +37,8 @@ public class Manifest {
3537
public final String license;
3638
public final PackageURL root;
3739
public final Set<String> dependencies;
40+
public final Map<String, String> peerDependencies;
41+
public final Map<String, String> optionalDependencies;
3842
public final Set<String> ignored;
3943
public final Path path;
4044

@@ -45,6 +49,8 @@ public Manifest(Path manifestPath) throws IOException {
4549
}
4650
var content = loadManifest(manifestPath);
4751
this.dependencies = loadDependencies(content);
52+
this.peerDependencies = loadDependencyMap(content, "peerDependencies");
53+
this.optionalDependencies = loadDependencyMap(content, "optionalDependencies");
4854
this.name = content.get("name").asText();
4955
this.version = content.get("version").asText();
5056
this.license = loadLicense(content);
@@ -63,12 +69,29 @@ private JsonNode loadManifest(Path manifestPath) throws IOException {
6369

6470
private Set<String> loadDependencies(JsonNode content) {
6571
var names = new HashSet<String>();
66-
if (content != null && content.has("dependencies")) {
67-
content.get("dependencies").fieldNames().forEachRemaining(names::add);
72+
if (content != null) {
73+
addFieldNames(names, content, "dependencies");
74+
addFieldNames(names, content, "optionalDependencies");
75+
addFieldNames(names, content, "peerDependencies");
6876
}
6977
return Collections.unmodifiableSet(names);
7078
}
7179

80+
private void addFieldNames(Set<String> names, JsonNode content, String field) {
81+
if (content.has(field)) {
82+
content.get(field).fieldNames().forEachRemaining(names::add);
83+
}
84+
}
85+
86+
private Map<String, String> loadDependencyMap(JsonNode content, String field) {
87+
if (content == null || !content.has(field)) {
88+
return Collections.emptyMap();
89+
}
90+
var map = new HashMap<String, String>();
91+
content.get(field).fields().forEachRemaining(e -> map.put(e.getKey(), e.getValue().asText()));
92+
return Collections.unmodifiableMap(map);
93+
}
94+
7295
private String loadLicense(JsonNode content) {
7396
if (content == null) {
7497
return null;

‎src/main/java/io/github/guacsec/trustifyda/sbom/CycloneDXSbom.java‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -337,18 +337,31 @@ public boolean checkIfPackageInsideDependsOnList(PackageURL component, String na
337337
List<Dependency> deps = targetComponent.getDependencies();
338338
List<PackageURL> allDirectDeps = Collections.emptyList();
339339
if (deps != null) {
340-
deps.stream()
341-
.map(
342-
dep -> {
343-
try {
344-
return new PackageURL(dep.getRef());
345-
} catch (MalformedPackageURLException e) {
346-
throw new RuntimeException(e);
347-
}
348-
})
349-
.collect(Collectors.toList());
340+
allDirectDeps =
341+
deps.stream()
342+
.map(
343+
dep -> {
344+
try {
345+
return new PackageURL(dep.getRef());
346+
} catch (MalformedPackageURLException e) {
347+
throw new RuntimeException(e);
348+
}
349+
})
350+
.collect(Collectors.toList());
350351
}
351-
result = allDirectDeps.stream().anyMatch(dep -> dep.getName().equals(name));
352+
result =
353+
allDirectDeps.stream()
354+
.anyMatch(
355+
dep -> {
356+
if (dep.getName().equals(name)) {
357+
return true;
358+
}
359+
var fullName =
360+
dep.getNamespace() != null
361+
? dep.getNamespace() + "/" + dep.getName()
362+
: dep.getName();
363+
return fullName.equals(name);
364+
});
352365
}
353366
return result;
354367
}

‎src/test/java/io/github/guacsec/trustifyda/providers/Javascript_Provider_Test.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class Javascript_Provider_Test extends ExhortTest {
4545
// - package.json: the target manifest for testing
4646
// - expected_sbom.json: the SBOM expected to be provided
4747
static Stream<String> testFolders() {
48-
return Stream.of("deps_with_ignore", "deps_with_no_ignore");
48+
return Stream.of("deps_with_ignore", "deps_with_no_ignore", "deps_with_mixed_dep_types");
4949
}
5050

5151
static Stream<String> providers() {

‎src/test/java/io/github/guacsec/trustifyda/providers/ManifestTest.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Copyright 2023-2025 Trustify Dependency Analytics Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
*
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package io.github.guacsec.trustifyda.providers;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
21+
import io.github.guacsec.trustifyda.ExhortTest;
22+
import io.github.guacsec.trustifyda.providers.javascript.model.Manifest;
23+
import java.io.IOException;
24+
import java.util.Map;
25+
import org.junit.jupiter.api.Test;
26+
27+
class ManifestTest extends ExhortTest {
28+
29+
@Test
30+
void loads_manifest_with_mixed_dependency_types() throws IOException {
31+
var manifestPath = resolveFile("tst_manifests/npm/deps_with_mixed_dep_types/package.json");
32+
var m = new Manifest(manifestPath);
33+
34+
assertThat(m.name).isEqualTo("mixed-deps-test");
35+
assertThat(m.version).isEqualTo("1.0.0");
36+
37+
// dependencies should include deps, peerDeps, and optionalDeps but NOT devDeps
38+
assertThat(m.dependencies).containsExactlyInAnyOrder("express", "axios", "minimist", "lodash");
39+
assertThat(m.dependencies).doesNotContain("jest", "eslint");
40+
41+
// peerDependencies and optionalDependencies maps
42+
assertThat(m.peerDependencies).isEqualTo(Map.of("minimist", "1.2.0"));
43+
assertThat(m.optionalDependencies).isEqualTo(Map.of("lodash", "4.17.19"));
44+
45+
assertThat(m.ignored).isEmpty();
46+
}
47+
}

‎src/test/resources/tst_manifests/golang/go_mod_with_all_ignore/expected_sbom_component_analysis.json‎

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,26 +12,10 @@
1212
"purl": "pkg:golang/github.com/devfile-samples/devfile-sample-go-basic@v0.0.0"
1313
}
1414
},
15-
"components": [
16-
{
17-
"type": "library",
18-
"bom-ref": "pkg:golang/github.com/miekg/dns@v1.1.12",
19-
"group": "github.com/miekg",
20-
"name": "dns",
21-
"version": "v1.1.12",
22-
"purl": "pkg:golang/github.com/miekg/dns@v1.1.12"
23-
}
24-
],
2515
"dependencies": [
2616
{
2717
"ref": "pkg:golang/github.com/devfile-samples/devfile-sample-go-basic@v0.0.0",
28-
"dependsOn": [
29-
"pkg:golang/github.com/miekg/dns@v1.1.12"
30-
]
31-
},
32-
{
33-
"ref": "pkg:golang/github.com/miekg/dns@v1.1.12",
34-
"dependsOn": []
18+
"dependsOn": [ ]
3519
}
3620
]
3721
}

0 commit comments

Comments
 (0)