Fix SpotBugs warnings in RandomHelper (#252)

- Added SpotBugs annotations dependency
- Suppressed MS_EXPOSE_REP warning on getRandom() method
- Suppressed EI_EXPOSE_STATIC_REP2 warning on setRandom() method
- Added test to document intentional mutable Random exposure
- Added detailed justification for the design decision

The RandomHelper class is designed for test environments where controlled
randomness is required. Exposing the mutable Random instance is intentional
to allow testing frameworks full control over randomness.

Co-authored-by: TLS-Attacker Developer <[email protected]>

Author: ic0ns

pom.xml

@@ -100,6 +100,11 @@
             <groupId>org.apache.logging.log4j</groupId>
             <artifactId>log4j-core</artifactId>
         </dependency>
+        <dependency>
+            <groupId>com.github.spotbugs</groupId>
+            <artifactId>spotbugs-annotations</artifactId>
+            <scope>provided</scope>
+        </dependency>
         <!-- scope: test -->
         <dependency>
             <groupId>org.glassfish.jaxb</groupId>

src/main/java/de/rub/nds/modifiablevariable/util/RandomHelper.java

@@ -7,6 +7,7 @@
  */
 package de.rub.nds.modifiablevariable.util;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Random;
 
 /**
@@ -30,8 +31,20 @@ public final class RandomHelper {
      * <p>The fixed seed ensures reproducible "random" behavior across test runs. If the Random
      * instance hasn't been initialized yet, this method initializes it.
      *
+     * <p><b>Note:</b> This method intentionally returns the mutable Random instance directly to
+     * allow for flexible testing scenarios. The setRandom() method is also provided to replace the
+     * instance entirely. This design is intentional for testing frameworks that need full control
+     * over randomness.
+     *
      * @return A Random instance with a fixed seed of 0
      */
+    @SuppressFBWarnings(
+            value = "MS_EXPOSE_REP",
+            justification =
+                    "Intentionally exposing mutable static Random for testing flexibility. "
+                            + "This class is designed for test environments where controlled randomness "
+                            + "is required, and the ability to modify or replace the Random instance "
+                            + "is a feature, not a bug.")
     public static synchronized Random getRandom() {
         if (random == null) {
             random = new Random(0);
@@ -60,6 +73,13 @@ public static BadRandom getBadSecureRandom() {
      *
      * @param randomInstance The Random instance to use as the singleton
      */
+    @SuppressFBWarnings(
+            value = "EI_EXPOSE_STATIC_REP2",
+            justification =
+                    "Intentionally allowing external Random instances to be set for testing flexibility. "
+                            + "This class is designed for test environments where controlled randomness "
+                            + "is required, and the ability to replace the Random instance "
+                            + "is a feature, not a bug.")
     public static synchronized void setRandom(Random randomInstance) {
         random = randomInstance;
     }

src/test/java/de/rub/nds/modifiablevariable/util/RandomHelperTest.java

@@ -108,6 +108,39 @@ void testBadRandomConstructors() {
         assertNotNull(badRandom2.nextInt()); // Just verify it returns a value
     }
 
+    @Test
+    void testIntentionalMutableRandomExposure() {
+        // This test documents the intentional design decision to expose the mutable Random instance
+        Random random1 = RandomHelper.getRandom();
+        Random random2 = RandomHelper.getRandom();
+
+        // Verify we get the same instance (singleton pattern)
+        assertTrue(random1 == random2, "Should return the same Random instance");
+
+        // Verify that external modifications affect the singleton
+        // This is intentional behavior for testing flexibility
+        random1.setSeed(42);
+
+        // Create a separate Random with same seed to compare expected values
+        Random expectedRandom = new Random(42);
+
+        // Both references should produce the same value as the expected Random
+        int expectedValue = expectedRandom.nextInt();
+        int random1Value = random1.nextInt();
+        assertEquals(expectedValue, random1Value, "Modified Random should produce expected value");
+
+        // Reset and get second value to verify both references see same state
+        random1.setSeed(42);
+        expectedRandom.setSeed(42);
+        assertEquals(
+                expectedRandom.nextInt(),
+                random2.nextInt(),
+                "All references to the singleton should see the same state");
+
+        // Reset for other tests
+        RandomHelper.setRandom(new Random(0));
+    }
+
     @Test
     void testBadFixedRandom() {
         byte fixedValue = 42; // Using a single byte as per the class implementation