security: address APS-19018, APS-19019, APS-19020, APS-19024

.github/workflows/ci.yml

@@ -8,14 +8,17 @@ on:
     branches:
     - master
 
+permissions:
+  contents: read
+
 jobs:
   build:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
       - name: Set up JDK 8
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@c1e323688fd81a25caa38c78aa6df2d33d3e20d9 # v4.8.0
         with:
           java-version: 8
           distribution: 'temurin'

CHANGELOG.md

@@ -0,0 +1,59 @@
+# Changelog
+
+All notable changes to this project will be documented in this file.
+
+## [Unreleased]
+
+### Security
+
+- **SSRF guard on `AutomateClient.getSessionLogs`** — the URL returned by the
+  BrowserStack API is now validated before being fetched. Only `https` URLs whose
+  host (parsed via `java.net.URI`) ends in `.browserstack.com` are accepted; all
+  other URLs throw `AutomateException`. This prevents a maliciously-crafted
+  session record from causing the client to issue requests against an attacker
+  endpoint. (APS-19018)
+
+- **`setProxy` is now per-instance** — previously, calling
+  `BrowserStackClient.setProxy(...)` mutated a JVM-wide `static HTTP_TRANSPORT`
+  field, so a proxy configured on one client instance leaked to every other
+  instance running in the same JVM (and a non-proxied client could end up using
+  another tenant's proxy). The transport is now an instance field. **Anyone
+  whose code relied on cross-instance proxy state must now call `setProxy` on
+  each `BrowserStackClient` / `AutomateClient` they construct.** (APS-19019)
+
+- **`checkAuthState` now throws when EITHER credential is missing** — previously
+  the guard only fired when both `username` and `accessKey` were `null`,
+  meaning a client with one credential set and one missing would proceed and
+  produce a malformed `Authorization: Basic` header. The condition is now
+  `username == null || accessKey == null`. (APS-19019)
+
+
+- **SSRF guard on JVM-property base URL override** — both `AutomateClient` and
+  `AppAutomateClient` constructors now validate the result of
+  `System.getProperty("browserstack.automate.api", ...)` /
+  `System.getProperty("browserstack.app-automate.api", ...)` before passing it
+  to the parent constructor. Only `https` URLs whose host (parsed via
+  `java.net.URI`) ends in `.browserstack.com` are accepted; all other values
+  throw `IllegalArgumentException`. This prevents an attacker who can set JVM
+  properties from redirecting every signed API request — and the Basic Auth
+  credentials those requests carry — to an attacker-controlled host. (APS-19024)
+
+- **Path canonicalization in `AppAutomateClient.uploadApp`** — the file path is
+  now resolved via `File.getCanonicalFile()` before applying the `.apk` /
+  `.ipa` extension check, and the canonical name (not the original input) is
+  what the extension check sees. This prevents a symlink such as
+  `/tmp/legit.apk -> /etc/passwd` from passing the suffix-only check and
+  causing an arbitrary local file to be streamed to the upload endpoint.
+  (APS-19024)
+
+### CI / Release
+
+- **GitHub Actions hardening** — `.github/workflows/ci.yml` actions are now
+  pinned to commit SHAs (`actions/checkout@v4.3.1`,
+  `actions/setup-java@v4.8.0`) and a top-level `permissions: contents: read`
+  block has been added. (APS-19020)
+
+- **Maven release auto-promote disabled** — `nexus-staging-maven-plugin`'s
+  `autoReleaseAfterClose` is now `false`. Staged releases must be manually
+  promoted via OSSRH after a `mvn release:perform`. See the BrowserStack
+  internal release runbook for the manual promotion step. (APS-19020)

pom.xml

@@ -89,7 +89,7 @@
 				<configuration>
 					<serverId>ossrh</serverId>
 					<nexusUrl>https://oss.sonatype.org/</nexusUrl>
-					<autoReleaseAfterClose>true</autoReleaseAfterClose>
+					<autoReleaseAfterClose>false</autoReleaseAfterClose>
 				</configuration>
 			</plugin>
 			<plugin>

src/main/java/com/browserstack/appautomate/AppAutomateClient.java

@@ -2,6 +2,8 @@
 
 import java.io.File;
 import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.net.URI;
 import java.util.List;
 import com.browserstack.automate.Automate.BuildStatus;
 import com.browserstack.automate.exception.AppAutomateException;
@@ -32,7 +34,8 @@ public class AppAutomateClient extends BrowserStackClient implements AppAutomate
    * @param accessKey Browserstack accessKey
    */
   public AppAutomateClient(String username, String accessKey) {
-    super(System.getProperty("browserstack.app-automate.api", BASE_URL), username, accessKey);
+    super(validateApiBaseUrl(System.getProperty("browserstack.app-automate.api", BASE_URL)),
+        username, accessKey);
   }
 
   /**
@@ -71,9 +74,22 @@ public AppUploadResponse uploadApp(String filePath)
         throw new FileNotFoundException("File not found at " + filePath);
       }
 
-      if (!filePath.endsWith(".apk") && !filePath.endsWith(".ipa")) {
+      // APS-19024 / INJ-003: canonicalize before validating extension. Without this,
+      // a symlink such as /tmp/legit.apk -> /etc/passwd would pass a suffix-only
+      // check on the supplied path and result in an arbitrary local file being
+      // streamed to the upload endpoint. getCanonicalFile() resolves symlinks
+      // and ".." segments; we then check the extension on the canonical name.
+      final File canonical;
+      try {
+        canonical = file.getCanonicalFile();
+      } catch (IOException e) {
+        throw new AppAutomateException("Unable to canonicalize file path: " + e.getMessage(), 0);
+      }
+      final String canonicalName = canonical.getName().toLowerCase();
+      if (!canonicalName.endsWith(".apk") && !canonicalName.endsWith(".ipa")) {
         throw new InvalidFileExtensionException("File extension should be only .apk or .ipa.");
       }
+      file = canonical;
 
       MultipartContent content = new MultipartContent().setMediaType(
           new HttpMediaType("multipart/form-data").setParameter("boundary", "__END_OF_PART__"));
@@ -282,5 +298,46 @@ public List<Session> getSessions(final String buildId, final BuildStatus status)
     return getSessions(buildId, status, 0);
   }
 
-}
 
+  /**
+   * Validates the App Automate API base URL read from the JVM system property
+   * {@code browserstack.app-automate.api} (or the built-in default).
+   * <p>
+   * Defends against SSRF (APS-19024 / INJ-002): an attacker who can set the
+   * JVM property could otherwise redirect every signed API request — and the
+   * Basic Auth credentials those requests carry — to an attacker-controlled
+   * host. Only {@code https} URLs whose host (parsed via
+   * {@link java.net.URI#getHost()}) ends in {@code .browserstack.com} are
+   * accepted.
+   *
+   * @param url URL to validate. Must be a syntactically-valid absolute URL.
+   * @return the same URL, unchanged, when validation passes.
+   * @throws IllegalArgumentException if the URL is null, malformed, uses a
+   *                                  non-https scheme, or has a host outside
+   *                                  {@code *.browserstack.com}.
+   */
+  private static String validateApiBaseUrl(String url) {
+    if (url == null) {
+      throw new IllegalArgumentException("App Automate API base URL is null");
+    }
+
+    final URI uri;
+    try {
+      uri = URI.create(url);
+    } catch (IllegalArgumentException e) {
+      throw new IllegalArgumentException("Malformed App Automate API base URL", e);
+    }
+
+    final String scheme = uri.getScheme();
+    if (!"https".equalsIgnoreCase(scheme)) {
+      throw new IllegalArgumentException("Insecure App Automate API base URL scheme: " + scheme);
+    }
+
+    final String host = uri.getHost();
+    if (host == null || !host.toLowerCase().endsWith(".browserstack.com")) {
+      throw new IllegalArgumentException("Untrusted App Automate API base URL host: " + host);
+    }
+
+    return url;
+  }
+}

