Skip to content

Commit 3995fe7

Browse files
Harden URL handling, credentials, and CI workflow
- Validate API base URL read from JVM system properties (browserstack.automate.api / browserstack.app-automate.api): require https scheme and a host under .browserstack.com. - Validate the BrowserStack-issued logs URL in AutomateClient.getSessionLogs before fetching, with the same scheme + host allowlist. - Make HttpTransport per-instance (was a JVM-wide static field) so setProxy on one client does not affect other client instances in the same process. - Tighten BrowserStackClient.checkAuthState so it throws when EITHER the username OR access key is missing (was: only when both were null). - AppAutomateClient.uploadApp now canonicalizes the file path via File.getCanonicalFile() before applying the .apk/.ipa extension check, and uses the canonical File for the upload. - Pin GitHub Actions in .github/workflows/ci.yml to commit SHAs and add a top-level permissions: contents: read block. - Set nexus-staging-maven-plugin autoReleaseAfterClose to false so staged releases must be promoted manually after mvn release:perform. Adds hermetic unit tests for the URL allowlist, credential check, per-instance HttpTransport, and canonical-path extension check.
1 parent 301163d commit 3995fe7

8 files changed

Lines changed: 586 additions & 13 deletions

File tree

.github/workflows/ci.yml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,17 @@ on:
88
branches:
99
- master
1010

11+
permissions:
12+
contents: read
13+
1114
jobs:
1215
build:
1316
runs-on: ubuntu-latest
1417

1518
steps:
16-
- uses: actions/checkout@v2
19+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
1720
- name: Set up JDK 8
18-
uses: actions/setup-java@v3
21+
uses: actions/setup-java@c1e323688fd81a25caa38c78aa6df2d33d3e20d9 # v4.8.0
1922
with:
2023
java-version: 8
2124
distribution: 'temurin'

pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@
8989
<configuration>
9090
<serverId>ossrh</serverId>
9191
<nexusUrl>https://oss.sonatype.org/</nexusUrl>
92-
<autoReleaseAfterClose>true</autoReleaseAfterClose>
92+
<autoReleaseAfterClose>false</autoReleaseAfterClose>
9393
</configuration>
9494
</plugin>
9595
<plugin>

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

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
import java.io.File;
44
import java.io.FileNotFoundException;
5+
import java.io.IOException;
6+
import java.net.URI;
57
import java.util.List;
68
import com.browserstack.automate.Automate.BuildStatus;
79
import com.browserstack.automate.exception.AppAutomateException;
@@ -32,7 +34,8 @@ public class AppAutomateClient extends BrowserStackClient implements AppAutomate
3234
* @param accessKey Browserstack accessKey
3335
*/
3436
public AppAutomateClient(String username, String accessKey) {
35-
super(System.getProperty("browserstack.app-automate.api", BASE_URL), username, accessKey);
37+
super(validateApiBaseUrl(System.getProperty("browserstack.app-automate.api", BASE_URL)),
38+
username, accessKey);
3639
}
3740

