Harden URL handling, credentials, and CI workflow#83
Merged
karanshah-browserstack merged 3 commits intomasterfrom May 8, 2026
Merged
Harden URL handling, credentials, and CI workflow#83karanshah-browserstack merged 3 commits intomasterfrom
karanshah-browserstack merged 3 commits intomasterfrom
Conversation
- 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.
Restores nexus-staging-maven-plugin's autoReleaseAfterClose to true so release-promotion behaviour matches master. Out of scope for this PR.
Customers may rely on the previous JVM-wide static HTTP_TRANSPORT behaviour where setProxy on any client applied to all instances. Restoring the static field to avoid breaking those setups. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
karanshah-browserstack
approved these changes
May 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR hardens URL handling, credential validation, and CI configuration in the
automate-client-javaclient.AutomateClient.getSessionLogshttpsURLs whose host ends with.browserstack.com(parsed viajava.net.URI, not a raw-string suffix) are accepted before fetching the logs URL returned by the API.AutomateClientandAppAutomateClientconstructorsbrowserstack.automate.api/browserstack.app-automate.apiis now validated against the same allowlist before being passed to the parent constructor.BrowserStackClient.checkAuthStateAppAutomateClient.uploadAppFile.getCanonicalFile()before the.apk/.ipaextension check; the canonical file is used for the upload..github/workflows/ci.ymlactions/checkout@v4.3.1,actions/setup-java@v4.8.0); top-levelpermissions: contents: readadded.Rationale on the CI changes
@v2/@v3) makes our CI builds reproducible and forces every action upgrade to be an explicit, reviewed change in this repo. The# v4.3.1/# v4.8.0trailing comments preserve human readability of the version that the SHA corresponds to. The top-levelpermissions: contents: readblock sets the defaultGITHUB_TOKENto read-only so the workflow only gets escalated permissions if a job explicitly asks for them.Breaking changes for consumers relying on undocumented behaviour
checkAuthStatenow throws when EITHER credential is missing. Previously the guard only fired whenusernameANDaccessKeywere bothnull. A client with one credential set and one missing now throwsIllegalStateExceptioninstead of silently producing a malformedAuthorizationheader.browserstack.automate.api/browserstack.app-automate.apiare now allowlist-validated. Anything outsidehttps://*.browserstack.commakes the constructor throwIllegalArgumentException. Effectively a no-op for production consumers; teams pointing at private mocks orlocalhostfor tests will need to update those tests.Tests
AutomateClientSecurityTest— 7 tests for the logs URL allowlist and the credential-presence check.ApiBaseUrlAndUploadSecurityTest— 10 tests for the JVM-property base URL allowlist and the canonical-path extension check.master.Replaces #82 (closed when its branch was renamed; review comments from that PR have been addressed in this diff —
CHANGELOG.mdremoved and inline source/test comments rewritten in customer-facing language).