Fix correctness + security findings from code review

README.md

@@ -14,7 +14,7 @@ This Jenkins plugin integrates TestingBot.com features inside Jenkins.
 
 ## Prerequisites
 
-* Minimum supported Jenkins version is 2.338
+* Minimum supported Jenkins version is 2.541.3 (the plugin builds and runs on Java 17+)
 * A TestingBot account
 
 ## Setting up the plugin

src/main/java/testingbot/TestingBotBuildObject.java

@@ -21,7 +21,7 @@ public TestingBotBuildObject(String sessionId, String className, String testName
       this.isPassed = isPassed;
       this.authHash = authHash;
       this.test = test;
-      this.environmentName = test.getBrowser() + " | " + test.getOs();
+      this.environmentName = test == null ? "" : test.getBrowser() + " | " + test.getOs();
     }
 
     /**

src/main/java/testingbot/TestingBotBuildSummary.java

@@ -15,6 +15,8 @@
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 public class TestingBotBuildSummary extends InvisibleAction implements Serializable {
     private static final long serialVersionUID = 1L;
@@ -79,9 +81,17 @@ public void onCompleted(AbstractBuild<?, ?> r, TaskListener listener) {
                         continue;
                     }
                     String sessionId = sessionIds.get(0);
-                    TestingbotTest test = apiClient.getTest(sessionId);
-                    TestingBotBuildObject tbo = new TestingBotBuildObject(sessionId, cr.getClassName(), cr.getName(), cr.isPassed(), apiClient.getAuthenticationHash(sessionId), test);
-                    ids.add(tbo);
+                    try {
+                        TestingbotTest test = apiClient.getTest(sessionId);
+                        TestingBotBuildObject tbo = new TestingBotBuildObject(sessionId, cr.getClassName(), cr.getName(), cr.isPassed(), apiClient.getAuthenticationHash(sessionId), test);
+                        ids.add(tbo);
+                    } catch (RuntimeException e) {
+                        // A single bad session id (typo, wrong account, expired/purged session) must not
+                        // drop the report for the other valid sessions in this build. Log at WARNING so
+                        // the failed lookup is visible (unlike the normal "no session" case).
+                        Logger.getLogger(TestingBotBuildSummary.class.getName())
+                                .log(Level.WARNING, "Skipping TestingBot session " + sessionId, e);
+                    }
                 }
             }
             if (ids.isEmpty()) {

src/main/java/testingbot/TestingBotBuildWrapper.java

@@ -133,7 +133,8 @@ public String getDisplayName() {
 
         @POST
         public ListBoxModel doFillCredentialsIdItems(@AncestorInPath final Item context) {
-            if (context != null && !context.hasPermission(Item.CONFIGURE)) {
+            if (context == null ? !Jenkins.get().hasPermission(Jenkins.ADMINISTER)
+                    : !context.hasPermission(Item.CONFIGURE)) {
                 return new StandardListBoxModel();
             }
 

src/main/java/testingbot/TestingBotCredentials.java

@@ -194,17 +194,14 @@ public static TestingBotCredentials getCredentials(final AbstractItem buildItem,
             return null;
         }
 
-        CredentialsMatcher matcher;
         if (credentialsId != null) {
-            matcher = CredentialsMatchers.allOf(CredentialsMatchers.withId(credentialsId));
-        } else {
-            matcher = CredentialsMatchers.always();
+            // A specific id was requested: return it or null — never silently fall back to a
+            // different account when the id does not match (renamed/mistyped/deleted credential).
+            return CredentialsMatchers.firstOrNull(available,
+                    CredentialsMatchers.withId(credentialsId));
         }
-
-        return CredentialsMatchers.firstOrDefault(
-                available,
-                matcher,
-                available.get(0));
+        // No id specified: use the first available credential.
+        return available.get(0);
     }
 
     public static List<TestingBotCredentials> availableCredentials(final AbstractItem abstractItem) {

src/main/java/testingbot/TestingBotReportFactory.java

@@ -54,7 +54,8 @@ public List<TestingBotReport> getTestAction(TestObject to) {
                 return Collections.singletonList(new TestingBotReport(credentials, cr, ids));
             }
         }
-        logger.log(Level.WARNING, "Empty list for TB sessions");
+        // Fires for the majority of test-tree nodes (any case without a TestingBotSessionID); keep it quiet.
+        logger.log(Level.FINE, "No TestingBot sessions for this test object");
         return Collections.emptyList();
     }
 

src/main/java/testingbot/TunnelManager.java

@@ -74,6 +74,7 @@ static void applyOptions(App app, String rawOptions, TaskListener listener) {
         }
         String[] tokens = tokenize(trimmed);
         List<String> ignored = new ArrayList<>();
+        List<String> auth = new ArrayList<>();
         for (int i = 0; i < tokens.length; i++) {
             String token = tokens[i];
             switch (token) {
@@ -120,7 +121,7 @@ static void applyOptions(App app, String rawOptions, TaskListener listener) {
                 case "-a":
                 case "--auth":
                     if (hasValue(tokens, i)) {
-                        app.setBasicAuth(new String[]{tokens[++i]});
+                        auth.add(tokens[++i]);
                     } else {
                         ignored.add(token + " (missing value)");
                     }
@@ -129,6 +130,10 @@ static void applyOptions(App app, String rawOptions, TaskListener listener) {
                     ignored.add(token);
             }
         }
+        if (!auth.isEmpty()) {
+            // --auth may be repeated for multiple hosts; apply them all at once.
+            app.setBasicAuth(auth.toArray(new String[0]));
+        }
         if (!ignored.isEmpty()) {
             listener.getLogger().println("[TestingBot] Ignoring invalid or unsupported tunnel option(s): "
                     + String.join(", ", ignored));
@@ -139,6 +144,23 @@ private static boolean hasValue(String[] tokens, int index) {
         return index + 1 < tokens.length && !tokens[index + 1].startsWith("-");
     }
 
+    /**
+     * Returns the value of a user-supplied {@code --tunnel-identifier} / {@code -i} option, or
+     * {@code null} if the options don't specify one.
+     */
+    public static String extractTunnelIdentifier(String rawOptions) {
+        if (rawOptions == null) {
+            return null;
+        }
+        String[] tokens = tokenize(rawOptions.trim());
+        for (int i = 0; i < tokens.length; i++) {
+            if (("-i".equals(tokens[i]) || "--tunnel-identifier".equals(tokens[i])) && hasValue(tokens, i)) {
+                return tokens[i + 1];
+            }
+        }
+        return null;
+    }
+
     /**
      * Configures and boots the given {@link App}, blocking until the tunnel
      * reports READY (or a poll error occurs). Credentials are passed as already

src/main/java/testingbot/pipeline/TestingBotTunnelStep.java

@@ -14,6 +14,7 @@
 import hudson.model.Run;
 import hudson.model.TaskListener;
 import hudson.model.TopLevelItem;
+import hudson.remoting.VirtualChannel;
 import hudson.security.ACL;
 import hudson.util.ListBoxModel;
 import java.util.ArrayList;
@@ -221,7 +222,12 @@ public boolean start() throws Exception {
                 run.addAction(new TestingBotBuildAction(tbCredentials));
             }
 
-            tunnelIdentifier = "jenkins-" + job.getName() + "-" + run.getNumber() + "-" + TUNNEL_SEQ.incrementAndGet();
+            // Respect a user-supplied --tunnel-identifier (their Selenium/Appium capabilities may
+            // target it); otherwise generate a unique one so parallel tunnels stay isolated.
+            String userIdentifier = TunnelManager.extractTunnelIdentifier(options);
+            tunnelIdentifier = (userIdentifier != null && !userIdentifier.isEmpty())
+                    ? userIdentifier
+                    : "jenkins-" + run.getParent().getFullName().replace('/', '-') + "-" + run.getNumber() + "-" + TUNNEL_SEQ.incrementAndGet();
 
             // Resolve secrets on the controller; only plaintext crosses the channel (over the encrypted remoting link).
             String key = tbCredentials.getKey();
@@ -239,7 +245,7 @@ public boolean start() throws Exception {
             env.put("SELENIUM_HOST", hubHost);
             env.put("SELENIUM_PORT", hubPort);
 
-            computer.getChannel().call(new TbStartTunnelHandler(key, secret, options, tunnelIdentifier, listener));
+            requireChannel(computer).call(new TbStartTunnelHandler(key, secret, options, tunnelIdentifier, listener));
 
             try {
                 body = getContext().newBodyInvoker()
@@ -251,17 +257,40 @@ public boolean start() throws Exception {
             } catch (Exception e) {
                 // The tunnel started but the body could not be invoked; stop it now, otherwise the
                 // Callback that would normally stop it never runs and the tunnel leaks.
-                try {
-                    computer.getChannel().call(new TbStopTunnelHandler(tunnelIdentifier, listener));
-                } catch (Exception ignored) {
-                    // best effort
-                }
+                stopTunnelQuietly(computer, tunnelIdentifier, listener);
                 throw e;
             }
 
             return false;
         }
 
+        private static VirtualChannel requireChannel(Computer computer) throws Exception {
+            VirtualChannel channel = computer == null ? null : computer.getChannel();
+            if (channel == null) {
+                throw new Exception("The agent is offline; cannot start the TestingBot tunnel");
+            }
+            return channel;
+        }
+
+        /** Best-effort tunnel teardown that tolerates an offline agent (no channel). */
+        private static void stopTunnelQuietly(Computer computer, String tunnelIdentifier, TaskListener listener) {
+            VirtualChannel channel = computer == null ? null : computer.getChannel();
+            if (channel == null) {
+                RUNNING_TUNNELS.remove(tunnelIdentifier);
+                if (listener != null) {
+                    listener.getLogger().println("[TestingBot] Agent offline: remote teardown of tunnel "
+                            + tunnelIdentifier + " could not be performed. It may still be running on the agent "
+                            + "and should stop when the agent process exits (best-effort cleanup).");
+                }
+                return;
+            }
+            try {
+                channel.call(new TbStopTunnelHandler(tunnelIdentifier, listener));
+            } catch (Exception ignored) {
+                // best effort
+            }
+        }
+
         @Override
         public void stop(@NonNull Throwable cause) throws Exception {
             if (body != null) {
@@ -286,7 +315,7 @@ private static final class Callback extends BodyExecutionCallback.TailCall {
             protected void finished(StepContext context) throws Exception {
                 TaskListener listener = context.get(TaskListener.class);
                 Computer computer = context.get(Computer.class);
-                computer.getChannel().call(new TbStopTunnelHandler(tunnelIdentifier, listener));
+                stopTunnelQuietly(computer, tunnelIdentifier, listener);
             }
         }
     }

src/test/java/testingbot/TunnelManagerOptionsTest.java

@@ -89,4 +89,19 @@ public void reportsInvalidMetricsPort() throws Exception {
         String log = applyAndCapture(app, "--metrics-port notanumber");
         assertThat(log).contains("metrics-port", "invalid value");
     }
+
+    @Test
+    public void accumulatesRepeatedAuthOptions() {
+        App app = new App();
+        TunnelManager.applyOptions(app, "--auth host1:80:u:p --auth host2:80:u:p", TaskListener.NULL);
+        assertThat(app.getBasicAuth()).containsExactly("host1:80:u:p", "host2:80:u:p");
+    }
+
+    @Test
+    public void extractsUserTunnelIdentifier() {
+        assertThat(TunnelManager.extractTunnelIdentifier("--debug --tunnel-identifier my-suite")).isEqualTo("my-suite");
+        assertThat(TunnelManager.extractTunnelIdentifier("-i short-id")).isEqualTo("short-id");
+        assertThat(TunnelManager.extractTunnelIdentifier("--debug --proxy host:8080")).isNull();
+        assertThat(TunnelManager.extractTunnelIdentifier(null)).isNull();
+    }
 }