feat!: set golang mvs true as default (#188)

## Description

Set the use of the MVS algorithm default to `true` as it is the default
behaviour in Go
* Fix how environment variables are parsed in Windows
* Fix NPE when processing dependencies
* The last dependency was being ignored

**Related issue (if any):** 
fixes #174 
fixes #192

## Checklist

- [x] I have followed this repository's contributing guidelines.
- [x] I will adhere to the project's code of conduct.

---------

Signed-off-by: Ruben Romero Montes <[email protected]>

Author: ruromero

README.md

@@ -506,22 +506,23 @@ Two possible values for this setting:
 
 #### Golang Support
 
-By default, all go.mod' packages' transitive modules will be taken to analysis with their original package version, that is,
-if go.mod has 2 modules, `a` and `b`, and each one of them has the same package c with same major version v1, but different minor versions:
-- namespace/c/[email protected]
-- namespace/c/[email protected]
+By default, Golang dependency resolution follows the [Minimal Version Selection (MVS) Algorithm](https://go.dev/ref/mod#minimal-version-selection).  
+This means that when analyzing a project, only the module versions that would actually be included in the final executable are considered.
 
+For example, if your `go.mod` file declares two modules, `a` and `b`, and both depend on the same package `c` (same major version `v1`) but with different minor versions:
 
-Then both of these packages will be entered to the generated sbom and will be included in analysis returned to client.
-In golang, in an actual build of an application into an actual application executable binary, only one of the minor versions will be included in the executable, as only packages with same name but different major versions considered different packages ,
-hence can co-exist together in the application executable.
+- `namespace/c/[email protected]`
+- `namespace/c/[email protected]`
 
-Go ecosystem knows how to select one minor version among all the minor versions of the same major version of a given package, using the [MVS Algorithm](https://go.dev/ref/mod#minimal-version-selection).
-
-In order to enable this behavior, that only shows in analysis modules versions that are actually built into the application executable, please set
-system property/environment variable - `EXHORT_GO_MVS_LOGIC_ENABLED=true`(Default is false)
+Only one of these versions — the minimal version selected by MVS — will be included in the generated SBOM and analysis results.  
+This mirrors the behavior of a real Go build, where only one minor version of a given major version can be present in the executable (since Go treats packages with the same name and major version as identical).
 
+The MVS-based resolution is **enabled by default**.  
+If you want to disable this behavior and instead include **all transitive module versions** (as listed in `go.mod` dependencies), set the system property or environment variable:
 
+```bash
+EXHORT_GO_MVS_LOGIC_ENABLED=false
+```
 
 ####  Python Support
 

pom.xml

@@ -492,6 +492,7 @@ limitations under the License.]]>
             <additionalOptions>
               <!--suppress UnresolvedMavenProperty -->
               <jacoco>${jacoco.java.option}</jacoco>
+              <option>-XX:+EnableDynamicAgentLoading</option>
             </additionalOptions>
           </javaOptions>
           <tags>
@@ -531,7 +532,7 @@ limitations under the License.]]>
               **/*ImageUtilsTest.java
             </exclude>
           </excludes>
-          <argLine>@{surefire.argLine}</argLine>
+          <argLine>@{surefire.argLine} -XX:+EnableDynamicAgentLoading</argLine>
         </configuration>
         <dependencies>
           <!-- https://mvnrepository.com/artifact/me.fabriciorby/maven-surefire-junit5-tree-reporter -->

src/main/java/com/redhat/exhort/providers/GoModulesProvider.java

@@ -271,13 +271,12 @@ private Sbom buildSbomFromGraph(
       if (!edges.containsKey(getParentVertex(line))) {
         // Collect all direct dependencies of the current module into a list.
         List<String> deps =
-            collectAllDirectDependencies(
-                linesList.subList(startingIndex, linesList.size() - 1), line);
+            collectAllDirectDependencies(linesList.subList(startingIndex, linesList.size()), line);
         edges.put(getParentVertex(line), deps);
         startingIndex += deps.size();
       }
     }
-    boolean goMvsLogicEnabled = Environment.getBoolean(PROP_EXHORT_GO_MVS_LOGIC_ENABLED, false);
+    boolean goMvsLogicEnabled = Environment.getBoolean(PROP_EXHORT_GO_MVS_LOGIC_ENABLED, true);
     if (goMvsLogicEnabled) {
       edges = getFinalPackagesVersionsForModule(edges, manifestPath);
     }
@@ -326,24 +325,35 @@ private Map<String, List<String>> getFinalPackagesVersionsForModule(
                 Collectors.toMap(
                     t -> t.split(" ")[0], t -> t.split(" ")[1], (first, second) -> second));
     Map<String, List<String>> listWithModifiedVersions = new HashMap<>();
-    edges.entrySet().stream()
-        .filter(string -> string.getKey().trim().split("@").length == 2)
-        .collect(Collectors.toList())
-        .forEach(
-            (entry) -> {
-              String packageWithSelectedVersion =
-                  getPackageWithFinalVersion(finalModulesVersions, entry.getKey());
-              List<String> packagesWithFinalVersions =
-                  getListOfPackagesWithFinalVersions(finalModulesVersions, entry);
-              listWithModifiedVersions.put(packageWithSelectedVersion, packagesWithFinalVersions);
-            });
+    // Process all entries, including those without versions (like the root module)
+    edges.forEach(
+        (key, value) -> {
+          // Handle both cases: module with version (module@version) and without (just module name)
+          String packageWithSelectedVersion;
+          if (key.contains("@")) {
+            packageWithSelectedVersion = getPackageWithFinalVersion(finalModulesVersions, key);
+          } else {
+            // For root module or modules without version, get version from finalModulesVersions
+            // or use default version
+            String version = finalModulesVersions.get(key);
+            if (version != null) {
+              packageWithSelectedVersion = String.format("%s@%s", key, version);
+            } else {
+              // If not found, keep original key and append default version
+              packageWithSelectedVersion = String.format("%s@%s", key, this.mainModuleVersion);
+            }
+          }
+          List<String> packagesWithFinalVersions =
+              getListOfPackagesWithFinalVersions(finalModulesVersions, value);
+          listWithModifiedVersions.put(packageWithSelectedVersion, packagesWithFinalVersions);
+        });
 
     return listWithModifiedVersions;
   }
 
   private List<String> getListOfPackagesWithFinalVersions(
-      Map<String, String> finalModulesVersions, Map.Entry<String, List<String>> entry) {
-    return entry.getValue().stream()
+      Map<String, String> finalModulesVersions, List<String> packages) {
+    return packages.stream()
         .map(
             (packageWithVersion) ->
                 getPackageWithFinalVersion(finalModulesVersions, packageWithVersion))

src/main/java/com/redhat/exhort/providers/JavaMavenProvider.java

@@ -37,6 +37,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.TreeMap;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import javax.xml.stream.XMLInputFactory;
@@ -426,7 +427,7 @@ public int hashCode() {
 
   private String selectMvnRuntime(final Path manifestPath) {
     boolean preferWrapper = Operations.getWrapperPreference(MVN);
-    if (preferWrapper) {
+    if (preferWrapper && manifestPath != null) {
       String wrapperName = Operations.isWindows() ? "mvnw.cmd" : "mvnw";
       String mvnw = traverseForMvnw(wrapperName, manifestPath.toString());
       if (mvnw != null) {
@@ -438,8 +439,10 @@ private String selectMvnRuntime(final Path manifestPath) {
           }
           return mvnw;
         } catch (Exception e) {
-          log.warning(
-              "Failed to check for mvnw due to: " + e.getMessage() + " Fall back to use mvn");
+          log.log(
+              Level.WARNING,
+              "Failed to check for mvnw due to: {0} Fall back to use mvn",
+              e.getMessage());
         }
       }
     }

src/main/java/com/redhat/exhort/sbom/CycloneDXSbom.java

@@ -23,6 +23,7 @@
 import com.redhat.exhort.utils.Environment;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import java.util.Objects;
@@ -288,18 +289,19 @@ public boolean checkIfPackageInsideDependsOnList(PackageURL component, String na
     if (comp.isPresent()) {
       Dependency targetComponent = comp.get();
       List<Dependency> deps = targetComponent.getDependencies();
-      List<PackageURL> allDirectDeps =
-          deps.stream()
-              .map(
-                  dep -> {
-                    try {
-                      return new PackageURL(dep.getRef());
-                    } catch (MalformedPackageURLException e) {
-                      throw new RuntimeException(e);
-                    }
-                  })
-              .collect(Collectors.toList());
-
+      List<PackageURL> allDirectDeps = Collections.emptyList();
+      if (deps != null) {
+        deps.stream()
+            .map(
+                dep -> {
+                  try {
+                    return new PackageURL(dep.getRef());
+                  } catch (MalformedPackageURLException e) {
+                    throw new RuntimeException(e);
+                  }
+                })
+            .collect(Collectors.toList());
+      }
       result = allDirectDeps.stream().anyMatch(dep -> dep.getName().equals(name));
     }
     return result;

src/main/java/com/redhat/exhort/tools/Operations.java

@@ -56,9 +56,19 @@ private Operations() {
    * @return the custom path from the relevant environment variable or the original argument.
    */
   public static String getCustomPathOrElse(String defaultExecutable) {
-    var target = defaultExecutable.toUpperCase().replaceAll(" ", "_").replaceAll("-", "_");
-    var executableKey = String.format("EXHORT_%s_PATH", target);
-    return Environment.get(executableKey, defaultExecutable);
+    String normalized = defaultExecutable;
+    int dotIndex = normalized.indexOf('.');
+    if (dotIndex > 0) {
+      normalized = normalized.substring(0, dotIndex);
+    }
+
+    var target = normalized.toUpperCase().replaceAll(" ", "_").replaceAll("-", "_");
+    var primaryKey = String.format("EXHORT_%s_PATH", target);
+    String primary = Environment.get(primaryKey);
+    if (primary != null) {
+      return primary;
+    }
+    return defaultExecutable;
   }
 
   /**

src/main/java/com/redhat/exhort/utils/Environment.java

@@ -22,8 +22,8 @@ public final class Environment {
   private Environment() {}
 
   public static String get(String name, String defaultValue) {
-    return Optional.ofNullable(System.getProperty(name))
-        .or(() -> Optional.ofNullable(System.getenv(name)))
+    return Optional.ofNullable(System.getenv(name))
+        .or(() -> Optional.ofNullable(System.getProperty(name)))
         .orElse(defaultValue);
   }
 

src/test/java/com/redhat/exhort/image/ImageUtilsTest.java

@@ -496,8 +496,12 @@ void test_get_syft_envs() {
   @Test
   @SetSystemProperty(key = "PATH", value = mockPath)
   void test_update_PATH_env() {
-    var path = ImageUtils.updatePATHEnv("test-exec-path");
-    assertEquals("PATH=test-path" + File.pathSeparator + "test-exec-path", path);
+    try (MockedStatic<Environment> mockEnv =
+        Mockito.mockStatic(Environment.class, Mockito.CALLS_REAL_METHODS)) {
+      mockEnv.when(() -> Environment.get("PATH")).thenReturn("test-path");
+      var path = ImageUtils.updatePATHEnv("test-exec-path");
+      assertEquals("PATH=test-path" + File.pathSeparator + "test-exec-path", path);
+    }
   }
 
   @Test

src/test/java/com/redhat/exhort/providers/Golang_Modules_Provider_Test.java

@@ -23,6 +23,10 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.SerializationFeature;
 import com.redhat.exhort.Api;
 import com.redhat.exhort.ExhortTest;
 import java.io.IOException;
@@ -38,6 +42,9 @@
 
 @ExtendWith(HelperExtension.class)
 class Golang_Modules_Provider_Test extends ExhortTest {
+  private static final ObjectMapper JSON_MAPPER =
+      new ObjectMapper().enable(SerializationFeature.INDENT_OUTPUT);
+
   // test folder are located at src/test/resources/tst_manifests/npm
   // each folder should contain:
   // - package.json: the target manifest for testing
@@ -78,7 +85,8 @@ void test_the_provideStack(String testFolder) throws IOException {
     Files.deleteIfExists(tmpGolangFile);
     // verify expected SBOM is returned
     assertThat(content.type).isEqualTo(Api.CYCLONEDX_MEDIA_TYPE);
-    assertThat(dropIgnored(new String(content.buffer))).isEqualTo(dropIgnored(expectedSbom));
+    assertThat(prettyJson(dropIgnoredKeepFormat(new String(content.buffer))))
+        .isEqualTo(prettyJson(dropIgnoredKeepFormat(expectedSbom)));
   }
 
   @ParameterizedTest
@@ -107,7 +115,8 @@ void test_the_provideComponent(String testFolder) throws IOException {
     Files.deleteIfExists(tmpGolangFile);
     // verify expected SBOM is returned
     assertThat(content.type).isEqualTo(Api.CYCLONEDX_MEDIA_TYPE);
-    assertThat(dropIgnored(new String(content.buffer))).isEqualTo(dropIgnored(expectedSbom));
+    assertThat(prettyJson(dropIgnoredKeepFormat(new String(content.buffer))))
+        .isEqualTo(prettyJson(dropIgnoredKeepFormat(expectedSbom)));
   }
 
   @Test
@@ -149,14 +158,15 @@ void Test_Golang_Modules_with_Match_Manifest_Version(boolean MatchManifestVersio
       String actualSbomWithTSStripped = dropIgnoredKeepFormat(sbomString);
 
       assertEquals(
-          dropIgnored(getStringFromFile("msc/golang/expected_sbom_ca.json")).trim(),
-          dropIgnored(actualSbomWithTSStripped));
+          prettyJson(
+              dropIgnoredKeepFormat(getStringFromFile("msc/golang/expected_sbom_ca.json").trim())),
+          prettyJson(actualSbomWithTSStripped));
     }
   }
 
   @Test
-  void Test_Golang_MvS_Logic_Enabled() throws IOException {
-    System.setProperty(GoModulesProvider.PROP_EXHORT_GO_MVS_LOGIC_ENABLED, "true");
+  void Test_Golang_MvS_Logic_Disabled() throws IOException {
+    System.setProperty(GoModulesProvider.PROP_EXHORT_GO_MVS_LOGIC_ENABLED, "false");
     String goModPath = getFileFromResource("go.mod", "msc/golang/mvs_logic/go.mod");
     Path manifest = Path.of(goModPath);
     GoModulesProvider goModulesProvider = new GoModulesProvider(manifest);
@@ -165,12 +175,10 @@ void Test_Golang_MvS_Logic_Enabled() throws IOException {
             goModulesProvider.getDependenciesSbom(manifest, true).getAsJsonString());
     String expectedSbom =
         getStringFromFile("msc/golang/mvs_logic/expected_sbom_stack_analysis.json").trim();
-    assertEquals(dropIgnored(expectedSbom), dropIgnored(resultSbom));
+    assertEquals(prettyJson(dropIgnoredKeepFormat(expectedSbom)), prettyJson(resultSbom));
 
-    // check that only one version of package golang/go.opencensus.io is in sbom for
-    // EXHORT_GO_MVS_LOGIC_ENABLED=true
     assertEquals(
-        1,
+        5,
         Arrays.stream(resultSbom.split(System.lineSeparator()))
             .filter(str -> str.contains("\"ref\" : \"pkg:golang/go.opencensus.io@"))
             .count());
@@ -186,17 +194,20 @@ void Test_Golang_MvS_Logic_Enabled() throws IOException {
         Arrays.stream(resultSbom.split(System.lineSeparator()))
                 .filter(str -> str.contains("\"ref\" : \"pkg:golang/go.opencensus.io@"))
                 .count()
-            > 1);
-  }
-
-  private String dropIgnored(String s) {
-    return s.replaceAll("goarch=\\w+&goos=\\w+&", "")
-        .replaceAll("\\s+", "")
-        .replaceAll("\"timestamp\":\"[a-zA-Z0-9\\-\\:]+\",", "");
+            == 1);
   }
 
   private String dropIgnoredKeepFormat(String s) {
     return s.replaceAll("goarch=\\w+&goos=\\w+&", "")
         .replaceAll("\"timestamp\" : \"[a-zA-Z0-9\\-\\:]+\",\n    ", "");
   }
+
+  private String prettyJson(String s) {
+    try {
+      JsonNode node = JSON_MAPPER.readTree(s);
+      return JSON_MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(node);
+    } catch (JsonProcessingException e) {
+      return s; // Fallback if not valid JSON after sanitization
+    }
+  }
 }

src/test/java/com/redhat/exhort/providers/Java_Envs_Test.java

@@ -30,8 +30,12 @@ public class Java_Envs_Test {
   @Test
   @SetSystemProperty(key = "JAVA_HOME", value = "test-java-home")
   void test_java_get_envs() {
-    var envs = new JavaMavenProvider(null).getMvnExecEnvs();
-    assertEquals(Collections.singletonMap("JAVA_HOME", "test-java-home"), envs);
+    try (MockedStatic<Environment> mockEnv =
+        Mockito.mockStatic(Environment.class, Mockito.CALLS_REAL_METHODS)) {
+      mockEnv.when(() -> Environment.get("JAVA_HOME")).thenReturn("test-java-home");
+      var envs = new JavaMavenProvider(null).getMvnExecEnvs();
+      assertEquals(Collections.singletonMap("JAVA_HOME", "test-java-home"), envs);
+    }
   }
 
   @Test