3841
/**
@@ -71,9 +74,21 @@ public AppUploadResponse uploadApp(String filePath)
7174
throw new FileNotFoundException("File not found at " + filePath);
7275
}
7376

74-
if (!filePath.endsWith(".apk") && !filePath.endsWith(".ipa")) {
77+
// Canonicalize before validating extension so a symlink whose name ends in
78+
// .apk/.ipa but whose target file does not is rejected. getCanonicalFile()
79+
// resolves symlinks and ".." segments; the extension check then runs on the
80+
// canonical name.
81+
final File canonical;
82+
try {
83+
canonical = file.getCanonicalFile();
84+
} catch (IOException e) {
85+
throw new AppAutomateException("Unable to canonicalize file path: " + e.getMessage(), 0);
86+
}
87+
final String canonicalName = canonical.getName().toLowerCase();
88+
if (!canonicalName.endsWith(".apk") && !canonicalName.endsWith(".ipa")) {
7589
throw new InvalidFileExtensionException("File extension should be only .apk or .ipa.");
7690
}
91+
file = canonical;
7792

7893
MultipartContent content = new MultipartContent().setMediaType(
7994
new HttpMediaType("multipart/form-data").setParameter("boundary", "__END_OF_PART__"));
@@ -282,5 +297,43 @@ public List<Session> getSessions(final String buildId, final BuildStatus status)
282297
return getSessions(buildId, status, 0);
283298
}
284299

285-
}
286300

301+
/**
302+
* Validates the App Automate API base URL read from the JVM system property
303+
* {@code browserstack.app-automate.api} (or the built-in default).
304+
* <p>
305+
* Only {@code https} URLs whose host (parsed via
306+
* {@link java.net.URI#getHost()}) ends in {@code .browserstack.com} are
307+
* accepted.
308+
*
309+
* @param url URL to validate. Must be a syntactically-valid absolute URL.
310+
* @return the same URL, unchanged, when validation passes.
311+
* @throws IllegalArgumentException if the URL is null, malformed, uses a
312+
* non-https scheme, or has a host outside
313+
* {@code *.browserstack.com}.
314+
*/
315+
private static String validateApiBaseUrl(String url) {
316+
if (url == null) {
317+
throw new IllegalArgumentException("App Automate API base URL is null");
318+
}
319+
320+
final URI uri;
321+
try {
322+
uri = URI.create(url);
323+
} catch (IllegalArgumentException e) {
324+
throw new IllegalArgumentException("Malformed App Automate API base URL", e);
325+
}
326+
327+
final String scheme = uri.getScheme();
328+
if (!"https".equalsIgnoreCase(scheme)) {
329+
throw new IllegalArgumentException("Insecure App Automate API base URL scheme: " + scheme);
330+
}
331+
332+
final String host = uri.getHost();
333+
if (host == null || !host.toLowerCase().endsWith(".browserstack.com")) {
334+
throw new IllegalArgumentException("Untrusted App Automate API base URL host: " + host);
335+
}
336+
337+
return url;
338+
}
339+
}

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

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import javax.annotation.Nonnull;
1616
import java.util.ArrayList;
1717
import java.util.Arrays;
18+
import java.net.URI;
1819
import java.util.HashMap;
1920
import java.util.List;
2021
import java.util.Map;
@@ -35,7 +36,8 @@ public final class AutomateClient extends BrowserStackClient implements Automate
3536
* @param accessKey Access Key for your BrowserStack Automate account.
3637
*/
3738
public AutomateClient(String username, String accessKey) {
38-
super(System.getProperty("browserstack.automate.api", BASE_URL), username, accessKey);
39+
super(validateApiBaseUrl(System.getProperty("browserstack.automate.api", BASE_URL)),
40+
username, accessKey);
3941
}
4042

