Skip to content

Commit cddf65b

Browse files
ruromeroruromero
authored
fix(npm): stop skipping dependency subtree when root entry has no version (guacsec#423)
## Summary - Fix `JavaScriptProvider.addDependenciesFromKey()` which skipped both the entry and its entire transitive dependency subtree when a root-level dependency had no version field (e.g., `file:` deps, workspace packages, linked packages) - Now treats null version as valid (produces a versionless PURL) and always recurses into children, matching the JS client's `_addDependenciesToSbom()` behavior - Updated reproducer test comments to reflect the fix (TC-3818 / TC-4128) ### Root cause The null-version check at line 181 did `return` inside a `forEachRemaining` lambda, which exited the lambda body entirely — skipping both the `sbom.addDependency()` call and the `addDependenciesOf()` recursion for that entry's children. ### Fix Replace the early-return guard with a null-safe version extraction that passes `null` to `toPurl()` (valid per PURL spec — version is optional), then unconditionally recurses into children. Implements [TC-4128](https://redhat.atlassian.net/browse/TC-4128) ## Test plan - [x] Reproducer test `test_provideStack_includes_deps_of_root_entry_without_version` passes (7 components) - [x] All 37 JavaScript provider tests pass (npm, pnpm, yarn-classic, yarn-berry) - [x] Spotless formatting clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) [TC-4128]: https://redhat.atlassian.net/browse/TC-4128?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
1 parent 1baa6a8 commit cddf65b

13 files changed

Lines changed: 209 additions & 68 deletions

File tree

‎src/main/java/io/github/guacsec/trustifyda/Provider.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,13 @@ public Content(byte[] buffer, String type) {
4747
/** The ecosystem of this provider, i.e. maven. */
4848
public final Ecosystem.Type ecosystem;
4949

50-
public final Path manifest;
50+
public final Path manifestPath;
5151

5252
protected final ObjectMapper objectMapper = new ObjectMapper();
5353

54-
protected Provider(Ecosystem.Type ecosystem, Path manifest) {
54+
protected Provider(Ecosystem.Type ecosystem, Path manifestPath) {
5555
this.ecosystem = ecosystem;
56-
this.manifest = manifest;
56+
this.manifestPath = manifestPath;
5757
}
5858

5959
/**

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

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ private void addDependencies(
146146
handleSingleCrate(sbom, root, metadata, nodeMap, packageMap, ignoredDeps, analysisType);
147147
}
148148

149-
} catch (Exception e) {
149+
} catch (IOException | InterruptedException e) {
150150
log.severe("Unexpected error during " + analysisType + " analysis: " + e.getMessage());
151151
}
152152
}
@@ -385,7 +385,7 @@ private Path findOutermostCargoTomlDirectory(Path startDir) {
385385
}
386386

387387
private CargoMetadata executeCargoMetadata() throws IOException, InterruptedException {
388-
Path workingDir = manifest.getParent();
388+
Path workingDir = manifestPath.getParent();
389389

390390
if (debugLoggingIsNeeded()) {
391391
log.info("Executing cargo metadata for full dependency resolution with resolved versions");
@@ -657,20 +657,21 @@ public CargoProvider(Path manifest) {
657657
@Override
658658
public String readLicenseFromManifest() {
659659
String manifestLicense = readLicenseFromToml(null);
660-
return LicenseUtils.getLicense(manifestLicense, manifest);
660+
return LicenseUtils.getLicense(manifestLicense, manifestPath);
661661
}
662662

663663
private String readLicenseFromToml(TomlParseResult existingResult) {
664664
try {
665-
TomlParseResult tomlResult = existingResult != null ? existingResult : Toml.parse(manifest);
665+
TomlParseResult tomlResult =
666+
existingResult != null ? existingResult : Toml.parse(manifestPath);
666667
if (tomlResult.hasErrors()) {
667668
return null;
668669
}
669670
String license = tomlResult.getString("package.license");
670-
return LicenseUtils.getLicense(license, manifest);
671+
return LicenseUtils.getLicense(license, manifestPath);
671672
} catch (IOException e) {
672673
log.warning("Failed to read license from Cargo.toml: " + e.getMessage());
673-
return LicenseUtils.getLicense(null, manifest);
674+
return LicenseUtils.getLicense(null, manifestPath);
674675
}
675676
}
676677

@@ -687,11 +688,11 @@ public Content provideStack() throws IOException {
687688
}
688689

689690
private Sbom createSbom(AnalysisType analysisType) throws IOException {
690-
if (!Files.exists(manifest) || !Files.isRegularFile(manifest)) {
691-
throw new IOException("Cargo.toml not found: " + manifest);
691+
if (!Files.exists(manifestPath) || !Files.isRegularFile(manifestPath)) {
692+
throw new IOException("Cargo.toml not found: " + manifestPath);
692693
}
693694

694-
TomlParseResult tomlResult = Toml.parse(manifest);
695+
TomlParseResult tomlResult = Toml.parse(manifestPath);
695696
if (tomlResult.hasErrors()) {
696697
throw new IOException(
697698
"Invalid Cargo.toml format: " + tomlResult.errors().get(0).getMessage());
@@ -706,7 +707,7 @@ private Sbom createSbom(AnalysisType analysisType) throws IOException {
706707
Type.CARGO.getType(), null, projectInfo.name(), projectInfo.version(), null, null);
707708
sbom.addRoot(root, readLicenseFromToml(tomlResult));
708709

709-
String cargoContent = Files.readString(manifest, StandardCharsets.UTF_8);
710+
String cargoContent = Files.readString(manifestPath, StandardCharsets.UTF_8);
710711
Set<String> ignoredDeps = getIgnoredDependencies(tomlResult, cargoContent);
711712
addDependencies(sbom, root, ignoredDeps, tomlResult, analysisType);
712713
return sbom;
@@ -744,7 +745,7 @@ private ProjectInfo parseCargoToml(TomlParseResult result) throws IOException {
744745
boolean hasWorkspace = result.contains("workspace");
745746
if (hasWorkspace) {
746747
String workspaceVersion = result.getString(WORKSPACE_PACKAGE_VERSION);
747-
String dirName = manifest.toAbsolutePath().getParent().getFileName().toString();
748+
String dirName = manifestPath.toAbsolutePath().getParent().getFileName().toString();
748749
if (debugLoggingIsNeeded()) {
749750
log.info(
750751
"Using workspace fallback: name="

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,27 +73,27 @@ public GoModulesProvider(Path manifest) {
7373

7474
@Override
7575
public String readLicenseFromManifest() {
76-
return LicenseUtils.readLicenseFile(manifest);
76+
return LicenseUtils.readLicenseFile(manifestPath);
7777
}
7878

7979
@Override
8080
public Content provideStack() throws IOException {
8181
// check for custom executable
82-
Sbom sbom = getDependenciesSbom(manifest, true);
82+
Sbom sbom = getDependenciesSbom(manifestPath, true);
8383
return new Content(
8484
sbom.getAsJsonString().getBytes(StandardCharsets.UTF_8), Api.CYCLONEDX_MEDIA_TYPE);
8585
}
8686

8787
@Override
8888
public Content provideComponent() throws IOException {
89-
if (!Files.exists(manifest)) {
90-
throw new IllegalArgumentException("Missing required go.mod file: " + manifest);
89+
if (!Files.exists(manifestPath)) {
90+
throw new IllegalArgumentException("Missing required go.mod file: " + manifestPath);
9191
}
92-
if (!Files.isRegularFile(manifest)) {
92+
if (!Files.isRegularFile(manifestPath)) {
9393
throw new IllegalArgumentException(
94-
"The provided manifest is not a regular file: " + manifest);
94+
"The provided manifest is not a regular file: " + manifestPath);
9595
}
96-
var sbom = getDependenciesSbom(manifest, false);
96+
var sbom = getDependenciesSbom(manifestPath, false);
9797
return new Content(
9898
sbom.getAsJsonString().getBytes(StandardCharsets.UTF_8), Api.CYCLONEDX_MEDIA_TYPE);
9999
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,12 @@ public GradleProvider(Path manifest) {
7070

7171
@Override
7272
public String readLicenseFromManifest() {
73-
return LicenseUtils.readLicenseFile(manifest);
73+
return LicenseUtils.readLicenseFile(manifestPath);
7474
}
7575

7676
@Override
7777
public Content provideStack() throws IOException {
78-
Path tempFile = getDependencies(manifest);
78+
Path tempFile = getDependencies(manifestPath);
7979
try {
8080
if (debugLoggingIsNeeded()) {
8181
String stackAnalysisDependencyTree = Files.readString(tempFile);
@@ -84,10 +84,10 @@ public Content provideStack() throws IOException {
8484
"Package Manager Gradle Stack Analysis Dependency Tree Output: %s %s",
8585
System.lineSeparator(), stackAnalysisDependencyTree));
8686
}
87-
Map<String, String> propertiesMap = extractProperties(manifest);
87+
Map<String, String> propertiesMap = extractProperties(manifestPath);
8888

8989
var sbom = buildSbomFromTextFormat(tempFile, propertiesMap, AnalysisType.STACK);
90-
var ignored = getIgnoredDeps(manifest);
90+
var ignored = getIgnoredDeps(manifestPath);
9191

9292
return new Content(
9393
sbom.filterIgnoredDeps(ignored).getAsJsonString().getBytes(), Api.CYCLONEDX_MEDIA_TYPE);
@@ -605,12 +605,12 @@ private List<String> extractLines(Path inputFilePath, String startMarker) throws
605605

606606
@Override
607607
public Content provideComponent() throws IOException {
608-
Path tempFile = getDependencies(manifest);
608+
Path tempFile = getDependencies(manifestPath);
609609
try {
610-
Map<String, String> propertiesMap = extractProperties(manifest);
610+
Map<String, String> propertiesMap = extractProperties(manifestPath);
611611

612612
Sbom sbom = buildSbomFromTextFormat(tempFile, propertiesMap, AnalysisType.COMPONENT);
613-
var ignored = getIgnoredDeps(manifest);
613+
var ignored = getIgnoredDeps(manifestPath);
614614

615615
return new Content(
616616
sbom.filterIgnoredDeps(ignored).getAsJsonString().getBytes(), Api.CYCLONEDX_MEDIA_TYPE);

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

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ public JavaMavenProvider(Path manifest) {
6868

6969
@Override
7070
public String readLicenseFromManifest() {
71-
String manifestLicense = readLicenseFromPom(manifest);
72-
return LicenseUtils.getLicense(manifestLicense, manifest);
71+
String manifestLicense = readLicenseFromPom(manifestPath);
72+
return LicenseUtils.getLicense(manifestLicense, manifestPath);
7373
}
7474

7575
/**
@@ -124,10 +124,11 @@ private String readLicenseFromPom(Path pomPath) {
124124

125125
@Override
126126
public Content provideStack() throws IOException {
127-
var mvnCleanCmd = buildMvnCommandArgs("clean", "-f", manifest.toString(), "--batch-mode", "-q");
127+
var mvnCleanCmd =
128+
buildMvnCommandArgs("clean", "-f", manifestPath.toString(), "--batch-mode", "-q");
128129
var mvnEnvs = getMvnExecEnvs();
129130
// execute the clean command
130-
Operations.runProcess(manifest.getParent(), mvnCleanCmd.toArray(String[]::new), mvnEnvs);
131+
Operations.runProcess(manifestPath.getParent(), mvnCleanCmd.toArray(String[]::new), mvnEnvs);
131132
// create a temp file for storing the dependency tree in
132133
var tmpFile = Files.createTempFile("TRUSTIFY_DA_dot_graph_", null);
133134
// the tree command will build the project and create the dependency tree in the temp file
@@ -140,12 +141,12 @@ public Content provideStack() throws IOException {
140141
"-DoutputType=text",
141142
String.format("-DoutputFile=%s", tmpFile.toString()),
142143
"-f",
143-
manifest.toString(),
144+
manifestPath.toString(),
144145
"--batch-mode",
145146
"-q"));
146147
// if we have dependencies marked as ignored, exclude them from the tree command
147148
var ignored =
148-
getDependencies(manifest).stream()
149+
getDependencies(manifestPath).stream()
149150
.filter(d -> d.ignored)
150151
.map(DependencyAggregator::toPurlWithoutVersion)
151152
.map(PackageURL::getCoordinates)
@@ -157,7 +158,7 @@ public Content provideStack() throws IOException {
157158

158159
// execute the tree command
159160
var mvnTreeCmd = buildMvnCommandArgs(mvnTreeCmdArgs.toArray(String[]::new));
160-
Operations.runProcess(manifest.getParent(), mvnTreeCmd.toArray(String[]::new), mvnEnvs);
161+
Operations.runProcess(manifestPath.getParent(), mvnTreeCmd.toArray(String[]::new), mvnEnvs);
161162
if (debugLoggingIsNeeded()) {
162163
String stackAnalysisDependencyTree = Files.readString(tmpFile);
163164
log.info(
@@ -201,12 +202,12 @@ private Content generateSbomFromEffectivePom() throws IOException {
201202
"help:effective-pom",
202203
String.format("-Doutput=%s", tmpEffPom.toString()),
203204
"-f",
204-
manifest.toString(),
205+
manifestPath.toString(),
205206
"--batch-mode",
206207
"-q");
207208
// execute the effective pom command
208209
Operations.runProcess(
209-
manifest.getParent(), mvnEffPomCmd.toArray(String[]::new), getMvnExecEnvs());
210+
manifestPath.getParent(), mvnEffPomCmd.toArray(String[]::new), getMvnExecEnvs());
210211
if (debugLoggingIsNeeded()) {
211212
String CaEffectivePoM = Files.readString(tmpEffPom);
212213
log.info(
@@ -216,7 +217,7 @@ private Content generateSbomFromEffectivePom() throws IOException {
216217
}
217218
// if we have dependencies marked as ignored grab ignored dependencies from the original pom
218219
// the effective-pom goal doesn't carry comments
219-
List<DependencyAggregator> dependencies = getDependencies(manifest);
220+
List<DependencyAggregator> dependencies = getDependencies(manifestPath);
220221
var ignored =
221222
dependencies.stream()
222223
.filter(d -> d.ignored)
@@ -437,7 +438,7 @@ private String selectMvnRuntime(final Path manifestPath) {
437438
if (mvnw != null) {
438439
try {
439440
// verify maven wrapper is accessible
440-
Operations.runProcess(manifest.getParent(), mvnw, ARG_VERSION);
441+
Operations.runProcess(manifestPath.getParent(), mvnw, ARG_VERSION);
441442
if (debugLoggingIsNeeded()) {
442443
log.info(String.format("using maven wrapper from : %s", mvnw));
443444
}

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

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import java.nio.file.Files;
3939
import java.nio.file.Path;
4040
import java.util.Collections;
41-
import java.util.Iterator;
4241
import java.util.Map;
4342
import java.util.Map.Entry;
4443
import java.util.TreeMap;
@@ -128,9 +127,7 @@ private void addDependenciesOf(Sbom sbom, PackageURL from, JsonNode node) {
128127
if (dependencies == null) {
129128
return;
130129
}
131-
Iterator<Entry<String, JsonNode>> fields = dependencies.fields();
132-
while (fields.hasNext()) {
133-
Entry<String, JsonNode> e = fields.next();
130+
for (var e : dependencies.properties()) {
134131
String name = e.getKey();
135132
JsonNode versionNode = e.getValue().get("version");
136133
if (versionNode == null) {
@@ -174,14 +171,13 @@ private void addDependenciesFromKey(Sbom sbom, JsonNode depTree, String key) {
174171
if (deps == null) {
175172
return;
176173
}
177-
deps.fields()
178-
.forEachRemaining(
174+
deps.properties()
175+
.forEach(
179176
e -> {
180177
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());
178+
String version =
179+
(versionNode != null && !versionNode.isNull()) ? versionNode.asText() : null;
180+
var target = toPurl(e.getKey(), version);
185181
sbom.addDependency(manifest.root, target, null);
186182
addDependenciesOf(sbom, target, e.getValue());
187183
});
@@ -216,16 +212,15 @@ private void addRootDependenciesFromKey(
216212
if (node == null) {
217213
return;
218214
}
219-
node.fields()
220-
.forEachRemaining(
215+
node.properties()
216+
.forEach(
221217
e -> {
222218
String name = e.getKey();
223219
JsonNode versionNode = e.getValue().get("version");
224-
if (versionNode != null) {
225-
String version = versionNode.asText();
226-
PackageURL purl = toPurl(name, version);
227-
direct.put(name, purl);
228-
}
220+
String version =
221+
(versionNode != null && !versionNode.isNull()) ? versionNode.asText() : null;
222+
PackageURL purl = toPurl(name, version);
223+
direct.put(name, purl);
229224
});
230225
}
231226

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,16 @@ public void setPythonController(PythonControllerBase pythonController) {
5757
@Override
5858
public Content provideStack() throws IOException {
5959
PythonControllerBase controller = getPythonController();
60-
List<Map<String, Object>> dependencies = controller.getDependencies(manifest.toString(), true);
60+
List<Map<String, Object>> dependencies =
61+
controller.getDependencies(manifestPath.toString(), true);
6162
printDependenciesTree(dependencies);
6263
Sbom sbom = SbomFactory.newInstance(Sbom.BelongingCondition.PURL, "sensitive");
6364
sbom.addRoot(
6465
toPurl(getRootComponentName(), getRootComponentVersion()), readLicenseFromManifest());
6566
for (Map<String, Object> component : dependencies) {
6667
addAllDependencies(sbom.getRoot(), component, sbom);
6768
}
68-
String manifestContent = Files.readString(manifest);
69+
String manifestContent = Files.readString(manifestPath);
6970
handleIgnoredDependencies(manifestContent, sbom);
7071
return new Content(
7172
sbom.getAsJsonString().getBytes(StandardCharsets.UTF_8), Api.CYCLONEDX_MEDIA_TYPE);
@@ -74,7 +75,8 @@ public Content provideStack() throws IOException {
7475
@Override
7576
public Content provideComponent() throws IOException {
7677
PythonControllerBase controller = getPythonController();
77-
List<Map<String, Object>> dependencies = controller.getDependencies(manifest.toString(), false);
78+
List<Map<String, Object>> dependencies =
79+
controller.getDependencies(manifestPath.toString(), false);
7880
printDependenciesTree(dependencies);
7981
Sbom sbom = SbomFactory.newInstance();
8082
sbom.addRoot(
@@ -85,7 +87,7 @@ public Content provideComponent() throws IOException {
8587
sbom.getRoot(),
8688
toPurl((String) component.get("name"), (String) component.get("version")),
8789
null));
88-
String manifestContent = Files.readString(manifest);
90+
String manifestContent = Files.readString(manifestPath);
8991
handleIgnoredDependencies(manifestContent, sbom);
9092
return new Content(
9193
sbom.getAsJsonString().getBytes(StandardCharsets.UTF_8), Api.CYCLONEDX_MEDIA_TYPE);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ protected PythonProvider(Path manifest) {
4343

4444
@Override
4545
public String readLicenseFromManifest() {
46-
return LicenseUtils.readLicenseFile(manifest);
46+
return LicenseUtils.readLicenseFile(manifestPath);
4747
}
4848

4949
protected String getRootComponentName() {

0 commit comments

Comments
 (0)