src/main/java/com/browserstack/automate/AutomateClient.java

@@ -15,6 +15,7 @@
 import javax.annotation.Nonnull;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.net.URI;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -35,7 +36,8 @@ public final class AutomateClient extends BrowserStackClient implements Automate
    * @param accessKey Access Key for your BrowserStack Automate account.
    */
   public AutomateClient(String username, String accessKey) {
-    super(System.getProperty("browserstack.automate.api", BASE_URL), username, accessKey);
+    super(validateApiBaseUrl(System.getProperty("browserstack.automate.api", BASE_URL)),
+        username, accessKey);
   }
 
   /**
@@ -458,6 +460,7 @@ public String getSessionLogs(final Session session) throws AutomateException {
     }
 
     try {
+      validateBrowserStackUrl(session.getLogUrl());
       BrowserStackRequest request = newRequest(Method.GET, session.getLogUrl(), false);
       request.getHttpRequest().getHeaders().setAccept("*/*");
       return request.asString();
@@ -529,4 +532,82 @@ public String recycleKey() throws AutomateException {
     setAccessKey(newAccessKey);
     return newAccessKey;
   }
+
+  /**
+   * Validates the API base URL read from the JVM system property
+   * {@code browserstack.automate.api} (or the built-in default).
+   * <p>
+   * Defends against SSRF (APS-19024 / INJ-002): an attacker who can set the
+   * JVM property could otherwise redirect every signed API request — and the
+   * Basic Auth credentials those requests carry — to an attacker-controlled
+   * host. Only {@code https} URLs whose host (parsed via
+   * {@link java.net.URI#getHost()}) ends in {@code .browserstack.com} are
+   * accepted.
+   *
+   * @param url URL to validate. Must be a syntactically-valid absolute URL.
+   * @return the same URL, unchanged, when validation passes.
+   * @throws IllegalArgumentException if the URL is null, malformed, uses a
+   *                                  non-https scheme, or has a host outside
+   *                                  {@code *.browserstack.com}.
+   */
+  private static String validateApiBaseUrl(String url) {
+    if (url == null) {
+      throw new IllegalArgumentException("API base URL is null");
+    }
+
+    final URI uri;
+    try {
+      uri = URI.create(url);
+    } catch (IllegalArgumentException e) {
+      throw new IllegalArgumentException("Malformed API base URL", e);
+    }
+
+    final String scheme = uri.getScheme();
+    if (!"https".equalsIgnoreCase(scheme)) {
+      throw new IllegalArgumentException("Insecure API base URL scheme: " + scheme);
+    }
+
+    final String host = uri.getHost();
+    if (host == null || !host.toLowerCase().endsWith(".browserstack.com")) {
+      throw new IllegalArgumentException("Untrusted API base URL host: " + host);
+    }
+
+    return url;
+  }
+
+  /**
+   * Validates that a URL is safe to fetch as a BrowserStack-issued logs URL.
+   * <p>
+   * Defends against SSRF (APS-19018): only allow {@code https} scheme and a host
+   * whose lowercased value ends with {@code .browserstack.com}. Host comparison
+   * uses {@link java.net.URI#getHost()} (NOT a string suffix on the raw URL) so that
+   * spoofed hosts such as {@code https://automate.browserstack.com.attacker.example/}
+   * are rejected.
+   *
+   * @param url URL to validate. Must be a syntactically-valid absolute URL.
+   * @throws AutomateException if the URL is malformed, uses a non-https scheme,
+   *                           or has a host outside {@code *.browserstack.com}.
+   */
+  private static void validateBrowserStackUrl(String url) throws AutomateException {
+    if (url == null) {
+      throw new AutomateException("Logs URL is null", 400);
+    }
+
+    final URI uri;
+    try {
+      uri = URI.create(url);
+    } catch (IllegalArgumentException e) {
+      throw new AutomateException("Malformed logs URL", 400);
+    }
+
+    final String scheme = uri.getScheme();
+    if (!"https".equalsIgnoreCase(scheme)) {
+      throw new AutomateException("Insecure logs URL scheme: " + scheme, 400);
+    }
+
+    final String host = uri.getHost();
+    if (host == null || !host.toLowerCase().endsWith(".browserstack.com")) {
+      throw new AutomateException("Untrusted logs URL host: " + host, 400);
+    }
+  }
 }

