From f882c50723f9f29e785ec39983ab7ab4ef78f1b8 Mon Sep 17 00:00:00 2001 From: Crash0v3rrid3 Date: Fri, 16 Oct 2026 15:54:23 +0530 Subject: [PATCH 1/2] fix(security): only follow redirects to https on browserstack.com [DEVA11Y-473] followRedirects()/followWithGet() returned any Location target, and URLSession.shared auto-followed every hop, so an https -> http (or off-domain) redirect would fetch the CLI archive in cleartext or from an arbitrary host before our code saw the URL. Route the HEAD/GET resolution and the archive download through a session whose delegate vetoes any hop that is not https on browserstack.com or a subdomain, and re-check the vetoed Location / final response URL after each request as a backstop. The initial URL is still governed by parseOverride(), so an HTTPS override to a mirror keeps working as long as it does not redirect off-domain. Co-Authored-By: Claude Opus 5.5 --- .../BrowserStackAccessibilityLint.swift | 79 +++++++++++++++++-- 1 file changed, 74 insertions(+), 5 deletions(-) diff --git a/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift b/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift index ab63afa..6725c27 100644 --- a/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift +++ b/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift @@ -109,6 +109,67 @@ private func parseOverride(urlString: String?) throws -> URL? { return url } +// MARK: - Redirect policy (DEVA11Y-473) + +/// A redirect may only land on HTTPS on browserstack.com or one of its subdomains. Without +/// this, a hop to an http:// URL (or an unrelated host) would be followed and the CLI +/// archive downloaded from it. The *initial* URL is governed by `parseOverride` instead, so +/// an explicit HTTPS override pointing at a mirror still works as long as it does not redirect +/// off-domain. +private func isAllowedRedirectTarget(_ url: URL) -> Bool { + guard url.scheme?.lowercased() == "https", let host = url.host?.lowercased() else { + return false + } + return host == "browserstack.com" || host.hasSuffix(".browserstack.com") +} + +@discardableResult +private func requireAllowedRedirectTarget(_ url: URL) throws -> URL { + guard isAllowedRedirectTarget(url) else { + throw PluginError("Refusing to follow redirect to \(url.absoluteString): only https:// URLs on browserstack.com are allowed.") + } + return url +} + +/// Vetoes every redirect hop URLSession would otherwise follow on its own, so a disallowed +/// target is never contacted. Callers additionally check the response's final URL, which is +/// the backstop on platforms whose Foundation does not consult the delegate. +private final class RedirectPolicyDelegate: NSObject, URLSessionTaskDelegate { + func urlSession( + _ session: URLSession, + task: URLSessionTask, + willPerformHTTPRedirection response: HTTPURLResponse, + newRequest request: URLRequest, + completionHandler: @escaping (URLRequest?) -> Void + ) { + if let url = request.url, isAllowedRedirectTarget(url) { + completionHandler(request) + } else { + completionHandler(nil) + } + } +} + +private let downloadSession = URLSession( + configuration: .default, + delegate: RedirectPolicyDelegate(), + delegateQueue: nil +) + +/// Validates where a request actually ended up: an unfollowed (vetoed) redirect's Location, +/// and the final URL if it differs from the one requested. +private func validateRedirects(response: URLResponse, requestedURL: URL) throws { + if let http = response as? HTTPURLResponse, + (300..<400).contains(http.statusCode), + let location = http.value(forHTTPHeaderField: "Location"), + let target = URL(string: location, relativeTo: requestedURL)?.absoluteURL { + try requireAllowedRedirectTarget(target) + } + if let finalURL = response.url, finalURL != requestedURL { + try requireAllowedRedirectTarget(finalURL) + } +} + private func sanitizeArguments(_ arguments: [String]) -> [String] { var result: [String] = [] var skipNext = false @@ -641,14 +702,15 @@ private struct BrowserStackCLIDownloader { request.timeoutInterval = 30 do { - let (_, response) = try await URLSession.shared.data(for: request) + let (_, response) = try await downloadSession.data(for: request) + try validateRedirects(response: response, requestedURL: url) if let http = response as? HTTPURLResponse { if http.statusCode == 405 || http.statusCode == 501 { return try await followWithGet(for: url) } if let location = http.value(forHTTPHeaderField: "Location"), let redirectURL = URL(string: location, relativeTo: url)?.absoluteURL { - return redirectURL + return try requireAllowedRedirectTarget(redirectURL) } } if let finalURL = response.url { @@ -671,11 +733,12 @@ private struct BrowserStackCLIDownloader { request.httpShouldHandleCookies = false request.timeoutInterval = 60 - let (_, response) = try await URLSession.shared.data(for: request) + let (_, response) = try await downloadSession.data(for: request) + try validateRedirects(response: response, requestedURL: url) if let http = response as? HTTPURLResponse, let location = http.value(forHTTPHeaderField: "Location"), let redirectURL = URL(string: location, relativeTo: url)?.absoluteURL { - return redirectURL + return try requireAllowedRedirectTarget(redirectURL) } guard let finalURL = response.url else { throw PluginError("Failed to resolve redirect for \(url.absoluteString).") @@ -692,7 +755,13 @@ private struct BrowserStackCLIDownloader { return } - let (tempURL, response) = try await URLSession.shared.download(from: url) + let (tempURL, response) = try await downloadSession.download(from: url) + do { + try validateRedirects(response: response, requestedURL: url) + } catch { + try? fileManager.removeItem(at: tempURL) + throw error + } if let httpResponse = response as? HTTPURLResponse, !(200..<300).contains(httpResponse.statusCode) { try? fileManager.removeItem(at: tempURL) throw PluginError("Failed to download BrowserStack CLI (HTTP \(httpResponse.statusCode)).") From 2e23e29f1712cabcd4b177a6056804ad16d8b8e8 Mon Sep 17 00:00:00 2001 From: Crash0v3rrid3 Date: Fri, 16 Oct 2026 15:56:05 +0530 Subject: [PATCH 2/2] fix(security): fail closed when the CLI checksum sidecar is unusable [DEVA11Y-473] verifyArchiveChecksum aborted only on a digest mismatch. A sidecar that could not be derived, returned non-2xx, failed to fetch, or was not a 64-char hex digest let the archive be extracted and executed unverified, so blocking or corrupting the .sha256 bypassed the check entirely (Security retest of DEVA11Y-473 @806f98cb). Every one of those paths now throws before extraction. The sidecar fetch also goes through the redirect-guarded session. README updated to describe the plugin's fail-closed behaviour. MERGE GATE: sdk-assets.browserstack.com does not publish .sha256 yet (403 for all six platform binaries as of 2026-10-16), so this commit makes every plugin run fail until SDK assets ships the sidecars. Co-Authored-By: Claude Opus 5.5 --- .../BrowserStackAccessibilityLint.swift | 36 +++++++++---------- README.md | 2 +- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift b/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift index 6725c27..c98d252 100644 --- a/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift +++ b/Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift @@ -409,10 +409,11 @@ private struct BrowserStackCLIDownloader { /// SHA-256 sidecar (`.sha256`) before it is extracted, made executable and run. /// api.browserstack.com (control plane) 302-redirects to a versioned, immutable asset on /// the CDN/S3 (data plane); a checksum published next to that asset lets us detect a - /// tampered or corrupted binary. Semantics mirror the launcher self-update: fail CLOSED on - /// a mismatch, fail OPEN (warn + proceed) when no sidecar is published yet, so this is - /// non-breaking until the SDK-assets team ships the sidecars (the server-side half of the - /// fix). This is a download-integrity check, NOT an authenticity signature. + /// tampered or corrupted binary. Fails CLOSED on every path: a mismatch, and equally a + /// sidecar that cannot be derived, fetched or parsed, aborts before extraction. Treating + /// "no sidecar" as "skip verification" let anyone able to block or corrupt the sidecar + /// bypass the check entirely (DEVA11Y-473 security retest). This is a download-integrity + /// check, NOT an authenticity signature. private func verifyArchiveChecksum(archiveURL: URL, resolvedURL: URL) async throws { // Derive the sidecar from the asset's scheme/host/path only. Stripping any query // string keeps signed/presigned URLs (…zip?token=) from deriving a permanently-404 @@ -422,39 +423,34 @@ private struct BrowserStackCLIDownloader { sidecarComponents?.fragment = nil guard let strippedURL = sidecarComponents?.url, let sidecarURL = URL(string: strippedURL.absoluteString + ".sha256") else { - Diagnostics.remark("BrowserStackAccessibilityLint: could not derive checksum URL; skipping integrity check (DEVA11Y-473/474).") - return + throw PluginError("Could not derive the BrowserStack CLI checksum URL from \(resolvedURL.absoluteString); refusing to use the downloaded binary.") } var request = URLRequest(url: sidecarURL) request.httpShouldHandleCookies = false request.timeoutInterval = 30 let body: Data do { - let (data, response) = try await URLSession.shared.data(for: request) + let (data, response) = try await downloadSession.data(for: request) + try validateRedirects(response: response, requestedURL: sidecarURL) guard let http = response as? HTTPURLResponse, (200..<300).contains(http.statusCode) else { - Diagnostics.remark("BrowserStackAccessibilityLint: no published checksum at \(sidecarURL.absoluteString); proceeding WITHOUT integrity verification (DEVA11Y-473/474).") - return + let status = (response as? HTTPURLResponse).map { "HTTP \($0.statusCode)" } ?? "no HTTP response" + throw PluginError("No BrowserStack CLI checksum available at \(sidecarURL.absoluteString) (\(status)); refusing to use the downloaded binary.") } body = data + } catch let error as PluginError { + throw error } catch { - Diagnostics.remark("BrowserStackAccessibilityLint: checksum fetch failed (\(error.localizedDescription)); proceeding WITHOUT integrity verification (DEVA11Y-473/474).") - return + throw PluginError("Failed to fetch the BrowserStack CLI checksum from \(sidecarURL.absoluteString) (\(error.localizedDescription)); refusing to use the downloaded binary.") } - // A published sidecar that is present but empty/unreadable is treated as a hard failure: - // once the server publishes checksums, a missing value must not silently downgrade to - // "no verification". guard let text = String(data: body, encoding: .utf8), let expected = text.split(whereSeparator: { $0 == " " || $0 == "\n" || $0 == "\r" || $0 == "\t" }).first.map(String.init), !expected.isEmpty else { throw PluginError("BrowserStack CLI checksum sidecar was empty or unreadable; refusing to use the downloaded binary.") } - // A non-empty body that is not a 64-char hex digest is a CDN/S3 error page answered - // 200 (e.g. an S3 `AccessDenied` XML), not a checksum. Fail OPEN rather than turning - // its first token into the "expected hash" and hard-failing every client on every - // run (DEVA11Y-473/474 review). + // Anything that is not a 64-char hex digest (e.g. a CDN/S3 error page answered 200) is + // not a checksum we can verify against, so it is a hard failure too. guard expected.count == 64, expected.allSatisfy({ $0.isHexDigit }) else { - Diagnostics.remark("BrowserStackAccessibilityLint: malformed checksum at \(sidecarURL.absoluteString); proceeding WITHOUT integrity verification (DEVA11Y-473/474).") - return + throw PluginError("Malformed BrowserStack CLI checksum at \(sidecarURL.absoluteString); refusing to use the downloaded binary.") } let actual = try sha256Hex(of: archiveURL) guard actual.caseInsensitiveCompare(expected) == .orderedSame else { diff --git a/README.md b/README.md index c1626f5..4fcb963 100644 --- a/README.md +++ b/README.md @@ -213,7 +213,7 @@ You can then edit the `.git/hooks/pre-commit` file to customise the registered p ## Updating the launcher script The launcher scripts no longer update themselves automatically on every run. To pull the latest launcher on demand, run the `self-update` subcommand — the download is checked against a published SHA-256 checksum (an integrity check against corruption in transit, not an authenticity signature, since the script and its checksum share one origin) and is only applied if it matches. -The BrowserStack CLI binary the scripts and the SwiftPM plugin download is verified the same way: when a `.sha256` checksum is published next to the binary, the download is checked against it (over HTTPS, on macOS/Linux/Windows) before it is extracted, made executable, or run, and a mismatch aborts. This is an integrity check, not an authenticity signature. Until that server-side checksum is published the verification is inert by design — the binary still downloads and runs — so it hardens the download as defense-in-depth rather than being a guarantee that holds today. +The BrowserStack CLI binary the scripts and the SwiftPM plugin download is verified the same way: the download is checked against the `.sha256` checksum published next to the binary (over HTTPS, on macOS/Linux/Windows) before it is extracted, made executable, or run, and a mismatch aborts. This is an integrity check, not an authenticity signature. The SwiftPM plugin fails closed: if the checksum is missing, unreachable or malformed it refuses to run the binary, and it only follows download redirects to `https://` URLs on `browserstack.com`. The launcher scripts still proceed without verification when no checksum is published. Zsh ```zsh