Skip to content

fix(maven): resolve version ranges in component analysis #491

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Strum355 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(maven): resolve version ranges in component analysis #491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions src/main/java/io/github/guacsec/trustifyda/providers/JavaMavenProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -228,6 +229,7 @@ private Content generateSbomFromEffectivePom() throws IOException {
.filter(DependencyAggregator::isTestDependency)
.collect(Collectors.toSet());
var deps = getDependencies(tmpEffPom);
deps = resolveVersionRanges(deps);
var sbom = SbomFactory.newInstance().addRoot(getRoot(tmpEffPom), readLicenseFromManifest());
deps.stream()
.filter(dep -> !testsDeps.contains(dep))
Expand All @@ -239,6 +241,82 @@ private Content generateSbomFromEffectivePom() throws IOException {
return new Content(sbom.getAsJsonString().getBytes(), Api.CYCLONEDX_MEDIA_TYPE);
}

/**
* Checks whether the given version string is a Maven version range expression (starts with '[' or
* '(').
*/
private static boolean isVersionRange(String version) {
if (version == null || version.isEmpty()) return false;
char first = version.charAt(0);
return first == '[' || first == '(';
}

/**
* Resolves Maven version ranges by running the dependency tree plugin and replacing range
* expressions with concrete resolved versions. If no version ranges are present, the original
* list is returned unchanged. On failure, the original list is returned with a warning logged.
*/
private List<DependencyAggregator> resolveVersionRanges(List<DependencyAggregator> deps)
throws IOException {
// Short-circuit: if no dep has a version range, return unchanged
boolean hasRanges = deps.stream().anyMatch(d -> isVersionRange(d.version));
if (!hasRanges) {
return deps;
}

Path tmpFile = Files.createTempFile("TRUSTIFY_DA_range_tree_", ".txt");
try {
Comment on lines +259 to +268
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Deleting the temp file can throw and violate the "return original list" fallback guarantee

Per the Javadoc, failures in resolving version ranges should result in the original list being returned unchanged. However, Files.deleteIfExists(tmpFile) in the finally block can throw an IOException that bypasses the existing catch (Exception e) and leaks out of the method, violating that contract. Please isolate the delete in its own try/catch (logging or ignoring failures) so cleanup errors don’t alter the method’s behavior, or adjust the signature/contract if you intend to let such IOExceptions propagate.

var cmd =
buildMvnCommandArgs(
"org.apache.maven.plugins:maven-dependency-plugin:3.6.0:tree",
"-Dscope=compile",
"-DoutputType=text",
String.format("-DoutputFile=%s", tmpFile.toString()),
"-f",
manifestPath.toString(),
"--batch-mode",
"-q");
Operations.runProcess(manifestPath.getParent(), cmd.toArray(String[]::new), getMvnExecEnvs());

// Read the dependency tree output and build a lookup map of resolved versions
List<String> lines = Files.readAllLines(tmpFile);
Map<String, String> resolvedVersions = new HashMap<>();
for (String line : lines) {
if (getDepth(line) == 1) {
DependencyAggregator resolved = parseDep(line);
resolvedVersions.put(resolved.groupId + ":" + resolved.artifactId, resolved.version);
}
}

// Replace version ranges with resolved concrete versions
for (DependencyAggregator dep : deps) {
if (isVersionRange(dep.version)) {
String key = dep.groupId + ":" + dep.artifactId;
String resolved = resolvedVersions.get(key);
if (resolved != null) {
if (debugLoggingIsNeeded()) {
log.info(
String.format(
"Resolved version range for %s: %s -> %s", key, dep.version, resolved));
}
dep.version = resolved;
}
}
}
} catch (Exception e) {
log.warning(
String.format(
"Failed to resolve version ranges via dependency tree, "
+ "using original versions: %s",
e.getMessage()));
return deps;
} finally {
Files.deleteIfExists(tmpFile);
}

return deps;
}

private PackageURL getRoot(final Path manifestPath) throws IOException {
XMLStreamReader reader = null;
try {
Expand Down
45 changes: 40 additions & 5 deletions src/test/java/io/github/guacsec/trustifyda/providers/Java_Maven_Provider_Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ static Stream<String> testFolders() {
"deps_with_ignore_on_version",
"deps_with_ignore_on_wrong",
"deps_with_no_ignore",
"pom_deps_with_no_ignore_common_paths");
"pom_deps_with_no_ignore_common_paths",
"deps_with_version_range");
}

