Merge pull request #83 from browserstack/harden/url-and-credential-validation

Harden URL handling, credentials, and CI workflow

Author: karanshah-browserstack

.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'

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,21 @@ public AppUploadResponse uploadApp(String filePath)
         throw new FileNotFoundException("File not found at " + filePath);
       }
 
-      if (!filePath.endsWith(".apk") && !filePath.endsWith(".ipa")) {
+      // Canonicalize before validating extension so a symlink whose name ends in
+      // .apk/.ipa but whose target file does not is rejected. getCanonicalFile()
+      // resolves symlinks and ".." segments; the extension check then runs 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 +297,43 @@ 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>
+   * 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,79 @@ 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>
+   * 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 a BrowserStack-issued logs URL before fetching.
+   * <p>
+   * Only {@code https} scheme is allowed, and the host (parsed via
+   * {@link java.net.URI#getHost()}, not a string suffix on the raw URL) must
+   * end with {@code .browserstack.com}. A URL whose host has
+   * {@code .browserstack.com} only as an internal substring — e.g.
+   * {@code https://automate.browserstack.com.example/} — is 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

@@ -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)");
         }
     }