+ * 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; + } +} diff --git a/src/main/java/com/browserstack/automate/AutomateClient.java b/src/main/java/com/browserstack/automate/AutomateClient.java index f979dc8..6a3e148 100644 --- a/src/main/java/com/browserstack/automate/AutomateClient.java +++ b/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). + *
+ * 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. + *
+ * 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);
+ }
+ }
}
diff --git a/src/main/java/com/browserstack/client/BrowserStackClient.java b/src/main/java/com/browserstack/client/BrowserStackClient.java
index a28a2b8..b5b5464 100644
--- a/src/main/java/com/browserstack/client/BrowserStackClient.java
+++ b/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
+ * Skipped on platforms that don't allow creating symlinks (e.g. Windows
+ * without developer mode).
+ */
+ @Test
+ public void uploadApp_rejectsSymlinkWhoseCanonicalNameLacksValidExtension()
+ throws IOException, FileNotFoundException, AppAutomateException {
+ Path tmp = Files.createTempDirectory("symlink-extension-");
+ try {
+ // Create a non-.apk target file.
+ Path target = tmp.resolve("secret.txt");
+ Files.write(target, "sensitive content".getBytes());
+
+ // Create the .apk symlink pointing at it.
+ Path symlink = tmp.resolve("legit.apk");
+ try {
+ Files.createSymbolicLink(symlink, target);
+ } catch (UnsupportedOperationException | IOException | SecurityException e) {
+ Assume.assumeNoException(
+ "Symlink creation not permitted on this platform/runtime", e);
+ }
+
+ AppAutomateClient client = new AppAutomateClient(DUMMY_USER, DUMMY_KEY);
+ try {
+ client.uploadApp(symlink.toAbsolutePath().toString());
+ fail("Expected InvalidFileExtensionException for symlink to non-.apk file");
+ } catch (InvalidFileExtensionException e) {
+ assertTrue("message should mention .apk/.ipa: " + e.getMessage(),
+ e.getMessage().toLowerCase().contains(".apk"));
+ }
+ } finally {
+ // Best-effort cleanup; ignore failures.
+ try { deleteRecursive(tmp); } catch (IOException ignored) {}
+ }
+ }
+
+ /**
+ * A regular .apk file (no symlink) must still be accepted by the extension
+ * check after canonicalization. The upload itself will fail with dummy
+ * creds, but specifically NOT with InvalidFileExtensionException — that
+ * proves the canonical-name extension check passed.
+ */
+ @Test
+ public void uploadApp_acceptsRealApkAfterCanonicalization()
+ throws IOException, FileNotFoundException {
+ Path tmp = Files.createTempDirectory("apk-extension-");
+ try {
+ Path apk = tmp.resolve("dummy.apk");
+ Files.write(apk, new byte[]{0x50, 0x4B, 0x03, 0x04}); // ZIP magic
+ AppAutomateClient client = new AppAutomateClient(DUMMY_USER, DUMMY_KEY);
+ try {
+ client.uploadApp(apk.toAbsolutePath().toString());
+ // unlikely with dummy creds, but if it returns, that also passes
+ } catch (InvalidFileExtensionException e) {
+ fail("Real .apk should not be rejected by extension check: "
+ + e.getMessage());
+ } catch (AppAutomateException e) {
+ // Expected — auth failure with dummy creds. Our extension check
+ // already passed by the time we got here.
+ }
+ } finally {
+ try { deleteRecursive(tmp); } catch (IOException ignored) {}
+ }
+ }
+
+ private static void deleteRecursive(Path p) throws IOException {
+ if (!Files.exists(p, java.nio.file.LinkOption.NOFOLLOW_LINKS)) return;
+ if (Files.isDirectory(p, java.nio.file.LinkOption.NOFOLLOW_LINKS)) {
+ try (java.util.stream.Stream
+ * For a URL like {@code https://automate.browserstack.com.example/foo},
+ * a naive {@code url.endsWith(".browserstack.com")} would fail to reject
+ * it because {@code .browserstack.com} only appears as an internal
+ * substring of the host. {@link java.net.URI#getHost()} returns the actual
+ * host {@code automate.browserstack.com.example}, which the suffix check
+ * on the host (not the URL) correctly rejects.
+ */
+ @Test
+ public void getSessionLogs_rejectsHostSuffixSpoof() throws Exception {
+ AutomateClient client = new AutomateClient(DUMMY_USER, DUMMY_KEY);
+ Session session = sessionWithLogUrl("https://automate.browserstack.com.example/foo");
+ try {
+ client.getSessionLogs(session);
+ fail("Expected AutomateException for non-allowlisted host");
+ } catch (AutomateException e) {
+ assertTrue("message mentions Untrusted host: " + e.getMessage(),
+ e.getMessage().toLowerCase().contains("untrusted"));
+ assertEquals(400, e.getStatusCode());
+ }
+ }
+
+ /**
+ * A trusted host should pass the validation guard. The HTTP request will
+ * still fail (DNS / 401 / etc.) but specifically NOT with the 400
+ * "Untrusted/Insecure/Malformed" guard message — that proves the URL
+ * cleared the allowlist.
+ */
+ @Test
+ public void getSessionLogs_acceptsTrustedHost() throws Exception {
+ AutomateClient client = new AutomateClient(DUMMY_USER, DUMMY_KEY);
+ Session session = sessionWithLogUrl("https://api.browserstack.com/automate/path");
+ try {
+ client.getSessionLogs(session);
+ // If somehow it succeeds (unlikely with dummy creds), that also passes the guard.
+ } catch (AutomateException e) {
+ String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase();
+ assertTrue("Trusted host should not be rejected by URL allowlist. Got: " + e.getMessage(),
+ !msg.contains("untrusted") && !msg.contains("insecure")
+ && !msg.contains("malformed"));
+ }
+ }
+
+ // ---------- checkAuthState requires both credentials ----------
+
+ private static void invokeCheckAuthState(BrowserStackClient client) throws Throwable {
+ Method m = BrowserStackClient.class.getDeclaredMethod("checkAuthState");
+ m.setAccessible(true);
+ try {
+ m.invoke(client);
+ } catch (InvocationTargetException e) {
+ throw e.getTargetException();
+ }
+ }
+
+ private static void setField(BrowserStackClient client, String fieldName, Object value)
+ throws ReflectiveOperationException {
+ Field f = BrowserStackClient.class.getDeclaredField(fieldName);
+ f.setAccessible(true);
+ f.set(client, value);
+ }
+
+ @Test
+ public void checkAuthState_throwsWhenUsernameNull() throws Throwable {
+ AutomateClient client = new AutomateClient(DUMMY_USER, DUMMY_KEY);
+ setField(client, "username", null);
+ try {
+ invokeCheckAuthState(client);
+ fail("Expected IllegalStateException when username is null");
+ } catch (IllegalStateException e) {
+ assertNotNull(e.getMessage());
+ }
+ }
+
+ @Test
+ public void checkAuthState_throwsWhenAccessKeyNull() throws Throwable {
+ AutomateClient client = new AutomateClient(DUMMY_USER, DUMMY_KEY);
+ setField(client, "accessKey", null);
+ try {
+ invokeCheckAuthState(client);
+ fail("Expected IllegalStateException when accessKey is null");
+ } catch (IllegalStateException e) {
+ assertNotNull(e.getMessage());
+ }
+ }
+
+ @Test
+ public void checkAuthState_passesWhenBothCredentialsPresent() throws Throwable {
+ AutomateClient client = new AutomateClient(DUMMY_USER, DUMMY_KEY);
+ // Should not throw
+ invokeCheckAuthState(client);
+ }
+}
diff --git a/src/test/java/com/browserstack/client/BrowserStackClientProxyIsolationTest.java b/src/test/java/com/browserstack/client/BrowserStackClientProxyIsolationTest.java
new file mode 100644
index 0000000..7f21ee3
--- /dev/null
+++ b/src/test/java/com/browserstack/client/BrowserStackClientProxyIsolationTest.java
@@ -0,0 +1,63 @@
+package com.browserstack.client;
+
+import com.browserstack.automate.AutomateClient;
+import com.google.api.client.http.HttpTransport;
+import org.junit.Test;
+
+import java.lang.reflect.Field;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
+
+/**
+ * Verifies that {@code setProxy} mutates per-instance state only.
+ *
+ * The {@code httpTransport} field is per-instance, so configuring a proxy
+ * on one client must not affect another client constructed in the same
+ * process.
+ *
+ * This test snapshots both clients' {@code httpTransport} references
+ * before and after a proxy is configured on one of them.
+ */
+public class BrowserStackClientProxyIsolationTest {
+
+ private static HttpTransport getHttpTransport(BrowserStackClient client) throws Exception {
+ Field f = BrowserStackClient.class.getDeclaredField("httpTransport");
+ f.setAccessible(true);
+ return (HttpTransport) f.get(client);
+ }
+
+ @Test
+ public void setProxy_isInstanceScoped_doesNotLeakAcrossClients() throws Exception {
+ AutomateClient clientA = new AutomateClient("user_a", "key_a");
+ AutomateClient clientB = new AutomateClient("user_b", "key_b");
+
+ HttpTransport beforeA = getHttpTransport(clientA);
+ HttpTransport beforeB = getHttpTransport(clientB);
+ assertNotNull("clientA must have a transport before setProxy", beforeA);
+ assertNotNull("clientB must have a transport before setProxy", beforeB);
+
+ // Configure a proxy on clientA only.
+ clientA.setProxy("proxy.example.invalid", 3128, null, null);
+
+ HttpTransport afterA = getHttpTransport(clientA);
+ HttpTransport afterB = getHttpTransport(clientB);
+
+ // clientA's transport should have been replaced.
+ assertNotSame(
+ "clientA.httpTransport must be replaced after setProxy",
+ beforeA, afterA);
+
+ // clientB's transport must NOT have changed.
+ assertSame(
+ "clientB.httpTransport must NOT change when setProxy is called on clientA "
+ + "(httpTransport must be per-instance, not shared across BrowserStackClient instances)",
+ beforeB, afterB);
+
+ // And clientA's new transport must not equal clientB's transport.
+ assertNotSame(
+ "clientA and clientB must have distinct transports after setProxy on A",
+ afterA, afterB);
+ }
+}