@ParameterizedTest
Expand Down Expand Up @@ -143,12 +144,29 @@ void test_the_provideComponent(String testFolder) throws IOException {
getClass(), String.format("tst_manifests/maven/%s/effectivePom.xml", testFolder))) {
effectivePom = new String(is.readAllBytes());
}

String depTree;
try (var is =
getResourceAsStreamDecision(
getClass(), String.format("tst_manifests/maven/%s/depTree.txt", testFolder))) {
depTree = new String(is.readAllBytes());
}

try (MockedStatic<Operations> mockedOperations = mockStatic(Operations.class)) {
mockedOperations
.when(() -> Operations.runProcess(any(), any(), any()))
.thenAnswer(
invocationOnMock ->
getOutputFileAndOverwriteItWithMock(effectivePom, invocationOnMock, "-Doutput"));
invocationOnMock -> {
Comment on lines 155 to +159
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add a test ensuring we fall back gracefully when the dependency:tree invocation fails

The current tests only cover the successful path where Operations.runProcess writes both the effective POM and dep tree outputs. Please add a test (or parameterized case) where the mock throws for the -DoutputFile invocation to verify that:

  • provideComponent / provideComponent_With_Path swallow the exception, and
  • the SBOM is still produced (with unresolved versions, as now).

This will lock in the intended failure behavior and guard against regressions in error handling.

Suggested implementation:

      mockedOperations
          .when(() -> Operations.runProcess(any(), any(), any()))
          .thenAnswer(
              invocationOnMock -> {
                String result =
                    getOutputFileAndOverwriteItWithMock(
                        effectivePom, invocationOnMock, "-Doutput=");
                if (result == null) {
                  result =
                      getOutputFileAndOverwriteItWithMock(
                          depTree, invocationOnMock, "-DoutputFile");
                }
                return result;
              });
      // Mock Operations.getCustomPathOrElse to return "mvn"
      mockedOperations.when(() -> Operations.getCustomPathOrElse(anyString())).thenReturn("mvn");

To implement the requested test coverage for the failure path of the dependency:tree invocation, add the following tests to Java_Maven_Provider_Test.java inside the test class (alongside the existing tests for provideComponent / provideComponent_With_Path). You may need to adjust variable names (testFolder, provider, etc.) to match your existing test setup.

  1. Add a test for provideComponent that simulates a failure of the -DoutputFile / dep tree invocation:
@Test
void testProvideComponent_DependencyTreeFailureFallsBackGracefully() throws Exception {
  String testFolder = "basic";

  String effectivePom;
  try (var is =
      getResourceAsStreamDecision(
          getClass(), String.format("tst_manifests/maven/%s/effectivePom.xml", testFolder))) {
    effectivePom = new String(is.readAllBytes(), StandardCharsets.UTF_8);
  }

  try (MockedStatic<Operations> mockedOperations = mockStatic(Operations.class)) {
    mockedOperations
        .when(() -> Operations.runProcess(any(), any(), any()))
        .thenAnswer(
            invocationOnMock -> {
              // First, handle the effective POM generation as usual
              String result =
                  getOutputFileAndOverwriteItWithMock(
                      effectivePom, invocationOnMock, "-Doutput=");
              if (result == null) {
                // Simulate failure of the dependency:tree invocation (-DoutputFile)
                throw new IOException("Simulated dependency:tree failure");
              }
              return result;
            });

    mockedOperations.when(() -> Operations.getCustomPathOrElse(anyString())).thenReturn("mvn");

    Java_Maven_Provider provider = new Java_Maven_Provider();
    Optional<Component> componentOpt = provider.provideComponent(testFolder);

    // Verify that the provider swallowed the failure and still produced a component / SBOM
    assertThat(componentOpt).isPresent();
    Component component = componentOpt.get();
    assertThat(component).isNotNull();

    // If your existing tests assert on SBOM contents, add similar assertions here.
    // For example, if unresolved versions are expected when dep tree is missing:
    // assertThat(component.getVersion()).isNull();
    // or check for whatever "unresolved" representation you're currently using.
  }
}
  1. Add a similar test for provideComponent_With_Path if that method exists and is covered by the existing success-path tests:
@Test
void testProvideComponentWithPath_DependencyTreeFailureFallsBackGracefully() throws Exception {
  String testFolder = "basic";

  String effectivePom;
  try (var is =
      getResourceAsStreamDecision(
          getClass(), String.format("tst_manifests/maven/%s/effectivePom.xml", testFolder))) {
    effectivePom = new String(is.readAllBytes(), StandardCharsets.UTF_8);
  }

  Path projectPath =
      Paths.get("src", "test", "resources", "tst_manifests", "maven", testFolder).toAbsolutePath();

  try (MockedStatic<Operations> mockedOperations = mockStatic(Operations.class)) {
    mockedOperations
        .when(() -> Operations.runProcess(any(), any(), any()))
        .thenAnswer(
            invocationOnMock -> {
              String result =
                  getOutputFileAndOverwriteItWithMock(
                      effectivePom, invocationOnMock, "-Doutput=");
              if (result == null) {
                // Simulate failure of the dependency:tree invocation (-DoutputFile)
                throw new IOException("Simulated dependency:tree failure");
              }
              return result;
            });

    mockedOperations.when(() -> Operations.getCustomPathOrElse(anyString())).thenReturn("mvn");

    Java_Maven_Provider provider = new Java_Maven_Provider();
    Optional<Component> componentOpt = provider.provideComponent_With_Path(projectPath);

    // Verify that the provider swallowed the failure and still produced a component / SBOM
    assertThat(componentOpt).isPresent();
    Component component = componentOpt.get();
    assertThat(component).isNotNull();

    // As above, assert on unresolved versions / missing dep-tree information as appropriate.
  }
}
  1. Notes / integration details:
  • Ensure you import any additional types used above if not already present: 
    import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Optional;
  • The helper getOutputFileAndOverwriteItWithMock(...) and getResourceAsStreamDecision(...) are reused exactly as in your existing tests.
  • Update the assertions at the end of each test to match however your SBOM represents unresolved dependency versions today (e.g., null version, placeholder string, absent dependencies, etc.). The key invariant is that:
    • No exception propagates from provideComponent / provideComponent_With_Path, and
    • A non-null SBOM / component is still returned despite the simulated dependency:tree failure.

String result =
getOutputFileAndOverwriteItWithMock(
effectivePom, invocationOnMock, "-Doutput=");
if (result == null) {
result =
getOutputFileAndOverwriteItWithMock(
depTree, invocationOnMock, "-DoutputFile");
}
return result;
});
// Mock Operations.getCustomPathOrElse to return "mvn"
mockedOperations.when(() -> Operations.getCustomPathOrElse(anyString())).thenReturn("mvn");
mockedOperations
Expand Down Expand Up @@ -189,12 +207,29 @@ void test_the_provideComponent_With_Path(String testFolder) throws IOException {
getClass(), String.format("tst_manifests/maven/%s/effectivePom.xml", testFolder))) {
effectivePom = new String(is.readAllBytes());
}