4143
/**
@@ -458,6 +460,7 @@ public String getSessionLogs(final Session session) throws AutomateException {
458460
}
459461

460462
try {
463+
validateBrowserStackUrl(session.getLogUrl());
461464
BrowserStackRequest request = newRequest(Method.GET, session.getLogUrl(), false);
462465
request.getHttpRequest().getHeaders().setAccept("*/*");
463466
return request.asString();
@@ -529,4 +532,79 @@ public String recycleKey() throws AutomateException {
529532
setAccessKey(newAccessKey);
530533
return newAccessKey;
531534
}
535+
536+
/**
537+
* Validates the API base URL read from the JVM system property
538+
* {@code browserstack.automate.api} (or the built-in default).
539+
* <p>
540+
* Only {@code https} URLs whose host (parsed via
541+
* {@link java.net.URI#getHost()}) ends in {@code .browserstack.com} are
542+
* accepted.
543+
*
544+
* @param url URL to validate. Must be a syntactically-valid absolute URL.
545+
* @return the same URL, unchanged, when validation passes.
546+
* @throws IllegalArgumentException if the URL is null, malformed, uses a
547+
* non-https scheme, or has a host outside
548+
* {@code *.browserstack.com}.
549+
*/
550+
private static String validateApiBaseUrl(String url) {
551+
if (url == null) {
552+
throw new IllegalArgumentException("API base URL is null");
553+
}
554+
555+
final URI uri;
556+
try {
557+
uri = URI.create(url);
558+
} catch (IllegalArgumentException e) {
559+
throw new IllegalArgumentException("Malformed API base URL", e);
560+
}
561+
562+
final String scheme = uri.getScheme();
563+
if (!"https".equalsIgnoreCase(scheme)) {
564+
throw new IllegalArgumentException("Insecure API base URL scheme: " + scheme);
565+
}
566+
567+
final String host = uri.getHost();
568+
if (host == null || !host.toLowerCase().endsWith(".browserstack.com")) {
569+
throw new IllegalArgumentException("Untrusted API base URL host: " + host);
570+
}
571+
572+
return url;
573+
}
574+
575+
/**
576+
* Validates that a URL is a BrowserStack-issued logs URL before fetching.
577+
* <p>
578+
* Only {@code https} scheme is allowed, and the host (parsed via
579+
* {@link java.net.URI#getHost()}, not a string suffix on the raw URL) must
580+
* end with {@code .browserstack.com}. A URL whose host has
581+
* {@code .browserstack.com} only as an internal substring — e.g.
582+
* {@code https://automate.browserstack.com.example/} — is rejected.
583+
*
584+
* @param url URL to validate. Must be a syntactically-valid absolute URL.
585+
* @throws AutomateException if the URL is malformed, uses a non-https scheme,
586+
* or has a host outside {@code *.browserstack.com}.
587+
*/
588+
private static void validateBrowserStackUrl(String url) throws AutomateException {
589+
if (url == null) {
590+
throw new AutomateException("Logs URL is null", 400);
591+
}
592+
593+
final URI uri;
594+
try {
595+
uri = URI.create(url);
596+
} catch (IllegalArgumentException e) {
597+
throw new AutomateException("Malformed logs URL", 400);
598+
}
599+
600+
final String scheme = uri.getScheme();
601+
if (!"https".equalsIgnoreCase(scheme)) {
602+
throw new AutomateException("Insecure logs URL scheme: " + scheme, 400);
603+
}
604+
605+
final String host = uri.getHost();
606+
if (host == null || !host.toLowerCase().endsWith(".browserstack.com")) {
607+
throw new AutomateException("Untrusted logs URL host: " + host, 400);
608+
}
609+
}
532610
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public Object parseAndClose(Reader reader, Type type) throws IOException {
6969
throw new IOException("Unsupported operation");
7070
}
7171
};
72-
private static HttpTransport HTTP_TRANSPORT = new ApacheHttpTransport();
72+
private HttpTransport httpTransport = new ApacheHttpTransport();
7373
protected final BrowserStackCache<String, Object> cacheMap;
7474

7575
private HttpRequestFactory requestFactory;
@@ -105,8 +105,8 @@ public BrowserStackClient(String baseUrl, String username, String accessKey) {
105105
this.accessKey = accessKey.trim();
106106
}
107107

108-
static HttpRequestFactory newRequestFactory() {
109-
return HTTP_TRANSPORT.createRequestFactory(httpRequest -> httpRequest.setParser(OBJECT_PARSER));
108+
HttpRequestFactory newRequestFactory() {
109+
return httpTransport.createRequestFactory(httpRequest -> httpRequest.setParser(OBJECT_PARSER));
110110
}
111111

112112
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
174174
}
175175

176176
final HttpClient client = clientBuilder.build();
177-
HTTP_TRANSPORT = new ApacheHttpTransport(client);
177+
this.httpTransport = new ApacheHttpTransport(client);
178178
this.requestFactory = newRequestFactory();
179179
}
180180

@@ -187,8 +187,8 @@ protected synchronized void setAccessKey(final String accessKey) {
187187
}
188188

189189
private void checkAuthState() {
190-
if (this.accessKey == null && this.username == null) {
191-
throw new IllegalStateException("Missing API credentials");
190+
if (this.accessKey == null || this.username == null) {
191+
throw new IllegalStateException("Missing API credentials (username or access key is null)");
192192
}
193193
}
194194

0 commit comments

Comments
 (0)