src/main/java/com/browserstack/client/BrowserStackClient.java

@@ -69,7 +69,7 @@ public Object parseAndClose(Reader reader, Type type) throws IOException {
             throw new IOException("Unsupported operation");
         }
     };
-    private static HttpTransport HTTP_TRANSPORT = new ApacheHttpTransport();
+    private HttpTransport httpTransport = new ApacheHttpTransport();
     protected final BrowserStackCache<String, Object> cacheMap;
 
     private HttpRequestFactory requestFactory;
@@ -105,8 +105,8 @@ public BrowserStackClient(String baseUrl, String username, String accessKey) {
         this.accessKey = accessKey.trim();
     }
 
-    static HttpRequestFactory newRequestFactory() {
-        return HTTP_TRANSPORT.createRequestFactory(httpRequest -> httpRequest.setParser(OBJECT_PARSER));
+    HttpRequestFactory newRequestFactory() {
+        return httpTransport.createRequestFactory(httpRequest -> httpRequest.setParser(OBJECT_PARSER));
     }
 
     static HttpRequest newRequest(final HttpRequestFactory requestFactory, final Method method, final GenericUrl url) throws BrowserStackException {
@@ -174,7 +174,7 @@ public void setProxy(final String proxyHost, final int proxyPort, final String p
         }
 
         final HttpClient client = clientBuilder.build();
-        HTTP_TRANSPORT = new ApacheHttpTransport(client);
+        this.httpTransport = new ApacheHttpTransport(client);
         this.requestFactory = newRequestFactory();
     }
 
@@ -187,8 +187,8 @@ protected synchronized void setAccessKey(final String accessKey) {
     }
 
     private void checkAuthState() {
-        if (this.accessKey == null && this.username == null) {
-            throw new IllegalStateException("Missing API credentials");
+        if (this.accessKey == null || this.username == null) {
+            throw new IllegalStateException("Missing API credentials (username or access key is null)");
         }
     }