String depTree;
try (var is =
getResourceAsStreamDecision(
getClass(), String.format("tst_manifests/maven/%s/depTree.txt", testFolder))) {
depTree = new String(is.readAllBytes());
}

try (MockedStatic<Operations> mockedOperations = mockStatic(Operations.class)) {
mockedOperations
.when(() -> Operations.runProcess(any(), any(), any()))
.thenAnswer(
invocationOnMock ->
getOutputFileAndOverwriteItWithMock(effectivePom, invocationOnMock, "-Doutput"));
invocationOnMock -> {
String result =
getOutputFileAndOverwriteItWithMock(
effectivePom, invocationOnMock, "-Doutput=");
if (result == null) {
result =
getOutputFileAndOverwriteItWithMock(
depTree, invocationOnMock, "-DoutputFile");
}
return result;
});
// Mock Operations.getCustomPathOrElse to return "mvn"
mockedOperations.when(() -> Operations.getCustomPathOrElse(anyString())).thenReturn("mvn");
mockedOperations
Expand Down
3 changes: 3 additions & 0 deletions src/test/resources/tst_manifests/maven/deps_with_version_range/depTree.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pom-with-deps-version-range:pom-with-version-range-for-tests:jar:0.0.1
+- log4j:log4j:jar:1.2.17:compile
\- org.xerial.snappy:snappy-java:jar:1.1.10.0:compile
Loading
Loading